[Libguestfs] [PATCH v2] New API: btrfs_replace

[Cao jin]

Hi, pino

在 2015年06月16日 21:58, Pino Toscano 写道:
> On Monday 15 June 2015 15:52:31 Cao jin wrote:
>> Signed-off-by: Cao jin <caoj.fnst at cn.fujitsu.com>
>> ---
>>   daemon/btrfs.c                    | 37 ++++++++++++++++++
>>   generator/actions.ml              | 16 ++++++++
>>   tests/btrfs/Makefile.am           |  3 +-
>>   tests/btrfs/test-btrfs-replace.sh | 82 +++++++++++++++++++++++++++++++++++++++
>>   4 files changed, 137 insertions(+), 1 deletion(-)
>>   create mode 100755 tests/btrfs/test-btrfs-replace.sh
>>
>> diff --git a/daemon/btrfs.c b/daemon/btrfs.c
>> index 39392f7..eba336b 100644
>> --- a/daemon/btrfs.c
>> +++ b/daemon/btrfs.c
>> @@ -2083,3 +2083,40 @@ do_btrfs_image (char *const *sources, const char *image,
>>
>>     return 0;
>>   }
>> +
>> +int
>> +do_btrfs_replace (const char *srcdev, const char *targetdev,
>> +                                const char* mntpoint)
>> +{
>> +  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");
>> +  ADD_ARG (argv, i, "-f");
>> +  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 7252295..ffc2bbf 100644
>> --- a/generator/actions.ml
>> +++ b/generator/actions.ml
>> @@ -12593,6 +12593,22 @@ numbered C<partnum> on device C<device>.
>>
>>   It returns C<primary>, C<logical>, or C<extended>." };
>>
>> +  { defaults with
>> +    name = "btrfs_replace"; added = (1, 29, 46);
>
> The next version now will be 1.29.47, but I guess this value will be
> changed when the patch is applied for push.
>

OK, will fix it in next update.

>> +    style = RErr, [Device "srcdev"; Device "targetdev"; Pathname "mntpoint"], [];
>> +    proc_nr = Some 455;
>> +    optional = Some "btrfs"; camel_name = "BTRFSReplace";
>> +    test_excuse = "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>." };
>
> Use C<> for parameters, as POD markup.
>

OK

>> diff --git a/tests/btrfs/Makefile.am b/tests/btrfs/Makefile.am
>> index bf4d7ae..b6ef794 100644
>> --- a/tests/btrfs/Makefile.am
>> +++ b/tests/btrfs/Makefile.am
>> @@ -20,7 +20,8 @@ include $(top_srcdir)/subdir-rules.mk
>>   TESTS = \
>>   	test-btrfs-misc.pl \
>>   	test-btrfs-devices.sh \
>> -	test-btrfs-subvolume-default.pl
>> +	test-btrfs-subvolume-default.pl \
>> +	test-btrfs-replace.sh
>>
>>   TESTS_ENVIRONMENT = $(top_builddir)/run --test
>>
>> diff --git a/tests/btrfs/test-btrfs-replace.sh b/tests/btrfs/test-btrfs-replace.sh
>> new file mode 100755
>> index 0000000..c095e0b
>> --- /dev/null
>> +++ b/tests/btrfs/test-btrfs-replace.sh
>> @@ -0,0 +1,82 @@
>> +#!/bin/bash -
>> +# libguestfs
>> +# Copyright (C) 2015 Fujitsu 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.
>> +
>> +# Test btrfs replace devices.
>> +
>> +set -e
>> +
>> +# Allow the test to be skipped since btrfs is often broken.
>> +if [ -n "$SKIP_TEST_BTRFS_REPLACE_SH" ]; then
>> +    echo "$0: skipping test because environment variable is set."
>> +    exit 77
>> +fi
>> +
>> +# If btrfs is not available, bail.
>> +if ! guestfish -a /dev/null run : available btrfs; then
>> +    echo "$0: skipping test because btrfs is not available"
>> +    exit 77
>> +fi
>> +
>> +rm -f test-btrfs-devices-{1,2}.img replace.output
>> +
>> +guestfish  <<EOF > replace.output
>> +# Add 2 empty disks
>> +sparse test-btrfs-devices-1.img 1G
>> +sparse test-btrfs-devices-2.img 1G
>> +run
>> +
>> +mkfs-btrfs /dev/sda
>> +mount /dev/sda /
>> +
>> +mkdir /data
>> +copy-in $srcdir/../data/filesanddirs-10M.tar.xz /data
>> +
>> +# now, sda is btrfs while sdb is blank.
>> +btrfs-replace /dev/sda /dev/sdb /
>> +
>> +# after replace: sda is wiped out, while sdb has btrfs with data
>> +list-filesystems
>
> Should at this point the content of sdb be checked, to see whether
> the data previously in sda is now there?

the behaviour of replace is: 1. wipe out src first. 2. and then move all 
the data to target.
So I think, after checked 1, it should be enough for test purpose. But 
yes, we can do the check fully, make it more accurately

>> +EOF
>> +
>> +if [ "$(cat replace.output)" != "/dev/sda: unknown
>> +/dev/sdb: btrfs" ]; then
>> +    echo "btrfs-repalce fail!"
>> +    cat replace.output
>> +    exit 1
>> +fi
>> +
>> +rm -f replace.output
>> +
>> +#now, we do switch, replace the device back.
>> +guestfish -a test-btrfs-devices-1.img -a test-btrfs-devices-2.img <<EOF > replace.output
>> +run
>> +mount /dev/sdb /
>> +btrfs-replace /dev/sdb /dev/sda /
>> +list-filesystems
>
> Ditto.
>
> TBH, I'm still not sure why there's this test to replace the devices
> back, since the guestfish above should check already that replace
> works. Is the replace back operation somehow special/critical?
>

the purpose of replace back is: double security, based on the principle 
"btrfs replace cmd may not be perfect, fully test better", nothing 
special. If checked the content after 1st replace, the 2nd replace back 
could be omitted. What`s your opinion?

> Thanks,
>

-- 
Yours Sincerely,

Cao Jin