Libguestfs - Jan 2012 - [PATCH v2 1/3] ext2: tweak the error returned message of resize2fs-M(BZ755729)

From: Wanlong Gao <gaowanlong at cn.fujitsu.com>

Tweak the error message "e2fsck -f" and "e2fsck -fy".
Indicate the user to use the correct and/or forceall options.

Signed-off-by: Wanlong Gao <gaowanlong at cn.fujitsu.com>
---
 daemon/ext2.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/daemon/ext2.c b/daemon/ext2.c
index 79fd354..c280ca2 100644
--- a/daemon/ext2.c
+++ b/daemon/ext2.c
@@ -277,9 +277,14 @@ do_resize2fs_M (const char *device)
   if (e2prog (prog) == -1)
     return -1;
 
-  r = command (NULL, &err, prog, "-M" , device, NULL);
+  r = command (NULL, &err, prog, "-M", device, NULL);
   if (r == -1) {
-    reply_with_error ("%s", err);
+    if (strstr (err, "e2fsck -f")) {
+      free (err);
+      reply_with_error ("you need to run e2fsck with the correct and/or
forceall options first");
+    } else {
+      reply_with_error ("%s", err);
+    }
     free (err);
     return -1;
   }
-- 
1.7.8

From: Wanlong Gao <gaowanlong at cn.fujitsu.com>

Add a new api e2fsck with two options:
correct: same as '-p' option of e2fsck
forceall: same as '-y' option of e2fsck

Thanks for Rich's idea.
v1->v2: use optargs_bitmask

Signed-off-by: Wanlong Gao <gaowanlong at cn.fujitsu.com>
---
 daemon/ext2.c                  |   44 ++++++++++++++++++++++++++++++++++++++++
 generator/generator_actions.ml |   24 +++++++++++++++++++++
 src/MAX_PROC_NR                |    2 +-
 3 files changed, 69 insertions(+), 1 deletions(-)

diff --git a/daemon/ext2.c b/daemon/ext2.c
index c280ca2..5a0ac9f 100644
--- a/daemon/ext2.c
+++ b/daemon/ext2.c
@@ -294,6 +294,50 @@ do_resize2fs_M (const char *device)
 }
 
 int
+do_e2fsck (const char *device,
+           int correct,
+           int forceall)
+{
+  const char *argv[MAX_ARGS];
+  char *err;
+  size_t i = 0;
+  int r;
+  char prog[] = "e2fsck";
+
+  if (e2prog (prog) == -1)
+    return -1;
+
+  if (optargs_bitmask & GUESTFS_E2FSCK_CORRECT_BITMASK &&
+      optargs_bitmask & GUESTFS_E2FSCK_FORCEALL_BITMASK) {
+    reply_with_error("Only one of the options may be specified");
+    return -1;
+  }
+
+
+  ADD_ARG (argv, i, prog);
+  ADD_ARG (argv, i, "-f");
+
+  if (optargs_bitmask & GUESTFS_E2FSCK_CORRECT_BITMASK && correct)
+    ADD_ARG (argv, i, "-p");
+
+  if (optargs_bitmask & GUESTFS_E2FSCK_FORCEALL_BITMASK &&
forceall)
+    ADD_ARG (argv, i, "-y");
+
+  ADD_ARG (argv, i, device);
+  ADD_ARG (argv, i, NULL);
+
+  r = commandv (NULL, &err, argv);
+  if (r == -1 || r >= 2) {
+    reply_with_error ("%s", err);
+    free (err);
+    return -1;
+  }
+
+  free (err);
+  return 0;
+}
+
+int
 do_e2fsck_f (const char *device)
 {
   char *err;
diff --git a/generator/generator_actions.ml b/generator/generator_actions.ml
index fb82bb6..2e40c39 100644
--- a/generator/generator_actions.ml
+++ b/generator/generator_actions.ml
@@ -3454,6 +3454,8 @@ are activated or deactivated.");
        ["umount"; "/"];
        ["lvresize"; "/dev/VG/LV"; "20"];
        ["e2fsck_f"; "/dev/VG/LV"];
+       ["e2fsck"; "/dev/VG/LV"; "true";
"false"];
+       ["e2fsck"; "/dev/VG/LV"; "false";
"true"];
        ["resize2fs"; "/dev/VG/LV"];
        ["mount_options"; ""; "/dev/VG/LV";
"/"];
        ["cat"; "/new"]], "test content");
@@ -6597,6 +6599,28 @@ The usage of this device, for example C<filesystem>
or C<raid>.
 
 =back");
 
