Libguestfs - Jan 2015 - Re: [PATCH 4/5] New API: add btrfs_filesystem_defragment

Hi,

There is one problem: btrfs_filesystem_set_label just doesnt work,
giving error message:

  libguestfs: error: btrfs_filesystem_set_label: /: ERROR: unable to set label
Bad address

I'm almost sure the patch has no problem, but can't figure out
what's
the cause. So patch 5 is only for review.

Other APIs have no problem.

Regards,
Hu

Hu Tao (5):
  New API: btrfs_scrub
  New API: btrfs_check
  New API: btrfs_filesystem_get_label
  New API: add btrfs_filesystem_defragment
  New API: btrfs_filesystem_set_label

 daemon/btrfs.c       | 153 +++++++++++++++++++++++++++++++++++++++++++++++++++
 generator/actions.ml |  82 +++++++++++++++++++++++++++
 src/MAX_PROC_NR      |   2 +-
 3 files changed, 236 insertions(+), 1 deletion(-)

-- 
2.2.1

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 daemon/btrfs.c       | 32 ++++++++++++++++++++++++++++++++
 generator/actions.ml | 17 +++++++++++++++++
 src/MAX_PROC_NR      |  2 +-
 3 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/daemon/btrfs.c b/daemon/btrfs.c
index 150c089..79de539 100644
--- a/daemon/btrfs.c
+++ b/daemon/btrfs.c
@@ -1343,3 +1343,35 @@ do_btrfs_qgroup_remove (const char *src, const char *dst,
const char *path)
 
   return 0;
 }
+
+int
+do_btrfs_scrub (const char *path)
+{
+  const size_t MAX_ARGS = 64;
+  const char *argv[MAX_ARGS];
+  size_t i = 0;
+  CLEANUP_FREE char *path_buf = NULL;
+  CLEANUP_FREE char *err = NULL;
+  CLEANUP_FREE char *out = NULL;
+  int r;
+
+  path_buf = sysroot_path (path);
+  if (path_buf == NULL) {
+    reply_with_perror ("malloc");
+    return -1;
+  }
+
+  ADD_ARG (argv, i, str_btrfs);
+  ADD_ARG (argv, i, "scrub");
+  ADD_ARG (argv, i, "start");
+  ADD_ARG (argv, i, path_buf);
+  ADD_ARG (argv, i, NULL);
+
+  r = commandv (&out, &err, argv);
+  if (r == -1) {
+    reply_with_error ("%s: %s", path, err);
+    return -1;
+  }
+
+  return 0;
+}
diff --git a/generator/actions.ml b/generator/actions.ml
index a6a6dad..b44ce1c 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -12250,6 +12250,23 @@ several qgroups into a parent qgroup to share common
limit." };
     longdesc = "\
 Remove qgroup C<src> from the parent qgroup C<dst>." };
 
+  { defaults with
+    name = "btrfs_scrub";
+    style = RErr, [Pathname "path"], [];
+    proc_nr = Some 435;
+    optional = Some "btrfs"; camel_name = "BTRFSScrub";
+    tests = [
+      InitPartition, Always, TestRun (
+        [["mkfs_btrfs"; "/dev/sda1"; "";
""; "NOARG"; ""; "NOARG";
"NOARG"; ""; ""];
+         ["mount"; "/dev/sda1"; "/"];
+         ["btrfs_scrub"; "/"]]), [];
+    ];
+    shortdesc = "read all data from all disks and verify checksums";
+    longdesc = "\
+Reads all the data and metadata on the filesystem, and uses checksums
+and the duplicate copies from RAID storage to identify and repair any
+corrupt data." };
+
 ]
 
 (* Non-API meta-commands available only in guestfish.
diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR
index e828e5d..5910394 100644
--- a/src/MAX_PROC_NR
+++ b/src/MAX_PROC_NR
@@ -1 +1 @@
-434
+435
-- 
2.2.1

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 daemon/btrfs.c       | 24 ++++++++++++++++++++++++
 generator/actions.ml | 14 ++++++++++++++
 src/MAX_PROC_NR      |  2 +-
 3 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/daemon/btrfs.c b/daemon/btrfs.c
index 79de539..de20bc3 100644
--- a/daemon/btrfs.c
+++ b/daemon/btrfs.c
@@ -1375,3 +1375,27 @@ do_btrfs_scrub (const char *path)
 
   return 0;
 }
