Re: [libvirt] [PATCH 15/34] Auto-add disk controllers based on defined disks

On Fri, Jan 08, 2010 at 05:23:11PM +0000, Daniel P. Berrange wrote: > Existing applications using libvirt are not aware of the disk > controller concept. Thus, after parsing the <disk> definitions > in the XML, it is neccessary to create <controller> elements > to satisfy all requested disks, as per their defined drive > addresses > > * src/conf/domain_conf.c, src/conf/domain_conf.h, > src/libvirt_private.syms: Add virDomainDefAddDiskControllers() > method for populating disk controllers, and call it after > parsing disk definitions. > * src/qemu/qemu_conf.c: Call virDomainDefAddDiskControllers() > when doing ARGV -> XML conversion > * tests/qemuxml2argvdata/qemuxml2argv*.xml: Add disk controller > data to all data files which don't have it already > +/* > + * Based on the declared <address type=drive> info for any disks, > + * add neccessary drive controllers which are not already present > + * in the XML. This is for compat with existing apps which will > + * not know/care about <controller> info in the XML > + */ > +int virDomainDefAddDiskControllers(virDomainDefPtr def) > +{ > + if (virDomainDefAddDiskControllersForType(def, > + VIR_DOMAIN_CONTROLLER_TYPE_SCSI, > + VIR_DOMAIN_DISK_BUS_SCSI) < 0) > + return -1; > + > + if (virDomainDefAddDiskControllersForType(def, > + VIR_DOMAIN_CONTROLLER_TYPE_FDC, > + VIR_DOMAIN_DISK_BUS_FDC) < 0) > + return -1; > + > + if (virDomainDefAddDiskControllersForType(def, > + VIR_DOMAIN_CONTROLLER_TYPE_IDE, > + VIR_DOMAIN_DISK_BUS_IDE) < 0) > + return -1; > + > + return 0; > +} IIRC a previous patch in the serie disk controler are not sorted, so the order here is just arbitrary, right ?

> --- a/tests/qemuxml2argvdata/qemuxml2argv-floppy-drive-fat.xml > +++ b/tests/qemuxml2argvdata/qemuxml2argv-floppy-drive-fat.xml > @@ -21,5 +21,7 @@ > <readonly/> > <address type='drive' controller='0' bus='0' unit='0'/> > </disk> > + <controller type='ide' index='0'/> > + <controller type='fdc' index='0'/> > </devices> > </domain> strange here in the output we get ide before fdc, I would have assume the reverse based on the new function. I hope the order in generated output won't change randomly. Except that remark looks fine, ACK,