thr3ads.net - Libguestfs - [Libguestfs] [PATCH v3 1/2] Modify the function: analyze

If this information is useful, please help other people find it:
Share via:

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

Cao jin

2015-Jun-19 06:18 UTC

head link

[Libguestfs] [PATCH v3 2/2] New API: btrfs_device_stats

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

head link

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

head link

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

Maybe Matching Threads

Search for more apparently analagous threads

Libguestfs - Jun 2015 - [PATCH v3 1/2] Modify the function: analyze_line, make it more flexible

[Libguestfs] [PATCH v3 1/2] Modify the function: analyze_line, make it more flexible

[Libguestfs] [PATCH v3 2/2] New API: btrfs_device_stats

Re: [Libguestfs] [PATCH v3 1/2] Modify the function: analyze_line, make it more flexible

Re: [Libguestfs] [PATCH v3 2/2] New API: btrfs_device_stats

Maybe Matching Threads