+
+int
+do_btrfs_check (const char *device)
+{
+  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_btrfs);
+  ADD_ARG (argv, i, "check");
+  ADD_ARG (argv, i, device);
+  ADD_ARG (argv, i, NULL);
+
+  r = commandv (&out, &err, argv);
+  if (r == -1) {
+    reply_with_error ("%s: %s", device, err);
+    return -1;
+  }
+
+  return 0;
+}
diff --git a/generator/actions.ml b/generator/actions.ml
index b44ce1c..c328319 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -12267,6 +12267,20 @@ Reads all the data and metadata on the filesystem, and
uses checksums
 and the duplicate copies from RAID storage to identify and repair any
 corrupt data." };
 
+  { defaults with
+    name = "btrfs_check";
+    style = RErr, [Device "device"], [];
+    proc_nr = Some 436;
+    optional = Some "btrfs"; camel_name = "BTRFSCheck";
+    tests = [
+      InitPartition, Always, TestRun (
+        [["mkfs_btrfs"; "/dev/sda1"; "";
""; "NOARG"; ""; "NOARG";
"NOARG"; ""; ""];
+         ["btrfs_check"; "/dev/sda1"]]), [];
+    ];
+    shortdesc = "check or repair a btrfs filesystem offline";
+    longdesc = "\
+Check or repair a btrfs filesystem offline." };
+
 ]
 
 (* Non-API meta-commands available only in guestfish.
diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR
index 5910394..246662b 100644
--- a/src/MAX_PROC_NR
+++ b/src/MAX_PROC_NR
@@ -1 +1 @@
-435
+436
-- 
2.2.1

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 daemon/btrfs.c       | 32 ++++++++++++++++++++++++++++++++
 generator/actions.ml | 15 +++++++++++++++
 src/MAX_PROC_NR      |  2 +-
 3 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/daemon/btrfs.c b/daemon/btrfs.c
index de20bc3..6c74892 100644
--- a/daemon/btrfs.c
+++ b/daemon/btrfs.c
@@ -1399,3 +1399,35 @@ do_btrfs_check (const char *device)
 
   return 0;
 }
+
+char *
+do_btrfs_filesystem_get_label (const char *path)
+{
+  const size_t MAX_ARGS = 64;
+  const char *argv[MAX_ARGS];
+  size_t i = 0;
+  CLEANUP_FREE char *path_buf = NULL;
+  CLEANUP_FREE char *err = NULL;
+  char *out;
+  int r;
+
+  path_buf = sysroot_path (path);
+  if (path_buf == NULL) {
+    reply_with_perror ("malloc");
+    return NULL;
+  }
+
+  ADD_ARG (argv, i, str_btrfs);
+  ADD_ARG (argv, i, "filesystem");
+  ADD_ARG (argv, i, "label");
+  ADD_ARG (argv, i, path_buf);
+  ADD_ARG (argv, i, NULL);
+
+  r = commandv (&out, &err, argv);
+  if (r == -1) {
+    reply_with_error ("%s: %s", path, err);
+    return NULL;
+  }
+
+  return out;
+}
diff --git a/generator/actions.ml b/generator/actions.ml
index c328319..5223eb8 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -12281,6 +12281,21 @@ corrupt data." };
     longdesc = "\
 Check or repair a btrfs filesystem offline." };
 
