[libvirt] [PATCHv4 1/2] virsh: add new --details option to pool-list

[Richard W.M. Jones]

On Thu, Jul 01, 2010 at 02:01:57PM +0100, Daniel P. Berrange wrote:
> On Thu, Jul 01, 2010 at 08:57:25AM -0400, Hugh O. Brock wrote:
> > On Thu, Jul 01, 2010 at 11:50:20AM +0100, Daniel P. Berrange wrote:
> > > On Thu, Jul 01, 2010 at 11:38:06AM +0100, Richard W.M. Jones wrote:
> > > > On Tue, Jun 22, 2010 at 12:35:32PM -0600, Eric Blake wrote:
> > > > > On 06/22/2010 12:24 PM, Hugh O. Brock wrote:
> > > > > >> Correct, we shouldn't change this behaviour - it'll break apps parsing
> > > > > >> the output
> > > > > > 
> > > > > > FWIW Rich Jones complains that the output as it stands is nigh on
> > > > > > unparseable anyway. Perhaps we should consider that a bug, and fix
> > > > > > it...
> > > > > 
> > > > > The new --details option is our chance to change output - it outputs
> > > > > whatever format we want, because it is a new flag; Rich, do you have any
> > > > > preferences about what it _should_ output?
> > > > > 
> > > > > Here's what pool-list --details would currently do, if we applied
> > > > > Justin's patch as-is (modulo no line wrapping added by my email client):
> > > > 
> > > > Sorry, been away for a couple of weeks.
> > > > 
> > > > > virsh # pool-list --details --all
> > > > > Name       State     Autostart  Persistent  Capacity  Allocation  Available
> > > > > ---------------------------------------------------------------------------
> > > > > default    running   yes        yes          1.79 TB     1.49 TB  304.77 GB
> > > > > image_dir  running   yes        yes          1.79 TB     1.49 TB  304.77 GB
> > > > > tmp        inactive  no         yes                -           -          -
> > > > 
> > > > One good thing, and several bad things about that.  The good thing is
> > > > that empty columns are presented with '-' which means you can use awk
> > > > and sort -k to parse the output columnwise.
> > > > 
> > > > The bad things:
> > > > 
> > > > * Space within fields "1.79 TB" (awk / sort -k in fact _won't_ work).
> > > > 
> > > > * Numeric fields aren't numbers: You can't sort -n on "1.79 TB", and
> > > >   you can't read that number into a script and do math on it.  Most
> > > >   tools have a "-h" or "--human" option in order to generate human-
> > > >   readable numbers (without spaces), but default to just printing the
> > > >   raw numbers.
> > > > 
> > > > * Unnecessary "-------" line.
> > > > 
> > > > * Title line should be optional.  Have a --no-title option or
> > > >   something like that to suppress it.
> > > > 
> > > > * Does virsh still print an unnecessary blank line after the output?
> > > >   If so, stop doing that.
> > > 
> > > All of these bad things are just an artifact of trying to use the same
> > > output format for humans & machines. To address all those would make
> > > the result unpleasant for humans. We really do just need to create a
> > > dedicated format for machines, csv or json, or somethingelse and leave
> > > the human format alone
> > 
> > Actually I think we *do* need the --details convenience method for
> > human-readable output, *and* a better machine-parseable format. Justin
> > is mostly interested in making virsh more usable for people, which I
> > fully support -- making it easier to script is also a bonus however.
> 
> Err, that's entirely my point

I'm not looking at this too closely, but isn't this discussion about a
potential patch (adding --details)?  If so then if it costs little to
make it machine friendly by default, why not just do it.

Changing existing output is a little difficult though.

I'm concerned about having two code paths (normal vs machine
readable), with the implication that the machine readable path will
bit rot because it gets less visible use.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
New in Fedora 11: Fedora Windows cross-compiler. Compile Windows
programs, test, and build Windows installers. Over 70 libraries supprt'd
http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw