[PATCH 0/5] btrfs: add API for btrfs filesystem, check and scrub
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
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
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@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.
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?
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. Regards, Hu
On Monday 12 January 2015 11:45:02 Hu Tao wrote:
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@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.
btrfs balance has the same problem. I'll fix it if possible.
Thanks.
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?
I guess it depends on what "async operation" means. All the own libguestfs communication (the commands) is sync, and commands transferring files (upload, download, etc) can be aborted from the library (see user_cancel) or in case of error from the daemon side.
From a quick glance, it seems such operations are not aborted when shutting down, but IIRC we don't have thread safety on handles either.
Regarding all the other possible async operations (like the btrfs scrubbing of this thread), unless libguestfs explicitly know about them then they are implicitly interrupted/killed with the appliance shutdown.
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.
This is the usual approach we have so far in e.g. file transfer APIs. A better idea here would be adding support for async operations/jobs... -- Pino Toscano
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
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 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@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 :)
I found btrfs_fsck has been already implemented. Do you mean we should extend btrfs to support btrfs? Regards, Hu
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
_______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
On Monday 12 January 2015 09:37:36 Hu Tao wrote:
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@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 :)
I found btrfs_fsck has been already implemented. Do you mean we should extend btrfs to support btrfs?
I guess you mean "... extend fsck to support btrfs", right? :) That could be an idea, but given that "btrfs check" may usually need additional parameter, then it might not be worth it. -- Pino Toscano
On Tue, Jan 13, 2015 at 01:36:56PM +0100, Pino Toscano wrote:
On Monday 12 January 2015 09:37:36 Hu Tao wrote:
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@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 :)
I found btrfs_fsck has been already implemented. Do you mean we should extend btrfs to support btrfs?
I guess you mean "... extend fsck to support btrfs", right? :)
Yes, I had "extend fsck" in mind but...
That could be an idea, but given that "btrfs check" may usually need additional parameter, then it might not be worth it.
Okay. Regards, Hu
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
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
On Mon, Jan 05, 2015 at 01:26:08PM +0100, Pino Toscano wrote:
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.
Yes, blkid just works. I think this patch can be dropped. Thanks for reminding! Regards, Hu
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@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. Thanks, -- Pino Toscano
On Wed, Jan 07, 2015 at 11:23:29AM +0100, Pino Toscano wrote:
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.
Yeah, I'm planning to update the bit for all btrfs-related patches. Thank you for reminding! Regards, Hu
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
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
On Tue, Jan 13, 2015 at 02:00:13PM +0100, Pino Toscano wrote:
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?
Agreed.
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?
Agreed too, 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.
Okay. Regards, Hu
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
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@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?
Yes. This patch ban be dropped. Regards, Hu
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@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@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.
I found that all btrfs APIs are in optgroup "btrfs", so it is OK to "Always" test them. Regards, Hu
On Friday 16 January 2015 09:21:49 Hu Tao wrote:
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@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.
I found that all btrfs APIs are in optgroup "btrfs", so it is OK to "Always" test them.
As long as they are part of a "optional/btrfs" API yes, but not if you add such btrfs-specific tests to a non-btrfs API such as set_label. -- Pino Toscano
participants (2)
-
Hu Tao -
Pino Toscano