[Libguestfs] [PATCH 5/6] New APIs: copy-in and copy-out

Pino Toscano ptoscano at redhat.com
Mon Feb 2 14:42:42 UTC 2015


On Monday 02 February 2015 13:40:44 Richard W.M. Jones wrote:
> On Mon, Jan 26, 2015 at 05:04:10PM +0100, Pino Toscano wrote:
> > Currently implemented as guestfish commands, provide them instead as
> > single source -> destination functions for the library, so they can be
> > used also in other places.
> > 
> > These functions are not added to guestfish, since guestfish has its own
> > implementation (which will soon switch to call copy-in and copy-out for
> > multiple paths).
> > ---
> >  generator/actions.ml |  28 ++++++
> >  po/POTFILES          |   1 +
> >  src/Makefile.am      |   1 +
> >  src/copy-in-out.c    | 244 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 274 insertions(+)
> >  create mode 100644 src/copy-in-out.c
> > 
> > diff --git a/generator/actions.ml b/generator/actions.ml
> > index 25f4bb5..ab97891 100644
> > --- a/generator/actions.ml
> > +++ b/generator/actions.ml
> > @@ -3310,6 +3310,34 @@ In non-C language bindings, this allows you to retrieve the underlying
> >  C pointer to the handle (ie. C<guestfs_h *>).  The purpose of this is
> >  to allow other libraries to interwork with libguestfs." };
> >  
> > +  { defaults with
> > +    name = "copy_in";
> > +    style = RErr, [String "localpath"; Pathname "remotedir"], [];
> > +    visibility = VPublicNoFish;
> > +    shortdesc = "copy local files or directories into an image";
> > +    longdesc = "\
> > +C<guestfs_copy_in> copies local files or directories recursively into
> > +the disk image, placing them in the directory called C</remotedir>
>                                                           ^
> You don't need the / here.

Fixed.

> > +(which must exist).
> > +
> > +Wildcards cannot be used." };
> > +
> > +  { defaults with
> > +    name = "copy_out";
> > +    style = RErr, [Pathname "remotepath"; String "localdir"], [];
> > +    visibility = VPublicNoFish;
> > +    shortdesc = "copy remote files or directories out of an image";
> > +    longdesc = "\
> > +C<guestfs_copy_out> copies remote files or directories recursively
> > +out of the disk image, placing them on the host disk in a local
> > +directory called C<localdir> (which must exist).
> > +
> > +To download to the current directory, use C<.> as in:
> > +
> > + C<guestfs_copy_out> /home .
> > +
> > +Wildcards cannot be used." };
> > +
> >  ]
> >  
> >  (* daemon_functions are any functions which cause some action
> > diff --git a/po/POTFILES b/po/POTFILES
> > index 4194e5f..6e65377 100644
> > --- a/po/POTFILES
> > +++ b/po/POTFILES
> > @@ -297,6 +297,7 @@ src/canonical-name.c
> >  src/cleanup.c
> >  src/command.c
> >  src/conn-socket.c
> > +src/copy-in-out.c
> >  src/create.c
> >  src/dbdump.c
> >  src/drives.c
> > diff --git a/src/Makefile.am b/src/Makefile.am
> > index a83f257..2496887 100644
> > --- a/src/Makefile.am
> > +++ b/src/Makefile.am
> > @@ -90,6 +90,7 @@ libguestfs_la_SOURCES = \
> >  	canonical-name.c \
> >  	command.c \
> >  	conn-socket.c \
> > +	copy-in-out.c \
> >  	create.c \
> >  	dbdump.c \
> >  	drives.c \
> > diff --git a/src/copy-in-out.c b/src/copy-in-out.c
> > new file mode 100644
> > index 0000000..8e2edd3
> > --- /dev/null
> > +++ b/src/copy-in-out.c
> > @@ -0,0 +1,244 @@
> > +/* libguestfs
> > + * Copyright (C) 2015 Red Hat Inc.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> > + */
> > +
> > +#include <config.h>
> > +
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <stdint.h>
> > +#include <inttypes.h>
> > +#include <unistd.h>
> > +#include <sys/types.h>
> > +#include <sys/stat.h>
> > +#include <sys/wait.h>
> > +#include <errno.h>
> > +#include <string.h>
> > +
> > +#include "guestfs.h"
> > +#include "guestfs-internal.h"
> > +#include "guestfs-internal-actions.h"
> > +
> > +static int split_path (char *buf, size_t buf_size, const char *path, const char **dirname, const char **basename);
> > +
> > +int
> > +guestfs__copy_in (guestfs_h *g, const char *localpath, const char *remotedir)
> > +{
> > +  CLEANUP_CMD_CLOSE struct command *cmd = guestfs___new_command (g);
> > +  int fd;
> > +  int r;
> > +  char fdbuf[64];
> > +  size_t buf_len = strlen (localpath) + 1;
> > +  char buf[buf_len];
> > +  const char *dirname, *basename;
> > +
> > +  if (guestfs_is_dir (g, remotedir) == -1)
> > +    return -1;
> 
> I suppose this is here to check that remotedir is a directory, but
> that's not what the code here does.  The original guestfish code is:
> 
>   int remote_is_dir = guestfs_is_dir (g, remote);
>   if (remote_is_dir == -1)
>     return -1;
> 
>   if (!remote_is_dir) {
>     fprintf (stderr, _("copy-in: target '%s' is not a directory\n"), remote);
>     return -1;
>   }

You're right, I overly simplified it, thanks.
Taken back, using error() instead.

> > +  if (split_path (buf, buf_len, localpath, &dirname, &basename) == -1)
> > +    return -1;
> > +
> > +  guestfs___cmd_add_arg (cmd, "tar");
> > +  if (dirname) {
> > +    guestfs___cmd_add_arg (cmd, "-C");
> > +    guestfs___cmd_add_arg (cmd, dirname);
> > +  }
> > +  guestfs___cmd_add_arg (cmd, "-cf");
> > +  guestfs___cmd_add_arg (cmd, "-");
> > +  guestfs___cmd_add_arg (cmd, basename);
> > +
> > +  r = guestfs___cmd_run_async (cmd, NULL, NULL, &fd, NULL);
> > +  if (r == -1)
> > +    return -1;
> > +
> > +  snprintf (fdbuf, sizeof fdbuf, "/dev/fd/%d", fd);
> > +
> > +  r = guestfs_tar_in (g, fdbuf, remotedir);
> > +
> > +  if (close (fd) == -1) {
> > +    perror ("close (tar subprocess)");
> > +    return -1;
> > +  }
> > +
> > +  r = guestfs___cmd_wait (cmd);
> > +  if (r == -1)
> > +    return -1;
> > +  if (!(WIFEXITED (r) && WEXITSTATUS (r) == 0))
> > +    return -1;
> > +
> > +  return 0;
> > +}
> > +
> > +struct copy_out_child_data {
> > +  const char *localdir;
> > +  const char *basename;
> > +};
> > +
> > +static int
> > +child_setup (guestfs_h *g, void *data)
> > +{
> > +  struct copy_out_child_data d = *(struct copy_out_child_data *) data;
> > +
> > +  if (chdir (d.localdir) == -1) {
> > +    perror (d.localdir);
> > +    return -1;
> > +  }
> > +
> > +  if (mkdir (d.basename, 0777) == -1 && errno != EEXIST) {
> > +    perror (d.basename);
> > +    return -1;
> > +  }
> > +
> > +  if (chdir (d.basename) == -1) {
> > +    perror (d.basename);
> > +    return -1;
> > +  }
> > +
> > +  return 0;
> > +}
> > +
> > +int
> > +guestfs__copy_out (guestfs_h *g, const char *remotepath, const char *localdir)
> > +{
> > +  struct stat statbuf;
> > +  int r;
> > +
> > +  if (stat (localdir, &statbuf) == -1 ||
> > +      ! (S_ISDIR (statbuf.st_mode))) {
> > +    error (g, _("target '%s' is not a directory"), localdir);
> > +    return -1;
> > +  }
> > +
> > +  /* If the remote is a file, download it.  If it's a directory,
> > +   * create the directory in localdir first before using tar-out.
> > +   */
> > +  r = guestfs_is_file (g, remotepath);
> > +  if (r == -1)
> > +    return -1;
> > +
> > +  if (r == 1) {               /* is file */
> > +    CLEANUP_FREE char *filename = NULL;
> > +    size_t buf_len = strlen (remotepath) + 1;
> > +    char buf[buf_len];
> > +    const char *basename;
> > +
> > +    if (split_path (buf, buf_len, remotepath, NULL, &basename) == -1)
> > +      return -1;
> > +
> > +    if (asprintf (&filename, "%s/%s", localdir, basename) == -1) {
> > +      perror ("asprintf");
> > +      return -1;
> > +    }
> > +    if (guestfs_download (g, remotepath, filename) == -1)
> > +      return -1;
> > +  } else {                    /* not a regular file */
> > +    CLEANUP_CMD_CLOSE struct command *cmd = guestfs___new_command (g);
> > +    struct copy_out_child_data data;
> > +    char fdbuf[64];
> > +    int fd;
> > +
> > +    r = guestfs_is_dir (g, remotepath);
> > +    if (r == -1)
> > +      return -1;
> > +
> > +    if (r == 0) {
> > +      error (g, _("'%s' is not a file or directory"), remotepath);
> > +      return -1;
> > +    }
> > +
> > +    size_t buf_len = strlen (remotepath) + 1;
> > +    char buf[buf_len];
> > +    const char *basename;
> > +    if (split_path (buf, buf_len, remotepath, NULL, &basename) == -1)
> > +      return -1;
> > +
> > +    /* RHBZ#845522: If remotepath == "/" then basename would be an empty
> > +     * string.  Replace it with "." so that make_tar_output writes
> > +     * to "localdir/."
> > +     */
> > +    if (STREQ (basename, ""))
> > +      basename = ".";
> > +
> > +    data.localdir = localdir;
> > +    data.basename = basename;
> > +
> > +    guestfs___cmd_set_child_callback (cmd, &child_setup, &data);
> > +
> > +    guestfs___cmd_add_arg (cmd, "tar");
> > +    guestfs___cmd_add_arg (cmd, "-xf");
> > +    guestfs___cmd_add_arg (cmd, "-");
> > +
> > +    r = guestfs___cmd_run_async (cmd, NULL, &fd, NULL, NULL);
> > +    if (r == -1)
> > +      return -1;
> > +
> > +    snprintf (fdbuf, sizeof fdbuf, "/dev/fd/%d", fd);
> > +
> > +    r = guestfs_tar_out (g, remotepath, fdbuf);
> > +
> > +    if (close (fd) == -1) {
> > +      perror ("close (tar-output subprocess)");
> > +      return -1;
> > +    }
> > +
> > +    r = guestfs___cmd_wait (cmd);
> > +    if (r == -1)
> > +      return -1;
> > +    if (!(WIFEXITED (r) && WEXITSTATUS (r) == 0))
> > +      return -1;
> > +  }
> > +
> > +  return 0;
> > +}
> > +
> > +/* Split path into directory name and base name, using the buffer
> > + * provided as a working area.  If there is no directory name
> > + * (eg. path == "/") then this can return dirname as NULL.
> > + */
> > +static int
> > +split_path (char *buf, size_t buf_size,
> > +            const char *path, const char **dirname, const char **basename)
> > +{
> > +  size_t len = strlen (path);
> > +  if (len == 0 || len > buf_size - 1) {
> > +    fprintf (stderr, _("error: argument is zero length or longer than maximum permitted\n"));
> > +    return -1;
> > +  }
> > +
> > +  strcpy (buf, path);
> > +
> > +  if (len >= 2 && buf[len-1] == '/') {
> > +    buf[len-1] = '\0';
> > +    len--;
> > +  }
> > +
> > +  char *p = strrchr (buf, '/');
> > +  if (p && p > buf) {           /* "foo/bar" */
> > +    *p = '\0';
> > +    p++;
> > +    if (dirname) *dirname = buf;
> > +    if (basename) *basename = p;
> > +  } else if (p && p == buf) {   /* "/foo" */
> > +    if (dirname) *dirname = "/";
> > +    if (basename) *basename = buf+1;
> > +  } else {
> > +    if (dirname) *dirname = NULL;
> > +    if (basename) *basename = buf;
> > +  }
> > +
> > +  return 0;
> > +}
> 
> The guestfish code (which took *ages* to get right) is a lot more
> complicated ...

Some of the guestfish code is about the handling of the tar child
processes, either by directing copy_out's output to tar, or piping
tar's output to copy_in. This is now replaced by using the internal
commands API, which has been improved to handle the use cases of the
previous copy_in & copy_out implementations.

The rest of the code has either been used directly, or slightly
adapted to use commands and other internal API. I should not have
missed anything else, I think.

-- 
Pino Toscano




More information about the Libguestfs mailing list