Signed-off-by: Hu Tao <hutao(a)cn.fujitsu.com&gt; --- 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; +}

On Wed, Jan 07, 2015 at 12:37:48PM +0100, Pino Toscano wrote: > In data venerdì 26 dicembre 2014 16:17:45, Hu Tao ha scritto: > > Signed-off-by: Hu Tao <hutao(a)cn.fujitsu.com&gt; > > --- > > 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. btrfs balance has the same problem. I'll fix it if possible.

> 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? It probably corrupts the filesystem, but let me test it. Do we have any way to prevent the appliance from being shutdown if there is any async operation is ongoing?

From a quick glance, it seems such operations are not aborted when

> Another option could be making it synchronous (start -B), if it doesn't > take too much time. I agree with you if the caller of the API doesn't care to wait.

Signed-off-by: Hu Tao <hutao(a)cn.fujitsu.com&gt; --- 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"]]), []; + ];

On Wed, Jan 07, 2015 at 01:25:50PM +0100, Pino Toscano wrote: > In data venerdì 26 dicembre 2014 16:17:46, Hu Tao ha scritto: > > Signed-off-by: Hu Tao <hutao(a)cn.fujitsu.com&gt; > > --- > > 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 :) I found btrfs_fsck has been already implemented. Do you mean we should extend btrfs to support btrfs?

In data venerdì 26 dicembre 2014 16:17:47, Hu Tao ha scritto: > Signed-off-by: Hu Tao <hutao(a)cn.fujitsu.com&gt; > --- > 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 _______________________________________________ Libguestfs mailing list Libguestfs(a)redhat.com https://www.redhat.com/mailman/listinfo/libguestfs

In data martedì 6 gennaio 2015 09:30:19, Hu Tao ha scritto: > > > 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. Feel free to send this bit (updated) anyway, also for patch 5.

Signed-off-by: Hu Tao <hutao(a)cn.fujitsu.com&gt; --- 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." };

In data venerdì 26 dicembre 2014 16:17:49, Hu Tao ha scritto: > Signed-off-by: Hu Tao <hutao(a)cn.fujitsu.com&gt; > --- > 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 _______________________________________________ Libguestfs mailing list Libguestfs(a)redhat.com https://www.redhat.com/mailman/listinfo/libguestfs

On Mon, Jan 05, 2015 at 01:01:52PM +0100, Pino Toscano wrote: > In data venerdì 26 dicembre 2014 16:17:49, Hu Tao ha scritto: > > Signed-off-by: Hu Tao <hutao(a)cn.fujitsu.com&gt; > > --- > > 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. I found that all btrfs APIs are in optgroup "btrfs", so it is OK to "Always" test them.