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