+  { defaults with
+    name = "btrfs_filesystem_get_label";
+    style = RString "label", [Pathname "path"], [];
+    proc_nr = Some 437;
+    optional = Some "btrfs"; camel_name =
"BTRFSFilesystemGetLabel";
+    tests = [
+      InitPartition, Always, TestResultString (
+        [["mkfs_btrfs"; "/dev/sda1"; "";
""; "NOARG"; ""; "label";
"NOARG"; ""; ""];
+         ["mount"; "/dev/sda1"; "/"];
+         ["btrfs_filesystem_get_label"; "/";]],
"label\n"), [];
+    ];
+    shortdesc = "get a btrfs filesystem's label";
+    longdesc = "\
+Get a btrfs filesystem's label." };
+
 ]
 
 (* Non-API meta-commands available only in guestfish.
diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR
index 246662b..ce9cd49 100644
--- a/src/MAX_PROC_NR
+++ b/src/MAX_PROC_NR
@@ -1 +1 @@
-436
+437
-- 
2.2.1

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 daemon/btrfs.c       | 32 ++++++++++++++++++++++++++++++++
 generator/actions.ml | 20 ++++++++++++++++++++
 src/MAX_PROC_NR      |  2 +-
 3 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/daemon/btrfs.c b/daemon/btrfs.c
index 6c74892..32b1b05 100644
--- a/daemon/btrfs.c
+++ b/daemon/btrfs.c
@@ -1431,3 +1431,35 @@ do_btrfs_filesystem_get_label (const char *path)
 
   return out;
 }
+
+int
+do_btrfs_filesystem_defragment (const char *path)
+{
+  const size_t MAX_ARGS = 64;
+  const char *argv[MAX_ARGS];
+  size_t i = 0;
+  CLEANUP_FREE char *path_buf = NULL;
+  CLEANUP_FREE char *err = NULL;
+  CLEANUP_FREE char *out = NULL;
+  int r;
+
+  path_buf = sysroot_path (path);
+  if (path_buf == NULL) {
+    reply_with_perror ("malloc");
+    return -1;
+  }
+
+  ADD_ARG (argv, i, str_btrfs);
+  ADD_ARG (argv, i, "filesystem");
+  ADD_ARG (argv, i, "defragment");
+  ADD_ARG (argv, i, path_buf);
+  ADD_ARG (argv, i, NULL);
+
+  r = commandv (&out, &err, argv);
+  if (r == -1) {
+    reply_with_error ("%s: %s", path, err);
+    return -1;
+  }
+
+  return 0;
+}
diff --git a/generator/actions.ml b/generator/actions.ml
index 5223eb8..4b24f8a 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -12296,6 +12296,26 @@ Check or repair a btrfs filesystem offline." };
     longdesc = "\
 Get a btrfs filesystem's label." };
 
+  { defaults with
+    name = "btrfs_filesystem_defragment";
+    style = RErr, [Pathname "path"], [];
+    proc_nr = Some 438;
+    optional = Some "btrfs"; camel_name =
"BTRFSFilesystemDefragment";
+    tests = [
+      InitPartition, Always, TestRun (
+        [["mkfs_btrfs"; "/dev/sda1"; "";
""; "NOARG"; ""; "NOARG";
"NOARG"; ""; ""];
+         ["mount"; "/dev/sda1"; "/"];
+         ["btrfs_filesystem_defragment"; "/"]]), [];
+      InitPartition, Always, TestRun (
+        [["mkfs_btrfs"; "/dev/sda1"; "";
""; "NOARG"; ""; "NOARG";
"NOARG"; ""; ""];
+         ["mount"; "/dev/sda1"; "/"];
+         ["touch"; "/hello"];
+         ["btrfs_filesystem_defragment"; "/hello"]]), [];
+    ];
+    shortdesc = "defragment a file or directory";
+    longdesc = "\
+Defragment a file or directory." };
+
 ]
 
 (* Non-API meta-commands available only in guestfish.
diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR
index ce9cd49..c8eb588 100644
--- a/src/MAX_PROC_NR
+++ b/src/MAX_PROC_NR
@@ -1 +1 @@
-437
+438
-- 
2.2.1

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 daemon/btrfs.c       | 33 +++++++++++++++++++++++++++++++++
 generator/actions.ml | 16 ++++++++++++++++
 src/MAX_PROC_NR      |  2 +-
 3 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/daemon/btrfs.c b/daemon/btrfs.c
index 32b1b05..f9e35be 100644
--- a/daemon/btrfs.c
+++ b/daemon/btrfs.c
@@ -1463,3 +1463,36 @@ do_btrfs_filesystem_defragment (const char *path)
 
   return 0;
 }
