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.