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

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.