+
+int
+do_btrfs_filesystem_set_label (const char *path, const char *label)
+{
+  const size_t MAX_ARGS = 64;
+  const char *argv[MAX_ARGS];
+  size_t i = 0;
+  CLEANUP_FREE char *path_buf = NULL;
+  CLEANUP_FREE char *err = NULL;
+  CLEANUP_FREE char *out = NULL;
+  int r;
+
+  path_buf = sysroot_path (path);
+  if (path_buf == NULL) {
+    reply_with_perror ("malloc");
+    return -1;
+  }
+
+  ADD_ARG (argv, i, str_btrfs);
+  ADD_ARG (argv, i, "filesystem");
+  ADD_ARG (argv, i, "label");
+  ADD_ARG (argv, i, path_buf);
+  ADD_ARG (argv, i, label);
+  ADD_ARG (argv, i, NULL);
+
+  r = commandv (&out, &err, argv);
+  if (r == -1) {
+    reply_with_error ("%s: %s", path, err);
+    return -1;
+  }
+
+  return 0;
+}
diff --git a/generator/actions.ml b/generator/actions.ml
index 4b24f8a..7e1f3ed 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -12316,6 +12316,22 @@ Get a btrfs filesystem's label." };
     longdesc = "\
 Defragment a file or directory." };
 
