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@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list