Hi Toscano,
    please see my comment inline in the last section:)

在 2015年06月05日 00:37, Pino Toscano 写道:
Hi,

In data giovedì 4 giugno 2015 11:56:41, Pino Tsao ha scritto:
     Disable the test case temporarily for 2 reasons:
     1. Because the default test disk size is 500M, while btrfs
     convert command think it is too small to convert it(actually,
     just add 10M or 20M more is enough).

If the base test disks that are available in tests/c-api/tests are
not enough, just write the test (creating handle, scratch disks,
partitions, etc) in shell or perl, just like the other ones available
in tests/btrfs.

This kind of API needs at least some basic coverage.

     2. Btrfs-progs has may have a tiny bug, when execute the command
     in guestfish, it report some error, but convert the filesystem to
     btrfs successfully.

I don't understand, are you saying that `btrfs convert` will exit with
failure but doing the actual changes? If so, is that a known/reported
upstream bug?


Signed-off-by: Pino Tsao <[email protected]>
---
  daemon/btrfs.c       | 29 +++++++++++++++++++++++++++++
  generator/actions.ml | 18 ++++++++++++++++++
  2 files changed, 47 insertions(+)

diff --git a/daemon/btrfs.c b/daemon/btrfs.c
index 39392f7..fd679cf 100644
--- a/daemon/btrfs.c
+++ b/daemon/btrfs.c
@@ -38,6 +38,7 @@ GUESTFSD_EXT_CMD(str_btrfsck, btrfsck);
  GUESTFSD_EXT_CMD(str_mkfs_btrfs, mkfs.btrfs);
  GUESTFSD_EXT_CMD(str_umount, umount);
  GUESTFSD_EXT_CMD(str_btrfsimage, btrfs-image);
+GUESTFSD_EXT_CMD(str_btrfsconvert, btrfs-convert);

  int
  optgroup_btrfs_available (void)
@@ -2083,3 +2084,31 @@ do_btrfs_image (char *const *sources, const char *image,

    return 0;
  }
+
+int
+do_btrfs_convert (const char *device, int rollback)
+{
+  const size_t MAX_ARGS = 64;
+  const char *argv[MAX_ARGS];
+  size_t i = 0;
+  CLEANUP_FREE char *err = NULL;
+  CLEANUP_FREE char *out = NULL;
+  int r;
+
+  ADD_ARG (argv, i, str_btrfsconvert);
+  ADD_ARG (argv, i, device);
+
+  if ((optargs_bitmask & GUESTFS_BTRFS_CONVERT_ROLLBACK_BITMASK) && rollback)
+    ADD_ARG (argv, i, "-r");
+
+  ADD_ARG (argv, i, NULL);
+
+  r = commandv (&out, &err, argv);
+  if (r == -1) {
+    reply_with_error ("%s: %s", device, err);
+    return -1;
+  }
+
+  return 0;
+}

"out" seems not used, so no need to collect it from commandv.

diff --git a/generator/actions.ml b/generator/actions.ml
index 1a89869..e42f02d 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -12579,6 +12579,24 @@ numbered C<partnum> on device C<device>.

  It returns C<primary>, C<logical>, or C<extended>." };

+  { defaults with
+    name = "btrfs_convert";
+    style = RErr, [Device "device"], [OBool "rollback"];
+    proc_nr = Some 455;
+    optional = Some "btrfs"; camel_name = "BTRFSConvert";
+    tests = [
+      InitEmpty, Disabled, TestRun (
+         [["mkfs"; "ext2"; "/dev/sda"; ""; "NOARG"; ""; ""; "NOARG"];
+         ["btrfs_convert"; "/dev/sda"; ""];
+         ["btrfs_convert"; "/dev/sda"; "true"]]), []
+    ];

          ^ extra white spaces at the end

+    shortdesc = "convert from ext2/3/4 filesystem to btrfs or rollback";
+    longdesc = "\
+This is used to convert existed ext2/3/4 to btrfs filesystem, and the original
+filesystem image is accessible as from separate subvolume named ext2_saved as 
file image.

While I understand you copied this description from the btrfs-convert(1)
man page, please at least make sure it is proper grammar:

   This is used to convert an existing ext2/3/4 filesystem to btrfs;
   a copy of the original filesystem image is still available in a
   separate subvolume named "ext2_saved".

Is it still correct?

I think I misunderstood you just now. Yes, it is still right. I think your correction is better and more clearly, the original description is a little complicated for guys who are not native speaker of English.

If the upstream documentation is still incorrect, please report the
mistakes so it can be fixed there.

Thanks,


--
Yours Sincerely,

Cao Jin

_______________________________________________
Libguestfs mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libguestfs

Reply via email to