+  { defaults with
+    name = "btrfs_filesystem_set_label";
+    style = RErr, [Pathname "path"; String "label"], [];
+    proc_nr = Some 439;
+    optional = Some "btrfs"; camel_name =
"BTRFSFilesystemSetLabel";
+    tests = [
+      InitPartition, Always, TestResultString (
+        [["mkfs_btrfs"; "/dev/sda1"; "";
""; "NOARG"; ""; "NOARG";
"NOARG"; ""; ""];
+         ["mount"; "/dev/sda1"; "/"];
+         ["btrfs_filesystem_set_label"; "/";
"newlabel"];
+         ["btrfs_filesystem_get_label"; "/"]],
"newlabel"), [];
+    ];
+    shortdesc = "set a btrfs filesystem's label";
+    longdesc = "\
+Set a btrfs filesystem's label." };
+
 ]
 
 (* Non-API meta-commands available only in guestfish.
diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR
index c8eb588..99dea3b 100644
--- a/src/MAX_PROC_NR
+++ b/src/MAX_PROC_NR
@@ -1 +1 @@
-438
+439
-- 
2.2.1

In data venerdì 26 dicembre 2014 16:17:49, Hu Tao ha
scritto:
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  daemon/btrfs.c       | 33 +++++++++++++++++++++++++++++++++
>  generator/actions.ml | 16 ++++++++++++++++
>  src/MAX_PROC_NR      |  2 +-
>  3 files changed, 50 insertions(+), 1 deletion(-)
> 
> diff --git a/daemon/btrfs.c b/daemon/btrfs.c
> index 32b1b05..f9e35be 100644
> --- a/daemon/btrfs.c
> +++ b/daemon/btrfs.c
> @@ -1463,3 +1463,36 @@ do_btrfs_filesystem_defragment (const char *path)
>  
>    return 0;
>  }
> +
> +int
> +do_btrfs_filesystem_set_label (const char *path, const char *label)
> +{
> +  const size_t MAX_ARGS = 64;
> +  const char *argv[MAX_ARGS];
> +  size_t i = 0;
> +  CLEANUP_FREE char *path_buf = NULL;
> +  CLEANUP_FREE char *err = NULL;
> +  CLEANUP_FREE char *out = NULL;
> +  int r;
> +
> +  path_buf = sysroot_path (path);
> +  if (path_buf == NULL) {
> +    reply_with_perror ("malloc");
> +    return -1;
> +  }
> +
> +  ADD_ARG (argv, i, str_btrfs);
> +  ADD_ARG (argv, i, "filesystem");
> +  ADD_ARG (argv, i, "label");
> +  ADD_ARG (argv, i, path_buf);
> +  ADD_ARG (argv, i, label);
> +  ADD_ARG (argv, i, NULL);
> +
> +  r = commandv (&out, &err, argv);
> +  if (r == -1) {
> +    reply_with_error ("%s: %s", path, err);
> +    return -1;
> +  }
> +
> +  return 0;
> +}
Isn't this alredy covered by set_label?
> diff --git a/generator/actions.ml b/generator/actions.ml
> index 4b24f8a..7e1f3ed 100644
> --- a/generator/actions.ml
> +++ b/generator/actions.ml
> @@ -12316,6 +12316,22 @@ Get a btrfs filesystem's label." };
>      longdesc = "\
>  Defragment a file or directory." };
>  
> +  { defaults with
> +    name = "btrfs_filesystem_set_label";
> +    style = RErr, [Pathname "path"; String "label"],
[];
> +    proc_nr = Some 439;
> +    optional = Some "btrfs"; camel_name =
"BTRFSFilesystemSetLabel";
> +    tests = [
> +      InitPartition, Always, TestResultString (
> +        [["mkfs_btrfs"; "/dev/sda1"; "";
""; "NOARG"; ""; "NOARG";
"NOARG"; ""; ""];
> +         ["mount"; "/dev/sda1"; "/"];
> +         ["btrfs_filesystem_set_label"; "/";
"newlabel"];
> +         ["btrfs_filesystem_get_label"; "/"]],
"newlabel"), [];
> +    ];
This test bit would be good to be added to the tests of set_label,
just with s/Always/IfAvailable "btrfs"/ so it is run only when btrfs
is available.

Thanks,
-- 
Pino Toscano

In data venerdì 26 dicembre 2014 16:17:47, Hu Tao ha
scritto:
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  daemon/btrfs.c       | 32 ++++++++++++++++++++++++++++++++
>  generator/actions.ml | 15 +++++++++++++++
>  src/MAX_PROC_NR      |  2 +-
>  3 files changed, 48 insertions(+), 1 deletion(-)
> 
> diff --git a/daemon/btrfs.c b/daemon/btrfs.c
> index de20bc3..6c74892 100644
> --- a/daemon/btrfs.c
> +++ b/daemon/btrfs.c
> @@ -1399,3 +1399,35 @@ do_btrfs_check (const char *device)
>  
>    return 0;
>  }
> +
> +char *
> +do_btrfs_filesystem_get_label (const char *path)
> +{
> +  const size_t MAX_ARGS = 64;
> +  const char *argv[MAX_ARGS];
> +  size_t i = 0;
> +  CLEANUP_FREE char *path_buf = NULL;
> +  CLEANUP_FREE char *err = NULL;
> +  char *out;
> +  int r;
> +
> +  path_buf = sysroot_path (path);
> +  if (path_buf == NULL) {
> +    reply_with_perror ("malloc");
> +    return NULL;
> +  }
> +
> +  ADD_ARG (argv, i, str_btrfs);
> +  ADD_ARG (argv, i, "filesystem");
> +  ADD_ARG (argv, i, "label");
> +  ADD_ARG (argv, i, path_buf);
> +  ADD_ARG (argv, i, NULL);
> +
> +  r = commandv (&out, &err, argv);
> +  if (r == -1) {
> +    reply_with_error ("%s: %s", path, err);
> +    return NULL;
> +  }
> +
> +  return out;
> +}
There's vfs_label already as command to get the label of a filesystem,
so this special case for btrfs should be added there (just like the
ones done in set_label).