+  ("e2fsck", (RErr, [Device "device"], [OBool
"correct"; OBool "forceall"]), 304, [],
+   [], (* lvresize tests this *)
+   "check an ext2/ext3 filesystem",
+   "\
+This runs the ext2/ext3 filesystem checker on C<device>.
+Force to check the filesystem even if it appears to be clean.
+
+=over 4
+
+=item C<correct>
+
+Automatically repair the file system. This option will cause e2fsck to
automatically
+fix any filesystem problems that can be safely fixed without human
intervention.
+This option may not be specified at the same time as the C<forceall>
option.
+
+=item C<forceall>
+
+Assume an answer of 'yes' to all questions; allows e2fsck to be used
non-interactively.
+This option may not be specified at the same time as the C<correct>
option.
+
+=back");
+
 ]
 
 let all_functions = non_daemon_functions @ daemon_functions
diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR
index 8160622..873b744 100644
--- a/src/MAX_PROC_NR
+++ b/src/MAX_PROC_NR
@@ -1 +1 @@
-303
+304
-- 
1.7.8

From: Wanlong Gao <gaowanlong at cn.fujitsu.com>

Since we implement the new api e2fsck, just change the
internal of e2fsck_f to use e2fsck now.
v1->v2: use optargs_bitmask

Signed-off-by: Wanlong Gao <gaowanlong at cn.fujitsu.com>
---
 daemon/ext2.c |   28 +++++++---------------------
 1 files changed, 7 insertions(+), 21 deletions(-)

diff --git a/daemon/ext2.c b/daemon/ext2.c
index 5a0ac9f..802d7c3 100644
--- a/daemon/ext2.c
+++ b/daemon/ext2.c
@@ -327,26 +327,6 @@ do_e2fsck (const char *device,
   ADD_ARG (argv, i, NULL);
 
   r = commandv (NULL, &err, argv);
-  if (r == -1 || r >= 2) {
-    reply_with_error ("%s", err);
-    free (err);
-    return -1;
-  }
-
-  free (err);
-  return 0;
-}
-
-int
-do_e2fsck_f (const char *device)
-{
-  char *err;
-  int r;
-
-  char prog[] = "e2fsck";
-  if (e2prog (prog) == -1)
-    return -1;
-
   /* 0 = no errors, 1 = errors corrected.
    *
    * >= 4 means uncorrected or other errors.
@@ -354,7 +334,6 @@ do_e2fsck_f (const char *device)
    * 2, 3 means errors were corrected and we require a reboot.  This is
    * a difficult corner case.
    */
-  r = commandr (NULL, &err, prog, "-p", "-f", device,
NULL);
   if (r == -1 || r >= 2) {
     reply_with_error ("%s", err);
     free (err);
@@ -366,6 +345,13 @@ do_e2fsck_f (const char *device)
 }
 
 int
+do_e2fsck_f (const char *device)
+{
+  optargs_bitmask = GUESTFS_E2FSCK_CORRECT_BITMASK;
+  return do_e2fsck (device, 1, 0);
+}
+
+int
 do_mke2journal (int blocksize, const char *device)
 {
   char *err;
-- 
1.7.8

On Sat, Jan 14, 2012 at 12:03:43AM +0800, Wanlong Gao
wrote:
> From: Wanlong Gao <gaowanlong at cn.fujitsu.com>
> 
> Tweak the error message "e2fsck -f" and "e2fsck -fy".
> Indicate the user to use the correct and/or forceall options.
> 
> Signed-off-by: Wanlong Gao <gaowanlong at cn.fujitsu.com>
> ---
>  daemon/ext2.c |    9 +++++++--
>  1 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/daemon/ext2.c b/daemon/ext2.c
> index 79fd354..c280ca2 100644
> --- a/daemon/ext2.c
> +++ b/daemon/ext2.c
> @@ -277,9 +277,14 @@ do_resize2fs_M (const char *device)
>    if (e2prog (prog) == -1)
>      return -1;
>  
> -  r = command (NULL, &err, prog, "-M" , device, NULL);
> +  r = command (NULL, &err, prog, "-M", device, NULL);
>    if (r == -1) {
> -    reply_with_error ("%s", err);
> +    if (strstr (err, "e2fsck -f")) {
> +      free (err);
> +      reply_with_error ("you need to run e2fsck with the correct
and/or forceall options first");
> +    } else {
> +      reply_with_error ("%s", err);
> +    }
>      free (err);
>      return -1;
>    }
ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top