[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