On Tuesday 16 June 2015 17:10:09 Cao jin wrote:
Also modified a public function: analyze_line, make it more flexible
The addition of the parameter to analyze_line should be in an own
patch. Can you please decouple it?
Signed-off-by: Cao jin <caoj.fnst(a)cn.fujitsu.com>
---
daemon/btrfs.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++-----
generator/actions.ml | 12 ++++++++
2 files changed, 88 insertions(+), 7 deletions(-)
diff --git a/daemon/btrfs.c b/daemon/btrfs.c
index 39392f7..f913f66 100644
--- a/daemon/btrfs.c
+++ b/daemon/btrfs.c
@@ -853,11 +853,11 @@ 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 delim)
{
char *p = line;
char *next = NULL;
- char delimiter = ':';
+ char delimiter = delim;
char *del_pos = NULL;
if (!line || *line == '\0') {
Why keep the local variable 'delimiter'? Just name the argument like
that.
@@ -964,7 +964,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 +984,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 +994,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 +1008,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 +1031,7 @@ do_btrfs_subvolume_show (const char *subvolume)
return NULL;
}
- p = analyze_line(p, &key, &value);
+ p = analyze_line(p, &key, &value, ':');
}
}
@@ -2083,3 +2083,72 @@ do_btrfs_image (char *const *sources, const char *image,
return 0;
}
+
+char **
+do_btrfs_device_stats (const char *path, int zero)
+{
+ 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);
'ret' is leaked if returning before "return ret.argv".
+ 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);
+
+ if ((optargs_bitmask & GUESTFS_BTRFS_DEVICE_STATS_ZERO_BITMASK) && zero)
{
+ ADD_ARG (argv, i, "-z");
+ }
+
+ 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
+ *
+ */
+
+ /* Read the lines and split into "key: value". */
+ p = analyze_line(out, &key, &value, ' ');
+ while(key)
+ {
Curly bracket goes to the line before, and please remember the space
between function and opening round bracket...
+ if (add_string (&ret, key) == -1)
+ return NULL;
+
+ if (add_string (&ret, value) == -1)
+ return NULL;
+
+ p = analyze_line(p, &key, &value, ' ');
+ }
This means that the return "hash" will have keys like:
[/dev/sda].write_io_errs
? Wouldn't it better to just return the name of the attribute, i.e.
write_io_errs
?
diff --git a/generator/actions.ml b/generator/actions.ml
index d5e5ccf..dfb42b5 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -12593,6 +12593,18 @@ 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, 46);
+ style = RHashtable "devicestats", [Dev_or_Path "path"], [OBool
"zero"];
+ 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>.
+
+If you want to reset stats to zero after reading them, use option: zero:true." };
The 'zero' parameter adds side-effects to a "query" API, and this does
not seem too good.
If you want a way to reset this kind of btrfs metadata, then please
add a separate / well distinct API for it. Just because btrfs
subcommands have that awkward parameters, it doesn't mean we need to
map them 1:1 in libguestfs.
Thanks,
--
Pino Toscano