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