[Libguestfs] [PATCH nbdkit v3] ssh: Allow the remote file to be created

Laszlo Ersek lersek at redhat.com
Tue Apr 19 10:04:56 UTC 2022


On 04/19/22 11:14, Richard W.M. Jones wrote:
> This adds new parameters, create=(true|false), create-size=SIZE and
> create-mode=MODE to create and truncate the remote file.
> ---
>  plugins/ssh/nbdkit-ssh-plugin.pod |  34 ++++++++-
>  plugins/ssh/ssh.c                 | 112 +++++++++++++++++++++++++++---
>  tests/test-ssh.sh                 |  13 +++-
>  3 files changed, 146 insertions(+), 13 deletions(-)
> 
> diff --git a/plugins/ssh/nbdkit-ssh-plugin.pod b/plugins/ssh/nbdkit-ssh-plugin.pod
> index 3f401c15..2bc2c4a7 100644
> --- a/plugins/ssh/nbdkit-ssh-plugin.pod
> +++ b/plugins/ssh/nbdkit-ssh-plugin.pod
> @@ -5,8 +5,10 @@ nbdkit-ssh-plugin - access disk images over the SSH protocol
>  =head1 SYNOPSIS
>  
>   nbdkit ssh host=HOST [path=]PATH
> -            [compression=true] [config=CONFIG_FILE] [identity=FILENAME]
> -            [known-hosts=FILENAME] [password=PASSWORD|-|+FILENAME]
> +            [compression=true] [config=CONFIG_FILE]
> +            [create=true] [create-mode=MODE] [create-size=SIZE]
> +            [identity=FILENAME] [known-hosts=FILENAME]
> +            [password=PASSWORD|-|+FILENAME]
>              [port=PORT] [timeout=SECS] [user=USER]
>              [verify-remote-host=false]
>  
> @@ -62,6 +64,34 @@ The C<config> parameter is optional.  If it is I<not> specified at all
>  then F<~/.ssh/config> and F</etc/ssh/ssh_config> are both read.
>  Missing or unreadable files are ignored.
>  
> +=item B<create=true>
> +
> +(nbdkit E<ge> 1.32)
> +
> +If set, the remote file will be created.  The remote file is created
> +on the first NBD connection to nbdkit, not when nbdkit starts up.  If
> +the file already exists, it will be replaced and any existing content
> +lost.
> +
> +If using this option, you must use C<create-size>.  C<create-mode> can
> +be used to control the permissions of the new file.
> +
> +=item B<create-mode=>MODE
> +
> +(nbdkit E<ge> 1.32)
> +
> +If using C<create=true> specify the default permissions of the new
> +remote file.  You can use octal modes like C<create-mode=0777> or
> +C<create-mode=0644>.  The default is C<0600>, ie. only readable and
> +writable by the remote user.
> +
> +=item B<create-size=>SIZE
> +
> +(nbdkit E<ge> 1.32)
> +
> +If using C<create=true>, specify the virtual size of the new disk.
> +C<SIZE> can use modifiers like C<100M> etc.
> +
>  =item B<host=>HOST
>  
>  Specify the name or IP address of the remote host.
> diff --git a/plugins/ssh/ssh.c b/plugins/ssh/ssh.c
> index 77cfcf6c..ac9cc230 100644
> --- a/plugins/ssh/ssh.c
> +++ b/plugins/ssh/ssh.c
> @@ -44,12 +44,15 @@
>  #include <fcntl.h>
>  #include <sys/stat.h>
>  
> +#include <pthread.h>
> +
>  #include <libssh/libssh.h>
>  #include <libssh/sftp.h>
>  #include <libssh/callbacks.h>
>  
>  #include <nbdkit-plugin.h>
>  
> +#include "cleanup.h"
>  #include "const-string-vector.h"
>  #include "minmax.h"
>  
> @@ -63,6 +66,9 @@ static const char *known_hosts = NULL;
>  static const_string_vector identities = empty_vector;
>  static uint32_t timeout = 0;
>  static bool compression = false;
> +static bool create = false;
> +static int64_t create_size = -1;
> +static unsigned create_mode = S_IRUSR | S_IWUSR /* 0600 */;
>  
>  /* config can be:
>   * NULL => parse options from default file
> @@ -167,6 +173,27 @@ ssh_config (const char *key, const char *value)
>        return -1;
>      compression = r;
>    }
> +  else if (strcmp (key, "create") == 0) {
> +    r = nbdkit_parse_bool (value);
> +    if (r == -1)
> +      return -1;
> +    create = r;
> +  }
> +  else if (strcmp (key, "create-size") == 0) {
> +    create_size = nbdkit_parse_size (value);
> +    if (create_size == -1)
> +      return -1;
> +  }
> +  else if (strcmp (key, "create-mode") == 0) {
> +    r = nbdkit_parse_unsigned (key, value, &create_mode);
> +    if (r == -1)
> +      return -1;
> +    /* OpenSSH checks this too. */
> +    if (create_mode > 0777) {
> +      nbdkit_error ("create-mode must be <= 0777");
> +      return -1;
> +    }
> +  }
>  
>    else {
>      nbdkit_error ("unknown parameter '%s'", key);
> @@ -186,6 +213,13 @@ ssh_config_complete (void)
>      return -1;
>    }
>  
> +  /* If create=true, create-size must be supplied. */
> +  if (create && create_size == -1) {
> +    nbdkit_error ("if using create=true, you must specify the size "
> +                  "of the new remote file using create-size=SIZE");
> +    return -1;
> +  }
> +
>    return 0;
>  }
>  
> @@ -200,7 +234,10 @@ ssh_config_complete (void)
>    "identity=<FILENAME>        Prepend private key (identity) file.\n" \
>    "timeout=SECS               Set SSH connection timeout.\n" \
>    "verify-remote-host=false   Ignore known_hosts.\n" \
> -  "compression=true           Enable compression."
> +  "compression=true           Enable compression.\n" \
> +  "create=true                Create the remote file.\n" \
> +  "create-mode=MODE           Set the permissions of the remote file.\n" \
> +  "create-size=SIZE           Set the size of the remote file."
>  
>  /* Since we must simulate atomic pread and pwrite using seek +
>   * read/write, calls on each handle must be serialized.
> @@ -329,6 +366,65 @@ authenticate (struct ssh_handle *h)
>    return -1;
>  }
>  
> +/* This function opens or creates the remote file (depending on
> + * create=false|true).  Parallel connections might call this function
> + * at the same time, and so we must hold a lock to ensure that the
> + * file is created at most once.
> + */
> +static pthread_mutex_t create_lock = PTHREAD_MUTEX_INITIALIZER;
> +
> +static sftp_file
> +open_or_create_path (ssh_session session, sftp_session sftp, int readonly)
> +{
> +  ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&create_lock);
> +  int access_type;
> +  int r;
> +  sftp_file file;
> +
> +  access_type = readonly ? O_RDONLY : O_RDWR;
> +  if (create) access_type |= O_CREAT | O_TRUNC;
> +
> +  file = sftp_open (sftp, path, access_type, S_IRWXU);
> +  if (!file) {
> +    nbdkit_error ("cannot %s file for %s: %s",
> +                  create ? "create" : "open",
> +                  readonly ? "reading" : "writing",
> +                  ssh_get_error (session));
> +    return NULL;
> +  }
> +
> +  if (create) {
> +    /* There's no sftp_truncate call.  However OpenSSH lets you call
> +     * SSH_FXP_SETSTAT + SSH_FILEXFER_ATTR_SIZE which invokes
> +     * truncate(2) on the server.  Libssh doesn't provide a binding
> +     * for SSH_FXP_FSETSTAT so we have to pass the session + path.
> +     */
> +    struct sftp_attributes_struct attrs = {
> +      .flags = SSH_FILEXFER_ATTR_SIZE |
> +               SSH_FILEXFER_ATTR_PERMISSIONS,
> +      .size = create_size,
> +      .permissions = create_mode,
> +    };
> +
> +    r = sftp_setstat (sftp, path, &attrs);
> +    if (r != SSH_OK) {
> +      nbdkit_error ("setstat failed: %s", ssh_get_error (session));
> +
> +      /* Best-effort attempt to delete the remote file on failure. */
> +      r = sftp_unlink (sftp, path);

There's still a very obscure path where the file exists originally, we
successfully truncate it in sftp_open() -- assuming O_TRUNC has an
effect there --, but then sftp_setstat() fails and we delete the file,
even though we did not create it.

But, I don't think this is too bad. After we truncated the file, its
original contents were lost anyway.

Reviewed-by: Laszlo Ersek <lersek at redhat.com>

Thanks!
Laszlo

> +      if (r != SSH_OK)
> +        nbdkit_debug ("unlink failed: %s", ssh_get_error (session));
> +
> +      return NULL;
> +    }
> +  }
> +
> +  /* On the next connection, don't create or truncate the file. */
> +  create = false;
> +
> +  return file;
> +}
> +
>  /* Create the per-connection handle. */
>  static void *
>  ssh_open (int readonly)
> @@ -337,7 +433,6 @@ ssh_open (int readonly)
>    const int set = 1;
>    size_t i;
>    int r;
> -  int access_type;
>  
>    h = calloc (1, sizeof *h);
>    if (h == NULL) {
> @@ -471,7 +566,7 @@ ssh_open (int readonly)
>    if (authenticate (h) == -1)
>      goto err;
>  
> -  /* Open the SFTP connection and file. */
> +  /* Open the SFTP connection. */
>    h->sftp = sftp_new (h->session);
>    if (!h->sftp) {
>      nbdkit_error ("failed to allocate sftp session: %s",
> @@ -484,14 +579,11 @@ ssh_open (int readonly)
>                    ssh_get_error (h->session));
>      goto err;
>    }
> -  access_type = readonly ? O_RDONLY : O_RDWR;
> -  h->file = sftp_open (h->sftp, path, access_type, S_IRWXU);
> -  if (!h->file) {
> -    nbdkit_error ("cannot open file for %s: %s",
> -                  readonly ? "reading" : "writing",
> -                  ssh_get_error (h->session));
> +
> +  /* Open or create the remote file. */
> +  h->file = open_or_create_path (h->session, h->sftp, readonly);
> +  if (!h->file)
>      goto err;
> -  }
>  
>    nbdkit_debug ("opened libssh handle");
>  
> diff --git a/tests/test-ssh.sh b/tests/test-ssh.sh
> index 6c0ce410..f04b4488 100755
> --- a/tests/test-ssh.sh
> +++ b/tests/test-ssh.sh
> @@ -36,6 +36,7 @@ set -x
>  
>  requires test -f disk
>  requires nbdcopy --version
> +requires stat --version
>  
>  # Check that ssh to localhost will work without any passwords or phrases.
>  #
> @@ -48,7 +49,7 @@ then
>      exit 77
>  fi
>  
> -files="ssh.img"
> +files="ssh.img ssh2.img"
>  rm -f $files
>  cleanup_fn rm -f $files
>  
> @@ -59,3 +60,13 @@ nbdkit -v -D ssh.log=2 -U - \
>  
>  # The output should be identical.
>  cmp disk ssh.img
> +
> +# Copy local file 'ssh.img' to newly created "remote" 'ssh2.img'
> +size="$(stat -c %s disk)"
> +nbdkit -v -D ssh.log=2 -U - \
> +       ssh host=localhost $PWD/ssh2.img \
> +       create=true create-size=$size \
> +       --run 'nbdcopy ssh.img "$uri"'
> +
> +# The output should be identical.
> +cmp disk ssh2.img
> 



More information about the Libguestfs mailing list