[libvirt] [PATCH] Add virtio-scsi to fallback models of scsi controller

[Daniel P. Berrange]

On Mon, Jul 15, 2013 at 09:50:45AM +0200, Martin Kletzander wrote:
> When user does not specify any model for scsi controller, or worse, no
> controller at all, but libvirt automatically adds scsi controller with
> no model, we are not searching for virtio-scsi and thus this can fail
> for example on qemu which doesn't support lsi logic adapter.
> 
> This means that when qemu on x86 doesn't support lsi53c895a and the
> user adds the following to an XML without any scsi controller:
> 
> <disk ...>
>   ...
>   <target dev='sda'>
> </disk>
> 
> libvirt fails like this:
>  # virsh define asdf.xml
>  error: Failed to define domain from asdf.xml
>  error: internal error Unable to determine model for scsi controller
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=974943
> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
> ---
>  src/qemu/qemu_command.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 0e517f2..aff1768 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -703,6 +703,8 @@ qemuSetScsiControllerModel(virDomainDefPtr def,
>              *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI;
>          } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI)) {
>              *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC;
> +        } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_SCSI)) {
> +            *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI;
>          } else {
>              virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                             _("Unable to determine model for scsi controller"));

I'm not really convinced this is a good idea.

The code as it stands is saying on PPC uses IBMVSCSI, otherwise
uses LSI SCSI. This is reasonable behaviour for an application to
rely on since it is predictable from the outside - ie the app knows
what architecture / machine type the VM is, so it knows exactly what
SCSI controller will be chosen by libvirt & will get an error if not
available.

You're adding in a layer of unpredictability there, because now there
are multiple choices of SCSI controller for all architectures.
Furthermore, there is no parity of guest OS support between these
different SCSI controllers. Thus if an application has a guest that
supports LSI Logic SCSI, it is not reasonable to assume that falling
back to VirtIO SCSI is going to be helpful behaviour.

IMHO it is better to keep an error as we have now, and require that an
application specify an explicit model name in the XML for anything
else.

We could improve the error message here though, to something like

   "LSI Logic SCSI controller not available on this host"


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|