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

Re: [libvirt] [PATCH 03/11] conf: Parse and for the tray attribute



On Fri, Mar 23, 2012 at 05:48:32PM +0800, Osier Yang wrote:
> On 03/23/2012 04:05 PM, Daniel Veillard wrote:
> >On Wed, Mar 14, 2012 at 11:26:47PM +0800, Osier Yang wrote:
> >>>  The "tray" is only allowed for removable disks, i.e. CDROM and
> >>>  Floppy disks.
> >>>
> >>>  As the value for "tray" defaults to "closed", lots of tests are
> >>>  updated to include "tray='closed'" in the disk target XML.
> >   I tend to think "tray='closed'" should be ommited since it's the
> >default value, no need to serialize it out systematically
> >
> >
> 
> Not sure which way is better, but from my point of view,
> it's more explicit for a user's eyes to see what the tray
> status is, and when updating the device, changing the value
> of "tray" between "open" and "closed" is more sensiable than
> "adding tray='open'" and "removing tray='open'" (supposing
> QEMU will support CD-ROM and Floppy device updating with
> tray status in future, e.g. a qemu monitor command "change-tray",
> and one uses "virsh edit" to change the XML).
> 
> But I acknowledge all these reasons are weak somehow, :-)
> so I'm fine with either.
> 
> By the way, it seems to me it's not consistent on whether
> to print the default value for tags or not, some tags do,
> some not. Such as rawio='no' for disk won't be printed,
> and managed='no' for hostdevs will be printed.

  Well in this case I would expect tray="closed" 99% of the
time (if not more as on a physical machine !) so to me the
only data worth noticing is tray="open", and actually not
dumping tray="closed" makes the fact that the tray is open
more ovious IMHO.

  So yeah I really think it's better even from an human POV
(well I'm firmly convinced that users should never see XML
 dumps but that's a different story :-)

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


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