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

2010-07-05 Thread Justin Clift
On 07/05/2010 07:01 PM, Daniel P. Berrange wrote: Thinking this would keep the number of columns consistent, but also be readable for people. IMHO your original formatting is the best. Both of these new alternatives looks worse. We should be picking the more human friendly option here and not

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

2010-07-05 Thread Daniel P. Berrange
On Sat, Jul 03, 2010 at 07:03:14PM +1000, Justin Clift wrote: > On 07/01/2010 08:38 PM, Richard W.M. Jones wrote: > > >>virsh # pool-list --details --all > >>Name State Autostart Persistent Capacity Allocation > >>Available > >>---

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

2010-07-03 Thread Justin Clift
On 07/01/2010 08:38 PM, Richard W.M. Jones wrote: virsh # pool-list --details --all Name State Autostart Persistent Capacity Allocation Available --- defaultrunning yesyes 1.79 TB 1.4

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

2010-07-01 Thread Richard W.M. Jones
On Thu, Jul 01, 2010 at 04:06:36PM +0100, Richard W.M. Jones wrote: > If a column was deleted from the output of virt-df, > then you'd get an empty result column. Actually this is not true. csvtool gives an error: sudo virt-df --csv | csvtool namedcol "Virtual Machine,X" - Fatal error: exception

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

2010-07-01 Thread Justin Clift
On 07/02/2010 01:11 AM, Richard W.M. Jones wrote: The CSV format is rather underspecified as it is. csvtool was written based on what a certain popular spreadsheet can do, plus much folk wisdom. Changing the delimiter doesn't particularly help. It seems like a really useful utility, useful i

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

2010-07-01 Thread Richard W.M. Jones
On Fri, Jul 02, 2010 at 01:17:05AM +1000, Justin Clift wrote: > On 07/02/2010 01:11 AM, Richard W.M. Jones wrote: > >> The CSV format is rather underspecified as it is. csvtool was written >> based on what a certain popular spreadsheet can do, plus much folk >> wisdom. Changing the delimiter doe

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

2010-07-01 Thread Richard W.M. Jones
On Fri, Jul 02, 2010 at 12:59:36AM +1000, Justin Clift wrote: > On 07/01/2010 08:42 PM, Richard W.M. Jones wrote: > >> CSV is a good format, but beware the many ways to shoot yourself in >> the foot. I recommend using my program "csvtool" (in Fedora/Debian) >> which can fully and safely parse CSV

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

2010-07-01 Thread Richard W.M. Jones
On Thu, Jul 01, 2010 at 10:51:00AM -0400, Dave Allan wrote: > On Thu, Jul 01, 2010 at 11:42:12AM +0100, Richard W.M. Jones wrote: > > On Wed, Jun 23, 2010 at 09:58:34AM +0100, Daniel P. Berrange wrote: > > > On Tue, Jun 22, 2010 at 12:35:32PM -0600, Eric Blake wrote: > > > > On 06/22/2010 12:24 PM,

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

2010-07-01 Thread Justin Clift
On 07/01/2010 08:42 PM, Richard W.M. Jones wrote: CSV is a good format, but beware the many ways to shoot yourself in the foot. I recommend using my program "csvtool" (in Fedora/Debian) which can fully and safely parse CSV output from shell scripts, or use a library (eg. Text::CSV for Perl, or

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

2010-07-01 Thread Justin Clift
On 07/02/2010 12:00 AM, Daniel P. Berrange wrote: 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 The concep

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

2010-07-01 Thread Dave Allan
On Thu, Jul 01, 2010 at 11:42:12AM +0100, Richard W.M. Jones wrote: > On Wed, Jun 23, 2010 at 09:58:34AM +0100, Daniel P. Berrange 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 beh

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

2010-07-01 Thread Richard W.M. Jones
On Thu, Jul 01, 2010 at 03:00:14PM +0100, Daniel P. Berrange wrote: > 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 would dispute that this makes it "very unfriendly to humans". If you

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

2010-07-01 Thread Daniel P. Berrange
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

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

2010-07-01 Thread 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

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

2010-07-01 Thread Hugh O. Brock
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

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

2010-07-01 Thread Daniel P. Berrange
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

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

2010-07-01 Thread Hugh O. Brock
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 beh

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

2010-07-01 Thread Daniel P. Berrange
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 J

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

2010-07-01 Thread Richard W.M. Jones
On Wed, Jun 23, 2010 at 09:58:34AM +0100, Daniel P. Berrange 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 J

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

2010-07-01 Thread Richard W.M. Jones
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. P

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

2010-06-23 Thread Daniel P. Berrange
On Tue, Jun 22, 2010 at 02:24:31PM -0400, Hugh O. Brock wrote: > On Tue, Jun 22, 2010 at 06:54:55PM +0100, Daniel P. Berrange wrote: > > On Tue, Jun 22, 2010 at 11:44:57AM -0600, Eric Blake wrote: > > > On 06/21/2010 03:47 PM, Justin Clift wrote: > > > > This patch adds a new --details option to th

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

2010-06-23 Thread Daniel P. Berrange
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. P

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

2010-06-22 Thread Justin Clift
On 06/23/2010 03:44 AM, Eric Blake wrote: This is a change in behavior, even when the user didn't specify --details. Might it cause any backward compatibility issues in scripting, or are we okay with the change? $ old-virsh -c test:///default pool-list --inactive Name State

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

2010-06-22 Thread Eric Blake
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

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

2010-06-22 Thread Hugh O. Brock
On Tue, Jun 22, 2010 at 06:54:55PM +0100, Daniel P. Berrange wrote: > On Tue, Jun 22, 2010 at 11:44:57AM -0600, Eric Blake wrote: > > On 06/21/2010 03:47 PM, Justin Clift wrote: > > > This patch adds a new --details option to the virsh pool-list > > > command, making its output more useful to peopl

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

2010-06-22 Thread Daniel P. Berrange
On Tue, Jun 22, 2010 at 11:44:57AM -0600, Eric Blake wrote: > On 06/21/2010 03:47 PM, Justin Clift wrote: > > This patch adds a new --details option to the virsh pool-list > > command, making its output more useful to people who use virsh > > for significant lengths of time. > > > > Addresses BZ #

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

2010-06-22 Thread Eric Blake
On 06/21/2010 03:47 PM, Justin Clift wrote: > This patch adds a new --details option to the virsh pool-list > command, making its output more useful to people who use virsh > for significant lengths of time. > > Addresses BZ # 605543 > > https://bugzilla.redhat.com/show_bug.cgi?id=605543 > > +

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

2010-06-21 Thread Justin Clift
This patch adds a new --details option to the virsh pool-list command, making its output more useful to people who use virsh for significant lengths of time. Addresses BZ # 605543 https://bugzilla.redhat.com/show_bug.cgi?id=605543 --- (This is the v3 patch version, but with some wonky spacing