Isn't blkid working on btrfs filesystems? From what I see, there is
support for it already.
> diff --git a/generator/actions.ml b/generator/actions.ml
> index c328319..5223eb8 100644
> --- a/generator/actions.ml
> +++ b/generator/actions.ml
> @@ -12281,6 +12281,21 @@ corrupt data." };
>      longdesc = "\
>  Check or repair a btrfs filesystem offline." };
>  
> +  { defaults with
> +    name = "btrfs_filesystem_get_label";
> +    style = RString "label", [Pathname "path"], [];
> +    proc_nr = Some 437;
> +    optional = Some "btrfs"; camel_name =
"BTRFSFilesystemGetLabel";
> +    tests = [
> +      InitPartition, Always, TestResultString (
> +        [["mkfs_btrfs"; "/dev/sda1"; "";
""; "NOARG"; ""; "label";
"NOARG"; ""; ""];
> +         ["mount"; "/dev/sda1"; "/"];
> +         ["btrfs_filesystem_get_label"; "/";]],
"label\n"), [];
> +    ];
This test bit would be good to be added to the tests of vfs_label,
just with s/Always/IfAvailable "btrfs"/ so it is run only when btrfs
is available.

Thanks,
-- 
Pino Toscano

In data venerdì 26 dicembre 2014 16:17:45, Hu Tao ha
scritto:
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  daemon/btrfs.c       | 32 ++++++++++++++++++++++++++++++++
>  generator/actions.ml | 17 +++++++++++++++++
>  src/MAX_PROC_NR      |  2 +-
>  3 files changed, 50 insertions(+), 1 deletion(-)
> 
> diff --git a/daemon/btrfs.c b/daemon/btrfs.c
> index 150c089..79de539 100644
> --- a/daemon/btrfs.c
> +++ b/daemon/btrfs.c
> @@ -1343,3 +1343,35 @@ do_btrfs_qgroup_remove (const char *src, const char
*dst, const char *path)
>  
>    return 0;
>  }
> +
> +int
> +do_btrfs_scrub (const char *path)
> +{
> +  const size_t MAX_ARGS = 64;
> +  const char *argv[MAX_ARGS];
> +  size_t i = 0;
> +  CLEANUP_FREE char *path_buf = NULL;
> +  CLEANUP_FREE char *err = NULL;
> +  CLEANUP_FREE char *out = NULL;
> +  int r;
> +
> +  path_buf = sysroot_path (path);
> +  if (path_buf == NULL) {
> +    reply_with_perror ("malloc");
> +    return -1;
> +  }
> +
> +  ADD_ARG (argv, i, str_btrfs);
> +  ADD_ARG (argv, i, "scrub");
> +  ADD_ARG (argv, i, "start");
> +  ADD_ARG (argv, i, path_buf);
> +  ADD_ARG (argv, i, NULL);
> +
> +  r = commandv (&out, &err, argv);
> +  if (r == -1) {
> +    reply_with_error ("%s: %s", path, err);
> +    return -1;
> +  }
> +
> +  return 0;
> +}
Reading the btrfs documentation, it seems like this is an asynchronous
operation by default, so I'm not sure whether it would be useful to
just start it without any way to monitor its status.

Also, what would it happen when you start a scrub operation on a
mounted filesystem, and then you either unmount it or shut down the
appliance while the scrubbing is still ongoing?

Another option could be making it synchronous (start -B), if it doesn't
take too much time.

Thanks,
-- 
Pino Toscano

