Hi, Pino 在 2015年06月17日 23:37, 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. >OK>>>> + 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? >Yes, I forward your suggestion and consult him, the result is not surprised:( Here is what I learned from him: For the btrfs-progs cmds who output strings, the output are plain, don`t have patterns. Seen some guys who want a formatted output, they do a filter by themself, it not reasonable to ask btrfs-progs to output formatted strings. What`s your opinion about the output pattern? I can try to implement it.> Thanks, >-- Yours Sincerely, Cao Jin
Hi, On Thursday 18 June 2015 11:01:37 Cao jin wrote:> > 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? > > > > Yes, I forward your suggestion and consult him, the result is not > surprised:( Here is what I learned from him: > For the btrfs-progs cmds who output strings, the output are plain, > don`t have patterns. Seen some guys who want a formatted output, they do > a filter by themself, it not reasonable to ask btrfs-progs to output > formatted strings.This is exactly the issue here: every btrfs command has a totally different formatting for its output. Just let him take a look at all the commands implemented so far in libguestfs, you can easily spot that: - btrfs subvolume list - btrfs subvolume show - btrfs qgroup show - btrfs balance status - btrfs scrub status - btrfs device stats all have different custom parsers for each of their outputs. Compare that to e.g. the -m parameter for parted, so it outputs fields separated by semi-colon. -- Pino Toscano
Hi, Pino 在 2015年06月18日 16:41, Pino Toscano 写道:> Hi, > > On Thursday 18 June 2015 11:01:37 Cao jin wrote: >>> 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? >>> >> >> Yes, I forward your suggestion and consult him, the result is not >> surprised:( Here is what I learned from him: >> For the btrfs-progs cmds who output strings, the output are plain, >> don`t have patterns. Seen some guys who want a formatted output, they do >> a filter by themself, it not reasonable to ask btrfs-progs to output >> formatted strings. > > This is exactly the issue here: every btrfs command has a totally > different formatting for its output. Just let him take a look at all > the commands implemented so far in libguestfs, you can easily spot > that: > - btrfs subvolume list > - btrfs subvolume show > - btrfs qgroup show > - btrfs balance status > - btrfs scrub status > - btrfs device stats > all have different custom parsers for each of their outputs. > > Compare that to e.g. the -m parameter for parted, so it outputs fields > separated by semi-colon. >Already forward your mail to my colleague. He think your comment is interesting, and also forward your option to btrfs-progs mail-list. Speaking of btrfs device stats output, what do you suggest to do for now? Since it may has multi devices. -- Yours Sincerely, Cao Jin
Richard W.M. Jones
2015-Jun-23 13:06 UTC
Re: [Libguestfs] [PATCH] New API: btrfs_device_stats
On Thu, Jun 18, 2015 at 10:41:56AM +0200, Pino Toscano wrote:> Hi, > > On Thursday 18 June 2015 11:01:37 Cao jin wrote: > > > 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? > > > > > > > Yes, I forward your suggestion and consult him, the result is not > > surprised:( Here is what I learned from him: > > For the btrfs-progs cmds who output strings, the output are plain, > > don`t have patterns. Seen some guys who want a formatted output, they do > > a filter by themself, it not reasonable to ask btrfs-progs to output > > formatted strings. > > This is exactly the issue here: every btrfs command has a totally > different formatting for its output. Just let him take a look at all > the commands implemented so far in libguestfs, you can easily spot > that: > - btrfs subvolume list > - btrfs subvolume show > - btrfs qgroup show > - btrfs balance status > - btrfs scrub status > - btrfs device stats > all have different custom parsers for each of their outputs. > > Compare that to e.g. the -m parameter for parted, so it outputs fields > separated by semi-colon.I have to agree with Pino T that btrfs-progs would be a lot better if it had parseable output, like json/XML/etc, especially parseable output that was stable over time. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/