Re: [Libguestfs] [PATCH v2] New API: btrfs_replace

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(a)cn.fujitsu.com&gt; >>> --- >>> 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, >> >