[virt-tools-list] [virt-manager PATCH RFC] Remove the difference between VirtIO-SCSI and pure SCSI disks

[Cole Robinson]

On 01/28/2014 07:32 AM, Martin Kletzander wrote:
> On Mon, Jan 27, 2014 at 10:37:44AM -0500, Cole Robinson wrote:
>> On 01/27/2014 09:01 AM, Martin Kletzander wrote:
>>> These disk types are differentiated by the *model* of the controller
>>> they are connected to.  I added a patch that ensures the controller
>>> which these disk are connected to has model='virtio-scsi', but that is
>>> breaking libvirt's address generation; having <address
>>> type='whatever'/> specified makes all unspecified values to fallback
>>> to zeroes.  Libvirt's address generation is very deterministic (values
>>> are taken from the disk's target), but will be reliable for an unknown
>>> period and it's dependant on the order of the controllers.  Other
>>> thing we can do is not to differentiate between those two and let the
>>> user decide since the controller model can now be changed using in UI.
>>>
>>
>> I'm having trouble following this exactly: is there currently a problem that
>> can be reproduced with virt-manager?
>>
> 
> Short answer: Yes, see Bug 1036716 [1].  However, such simplification
> would miss some important points.  Sorry for the confusion in the
> first mail, I'll try to be more clear this time.
> 
> At first, there was a problem with multiple VirtIO-SCSI disks (Bug
> 950330 [2]).  When Virtio-SCSI disk was added, we added a virtio-scsi
> controller, but if there was a default-scsi controller in the XML and
> virt-manager specified 'index=0', those two clashed and the XML
> couldn't be defined.  When fixing this fairly simple issue, I tried to
> foresee the future a little bit and apart from only specifying the
> index, I also modified the address of the disk to point at that exact
> controller (since, in libvirt, by default the disks are being attached
> to controllers sequentially or depending on it's 'target' value) which
> resulted into this address:
> 
> <address type='scsi' controller='X'/>
> 
> where 'X' is the virtio-scsi controller index.  But what I didn't know
> was that if you add this address to libvirt, it takes the address as
> granted and all unspecified parameters (bus, target, unit) default to
> zeroes.  That makes all disks assigned to the same address, which is,
> of course, invalid.
> 
> So when this issue was discovered with multiple VirtIO-SCSI disks (Bug
> 1036716 [1]), I started investigating again.  I came to conclusion
> that we can do two things:
> 
>  a) Start tracking all the addresses for SCSI disks, check what
>     controllers they are assigned to and if there is new one added,
>     we'd either change the address or re-calculate the 'target'.  We'd
>     also have to check what scsi controllers the underlying qemu
>     supports and what libvirt will select as default.  OT: Depending
>     on the address we could pretty-print the disk properly as
>     VirtIO-SCSI disk (which we currently do not).
> 
>  b) Remove the differences between these disks and let libvirt and
>     qemu do their job of selecting the best option.
> 

Yes, if virt-manager has to get in the business of dealing with bus/unit blah,
it will be a pain. Unfortunately in the face of multiple scsi controllers,
there isn't much sensible that we can do.

We could partly offset this issue. If there's an existing scsi controller, and
the user requests virtio-scsi, throw up and error and tell them that they need
to change the model of their existing scsi controller. Or we could even offer
to do it for them but I don't care that much about that. We could similarly
throw an error if there are 2 scsi controllers available, since virt-manager
doesn't know how to add a scsi device to anything but the first controller.

The most common scenario will be someone attempting to add virtio-scsi, but
they have no pre-existing scsi controller attached to the guest. Anything else
is a corner case as far as I'm concerned.

Though really I'd say libvirt should also be fixed to handle filling in the
rest of the address info if just a 'unit' is specified. Obviously we won't be
able to fully depend on it in virt-manager, but if we add a support.py version
check we can just say adding disks to anything but the first controller
requires libvirt version X.

> To explain point (b) a bit more; if the controller model is 'default',
> it can be either some legacy SCSI controller or virtio-scsi.  That
> depends on what qemu supports and what libvirt chooses.  Newer
> upstream x86 qemu has only virtio-scsi IIRC (I had to fix this in
> libvirt commit b7f1c0c3 [3]), so with such qemu all SCSI disks are
> virtio-scsi anyway.

Upstream qemu, or RHEL? My understanding is upstream qemu has at least 3 scsi
controller models on x86.

Also some qemu versions might not have
> virtio-scsi at all.
> 

That can be said about many features we expose in virt-manager :) Until
there's some coordinated capabilities reporting between libvirt and
virt-manager, clients will never have a full picture. but I really only care
about it for what we use as defaults when creating new VMs. If some
non-default feature like virtio-scsi isn't available, I'm content with libvirt
just throwing an exception.

> Sorry to waste your time with such silliness, but it's something
> that's been worrying me for a while there.
> 

Not time wasting, this was informative.

- Cole