[Libguestfs] [PATCH] New API: btrfs_replace_start

[Pino Toscano]

Hi,

On Wednesday 10 June 2015 17:54:18 Pino Tsao wrote:
> Signed-off-by: Pino Tsao <caoj.fnst at cn.fujitsu.com>
> ---
>  daemon/btrfs.c                    | 40 +++++++++++++++++++++++++++++++++++++++
>  generator/actions.ml              | 19 +++++++++++++++++++
>  tests/btrfs/test-btrfs-devices.sh |  8 ++++++++
>  3 files changed, 67 insertions(+)
> 
> diff --git a/daemon/btrfs.c b/daemon/btrfs.c
> index 39392f7..acc300d 100644
> --- a/daemon/btrfs.c
> +++ b/daemon/btrfs.c
> @@ -2083,3 +2083,43 @@ do_btrfs_image (char *const *sources, const char *image,
>  
>    return 0;
>  }
> +
> +int
> +do_btrfs_replace_start (const char *srcdev, const char *targetdev,
> +                        const char* mntpoint, int force)
> +{
> +  const size_t MAX_ARGS = 64;
> +  const char *argv[MAX_ARGS];
> +  size_t i = 0;
> +  CLEANUP_FREE char *err = NULL;
> +  CLEANUP_FREE char *path_buf = NULL;
> +  int r;
> +
> +  path_buf = sysroot_path (mntpoint);
> +  if (path_buf == NULL) {
> +    reply_with_perror ("malloc");
> +    return -1;
> +  }
> +
> +  ADD_ARG (argv, i, str_btrfs);
> +  ADD_ARG (argv, i, "replace");
> +  ADD_ARG (argv, i, "start");
> +  ADD_ARG (argv, i, srcdev);
> +  ADD_ARG (argv, i, targetdev);
> +  ADD_ARG (argv, i, path_buf);
> +  ADD_ARG (argv, i, "-B");

I get that -B turns the operation from a background one to synchronous,
so why does this API have a _start suffix?

> +
> +  if ((optargs_bitmask & GUESTFS_BTRFS_REPLACE_START_FORCE_BITMASK) && force)
> +    ADD_ARG (argv, i, "-f");

Shouldn't -f be always passed, instead of having to choose it?

> +
> +  ADD_ARG (argv, i, NULL);
> +
> +  r = commandv (NULL, &err, argv);
> +  if (r == -1) {
> +    reply_with_error ("%s: %s", mntpoint, err);
> +    return -1;
> +  }
> +
> +  return 0;
> +}
> +
> diff --git a/generator/actions.ml b/generator/actions.ml
> index 1a89869..4443600 100644
> --- a/generator/actions.ml
> +++ b/generator/actions.ml
> @@ -12579,6 +12579,25 @@ numbered C<partnum> on device C<device>.
>  
>  It returns C<primary>, C<logical>, or C<extended>." };
>  
> +  { defaults with
> +    name = "btrfs_replace_start"; added = (1, 29, 46);
> +    style = RErr, [Device "srcdev"; Device "targetdev"; Pathname "mntpoint"], [OBool "force"];
> +    proc_nr = Some 455;
> +    optional = Some "btrfs"; camel_name = "BTRFSReplaceStart";
> +    test_excuse = "It is better to have 3+ test disk to do the test, so put the test in 'tests/btrfs' directory";
> +    shortdesc = "replace a btrfs managed device with another device";
> +    longdesc = "\
> +Replace device of a btrfs filesystem. On a live filesystem, duplicate the data
> +to the target device which is currently stored on the source device.
> +After completion of the operation, the source device is wiped out and
> +removed from the filesystem.
> +
> +The <targetdev> needs to be same size or larger than the <srcdev>. Devices
> +which are currently mounted are never allowed to be used as the <targetdev>
> +
> +Option 'force=true' means using and overwriting <targetdev> even if
> +it looks like containing a valid btrfs filesystem." };
> +
>  ]
>  
>  (* Non-API meta-commands available only in guestfish.
> diff --git a/tests/btrfs/test-btrfs-devices.sh b/tests/btrfs/test-btrfs-devices.sh
> index 3935c60..463b0a8 100755
> --- a/tests/btrfs/test-btrfs-devices.sh
> +++ b/tests/btrfs/test-btrfs-devices.sh
> @@ -66,6 +66,8 @@ btrfs-device-add "/dev/sdc1 /dev/sdd1" /
>  btrfs-device-delete "/dev/sdb1" /
>  btrfs-device-add "/dev/sdb1" /
>  btrfs-device-delete "/dev/sdc1 /dev/sdd1" /
> +btrfs-replace-start "/dev/sda1" "/dev/sdd1" / force:true
> +btrfs-replace-start "/dev/sdd1" "/dev/sda1" / force:true
>  
>  mkdir /data2
>  tar-in $srcdir/../data/filesanddirs-10M.tar.xz /data2 compress:xz
> @@ -74,6 +76,8 @@ btrfs-device-add "/dev/sdc1 /dev/sdd1" /
>  btrfs-device-delete "/dev/sdb1" /
>  btrfs-device-add "/dev/sdb1" /
>  btrfs-device-delete "/dev/sdc1 /dev/sdd1" /
> +btrfs-replace-start "/dev/sda1" "/dev/sdd1" / force:true
> +btrfs-replace-start "/dev/sdd1" "/dev/sda1" / force:true
>  
>  mkdir /data3
>  tar-in $srcdir/../data/filesanddirs-10M.tar.xz /data3 compress:xz
> @@ -82,6 +86,8 @@ btrfs-device-add "/dev/sdc1 /dev/sdd1" /
>  btrfs-device-delete "/dev/sdb1" /
>  btrfs-device-add "/dev/sdb1" /
>  btrfs-device-delete "/dev/sdc1 /dev/sdd1" /
> +btrfs-replace-start "/dev/sda1" "/dev/sdd1" / force:true
> +btrfs-replace-start "/dev/sdd1" "/dev/sda1" / force:true
>  
>  mkdir /data4
>  tar-in $srcdir/../data/filesanddirs-10M.tar.xz /data4 compress:xz
> @@ -90,6 +96,8 @@ btrfs-device-add "/dev/sdc1 /dev/sdd1" /
>  btrfs-device-delete "/dev/sdb1" /
>  btrfs-device-add "/dev/sdb1" /
>  btrfs-device-delete "/dev/sdc1 /dev/sdd1" /
> +btrfs-replace-start "/dev/sda1" "/dev/sdd1" / force:true
> +btrfs-replace-start "/dev/sdd1" "/dev/sda1" / force:true

What are these tests supposed to check? Other than calling
btrfs-replace-start and checking it does not fail, how can the result
of this operation be actually checked?

Thanks,
-- 
Pino Toscano