In data venerdì 26 dicembre 2014 16:17:46, Hu Tao ha
scritto:
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  daemon/btrfs.c       | 24 ++++++++++++++++++++++++
>  generator/actions.ml | 14 ++++++++++++++
>  src/MAX_PROC_NR      |  2 +-
>  3 files changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/daemon/btrfs.c b/daemon/btrfs.c
> index 79de539..de20bc3 100644
> --- a/daemon/btrfs.c
> +++ b/daemon/btrfs.c
> @@ -1375,3 +1375,27 @@ do_btrfs_scrub (const char *path)
>  
>    return 0;
>  }
> +
> +int
> +do_btrfs_check (const char *device)
> +{
> +  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_btrfs);
> +  ADD_ARG (argv, i, "check");
> +  ADD_ARG (argv, i, device);
> +  ADD_ARG (argv, i, NULL);
> +
> +  r = commandv (&out, &err, argv);
> +  if (r == -1) {
> +    reply_with_error ("%s: %s", device, err);
> +    return -1;
> +  }
> +
> +  return 0;
> +}
We do have already a "fsck" action; OTOH fsck.btrfs does nothing, so
I would rather call `btrfs check` there, instead of adding a new
btrfs-specific API.
Let's make our fsck useful on btrfs :)
> diff --git a/generator/actions.ml b/generator/actions.ml
> index b44ce1c..c328319 100644
> --- a/generator/actions.ml
> +++ b/generator/actions.ml
> @@ -12267,6 +12267,20 @@ Reads all the data and metadata on the filesystem,
and uses checksums
>  and the duplicate copies from RAID storage to identify and repair any
>  corrupt data." };
>  
> +  { defaults with
> +    name = "btrfs_check";
> +    style = RErr, [Device "device"], [];
> +    proc_nr = Some 436;
> +    optional = Some "btrfs"; camel_name =
"BTRFSCheck";
> +    tests = [
> +      InitPartition, Always, TestRun (
> +        [["mkfs_btrfs"; "/dev/sda1"; "";
""; "NOARG"; ""; "NOARG";
"NOARG"; ""; ""];
> +         ["btrfs_check"; "/dev/sda1"]]), [];
> +    ];
This test bit would be good to be added to the tests of fsck,
just with s/Always/IfAvailable "btrfs"/ so it is run only when btrfs
is available.

Thanks,
-- 
Pino Toscano

On Friday 26 December 2014 16:17:48 Hu Tao wrote:
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  daemon/btrfs.c       | 32 ++++++++++++++++++++++++++++++++
>  generator/actions.ml | 20 ++++++++++++++++++++
>  src/MAX_PROC_NR      |  2 +-
>  3 files changed, 53 insertions(+), 1 deletion(-)
> 
> diff --git a/daemon/btrfs.c b/daemon/btrfs.c
> index 6c74892..32b1b05 100644
> --- a/daemon/btrfs.c
> +++ b/daemon/btrfs.c
> @@ -1431,3 +1431,35 @@ do_btrfs_filesystem_get_label (const char *path)
>  
>    return out;
>  }
> +
> +int
> +do_btrfs_filesystem_defragment (const char *path)
> +{
> +  const size_t MAX_ARGS = 64;
> +  const char *argv[MAX_ARGS];
> +  size_t i = 0;
> +  CLEANUP_FREE char *path_buf = NULL;
> +  CLEANUP_FREE char *err = NULL;
> +  CLEANUP_FREE char *out = NULL;
> +  int r;
> +
> +  path_buf = sysroot_path (path);
> +  if (path_buf == NULL) {
> +    reply_with_perror ("malloc");
> +    return -1;
> +  }
> +
> +  ADD_ARG (argv, i, str_btrfs);
> +  ADD_ARG (argv, i, "filesystem");
> +  ADD_ARG (argv, i, "defragment");
> +  ADD_ARG (argv, i, path_buf);
> +  ADD_ARG (argv, i, NULL);
> +
> +  r = commandv (&out, &err, argv);
> +  if (r == -1) {
> +    reply_with_error ("%s: %s", path, err);
> +    return -1;
> +  }
> +
> +  return 0;
> +}
Generally this sounds fine for me.

It seems that directories are not recursively handled by default
(-r is needed); should btrfs_filesystem_defragment do that
automatically?

What about the options like -f (to flush) and -c (to compress), are
they options which get used often/usually, and thus worthy to be
passed as optargs?
> diff --git a/generator/actions.ml b/generator/actions.ml
> index 5223eb8..4b24f8a 100644
> --- a/generator/actions.ml
> +++ b/generator/actions.ml
> @@ -12296,6 +12296,26 @@ Check or repair a btrfs filesystem offline."
};
>      longdesc = "\
>  Get a btrfs filesystem's label." };
>  
> +  { defaults with
> +    name = "btrfs_filesystem_defragment";
> +    style = RErr, [Pathname "path"], [];
> +    proc_nr = Some 438;
> +    optional = Some "btrfs"; camel_name =
"BTRFSFilesystemDefragment";
> +    tests = [
> +      InitPartition, Always, TestRun (
> +        [["mkfs_btrfs"; "/dev/sda1"; "";
""; "NOARG"; ""; "NOARG";
"NOARG"; ""; ""];
> +         ["mount"; "/dev/sda1"; "/"];
> +         ["btrfs_filesystem_defragment"; "/"]]), [];
> +      InitPartition, Always, TestRun (
> +        [["mkfs_btrfs"; "/dev/sda1"; "";
""; "NOARG"; ""; "NOARG";
"NOARG"; ""; ""];
> +         ["mount"; "/dev/sda1"; "/"];
> +         ["touch"; "/hello"];
> +         ["btrfs_filesystem_defragment"; "/hello"]]),
[];
> +    ];
> +    shortdesc = "defragment a file or directory";
> +    longdesc = "\
> +Defragment a file or directory." };
Maybe it would be worth mentioning btrfs in the descriptions.

-- 
Pino Toscano