[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On Thu, Jul 01, 2010 at 02:44:35PM +0100, Richard W.M. Jones wrote:
> 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:
> > > > > 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.

[snip]

> 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.

The changes noted above to make it machine friendly, make it very 
unfriendly to humans. The patch was focused on making this more
friendly for humans.

> 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.

virsh data output is easily tested using the test:///default drive - we
already have a bunch of test cases for several virsh commands.

If we did add machine readable code though, I agree we want to avoid
having two parallel code paths.  The current code is rather mixing
up the jobs of fetching the data from libvirt with the job of printing
the data out.

I'd say we should separate this logic out, so the presentation code
is completely isolated & sharable across commands.

We have 3 data formats commonly used  in human friendly output

a. Table format

 # virsh list
  Id Name                 State
 ----------------------------------
  - f12i686              shut off
  - f12i686_v1           shut off
  - f13i686              shut off
  - f13x86_64            shut off

b. key value pair format

  # virsh dominfo f12i686
  Id:             -
  Name:           f12i686
  UUID:           0301f2c1-05fc-dddf-089b-60e874f11e64
  OS Type:        hvm
  State:          shut off
  CPU(s):         1
  Max memory:     1048576 kB
  Used memory:    1048576 kB
  Persistent:     yes
  
c. single value

  # virsh domuuid f12i686
  0301f2c1-05fc-dddf-089b-60e874f11e64


In terms of data structures, we could represent the first using an array of
hashes, the second using a single hash, and the third using a string.
We can then build re-usable data formatters for each of these data structures.

   virshPrintString(const char * value)
   virshPrintHash(const char ** headings, virHashPtr data)
   virshPrintHashList(const char ** headings, virHashPtr *data)

A global '--format=plain|csv|json|xml' flag could apply to every single
virsh command, causing these methods to print in the corresponding
data format. Thus we'd have no code paths duplicated & flexible data
formatting in many styles


Regards,
Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]