[libvirt] [PATCH] Add virtio-scsi to fallback models of scsi controller
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@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")); -- 1.8.3.2
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@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 :|
On 07/15/2013 02:24 PM, Daniel P. Berrange wrote:
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@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.
Since this information is visible from the code only, I don't think we need to guarantee that. If an application wants a SCSI controller of a particular type, it should specify that. Mgmt application IMHO can't rely on the fact that due to historical reasons there is controller model left as a default. We wouldn't be able to change anything then and whenever would QEMU decided to drop the model (like now with all "lsi*" controllers), we wouldn't be able to do anything automatically. The main problem now is that if only disk gets inserted into the XML, we will automatically add invalid controller (without any model), so the automatic addition of controllers is now broken due to this reason as well.
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.
Can we at least fallback to the 'virtio-scsi' model for controllers that were added automatically as a requisite of another device?
We could improve the error message here though, to something like
"LSI Logic SCSI controller not available on this host"
OK, at least a message like "No valid default SCSI controller available on host" (so it is sensible even on ppc64) would help a bit with other cases. Thanks, Martin
On Mon, Jul 15, 2013 at 02:54:58PM +0200, Martin Kletzander wrote:
On 07/15/2013 02:24 PM, Daniel P. Berrange wrote:
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@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.
Since this information is visible from the code only, I don't think we need to guarantee that. If an application wants a SCSI controller of a particular type, it should specify that. Mgmt application IMHO can't rely on the fact that due to historical reasons there is controller model left as a default. We wouldn't be able to change anything then and whenever would QEMU decided to drop the model (like now with all "lsi*" controllers), we wouldn't be able to do anything automatically.
The main problem now is that if only disk gets inserted into the XML, we will automatically add invalid controller (without any model), so the automatic addition of controllers is now broken due to this reason as well.
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.
Can we at least fallback to the 'virtio-scsi' model for controllers that were added automatically as a requisite of another device?
We could improve the error message here though, to something like
"LSI Logic SCSI controller not available on this host"
OK, at least a message like "No valid default SCSI controller available on host" (so it is sensible even on ppc64) would help a bit with other cases.
Hmm, I had forgotten that we auto-added controllers as a side effect of adding disks. If we were doing hotplug today, we would not do the auto-add of controllers, but we have no choice due to back compat issues. So I guess, reluctantly, we have to do what you suggest in your patch to cope with distros disabling other SCSI controllers. ACK 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 :|
On 07/18/2013 02:24 PM, Daniel P. Berrange wrote: [...]
Hmm, I had forgotten that we auto-added controllers as a side effect of adding disks. If we were doing hotplug today, we would not do the auto-add of controllers, but we have no choice due to back compat issues.
So I guess, reluctantly, we have to do what you suggest in your patch to cope with distros disabling other SCSI controllers.
ACK
I agree this is not the cleanest way, but this eases the use to some extent wrt back compat. Thanks for the review, pushed now. Martin
participants (2)
-
Daniel P. Berrange -
Martin Kletzander