[PATCH] NEW API: add new api btrfs-fsck
Add the new API btrfs-fsck to check the btrfs filesystem. Btrfs is currently under heavy development, and not suitable for any uses other than benchmarking and review. But it'll be useful in the near future. Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com> --- daemon/btrfs.c | 17 +++++++++++++++++ generator/generator_actions.ml | 11 +++++++++++ src/MAX_PROC_NR | 2 +- 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/daemon/btrfs.c b/daemon/btrfs.c index ab3babc..62f07f3 100644 --- a/daemon/btrfs.c +++ b/daemon/btrfs.c @@ -619,3 +619,20 @@ do_btrfs_set_seeding (const char *device, int svalue) free (err); return r; } + +int +do_btrfs_fsck (const char *device) +{ + char *err; + int r; + + r = commandr (NULL, &err, "btrfsck", device, NULL); + if (r == -1) { + reply_with_error ("%s: %s", device, err); + free (err); + return -1; + } + + free (err); + return r; +} diff --git a/generator/generator_actions.ml b/generator/generator_actions.ml index fc2df42..33ffd6b 100644 --- a/generator/generator_actions.ml +++ b/generator/generator_actions.ml @@ -7232,6 +7232,17 @@ If C<devices> is an empty list, this does nothing."); Enable or disable the seeding feature of a device that contains a btrfs filesystem."); + ("btrfs_fsck", (RErr, [Device "device"], []), 332, [Optional "btrfs"], + [InitPartition, IfAvailable "btrfs", TestRun ( + [["mkfs_btrfs"; "/dev/sda1"; ""; ""; "NOARG"; ""; "NOARG"; "NOARG"; ""; ""]; + ["btrfs_fsck"; "/dev/sda1"]])], + "check a btrfs filesystem", + "\ +Used to check a btrfs filesystem, C<device> is the device file where the +filesystem is stored. +NOTE: Btrfs is currently under heavy development, and not suitable for any +uses other than benchmarking and review."); + ] let all_functions = non_daemon_functions @ daemon_functions diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR index ec6cab0..6f96da6 100644 --- a/src/MAX_PROC_NR +++ b/src/MAX_PROC_NR @@ -1 +1 @@ -331 +332 -- 1.7.10
On Tue, May 15, 2012 at 10:33:42AM +0800, Wanlong Gao wrote:
Add the new API btrfs-fsck to check the btrfs filesystem. Btrfs is currently under heavy development, and not suitable for any uses other than benchmarking and review. But it'll be useful in the near future.
Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com> --- daemon/btrfs.c | 17 +++++++++++++++++ generator/generator_actions.ml | 11 +++++++++++ src/MAX_PROC_NR | 2 +- 3 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/daemon/btrfs.c b/daemon/btrfs.c index ab3babc..62f07f3 100644 --- a/daemon/btrfs.c +++ b/daemon/btrfs.c @@ -619,3 +619,20 @@ do_btrfs_set_seeding (const char *device, int svalue) free (err); return r; } + +int +do_btrfs_fsck (const char *device) +{ + char *err; + int r; + + r = commandr (NULL, &err, "btrfsck", device, NULL); + if (r == -1) { + reply_with_error ("%s: %s", device, err); + free (err); + return -1; + } + + free (err); + return r; +}
This worries me. Isn't btrfsck interactive? Ordinary fsck has a meaningful exit code, but btrfsck doesn't. Will it get one in future?
diff --git a/generator/generator_actions.ml b/generator/generator_actions.ml index fc2df42..33ffd6b 100644 --- a/generator/generator_actions.ml +++ b/generator/generator_actions.ml @@ -7232,6 +7232,17 @@ If C<devices> is an empty list, this does nothing."); Enable or disable the seeding feature of a device that contains a btrfs filesystem.");
+ ("btrfs_fsck", (RErr, [Device "device"], []), 332, [Optional "btrfs"], + [InitPartition, IfAvailable "btrfs", TestRun ( + [["mkfs_btrfs"; "/dev/sda1"; ""; ""; "NOARG"; ""; "NOARG"; "NOARG"; ""; ""]; + ["btrfs_fsck"; "/dev/sda1"]])], + "check a btrfs filesystem", + "\ +Used to check a btrfs filesystem, C<device> is the device file where the +filesystem is stored.
Put a blank line between paragraphs.
+NOTE: Btrfs is currently under heavy development, and not suitable for any +uses other than benchmarking and review.");
Instead of "NOTE:", it's better to use I<Note:> or B<Note:> (see perlpod(1)). Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones New in Fedora 11: Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 70 libraries supprt'd http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw
On 05/15/2012 03:43 PM, Richard W.M. Jones wrote:
On Tue, May 15, 2012 at 10:33:42AM +0800, Wanlong Gao wrote:
Add the new API btrfs-fsck to check the btrfs filesystem. Btrfs is currently under heavy development, and not suitable for any uses other than benchmarking and review. But it'll be useful in the near future.
Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com> --- daemon/btrfs.c | 17 +++++++++++++++++ generator/generator_actions.ml | 11 +++++++++++ src/MAX_PROC_NR | 2 +- 3 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/daemon/btrfs.c b/daemon/btrfs.c index ab3babc..62f07f3 100644 --- a/daemon/btrfs.c +++ b/daemon/btrfs.c @@ -619,3 +619,20 @@ do_btrfs_set_seeding (const char *device, int svalue) free (err); return r; } + +int +do_btrfs_fsck (const char *device) +{ + char *err; + int r; + + r = commandr (NULL, &err, "btrfsck", device, NULL); + if (r == -1) { + reply_with_error ("%s: %s", device, err); + free (err); + return -1; + } + + free (err); + return r; +}
This worries me. Isn't btrfsck interactive? Ordinary fsck has a meaningful exit code, but btrfsck doesn't. Will it get one in future?
It may because btrfsck is a experimental tool and will change a lot in the future.
diff --git a/generator/generator_actions.ml b/generator/generator_actions.ml index fc2df42..33ffd6b 100644 --- a/generator/generator_actions.ml +++ b/generator/generator_actions.ml @@ -7232,6 +7232,17 @@ If C<devices> is an empty list, this does nothing."); Enable or disable the seeding feature of a device that contains a btrfs filesystem.");
+ ("btrfs_fsck", (RErr, [Device "device"], []), 332, [Optional "btrfs"], + [InitPartition, IfAvailable "btrfs", TestRun ( + [["mkfs_btrfs"; "/dev/sda1"; ""; ""; "NOARG"; ""; "NOARG"; "NOARG"; ""; ""]; + ["btrfs_fsck"; "/dev/sda1"]])], + "check a btrfs filesystem", + "\ +Used to check a btrfs filesystem, C<device> is the device file where the +filesystem is stored.
Put a blank line between paragraphs.
+NOTE: Btrfs is currently under heavy development, and not suitable for any +uses other than benchmarking and review.");
Instead of "NOTE:", it's better to use I<Note:> or B<Note:> (see perlpod(1)).
Will send V2. Thanks, Wanlong Gao
Rich.
Add the new API btrfs-fsck to check the btrfs filesystem. Btrfs is currently under heavy development, and not suitable for any uses other than benchmarking and review. But it'll be useful in the near future. Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com> --- daemon/btrfs.c | 17 +++++++++++++++++ generator/generator_actions.ml | 12 ++++++++++++ src/MAX_PROC_NR | 2 +- 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/daemon/btrfs.c b/daemon/btrfs.c index ab3babc..62f07f3 100644 --- a/daemon/btrfs.c +++ b/daemon/btrfs.c @@ -619,3 +619,20 @@ do_btrfs_set_seeding (const char *device, int svalue) free (err); return r; } + +int +do_btrfs_fsck (const char *device) +{ + char *err; + int r; + + r = commandr (NULL, &err, "btrfsck", device, NULL); + if (r == -1) { + reply_with_error ("%s: %s", device, err); + free (err); + return -1; + } + + free (err); + return r; +} diff --git a/generator/generator_actions.ml b/generator/generator_actions.ml index fc2df42..d4c1385 100644 --- a/generator/generator_actions.ml +++ b/generator/generator_actions.ml @@ -7232,6 +7232,18 @@ If C<devices> is an empty list, this does nothing."); Enable or disable the seeding feature of a device that contains a btrfs filesystem."); + ("btrfs_fsck", (RErr, [Device "device"], []), 332, [Optional "btrfs"], + [InitPartition, IfAvailable "btrfs", TestRun ( + [["mkfs_btrfs"; "/dev/sda1"; ""; ""; "NOARG"; ""; "NOARG"; "NOARG"; ""; ""]; + ["btrfs_fsck"; "/dev/sda1"]])], + "check a btrfs filesystem", + "\ +Used to check a btrfs filesystem, C<device> is the device file where the +filesystem is stored. + +I<Note:> Btrfs is currently under heavy development, and not suitable for any +uses other than benchmarking and review."); + ] let all_functions = non_daemon_functions @ daemon_functions diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR index ec6cab0..6f96da6 100644 --- a/src/MAX_PROC_NR +++ b/src/MAX_PROC_NR @@ -1 +1 @@ -331 +332 -- 1.7.10
I had a look at the code for btrfsck. My first impression was: OMG! This has 3,500 lines of code with a total of 20 comments! It returns negative errno numbers as exit codes. The 'main' function literally contains lines like 'return -EIO;' I've no idea wtf they were thinking there. There are also regular exit codes (calls to 'exit (1);' and 'return 1;'). Since the return codes are effectively random, there doesn't seem to be any meaning behind the values, unlike regular fsck. It doesn't appear to be interactive. It looks as if stderr is used correctly for error messages. There are some command line options which we should bind. I think our binding should use optional arguments which will allow us some room for growth in future. We should start by binding at least the '--super <int64>' and '--repair' options. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v
Add the new API btrfs-fsck to check the btrfs filesystem. Btrfs is currently under heavy development, and not suitable for any uses other than benchmarking and review. But it'll be useful in the near future. Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com> --- daemon/btrfs.c | 43 ++++++++++++++++++++++++++++++++++++++++ generator/generator_actions.ml | 12 +++++++++++ gobject/Makefile.inc | 6 ++++-- po/POTFILES | 1 + src/MAX_PROC_NR | 2 +- 5 files changed, 61 insertions(+), 3 deletions(-) diff --git a/daemon/btrfs.c b/daemon/btrfs.c index ab3babc..23958f6 100644 --- a/daemon/btrfs.c +++ b/daemon/btrfs.c @@ -619,3 +619,46 @@ do_btrfs_set_seeding (const char *device, int svalue) free (err); return r; } + +int +do_btrfs_fsck (const char *device, int64_t superblock, int repair) +{ + char *err; + int r; + size_t i = 0; + const size_t MAX_ARGS = 64; + const char *argv[MAX_ARGS]; + char super_s[64]; + + ADD_ARG (argv, i, "btrfsck"); + + /* Optional arguments. */ + if (optargs_bitmask & GUESTFS_BTRFS_FSCK_SUPERBLOCK_BITMASK) { + if (superblock < 0) { + reply_with_error ("super block offset must be >= 0"); + return -1; + } + snprintf (super_s, sizeof super_s, "%" PRIi64, superblock); + ADD_ARG (argv, i, "--super"); + ADD_ARG (argv, i, super_s); + } + + if (!(optargs_bitmask & GUESTFS_BTRFS_FSCK_REPAIR_BITMASK)) + repair = 0; + + if (repair) + ADD_ARG (argv, i, "--repair"); + + ADD_ARG (argv, i, device); + ADD_ARG (argv, i, NULL); + + r = commandv (NULL, &err, argv); + if (r == -1) { + reply_with_error ("%s: %s", device, err); + free (err); + return -1; + } + + free (err); + return 0; +} diff --git a/generator/generator_actions.ml b/generator/generator_actions.ml index fc2df42..4e88704 100644 --- a/generator/generator_actions.ml +++ b/generator/generator_actions.ml @@ -7232,6 +7232,18 @@ If C<devices> is an empty list, this does nothing."); Enable or disable the seeding feature of a device that contains a btrfs filesystem."); + ("btrfs_fsck", (RErr, [Device "device"], [OInt64 "superblock"; OBool "repair"]), 332, [Optional "btrfs"], + [InitPartition, IfAvailable "btrfs", TestRun ( + [["mkfs_btrfs"; "/dev/sda1"; ""; ""; "NOARG"; ""; "NOARG"; "NOARG"; ""; ""]; + ["btrfs_fsck"; "/dev/sda1"; ""; ""]])], + "check a btrfs filesystem", + "\ +Used to check a btrfs filesystem, C<device> is the device file where the +filesystem is stored. + +I<Note:> Btrfs is currently under heavy development, and not suitable for any +uses other than benchmarking and review."); + ] let all_functions = non_daemon_functions @ daemon_functions diff --git a/gobject/Makefile.inc b/gobject/Makefile.inc index c912fef..d4db407 100644 --- a/gobject/Makefile.inc +++ b/gobject/Makefile.inc @@ -60,7 +60,8 @@ guestfs_gobject_headers= \ include/guestfs-gobject/optargs-ntfsfix.h \ include/guestfs-gobject/optargs-ntfsclone_out.h \ include/guestfs-gobject/optargs-mkfs_btrfs.h \ - include/guestfs-gobject/optargs-set_e2attrs.h + include/guestfs-gobject/optargs-set_e2attrs.h \ + include/guestfs-gobject/optargs-btrfs_fsck.h guestfs_gobject_sources= \ src/session.c \ @@ -102,4 +103,5 @@ guestfs_gobject_sources= \ src/optargs-ntfsfix.c \ src/optargs-ntfsclone_out.c \ src/optargs-mkfs_btrfs.c \ - src/optargs-set_e2attrs.c + src/optargs-set_e2attrs.c \ + src/optargs-btrfs_fsck.c diff --git a/po/POTFILES b/po/POTFILES index 2b4b586..9d3282e 100644 --- a/po/POTFILES +++ b/po/POTFILES @@ -134,6 +134,7 @@ fuse/guestmount.c gobject/src/optargs-add_domain.c gobject/src/optargs-add_drive_opts.c gobject/src/optargs-btrfs_filesystem_resize.c +gobject/src/optargs-btrfs_fsck.c gobject/src/optargs-compress_device_out.c gobject/src/optargs-compress_out.c gobject/src/optargs-copy_device_to_device.c diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR index ec6cab0..6f96da6 100644 --- a/src/MAX_PROC_NR +++ b/src/MAX_PROC_NR @@ -1 +1 @@ -331 +332 -- 1.7.10
Change the return value from "r" to "0". Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com> --- daemon/btrfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/daemon/btrfs.c b/daemon/btrfs.c index 23958f6..99d886f 100644 --- a/daemon/btrfs.c +++ b/daemon/btrfs.c @@ -617,7 +617,7 @@ do_btrfs_set_seeding (const char *device, int svalue) } free (err); - return r; + return 0; } int -- 1.7.10
Both patches look fine. I'll push them in a minute, just running the tests. Thanks, Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://et.redhat.com/~rjones/virt-df/
participants (2)
-
Richard W.M. Jones -
Wanlong Gao