[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
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
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
participants (2)
-
Cao jin -
Pino Toscano