[libvirt] [PATCH 0/2] Add lsi and virtio-scsi qemu caps
On qemu-kvm-0.15.1, it supports only lsi scsi controller model. On qemu-kvm-0.12.1.2, it supports only virtio-scsi-pci scsi model On qemu 1.1.50, it supports both. So, instead of using the lsilogic model by default, the patch tries to check which model the current QEMU supports, then choose it, lsi has the priority. If a scsi model is given in XML explicitly, we try to check if the underlying QEMU supports it or not, raise an error on checking failure. Guannan Ren(2) (1/2)qemu: add two qemu caps for lsi and virtio-scsi SCSI controllers (2/2)test: add lsi and virtio-scsi qemu caps in testcases. src/qemu/qemu_capabilities.c | 7 --- src/qemu/qemu_capabilities.h | 2 - src/qemu/qemu_command.c | 88 +++++++++++------------------------------ src/qemu/qemu_command.h | 3 +- tests/qemuhelptest.c | 10 +--- tests/qemuxml2argvtest.c | 16 +++----- 6 files changed, 34 insertions(+), 92 deletions(-)
Rename qemuDefaultScsiControllerModel to qemuCheckScsiControllerModel. When scsi model is given explicitly in XML(model > 0) checking if the underlying QEMU support it or not first, raise error on checking failure. If the model is not given(mode <= 0), return lsi or virtio-scsi which is supported, lsi take the priority. --- src/qemu/qemu_capabilities.c | 7 +++ src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_command.c | 88 ++++++++++++++++++++++++++++++----------- src/qemu/qemu_command.h | 3 +- 4 files changed, 75 insertions(+), 25 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 85c49a2..8282ad8 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -169,6 +169,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, "virtio-s390", "balloon-event", + "lsi", /*100*/ + "virtio-scsi-pci", ); struct qemu_feature_flags { @@ -1450,6 +1452,11 @@ qemuCapsParseDeviceStr(const char *str, virBitmapPtr flags) strstr(str, "name \"virtio-serial-s390\"")) qemuCapsSet(flags, QEMU_CAPS_VIRTIO_S390); + if (strstr(str, "name \"lsi53c895a\"")) + qemuCapsSet(flags, QEMU_CAPS_SCSI_LSI); + if (strstr(str, "name \"virtio-scsi-pci\"")) + qemuCapsSet(flags, QEMU_CAPS_VIRIO_SCSI_PCI); + /* Prefer -chardev spicevmc (detected earlier) over -device spicevmc */ if (!qemuCapsGet(flags, QEMU_CAPS_CHARDEV_SPICEVMC) && strstr(str, "name \"spicevmc\"")) diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index e8251dc..d2d68a9 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -135,6 +135,8 @@ enum qemuCapsFlags { QEMU_CAPS_NEC_USB_XHCI = 97, /* -device nec-usb-xhci */ QEMU_CAPS_VIRTIO_S390 = 98, /* -device virtio-*-s390 */ QEMU_CAPS_BALLOON_EVENT = 99, /* Async event for balloon changes */ + QEMU_CAPS_SCSI_LSI = 100, /* -device lsi */ + QEMU_CAPS_VIRIO_SCSI_PCI = 101, /* -device virtio-scsi-pci */ QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6ad65a6..6a4578d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -469,19 +469,60 @@ static int qemuAssignDeviceDiskAliasFixed(virDomainDiskDefPtr disk) } static int -qemuDefaultScsiControllerModel(virDomainDefPtr def) { - if (STREQ(def->os.arch, "ppc64") && - STREQ(def->os.machine, "pseries")) { - return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI; +qemuCheckScsiControllerModel(virDomainDefPtr def, + virBitmapPtr qemuCaps, + int *model) +{ + if (*model > 0) { + switch (*model) { + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC: + if (!qemuCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU doesn't support " + "lsi scsi controller")); + return -1; + } + break; + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI: + if (!qemuCapsGet(qemuCaps, QEMU_CAPS_VIRIO_SCSI_PCI)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU doesn't support " + "virtio scsi controller")); + return -1; + } + break; + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI: + /*TODO: need checking work here if necessary */ + break; + default: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unsupported controller model: %s"), + virDomainControllerModelSCSITypeToString(*model)); + return -1; + } } else { - return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC; + if (qemuCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI)) { + *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC; + } else if (qemuCapsGet(qemuCaps, QEMU_CAPS_VIRIO_SCSI_PCI)) { + *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI; + } else if (STREQ(def->os.arch, "ppc64") && + STREQ(def->os.machine, "pseries")) { + *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI; + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to determine model for scsi controller")); + return -1; + } } + + return 0; } /* Our custom -drive naming scheme used with id= */ static int qemuAssignDeviceDiskAliasCustom(virDomainDefPtr def, - virDomainDiskDefPtr disk) + virDomainDiskDefPtr disk, + virBitmapPtr qemuCaps) { const char *prefix = virDomainDiskBusTypeToString(disk->bus); int controllerModel = -1; @@ -491,11 +532,10 @@ qemuAssignDeviceDiskAliasCustom(virDomainDefPtr def, controllerModel = virDomainDiskFindControllerModel(def, disk, VIR_DOMAIN_CONTROLLER_TYPE_SCSI); - } - if (controllerModel == -1 || - controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO) - controllerModel = qemuDefaultScsiControllerModel(def); + if ((qemuCheckScsiControllerModel(def, qemuCaps, &controllerModel)) < 0) + return -1; + } if (disk->bus != VIR_DOMAIN_DISK_BUS_SCSI || controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC) { @@ -533,7 +573,7 @@ qemuAssignDeviceDiskAlias(virDomainDefPtr vmdef, { if (qemuCapsGet(qemuCaps, QEMU_CAPS_DRIVE)) { if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) - return qemuAssignDeviceDiskAliasCustom(vmdef, def); + return qemuAssignDeviceDiskAliasCustom(vmdef, def, qemuCaps); else return qemuAssignDeviceDiskAliasFixed(def); } else { @@ -850,9 +890,10 @@ qemuAssignSpaprVIOAddress(virDomainDefPtr def, virDomainDeviceInfoPtr info, return 0; } -int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def) +int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, + virBitmapPtr qemuCaps) { - int i, rc; + int i, rc = -1; int model; /* Default values match QEMU. See spapr_(llan|vscsi|vty).c */ @@ -869,9 +910,10 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def) for (i = 0 ; i < def->ncontrollers; i++) { model = def->controllers[i]->model; - if (model == -1 && - def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) - model = qemuDefaultScsiControllerModel(def); + if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) + if ((qemuCheckScsiControllerModel(def, qemuCaps, &model)) < 0) + return rc; + if (model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI && def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; @@ -1059,7 +1101,7 @@ int qemuDomainAssignAddresses(virDomainDefPtr def, { int rc; - rc = qemuDomainAssignSpaprVIOAddresses(def); + rc = qemuDomainAssignSpaprVIOAddresses(def, qemuCaps); if (rc) return rc; @@ -2434,9 +2476,8 @@ qemuBuildDriveDevStr(virDomainDefPtr def, controllerModel = virDomainDiskFindControllerModel(def, disk, VIR_DOMAIN_CONTROLLER_TYPE_SCSI); - if (controllerModel == -1 || - controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO) - controllerModel = qemuDefaultScsiControllerModel(def); + if ((qemuCheckScsiControllerModel(def, qemuCaps, &controllerModel)) < 0) + goto error; if (controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC) { if (disk->info.addr.drive.target != 0) { @@ -2765,10 +2806,9 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, switch (def->type) { case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: model = def->model; - if (model == -1 || - model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO) { - model = qemuDefaultScsiControllerModel(domainDef); - } + if ((qemuCheckScsiControllerModel(domainDef, qemuCaps, &model)) < 0) + return NULL; + switch (model) { case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI: virBufferAddLit(&buf, "virtio-scsi-pci"); diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 3ccf4d7..7068a81 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -178,7 +178,8 @@ virDomainDefPtr qemuParseCommandLinePid(virCapsPtr caps, int qemuDomainAssignAddresses(virDomainDefPtr def, virBitmapPtr qemuCaps, virDomainObjPtr); -int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def); +int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, + virBitmapPtr qemuCaps); int qemuDomainAssignPCIAddresses(virDomainDefPtr def, virBitmapPtr qemuCaps, -- 1.7.7.6
--- tests/qemuhelptest.c | 10 +++++++--- tests/qemuxml2argvtest.c | 16 ++++++++++------ 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index 012ba26..62afabf 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -482,7 +482,8 @@ mymain(void) QEMU_CAPS_PCI_ROMBAR, QEMU_CAPS_NO_ACPI, QEMU_CAPS_VIRTIO_BLK_SG_IO, - QEMU_CAPS_CPU_HOST); + QEMU_CAPS_CPU_HOST, + QEMU_CAPS_SCSI_LSI); DO_TEST("qemu-kvm-0.12.1.2-rhel61", 12001, 1, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -677,7 +678,8 @@ mymain(void) QEMU_CAPS_FSDEV_WRITEOUT, QEMU_CAPS_SCSI_BLOCK, QEMU_CAPS_SCSI_CD, - QEMU_CAPS_IDE_CD); + QEMU_CAPS_IDE_CD, + QEMU_CAPS_SCSI_LSI); DO_TEST("qemu-1.1.0", 1001000, 0, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -754,7 +756,9 @@ mymain(void) QEMU_CAPS_IDE_CD, QEMU_CAPS_NO_USER_CONFIG, QEMU_CAPS_HDA_MICRO, - QEMU_CAPS_NEC_USB_XHCI); + QEMU_CAPS_NEC_USB_XHCI, + QEMU_CAPS_SCSI_LSI, + QEMU_CAPS_VIRIO_SCSI_PCI); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 39fcd9f..af996f3 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -469,16 +469,19 @@ mymain(void) DO_TEST("disk-usb-device", QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST("disk-scsi-device", - QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); + QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_SCSI_LSI); DO_TEST("disk-scsi-device-auto", - QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); + QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_SCSI_LSI); DO_TEST("disk-scsi-disk-split", QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, - QEMU_CAPS_SCSI_CD); + QEMU_CAPS_SCSI_CD, QEMU_CAPS_SCSI_LSI, QEMU_CAPS_VIRIO_SCSI_PCI); DO_TEST("disk-scsi-vscsi", QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST("disk-scsi-virtio-scsi", - QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); + QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_VIRIO_SCSI_PCI); DO_TEST("disk-sata-device", QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_ICH9_AHCI); @@ -508,7 +511,8 @@ mymain(void) DO_TEST("disk-scsi-lun-passthrough", QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, - QEMU_CAPS_SCSI_BLOCK, QEMU_CAPS_VIRTIO_BLK_SG_IO); + QEMU_CAPS_SCSI_BLOCK, QEMU_CAPS_VIRTIO_BLK_SG_IO, + QEMU_CAPS_SCSI_LSI, QEMU_CAPS_VIRIO_SCSI_PCI); DO_TEST("graphics-vnc", NONE); DO_TEST("graphics-vnc-socket", NONE); @@ -768,7 +772,7 @@ mymain(void) DO_TEST("multifunction-pci-device", QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, - QEMU_CAPS_PCI_MULTIFUNCTION); + QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_SCSI_LSI); DO_TEST("monitor-json", QEMU_CAPS_DEVICE, QEMU_CAPS_CHARDEV, QEMU_CAPS_MONITOR_JSON, QEMU_CAPS_NODEFCONFIG); -- 1.7.7.6
On Tue, Aug 07, 2012 at 02:54:33PM +0800, Guannan Ren wrote:
On qemu-kvm-0.15.1, it supports only lsi scsi controller model. On qemu-kvm-0.12.1.2, it supports only virtio-scsi-pci scsi model
This doesn't make sense. virtio-scsi did not exist until long after qemu-kvm-0.12.1.2.
On qemu 1.1.50, it supports both. So, instead of using the lsilogic model by default, the patch tries to check which model the current QEMU supports, then choose it, lsi has the priority.
I'm not convinced we should ever be trying to use virtio scsi by default, given its wide lack of guest support. If LSI scsi does not exist, just report an error IMHO, and let virtio scsi require an explicit config choice. 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 08/07/2012 07:37 PM, Daniel P. Berrange wrote:
On Tue, Aug 07, 2012 at 02:54:33PM +0800, Guannan Ren wrote:
On qemu-kvm-0.15.1, it supports only lsi scsi controller model. On qemu-kvm-0.12.1.2, it supports only virtio-scsi-pci scsi model This doesn't make sense. virtio-scsi did not exist until long after qemu-kvm-0.12.1.2.
This is the version on my RHEL6.3 , maybe not upstream version code. # rpm -qa|grep kvm qemu-kvm-0.12.1.2-2.302.el6.x86_64 # /usr/libexec/qemu-kvm --version QEMU PC emulator version 0.12.1 (qemu-kvm-0.12.1.2)
On qemu 1.1.50, it supports both. So, instead of using the lsilogic model by default, the patch tries to check which model the current QEMU supports, then choose it, lsi has the priority. I'm not convinced we should ever be trying to use virtio scsi by default, given its wide lack of guest support. If LSI scsi does not exist, just report an error IMHO, and let virtio scsi require an explicit config choice.
Get it. the virtio-scsi is not used widely. Thanks for the review. Guannan
participants (2)
-
Daniel P. Berrange -
Guannan Ren