[Libguestfs] [PATCH v2] New API: btrfs_replace

Cao jin caoj.fnst at cn.fujitsu.com
Tue Jun 23 11:56:13 UTC 2015


Hi,

Ping 2.
please see my last comment:)

在 2015年06月18日 20:02, 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




More information about the Libguestfs mailing list