On Wed, Apr 13, 2016 at 06:39:55PM +0100, Richard W.M. Jones wrote: > On Wed, Apr 13, 2016 at 07:45:42PM +0300, Roman Kagan wrote: > > On Wed, Apr 13, 2016 at 10:11:54AM +0100, Richard W.M. Jones wrote: > > > On Tue, Apr 12, 2016 at 06:46:31PM +0300, Roman Kagan wrote: > > > > + (* Presence of virtio-scsi controller. *) > > > > + let has_virtio_scsi = > > > > + let obj = Xml.xpath_eval_expression xpathctx > > > > + "/domain/devices/controller[@model='virtio-scsi']" in > > > > > > I guess this short cut is OK. A true test would involve checking the > > > <target bus="scsi"> on each disk and matching it back to the > > > controller. In other words, a huge pain! Maybe you can add an "XXX" > > > comment in the source about this. > > > > On a second look, I'm not sure I got your comment right. > > > > AFAICS there's no way in libvirt xml to define multiple scsi buses, > > I think this is possible, although massively unlikely in practice. I > think the different controllers would have different > index="0"/index="1"/... attributes.
How the binding of drives to controllers is done, then? Is it specified in drive's "address"? And indeed we probably don't have to worry about this case. > > so all the matching I can do is just to notice the presence of > > virtio-scsi controller, and attribute all SCSI drives to it. This > > is exactly what happens in the patch. > > > > Am I missing something? > > So first off I think this part of the patch and your assumption is > fine, there's no need to change the code. > > However there is a possibility that the source guest might have > multiple mixed virtio-blk + virtio-scsi disks. In that case the disks > would look like this: > > <disk type='file' device='disk'> > <driver name='qemu' type='raw'/> > <source file=...> > <target dev='vda' bus='virtio'/> > </disk> > <disk type='file' device='disk'> > <driver name='qemu' type='raw'/> > <source file=...> > <target dev='sda' bus='scsi'/> > </disk> > <controller type="scsi" index="0" model="virtio-scsi"/> > > It's not something that we should worry about, and a simple comment > with "XXX" in it is fine, if you agree with my analysis. This case is handled fine in the patch, isn't it? has_virtio_scsi is a local variable which is only consulted when populating per-drive s_controller field: for drives with bus='scsi' it differentiates between Source_virtio_SCSI and Source_SCSI. The place where we make up a single disk type is v2v.ml:rcaps_from_source, but there I explicitly error out if the guest has multiple disk types (which may be too harsh but simplifies things for now). Roman. _______________________________________________ Libguestfs mailing list [email protected] https://www.redhat.com/mailman/listinfo/libguestfs
