4:41 a.m.
v3:
- rebase on upstream
- fix some comments
v2:
- add check for the length of lines[]
- the code parsing 'btrfs scrub -R status' output is changed into a loop
Hu Tao (2):
New API: btrfs_balance_status
New API: btfs_scrub_status
daemon/btrfs.c | 263 +++++++++++++++++++++++++++++++
generator/actions.ml | 26 +++
generator/structs.ml | 34 ++++
gobject/Makefile.inc | 4 +
java/Makefile.inc | 2 +
java/com/redhat/et/libguestfs/.gitignore | 2 +
po/POTFILES | 2 +
src/MAX_PROC_NR | 2 +-
8 files changed, 334 insertions(+), 1 deletion(-)
--
2.1.0
4:41 a.m.
New subject: [PATCH v3 1/2] New API: btrfs_balance_status
From: Hu Tao <hutao(a)cn.fujitsu.com>
Signed-off-by: Hu Tao <hutao(a)cn.fujitsu.com>
Signed-off-by: Chen Hanxiao <chenhanxiao(a)cn.fujitsu.com>
---
daemon/btrfs.c | 123 +++++++++++++++++++++++++++++++
generator/actions.ml | 10 +++
generator/structs.ml | 12 +++
gobject/Makefile.inc | 2 +
java/Makefile.inc | 1 +
java/com/redhat/et/libguestfs/.gitignore | 1 +
po/POTFILES | 1 +
src/MAX_PROC_NR | 2 +-
8 files changed, 151 insertions(+), 1 deletion(-)
diff --git a/daemon/btrfs.c b/daemon/btrfs.c
index 5cab52a..5cebd91 100644
--- a/daemon/btrfs.c
+++ b/daemon/btrfs.c
@@ -1659,3 +1659,126 @@ do_btrfs_rescue_super_recover (const char *device)
return 0;
}
+
+guestfs_int_btrfsbalance *
+do_btrfs_balance_status (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;
+ guestfs_int_btrfsbalance *ret;
+ char **lines;
+ size_t nlines;
+ const char *errptr;
+ int erroffset;
+#define N_MATCH 2
+ int ovector[N_MATCH * 3];
+ pcre *re = NULL;
+
+ 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, "balance");
+ ADD_ARG (argv, i, "status");
+ 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;
+ }
+
+ lines = split_lines (out);
+ if (!lines)
+ return NULL;
+
+ nlines = count_strings (lines);
+
+ ret = malloc(sizeof *ret);
+ if (ret == NULL) {
+ reply_with_perror ("malloc");
+ goto error;
+ }
+ memset (ret, 0, sizeof(*ret));
+
+ /* Output of `btrfs balance status' is like:
+ *
+ * running:
+ *
+ * Balance on '/' is running
+ * 3 out of about 8 chunks balanced (3 considered), 62% left
+ *
+ * paused:
+ *
+ * Balance on '/' is paused
+ * 3 out of about 8 chunks balanced (3 considered), 62% left
+ *
+ * no balance running:
+ *
+ * No Balance found on '/'
+ *
+ */
+ if (nlines < 1)
+ return NULL;
+ if (strstr (lines[0], "No balance found on")) {
+ ret->btrfsbalance_status = strdup("none");
+ if (ret->btrfsbalance_status == NULL) {
+ reply_with_perror ("strdup");
+ return NULL;
+ }
+ return ret;
+ }
+
+ re = pcre_compile ("Balance on '.*' is (.*)", 0, &errptr,
&erroffset, NULL);
+ if (re == NULL) {
+ reply_with_error ("pcre_compile (%i): %s", erroffset, errptr);
+ goto error;
+ }
+ if (pcre_exec (re, NULL, lines[0], strlen (lines[0]), 0, 0,
+ ovector, N_MATCH * 3) < 0) {
+ reply_with_error ("unexpected output from 'btrfs balance status'
command: %s", lines[0]);
+ goto error;
+ }
+#undef N_MATCH
+
+ if (STREQ (lines[0] + ovector[2], "running"))
+ ret->btrfsbalance_status = strdup("running");
+ else if (STREQ (lines[0] + ovector[2], "paused"))
+ ret->btrfsbalance_status = strdup("paused");
+ else {
+ reply_with_error ("unexpected output from 'btrfs balance status'
command: %s", lines[0]);
+ goto error;
+ }
+
+ if (nlines < 2) {
+ reply_with_error ("truncated output from 'btrfs balance status'
command");
+ goto error;
+ }
+
+ if (sscanf (lines[1], "%" SCNu64 " out of about %" SCNu64
+ " chunks balanced (%" SCNu64 " considered), %" SCNu64
"%% left",
+ &ret->btrfsbalance_balanced, &ret->btrfsbalance_total,
+ &ret->btrfsbalance_considered, &ret->btrfsbalance_left) != 4)
{
+ reply_with_perror ("sscanf");
+ goto error;
+ }
+
+ pcre_free (re);
+ return ret;
+
+error:
+ free (ret->btrfsbalance_status);
+ free (ret);
+ pcre_free (re);
+
+ return NULL;
+}
diff --git a/generator/actions.ml b/generator/actions.ml
index 6b37712..5301a29 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -12449,6 +12449,16 @@ valid GUID." };
longdesc = "\
Return the GUID of numbered GPT partition C<partnum>." };
+{ defaults with
+ name = "btrfs_balance_status";
+ style = RStruct ("status", "btrfsbalance"), [Pathname
"path"], [];
+ proc_nr = Some 448;
+ optional = Some "btrfs"; camel_name = "BTRFSBalanceStatus";
+ test_excuse = "test disk isn't large enough that btrfs_balance completes
before we can get its status";
+ shortdesc = "show the status of a running or paused balance";
+ longdesc = "\
+Show the status of a running or paused balance on a btrfs filesystem." };
+
]
(* Non-API meta-commands available only in guestfish.
diff --git a/generator/structs.ml b/generator/structs.ml
index 5b466a2..af42529 100644
--- a/generator/structs.ml
+++ b/generator/structs.ml
@@ -340,6 +340,18 @@ let structs = [
];
s_camel_name = "BTRFSQgroup" };
+ (* btrfs balance status output *)
+ { defaults with
+ s_name = "btrfsbalance";
+ s_cols = [
+ "btrfsbalance_status", FString;
+ "btrfsbalance_total", FUInt64;
+ "btrfsbalance_balanced", FUInt64;
+ "btrfsbalance_considered", FUInt64;
+ "btrfsbalance_left", FUInt64;
+ ];
+ s_camel_name = "BTRFSBalance" };
+
(* XFS info descriptor. *)
{ defaults with
s_name = "xfsinfo";
diff --git a/gobject/Makefile.inc b/gobject/Makefile.inc
index e0c0a5f..3ce10f9 100644
--- a/gobject/Makefile.inc
+++ b/gobject/Makefile.inc
@@ -25,6 +25,7 @@ guestfs_gobject_headers= \
include/guestfs-gobject/tristate.h \
include/guestfs-gobject/struct-application.h \
include/guestfs-gobject/struct-application2.h \
+ include/guestfs-gobject/struct-btrfsbalance.h \
include/guestfs-gobject/struct-btrfsqgroup.h \
include/guestfs-gobject/struct-btrfssubvolume.h \
include/guestfs-gobject/struct-dirent.h \
@@ -108,6 +109,7 @@ guestfs_gobject_sources= \
src/tristate.c \
src/struct-application.c \
src/struct-application2.c \
+ src/struct-btrfsbalance.c \
src/struct-btrfsqgroup.c \
src/struct-btrfssubvolume.c \
src/struct-dirent.c \
diff --git a/java/Makefile.inc b/java/Makefile.inc
index 103010f..499d3de 100644
--- a/java/Makefile.inc
+++ b/java/Makefile.inc
@@ -22,6 +22,7 @@
java_built_sources = \
com/redhat/et/libguestfs/Application.java \
com/redhat/et/libguestfs/Application2.java \
+ com/redhat/et/libguestfs/BTRFSBalance.java \
com/redhat/et/libguestfs/BTRFSQgroup.java \
com/redhat/et/libguestfs/BTRFSSubvolume.java \
com/redhat/et/libguestfs/Dirent.java \
diff --git a/java/com/redhat/et/libguestfs/.gitignore
b/java/com/redhat/et/libguestfs/.gitignore
index 1d4accc..b2b1b22 100644
--- a/java/com/redhat/et/libguestfs/.gitignore
+++ b/java/com/redhat/et/libguestfs/.gitignore
@@ -1,5 +1,6 @@
Application.java
Application2.java
+BTRFSBalance.java
BTRFSQgroup.java
BTRFSSubvolume.java
Dirent.java
diff --git a/po/POTFILES b/po/POTFILES
index 6e65377..3f66c9f 100644
--- a/po/POTFILES
+++ b/po/POTFILES
@@ -230,6 +230,7 @@ gobject/src/optargs-xfs_repair.c
gobject/src/session.c
gobject/src/struct-application.c
gobject/src/struct-application2.c
+gobject/src/struct-btrfsbalance.c
gobject/src/struct-btrfsqgroup.c
gobject/src/struct-btrfssubvolume.c
gobject/src/struct-dirent.c
diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR
index e9b7520..5379c47 100644
--- a/src/MAX_PROC_NR
+++ b/src/MAX_PROC_NR
@@ -1 +1 @@
-447
+448
--
2.1.0
8 a.m.
New subject: [PATCH v3 1/2] New API: btrfs_balance_status
On Wed, Feb 11, 2015 at 06:41:19PM +0800, Chen Hanxiao wrote:
+ if (nlines < 1)
+ return NULL;
This error handling is wrong. It would cause the protocol between the
appliance and library to become desynchronized. You have to call one
of the error functions -- eg. `reply_with_error' before returning
NULL.
The rest of this patch looks fine, if this change was made.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
1:53 a.m.
New subject: [PATCH v3 1/2] New API: btrfs_balance_status
-----Original Message-----
From: Richard W.M. Jones [mailto:rjones@redhat.com]
Sent: Wednesday, February 11, 2015 10:00 PM
To: Chen, Hanxiao/陈 晗霄
Cc: libguestfs(a)redhat.com
Subject: Re: [Libguestfs] [PATCH v3 1/2] New API: btrfs_balance_status
On Wed, Feb 11, 2015 at 06:41:19PM +0800, Chen Hanxiao wrote:
> + if (nlines < 1)
> + return NULL;
This error handling is wrong. It would cause the protocol between the
appliance and library to become desynchronized. You have to call one
of the error functions -- eg. `reply_with_error' before returning
NULL.
The rest of this patch looks fine, if this change was made.
Will be fixed in next version.
Thanks,
- Chen
4:41 a.m.
New subject: [PATCH v3 2/2] New API: btfs_scrub_status
From: Hu Tao <hutao(a)cn.fujitsu.com>
Signed-off-by: Hu Tao <hutao(a)cn.fujitsu.com>
Signed-off-by: Chen Hanxiao <chenhanxiao(a)cn.fujitsu.com>
---
daemon/btrfs.c | 140 +++++++++++++++++++++++++++++++
generator/actions.ml | 16 ++++
generator/structs.ml | 22 +++++
gobject/Makefile.inc | 2 +
java/Makefile.inc | 1 +
java/com/redhat/et/libguestfs/.gitignore | 1 +
po/POTFILES | 1 +
src/MAX_PROC_NR | 2 +-
8 files changed, 184 insertions(+), 1 deletion(-)
diff --git a/daemon/btrfs.c b/daemon/btrfs.c
index 5cebd91..e16aa13 100644
--- a/daemon/btrfs.c
+++ b/daemon/btrfs.c
@@ -1782,3 +1782,143 @@ error:
return NULL;
}
+
+guestfs_int_btrfsscrub *
+do_btrfs_scrub_status (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;
+ guestfs_int_btrfsscrub *ret;
+ char **lines;
+
+ 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, "scrub");
+ ADD_ARG (argv, i, "status");
+ ADD_ARG (argv, i, "-R");
+ 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;
+ }
+
+ if (verbose)
+ fprintf (stderr, "output from 'btrfs scrub status -R %s' is:\n%s",
path, out);
+
+ lines = split_lines (out);
+ if (!lines)
+ return NULL;
+
+ if (count_strings (lines) < 2) {
+ reply_with_error ("truncated output from 'btrfs scrub status -R'
command");
+ return NULL;
+ }
+
+ ret = malloc (sizeof *ret);
+ if (ret == NULL) {
+ reply_with_perror ("malloc");
+ return NULL;
+ }
+ memset (ret, 0, sizeof(*ret));
+
+ /* Output of `btrfs scrub -R status' is like:
+ *
+ * scrub status for 346121d1-1847-40f8-9b7b-2bf3d539c68f
+ * scrub started at Mon Feb 2 17:39:38 2015, running for 93 seconds
+ * data_extents_scrubbed: 136670
+ * tree_extents_scrubbed: 30023
+ * data_bytes_scrubbed: 4474441728
+ * tree_bytes_scrubbed: 491896832
+ * read_errors: 0
+ * csum_errors: 0
+ * verify_errors: 0
+ * no_csum: 17760
+ * csum_discards: 197622
+ * super_errors: 0
+ * malloc_errors: 0
+ * uncorrectable_errors: 0
+ * unverified_errors: 0
+ * corrected_errors: 0
+ * last_physical: 10301341696
+ *
+ * or:
+ *
+ * scrub status for 346121d1-1847-40f8-9b7b-2bf3d539c68f
+ * no stats available
+ */
+ for (i = 1; lines[i] != NULL; ++i) {
+ if ((i == 1) && STREQ (lines[i], "\tno stats available"))
+ return ret;
+ else if ((i == 2) && sscanf (lines[i], "\tdata_extents_scrubbed: %"
SCNu64,
+ &ret->btrfsscrub_data_extents_scrubbed) != 1)
+ goto error;
+ else if ((i == 3) && sscanf (lines[i], "\ttree_extents_scrubbed: %"
SCNu64,
+ &ret->btrfsscrub_tree_extents_scrubbed) != 1)
+ goto error;
+ else if ((i == 4) && sscanf (lines[i], "\tdata_bytes_scrubbed: %"
SCNu64,
+ &ret->btrfsscrub_data_bytes_scrubbed) != 1)
+ goto error;
+ else if ((i == 5) && sscanf (lines[i], "\ttree_bytes_scrubbed: %"
SCNu64,
+ &ret->btrfsscrub_tree_bytes_scrubbed) != 1)
+ goto error;
+ else if ((i == 6) && sscanf (lines[i], "\tread_errors: %" SCNu64,
+ &ret->btrfsscrub_read_errors) != 1)
+ goto error;
+ else if ((i == 7) && sscanf (lines[i], "\tcsum_errors: %" SCNu64,
+ &ret->btrfsscrub_csum_errors) != 1)
+ goto error;
+ else if ((i == 8) && sscanf (lines[i], "\tverify_errors: %"
SCNu64,
+ &ret->btrfsscrub_verify_errors) != 1)
+ goto error;
+ else if ((i == 9) && sscanf (lines[i], "\tno_csum: %" SCNu64,
+ &ret->btrfsscrub_no_csum) != 1)
+ goto error;
+ else if ((i == 10) && sscanf (lines[i], "\tcsum_discards: %"
SCNu64,
+ &ret->btrfsscrub_csum_discards) != 1)
+ goto error;
+ else if ((i == 11) && sscanf (lines[i], "\tsuper_errors: %"
SCNu64,
+ &ret->btrfsscrub_super_errors) != 1)
+ goto error;
+ else if ((i == 12) && sscanf (lines[i], "\tmalloc_errors: %"
SCNu64,
+ &ret->btrfsscrub_malloc_errors) != 1)
+ goto error;
+ else if ((i == 13) && sscanf (lines[i], "\tuncorrectable_errors: %"
SCNu64,
+ &ret->btrfsscrub_uncorrectable_errors) != 1)
+ goto error;
+ else if ((i == 14) && sscanf (lines[i], "\tunverified_errors: %"
SCNu64,
+ &ret->btrfsscrub_unverified_errors) != 1)
+ goto error;
+ else if ((i == 15) && sscanf (lines[i], "\tcorrected_errors: %"
SCNu64,
+ &ret->btrfsscrub_corrected_errors) != 1)
+ goto error;
+ else if ((i == 16) && sscanf (lines[i], "\tlast_physical: %"
SCNu64,
+ &ret->btrfsscrub_last_physical) != 1)
+ goto error;
+ }
+
+ if (i < 17) {
+ reply_with_error ("truncated output from 'btrfs scrub status -R'
command");
+ free (ret);
+ return NULL;
+ }
+
+ return ret;
+
+error:
+ reply_with_error ("%s: could not parse btrfs scrub status.", lines[i]);
+ free (ret);
+ return NULL;
+}
diff --git a/generator/actions.ml b/generator/actions.ml
index 5301a29..f82ac32 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -12459,6 +12459,22 @@ Return the GUID of numbered GPT partition C<partnum>."
};
longdesc = "\
Show the status of a running or paused balance on a btrfs filesystem." };
+ { defaults with
+ name = "btrfs_scrub_status";
+ style = RStruct ("status", "btrfsscrub"), [Pathname
"path"], [];
+ proc_nr = Some 449;
+ optional = Some "btrfs"; camel_name = "BTRFSScrubStatus";
+ tests = [
+ InitPartition, Always, TestRun (
+ [["mkfs_btrfs"; "/dev/sda1"; ""; "";
"NOARG"; ""; "NOARG"; "NOARG"; "";
""];
+ ["mount"; "/dev/sda1"; "/"];
+ ["btrfs_scrub_start"; "/"];
+ ["btrfs_scrub_status"; "/"]]), [];
+ ];
+ shortdesc = "show status of running or finished scrub";
+ longdesc = "\
+Show status of running or finished scrub on a btrfs filesystem." };
+
]
(* Non-API meta-commands available only in guestfish.
diff --git a/generator/structs.ml b/generator/structs.ml
index af42529..ea110a1 100644
--- a/generator/structs.ml
+++ b/generator/structs.ml
@@ -352,6 +352,28 @@ let structs = [
];
s_camel_name = "BTRFSBalance" };
+ (* btrfs scrub status output *)
+ { defaults with
+ s_name = "btrfsscrub";
+ s_cols = [
+ "btrfsscrub_data_extents_scrubbed", FUInt64;
+ "btrfsscrub_tree_extents_scrubbed", FUInt64;
+ "btrfsscrub_data_bytes_scrubbed", FUInt64;
+ "btrfsscrub_tree_bytes_scrubbed", FUInt64;
+ "btrfsscrub_read_errors", FUInt64;
+ "btrfsscrub_csum_errors", FUInt64;
+ "btrfsscrub_verify_errors", FUInt64;
+ "btrfsscrub_no_csum", FUInt64;
+ "btrfsscrub_csum_discards", FUInt64;
+ "btrfsscrub_super_errors", FUInt64;
+ "btrfsscrub_malloc_errors", FUInt64;
+ "btrfsscrub_uncorrectable_errors", FUInt64;
+ "btrfsscrub_unverified_errors", FUInt64;
+ "btrfsscrub_corrected_errors", FUInt64;
+ "btrfsscrub_last_physical", FUInt64;
+ ];
+ s_camel_name = "BTRFSScrub" };
+
(* XFS info descriptor. *)
{ defaults with
s_name = "xfsinfo";
diff --git a/gobject/Makefile.inc b/gobject/Makefile.inc
index 3ce10f9..6bbd72c 100644
--- a/gobject/Makefile.inc
+++ b/gobject/Makefile.inc
@@ -27,6 +27,7 @@ guestfs_gobject_headers= \
include/guestfs-gobject/struct-application2.h \
include/guestfs-gobject/struct-btrfsbalance.h \
include/guestfs-gobject/struct-btrfsqgroup.h \
+ include/guestfs-gobject/struct-btrfsscrub.h \
include/guestfs-gobject/struct-btrfssubvolume.h \
include/guestfs-gobject/struct-dirent.h \
include/guestfs-gobject/struct-hivex_node.h \
@@ -111,6 +112,7 @@ guestfs_gobject_sources= \
src/struct-application2.c \
src/struct-btrfsbalance.c \
src/struct-btrfsqgroup.c \
+ src/struct-btrfsscrub.c \
src/struct-btrfssubvolume.c \
src/struct-dirent.c \
src/struct-hivex_node.c \
diff --git a/java/Makefile.inc b/java/Makefile.inc
index 499d3de..477842a 100644
--- a/java/Makefile.inc
+++ b/java/Makefile.inc
@@ -24,6 +24,7 @@ java_built_sources = \
com/redhat/et/libguestfs/Application2.java \
com/redhat/et/libguestfs/BTRFSBalance.java \
com/redhat/et/libguestfs/BTRFSQgroup.java \
+ com/redhat/et/libguestfs/BTRFSScrub.java \
com/redhat/et/libguestfs/BTRFSSubvolume.java \
com/redhat/et/libguestfs/Dirent.java \
com/redhat/et/libguestfs/HivexNode.java \
diff --git a/java/com/redhat/et/libguestfs/.gitignore
b/java/com/redhat/et/libguestfs/.gitignore
index b2b1b22..8389269 100644
--- a/java/com/redhat/et/libguestfs/.gitignore
+++ b/java/com/redhat/et/libguestfs/.gitignore
@@ -2,6 +2,7 @@ Application.java
Application2.java
BTRFSBalance.java
BTRFSQgroup.java
+BTRFSScrub.java
BTRFSSubvolume.java
Dirent.java
HivexNode.java
diff --git a/po/POTFILES b/po/POTFILES
index 3f66c9f..f1c42fe 100644
--- a/po/POTFILES
+++ b/po/POTFILES
@@ -232,6 +232,7 @@ gobject/src/struct-application.c
gobject/src/struct-application2.c
gobject/src/struct-btrfsbalance.c
gobject/src/struct-btrfsqgroup.c
+gobject/src/struct-btrfsscrub.c
gobject/src/struct-btrfssubvolume.c
gobject/src/struct-dirent.c
gobject/src/struct-hivex_node.c
diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR
index 5379c47..2b20fd0 100644
--- a/src/MAX_PROC_NR
+++ b/src/MAX_PROC_NR
@@ -1 +1 @@
-448
+449
--
2.1.0
8:03 a.m.
New subject: [PATCH v3 2/2] New API: btfs_scrub_status
On Wed, Feb 11, 2015 at 06:41:20PM +0800, Chen Hanxiao wrote:
+ /* Output of `btrfs scrub -R status' is like:
+ *
+ * scrub status for 346121d1-1847-40f8-9b7b-2bf3d539c68f
+ * scrub started at Mon Feb 2 17:39:38 2015, running for 93 seconds
+ * data_extents_scrubbed: 136670
+ * tree_extents_scrubbed: 30023
+ * data_bytes_scrubbed: 4474441728
+ * tree_bytes_scrubbed: 491896832
+ * read_errors: 0
+ * csum_errors: 0
+ * verify_errors: 0
+ * no_csum: 17760
+ * csum_discards: 197622
+ * super_errors: 0
+ * malloc_errors: 0
+ * uncorrectable_errors: 0
+ * unverified_errors: 0
+ * corrected_errors: 0
+ * last_physical: 10301341696
+ *
+ * or:
+ *
+ * scrub status for 346121d1-1847-40f8-9b7b-2bf3d539c68f
+ * no stats available
+ */
+ for (i = 1; lines[i] != NULL; ++i) {
+ if ((i == 1) && STREQ (lines[i], "\tno stats available"))
+ return ret;
+ else if ((i == 2) && sscanf (lines[i], "\tdata_extents_scrubbed:
%" SCNu64,
+ &ret->btrfsscrub_data_extents_scrubbed) != 1)
+ goto error;
+ else if ((i == 3) && sscanf (lines[i], "\ttree_extents_scrubbed:
%" SCNu64,
+ &ret->btrfsscrub_tree_extents_scrubbed) != 1)
+ goto error;
[etc]
In my previous comment[1], I said it's better to turn this into a
loop, and that's what you've done.
[1] https://www.redhat.com/archives/libguestfs/2015-February/msg00008.html
Unfortunately testing `i == 2' etc means the loop isn't useful. The
code is effectively the same as the old code. It's still fragile if
the output of scrub status changes slightly.
I suggest removing the `i == X' tests.
Rest of the patch looks OK.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
3:22 a.m.
New subject: [PATCH v3 2/2] New API: btfs_scrub_status
-----Original Message-----
From: Richard W.M. Jones [mailto:rjones@redhat.com]
Sent: Wednesday, February 11, 2015 10:04 PM
To: Chen, Hanxiao/陈 晗霄
Cc: libguestfs(a)redhat.com
Subject: Re: [Libguestfs] [PATCH v3 2/2] New API: btfs_scrub_status
On Wed, Feb 11, 2015 at 06:41:20PM +0800, Chen Hanxiao wrote:
> + /* Output of `btrfs scrub -R status' is like:
> + *
> + * scrub status for 346121d1-1847-40f8-9b7b-2bf3d539c68f
> + * scrub started at Mon Feb 2 17:39:38 2015, running for 93 seconds
> + * data_extents_scrubbed: 136670
> + * tree_extents_scrubbed: 30023
> + * data_bytes_scrubbed: 4474441728
> + * tree_bytes_scrubbed: 491896832
> + * read_errors: 0
> + * csum_errors: 0
> + * verify_errors: 0
> + * no_csum: 17760
> + * csum_discards: 197622
> + * super_errors: 0
> + * malloc_errors: 0
> + * uncorrectable_errors: 0
> + * unverified_errors: 0
> + * corrected_errors: 0
> + * last_physical: 10301341696
> + *
> + * or:
> + *
> + * scrub status for 346121d1-1847-40f8-9b7b-2bf3d539c68f
> + * no stats available
> + */
> + for (i = 1; lines[i] != NULL; ++i) {
> + if ((i == 1) && STREQ (lines[i], "\tno stats available"))
> + return ret;
> + else if ((i == 2) && sscanf (lines[i], "\tdata_extents_scrubbed:
%" SCNu64,
> + &ret->btrfsscrub_data_extents_scrubbed) !=
1)
> + goto error;
> + else if ((i == 3) && sscanf (lines[i], "\ttree_extents_scrubbed:
%" SCNu64,
> + &ret->btrfsscrub_tree_extents_scrubbed) !=
1)
> + goto error;
[etc]
In my previous comment[1], I said it's better to turn this into a
loop, and that's what you've done.
[1] https://www.redhat.com/archives/libguestfs/2015-February/msg00008.html
Unfortunately testing `i == 2' etc means the loop isn't useful. The
code is effectively the same as the old code. It's still fragile if
the output of scrub status changes slightly.
I suggest removing the `i == X' tests.
Will do.
Thanks,
- Chen
Rest of the patch looks OK.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
3494
days inactive
3496
days old
6 comments
3 participants
participants (3)
-
Chen Hanxiao -
Chen, Hanxiao
-
Richard W.M. Jones