[Libguestfs] [PATCH v3 01/11] daemon: btrfs: add helper functions mount and umount

[Richard W.M. Jones]

On Fri, Dec 12, 2014 at 03:03:23PM +0800, Hu Tao wrote:
> Signed-off-by: Hu Tao <hutao at cn.fujitsu.com>
> ---
>  daemon/btrfs.c | 106 +++++++++++++++++++++++++++++++--------------------------
>  1 file changed, 58 insertions(+), 48 deletions(-)
> 
> diff --git a/daemon/btrfs.c b/daemon/btrfs.c
> index 754fdcd..514ba37 100644
> --- a/daemon/btrfs.c
> +++ b/daemon/btrfs.c
> +    if (mount_vfs_nochroot ("", NULL, fs, fs_buf, "<internal>") == -1) {
> +      if (rmdir (fs_buf) == -1 && errno != ENOENT)
> +        reply_with_error ("rmdir: %m\n");

This error path calls reply_with_error twice (the first time in
mount_vfs_nochroot).  That's a problem because it would cause protocol
desychronization.

[...]
> +    if (command (NULL, &err, str_umount, fs_buf, NULL) == -1) {
> +      reply_with_error ("%s", err ? err : "malloc");
> +      return -1;
> +    }

'err' is always non-NULL here.

There's no need to send an updated patch, as I'm just going to change
this to an fprintf statement before I push it (very soon).

Thanks,

Rich.

> +    if (rmdir (fs_buf) == -1 && errno != ENOENT) {
> +      reply_with_error ("rmdir: %m\n");
> +      return -1;
> +    }
> +  }
> +  free (fs_buf);
> +  return 0;
> +}
> +
>  guestfs_int_btrfssubvolume_list *
>  do_btrfs_subvolume_list (const mountable_t *fs)
>  {
> @@ -336,42 +388,10 @@ do_btrfs_subvolume_list (const mountable_t *fs)
>  
>    /* Execute 'btrfs subvolume list <fs>', and split the output into lines */
>    {
> -    CLEANUP_FREE char *fs_buf = NULL;
> -
> -    if (fs->type == MOUNTABLE_PATH) {
> -      fs_buf = sysroot_path (fs->device);
> -      if (fs_buf == NULL) {
> -        reply_with_perror ("malloc");
> -
> -      cmderror:
> -        if (fs->type != MOUNTABLE_PATH && fs_buf) {
> -          CLEANUP_FREE char *err = NULL;
> -          if (command (NULL, &err, str_umount, fs_buf, NULL) == -1)
> -            fprintf (stderr, "%s\n", err);
> -
> -          if (rmdir (fs_buf) == -1 && errno != ENOENT)
> -            fprintf (stderr, "rmdir: %m\n");
> -        }
> -        return NULL;
> -      }
> -    }
> -
> -    else {
> -      fs_buf = strdup ("/tmp/btrfs.XXXXXX");
> -      if (fs_buf == NULL) {
> -        reply_with_perror ("strdup");
> -        goto cmderror;
> -      }
> +    char *fs_buf = mount (fs);
>  
> -      if (mkdtemp (fs_buf) == NULL) {
> -        reply_with_perror ("mkdtemp");
> -        goto cmderror;
> -      }
> -
> -      if (mount_vfs_nochroot ("", NULL, fs, fs_buf, "<internal>") == -1) {
> -        goto cmderror;
> -      }
> -    }
> +    if (!fs_buf)
> +      return NULL;
>  
>      ADD_ARG (argv, i, str_btrfs);
>      ADD_ARG (argv, i, "subvolume");
> @@ -382,18 +402,8 @@ do_btrfs_subvolume_list (const mountable_t *fs)
>      CLEANUP_FREE char *out = NULL, *errout = NULL;
>      int r = commandv (&out, &errout, argv);
>  
> -    if (fs->type != MOUNTABLE_PATH) {
> -      CLEANUP_FREE char *err = NULL;
> -      if (command (NULL, &err, str_umount, fs_buf, NULL) == -1) {
> -        reply_with_error ("%s", err ? err : "malloc");
> -        goto cmderror;
> -      }
> -
> -      if (rmdir (fs_buf) == -1 && errno != ENOENT) {
> -        reply_with_error ("rmdir: %m\n");
> -        goto cmderror;
> -      }
> -    }
> +    if (umount (fs_buf, fs) != 0)
> +      return NULL;
>  
>      if (r == -1) {
>        CLEANUP_FREE char *fs_desc = mountable_to_string (fs);
> @@ -401,7 +411,7 @@ do_btrfs_subvolume_list (const mountable_t *fs)
>          fprintf (stderr, "malloc: %m");
>        }
>        reply_with_error ("%s: %s", fs_desc ? fs_desc : "malloc", errout);
> -      goto cmderror;
> +      return NULL;
>      }
>  
>      lines = split_lines (out);
> -- 
> 1.9.3
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs at redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW