[Libguestfs] [PATCH v2] New API: btrfs_replace

[Cao jin]

ping

在 2015年06月17日 15:31, 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