[Libguestfs] [PATCH] New API: btrfs_device_stats

[Pino Toscano]

Hi,

On Wednesday 17 June 2015 10:12:59 Cao jin wrote:
> >> @@ -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".
> >
> 
> yup...will fix this. see some other APIs have the same problem.

I will send something to help with these issues, so no need to change
this for now.

> >> +    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
> > ?
> 
> In the condition that the btrfs have multi devices, its original output 
> is going to this way:
>    [/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]...
>    [/dev/sdc]...
>    [/dev/sdc]...
>    [/dev/sdc]...
>    ...
> So. I think the [/dev/sd..] is necessary, how to think?

Possibly, but the user (as in caller for this API) still need to do
some kind of parsing; given that you are basically copying bits from
the btrfs output, they might change breaking users.

Speaking of this: you said that you have a colleague working on
btrfs-progs? What about suggesting to create some machine-parseable
output (csv, xml, yaml, json, whatever) so extracting the results of
btrfs tools is a lot more easy?

Thanks,
-- 
Pino Toscano