Cao jin
2015-Jun-19 06:18 UTC
[Libguestfs] [PATCH v3 1/2] Modify the function: analyze_line, make it more flexible
Mofify the function from a fixed delimiter to a variable. So,
it can be used in more APIs later. Also modified the existing
callers
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
---
daemon/btrfs.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/daemon/btrfs.c b/daemon/btrfs.c
index 39392f7..caa28ca 100644
--- a/daemon/btrfs.c
+++ b/daemon/btrfs.c
@@ -853,11 +853,10 @@ do_btrfs_fsck (const char *device, int64_t superblock, int
repair)
* returns the next position following \n.
*/
static char *
-analyze_line (char *line, char **key, char **value)
+analyze_line (char *line, char **key, char **value, char delimiter)
{
char *p = line;
char *next = NULL;
- char delimiter = ':';
char *del_pos = NULL;
if (!line || *line == '\0') {
@@ -964,7 +963,7 @@ do_btrfs_subvolume_show (const char *subvolume)
* snapshots/test3
*
*/
- p = analyze_line(out, &key, &value);
+ p = analyze_line(out, &key, &value, ':');
if (!p) {
reply_with_error ("truncated output: %s", out);
return NULL;
@@ -984,7 +983,7 @@ do_btrfs_subvolume_show (const char *subvolume)
}
/* Read the lines and split into "key: value". */
- p = analyze_line(p, &key, &value);
+ p = analyze_line(p, &key, &value, ':');
while (key) {
/* snapshot is special, see the output above */
if (STREQLEN (key, "Snapshot(s)", sizeof
("Snapshot(s)") - 1)) {
@@ -994,7 +993,7 @@ do_btrfs_subvolume_show (const char *subvolume)
if (add_string (&ret, key) == -1)
return NULL;
- p = analyze_line(p, &key, &value);
+ p = analyze_line(p, &key, &value, ':');
while (key && !value) {
ss = realloc (ss, ss_len + strlen (key) + 1);
@@ -1008,7 +1007,7 @@ do_btrfs_subvolume_show (const char *subvolume)
ss_len += strlen (key);
ss[ss_len] = '\0';
- p = analyze_line(p, &key, &value);
+ p = analyze_line(p, &key, &value, ':');
}
if (ss) {
@@ -1031,7 +1030,7 @@ do_btrfs_subvolume_show (const char *subvolume)
return NULL;
}
- p = analyze_line(p, &key, &value);
+ p = analyze_line(p, &key, &value, ':');
}
}
@@ -2083,3 +2082,4 @@ do_btrfs_image (char *const *sources, const char *image,
return 0;
}
+
--
2.1.0
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
---
daemon/btrfs.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++
generator/actions.ml | 10 ++++++++
2 files changed, 81 insertions(+)
diff --git a/daemon/btrfs.c b/daemon/btrfs.c
index caa28ca..4288fc9 100644
--- a/daemon/btrfs.c
+++ b/daemon/btrfs.c
@@ -2083,3 +2083,74 @@ do_btrfs_image (char *const *sources, const char *image,
return 0;
}
+char **
+do_btrfs_device_stats (const char *path)
+{
+ const size_t MAX_ARGS = 64;
+ const char *argv[MAX_ARGS];
+ size_t i = 0;
+ CLEANUP_FREE char *buf = NULL;
+ CLEANUP_FREE char *err = NULL;
+ CLEANUP_FREE char *out = NULL;
+ char *p, *key = NULL, *value = NULL;
+ DECLARE_STRINGSBUF (ret);
+ int r;
+ int is_dev;
+
+ is_dev = STREQLEN (path, "/dev/", 5);
+ buf = is_dev ? strdup (path) : sysroot_path (path);
+ if (buf == NULL) {
+ reply_with_perror ("malloc");
+ return NULL;
+ }
+
+ ADD_ARG (argv, i, str_btrfs);
+ ADD_ARG (argv, i, "device");
+ ADD_ARG (argv, i, "stats");
+ ADD_ARG (argv, i, buf);
+ ADD_ARG (argv, i, NULL);
+
+ r = commandv (&out, &err, argv);
+ if (r == -1) {
+ reply_with_error ("%s: %s", path, err);
+ return NULL;
+ }
+
+ /* Output pattern is:
+ * [/dev/sda].write_io_errs 0
+ * [/dev/sda].read_io_errs 0
+ * [/dev/sda].flush_io_errs 0
+ * [/dev/sda].corruption_errs 0
+ * [/dev/sda].generation_errs 0
+ * [/dev/sdb].write_io_errs 0
+ * [/dev/sdb].read_io_errs 0
+ * [/dev/sdb].flush_io_errs 0
+ * [/dev/sdb].corruption_errs 0
+ * [/dev/sdb].generation_errs 0
+ * [/dev/sdc]...
+ * ...
+ */
+
+ /* Read the lines and split into "key: value". */
+ p = analyze_line(out, &key, &value, ' ');
+ while (key) {
+ if (add_string (&ret, key) == -1)
+ goto error;
+
+ if (add_string (&ret, value) == -1)
+ goto error;
+
+ p = analyze_line(p, &key, &value, ' ');
+ }
+
+ if (end_stringsbuf (&ret) == -1)
+ goto error;
+
+ return ret.argv;
+
+error:
+ if (ret.argv != NULL)
+ free_stringslen (ret.argv, ret.size);
+
+ return NULL;
+}
diff --git a/generator/actions.ml b/generator/actions.ml
index d5e5ccf..8d279fd 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -12593,6 +12593,16 @@ numbered C<partnum> on device C<device>.
It returns C<primary>, C<logical>, or C<extended>." };
+ { defaults with
+ name = "btrfs_device_stats"; added = (1, 29, 47);
+ style = RHashtable "devicestats", [Dev_or_Path "path"],
[];
+ proc_nr = Some 456;
+ optional = Some "btrfs"; camel_name =
"BTRFSDeviceStats";
+ shortdesc = "read and print the device IO stats for mounted btrfs
device";
+ longdesc = "\
+Read and print the device IO stats for all mounted devices of the filesystem
+identified by C<path>, or for a single device identified by
C<path>." };
+
]
(* Non-API meta-commands available only in guestfish.
--
2.1.0
Pino Toscano
2015-Jun-23 10:19 UTC
Re: [Libguestfs] [PATCH v3 1/2] Modify the function: analyze_line, make it more flexible
In data venerdì 19 giugno 2015 14:18:21, Cao jin ha scritto:> Mofify the function from a fixed delimiter to a variable. So, > it can be used in more APIs later. Also modified the existing > callers > > Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com> > --- > daemon/btrfs.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-)This seems fine, can you please rebase it to upstream so I can apply it?> > diff --git a/daemon/btrfs.c b/daemon/btrfs.c > index 39392f7..caa28ca 100644 > --- a/daemon/btrfs.c > +++ b/daemon/btrfs.c > @@ -2083,3 +2082,4 @@ do_btrfs_image (char *const *sources, const char *image, > > return 0; > } > + >Can you please remove this extra line addition, while you are there? :) Thanks, -- Pino Toscano
Pino Toscano
2015-Jun-23 10:26 UTC
Re: [Libguestfs] [PATCH v3 2/2] New API: btrfs_device_stats
In data venerdì 19 giugno 2015 14:18:22, Cao jin ha scritto:> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com> > --- > daemon/btrfs.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > generator/actions.ml | 10 ++++++++ > 2 files changed, 81 insertions(+) > > diff --git a/daemon/btrfs.c b/daemon/btrfs.c > index caa28ca..4288fc9 100644 > --- a/daemon/btrfs.c > +++ b/daemon/btrfs.c > @@ -2083,3 +2083,74 @@ do_btrfs_image (char *const *sources, const char *image, > return 0; > } > > +char ** > +do_btrfs_device_stats (const char *path) > +{ > + const size_t MAX_ARGS = 64; > + const char *argv[MAX_ARGS]; > + size_t i = 0; > + CLEANUP_FREE char *buf = NULL; > + CLEANUP_FREE char *err = NULL; > + CLEANUP_FREE char *out = NULL; > + char *p, *key = NULL, *value = NULL; > + DECLARE_STRINGSBUF (ret);Now that my stringsbuf improvements in the daemon are in, you can make use of them to avoid leaking stringsbuf's. Declare the stringsbuf as: CLEANUP_FREE_STRINGSBUF DECLARE_STRINGSBUF (ret); and then when you want to return the strings in it, call e.g.: return take_stringsbuf (&ret);> + int r; > + int is_dev; > + > + is_dev = STREQLEN (path, "/dev/", 5); > + buf = is_dev ? strdup (path) : sysroot_path (path); > + if (buf == NULL) { > + reply_with_perror ("malloc"); > + return NULL; > + } > + > + ADD_ARG (argv, i, str_btrfs); > + ADD_ARG (argv, i, "device"); > + ADD_ARG (argv, i, "stats"); > + ADD_ARG (argv, i, buf); > + ADD_ARG (argv, i, NULL); > + > + r = commandv (&out, &err, argv); > + if (r == -1) { > + reply_with_error ("%s: %s", path, err); > + return NULL; > + } > + > + /* Output pattern is: > + * [/dev/sda].write_io_errs 0 > + * [/dev/sda].read_io_errs 0 > + * [/dev/sda].flush_io_errs 0 > + * [/dev/sda].corruption_errs 0 > + * [/dev/sda].generation_errs 0 > + * [/dev/sdb].write_io_errs 0 > + * [/dev/sdb].read_io_errs 0 > + * [/dev/sdb].flush_io_errs 0 > + * [/dev/sdb].corruption_errs 0 > + * [/dev/sdb].generation_errs 0 > + * [/dev/sdc]... > + * ... > + */ > + > + /* Read the lines and split into "key: value". */ > + p = analyze_line(out, &key, &value, ' '); > + while (key) { > + if (add_string (&ret, key) == -1) > + goto error; > + > + if (add_string (&ret, value) == -1) > + goto error; > + > + p = analyze_line(p, &key, &value, ' '); > + } > + > + if (end_stringsbuf (&ret) == -1) > + goto error; > + > + return ret.argv; > + > +error: > + if (ret.argv != NULL) > + free_stringslen (ret.argv, ret.size);With the above changes, this label is no more needed.> + > + return NULL; > +} > diff --git a/generator/actions.ml b/generator/actions.ml > index d5e5ccf..8d279fd 100644 > --- a/generator/actions.ml > +++ b/generator/actions.ml > @@ -12593,6 +12593,16 @@ numbered C<partnum> on device C<device>. > > It returns C<primary>, C<logical>, or C<extended>." }; > > + { defaults with > + name = "btrfs_device_stats"; added = (1, 29, 47); > + style = RHashtable "devicestats", [Dev_or_Path "path"], []; > + proc_nr = Some 456; > + optional = Some "btrfs"; camel_name = "BTRFSDeviceStats"; > + shortdesc = "read and print the device IO stats for mounted btrfs device"; > + longdesc = "\ > +Read and print the device IO stats for all mounted devices of the filesystem > +identified by C<path>, or for a single device identified by C<path>." };"I/O" I'm still not convinced that the return value of this new API is good, leaving its users to parse again strings that may change because of changes in future outputs of `btrfs device stats`. I understand that you want/need it, but we have to find an acceptable structure for it. -- Pino Toscano