[RFC] use curl for ssh dumps #59
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Checking for the Opalcore currently requires duplicate code. Simplify it by always checking if an Opalcore exists at the beginning of the script. Signed-off-by: Philipp Rudo <[email protected]>
Move the ssh host to a global variable to make it available for new functions introduced in following commits. While at it rename $SSH_KEY_LOCATION to $SSH_KEY to shorten the name and be consistent with the naming schema of the other variables. Signed-off-by: Philipp Rudo <[email protected]>
In dump_ssh the _ssh_opts need to be word split, which triggers ShellChecks SC2086 warning. This warning is currently disabled for all calls to ssh/scp. However in the ShellCheck wiki [1] the suggested solution for POSIX compatible code is to make the call in a separate function. This also makes disabling of SC2029 obsolete. One big benefit of this solution is that now calls to ssh/scp can also be made outside of dump_ssh without redefining _ssh_opts. One small downside is that the options passed to ssh/scp must now be maintained in each new function separately. Compared to the benefit described above this is a small price to pay. Note: Currently ssh/scp option -q,--quiet is used inconsistently. With this commit it will always be set. [1] https://www.shellcheck.net/wiki/SC2086 Signed-off-by: Philipp Rudo <[email protected]>
The mount point for file system dumps is currently passed as an argument to dump_fs. But there are only two possible values for the mount point. Its either $NEWROOT for failure_action dump_to_rootfs or whatever the user provided in kdump.conf. Thus instead of passing the mount point as argument use a global variable and update it when parsing the config and use that in dump_fs. Reuse $NEWROOT for this. Signed-off-by: Philipp Rudo <[email protected]>
Both dump_fs and dump_ssh currently define their own, almost identical,
directories they use to write the dump to. This not only adds duplicate
code but also prevents any code outside of dump_{fs,ssh} to access the
dump directory. Thus use a global definition for said directory. Reuse
the already existing $KDUMP_PATH for that.
Signed-off-by: Philipp Rudo <[email protected]>
This commit contains two major changes for dumps via ssh. One is the
switch from scp to sftp as protocol to transfer files. Using sftp has
two big advantages
1. It allows a wide variety of file operations, e.g. renaming files,
on the remote host.
2. It can work with files of unknown size, i.e. read from a pipe.
sftp is fully supported by OpenSSH, in fact since OpenSSH 9.0 (released
April 2022) the 'scp' command uses sftp internally by default.
The other big change is to make use of curl rather than ssh/scp/sftp
directly. This is mainly because curl provides a cleaner user interface
for now. But curl also supports a big variety of different protocols and
thus could be used to extend support to dump via e.g. http in the future.
Signed-off-by: Philipp Rudo <[email protected]>
When fetching the test status from the target a missing status file is
always considered a 'failure'. So there is no need to explicitly setting
the status to 'failure' in the initrd. This allows simplifying the code
a bit. For example we can now assume that the directory for $KDUMP_PATH
always exists (otherwise dump_{fs,ssh} would have returned with an
error).
Signed-off-by: Philipp Rudo <[email protected]>
prudo1
force-pushed
the
features/curl/main
branch
from
77987c3 to
e6e421a
Compare
|
Rebased to current main as PR #53 was merged. Updated the description accordingly. No code changes made. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi everybody,
This PR started out as a cleanup for ssh dumps by using
curlbut picked up a few other cleanups to support thekdumpctl testchanges made by @liutgnu in PR #53 on its way. The rational behind usingcurlis that it has a much cleaner UI compared to the home grown mix ofsshandscp. In addition it also supports a wide range of other protocols which could be leveraged to extend support to dump via e.g. http in the future.I can imagine that the switch to
curlcan be controversial. So I'm posting this as a RFC for now.