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@xxxxxxxxxx> > >> --- > >> 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list