[libvirt] [PATCH 0/3 v2] Add lsi and virtio-scsi qemu caps
Add two capabilities flags for the model of scsi controller LSI and virtio-scsi. Use the lsilogic model by default, if current QEMU supports doesn't it, raise an error. When 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(3) qemu: add capabilities flags related to scsi controller qemu: add two qemu caps for lsi and virtio-scsi SCSI 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 | 106 +++++++++++++++++++++++++++++++---------- src/qemu/qemu_command.h | 3 +- src/qemu/qemu_process.c | 9 ++- tests/qemuhelptest.c | 10 +++- tests/qemuxml2argvtest.c | 16 ++++-- 7 files changed, 114 insertions(+), 39 deletions(-)
QEMU_CAPS_SCSI_LSI set the flag when "lsi53c895a", bus PCI, alias "lsi" in the output of "qemu -device ?" -device lsi in qemu command line QEMU_CAPS_VIRIO_SCSI_PCI set the flag when "name "virtio-scsi-pci", bus PCI" in the output of qemu devices query. -device virtio-scsi-pci in qemu command line --- src/qemu/qemu_capabilities.c | 7 +++++++ src/qemu/qemu_capabilities.h | 2 ++ 2 files changed, 9 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 82a2870..ac90162 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -170,6 +170,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, "balloon-event", "bridge", /* 100 */ + "lsi", + "virtio-scsi-pci", ); @@ -1455,6 +1457,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 c1b67a6..05e942e 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -136,6 +136,8 @@ enum qemuCapsFlags { QEMU_CAPS_VIRTIO_S390 = 98, /* -device virtio-*-s390 */ QEMU_CAPS_BALLOON_EVENT = 99, /* Async event for balloon changes */ QEMU_CAPS_NETDEV_BRIDGE = 100, /* bridge helper support */ + QEMU_CAPS_SCSI_LSI = 101, /* -device lsi */ + QEMU_CAPS_VIRIO_SCSI_PCI = 102, /* -device virtio-scsi-pci */ QEMU_CAPS_LAST, /* this must always be the last item */ }; -- 1.7.7.6
On 08/07/2012 11:21 AM, Guannan Ren wrote:
QEMU_CAPS_SCSI_LSI set the flag when "lsi53c895a", bus PCI, alias "lsi" in the output of "qemu -device ?" -device lsi in qemu command line
QEMU_CAPS_VIRIO_SCSI_PCI set the flag when "name "virtio-scsi-pci", bus PCI" in the output of qemu devices query. -device virtio-scsi-pci in qemu command line --- src/qemu/qemu_capabilities.c | 7 +++++++ src/qemu/qemu_capabilities.h | 2 ++ 2 files changed, 9 insertions(+), 0 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 08/07/2012 03:46 PM, Eric Blake wrote:
On 08/07/2012 11:21 AM, Guannan Ren wrote:
QEMU_CAPS_SCSI_LSI set the flag when "lsi53c895a", bus PCI, alias "lsi" in the output of "qemu -device ?" -device lsi in qemu command line
QEMU_CAPS_VIRIO_SCSI_PCI set the flag when "name "virtio-scsi-pci", bus PCI" in the output of qemu devices query. -device virtio-scsi-pci in qemu command line --- src/qemu/qemu_capabilities.c | 7 +++++++ src/qemu/qemu_capabilities.h | 2 ++ 2 files changed, 9 insertions(+), 0 deletions(-)
ACK.
I spoke too soon. In isolation, this patch fails 'make check'. Please take the appropriate hunks out of 3/3 and squash them into this patch so that the testsuite passes (that way, 'git bisect' is easier to run without hitting false failures). -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Rename qemuDefaultScsiControllerModel to qemuCheckScsiControllerModel. When scsi model is given explicitly in XML(model > 0) checking if the underlying QEMU supports it or not first, raise an error on checking failure. When the model is not given(mode <= 0), return LSI by default, if the QEMU doesn't support it, raise an error. --- src/qemu/qemu_command.c | 106 +++++++++++++++++++++++++++++++++++----------- src/qemu/qemu_command.h | 3 +- src/qemu/qemu_process.c | 9 +++- 3 files changed, 88 insertions(+), 30 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9999a05..d4791c6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -469,19 +469,58 @@ 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 (STREQ(def->os.arch, "ppc64") && + STREQ(def->os.machine, "pseries")) { + *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI; + } else if (qemuCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI)) { + *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC; + } 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 +530,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 +571,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,13 +888,24 @@ qemuAssignSpaprVIOAddress(virDomainDefPtr def, virDomainDeviceInfoPtr info, return 0; } -int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def) +int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, + virBitmapPtr qemuCaps) { int i, rc; int model; + virBitmapPtr localCaps = NULL; /* Default values match QEMU. See spapr_(llan|vscsi|vty).c */ + if (!qemuCaps) { + /* need to get information from real environment */ + if (qemuCapsExtractVersionInfo(def->emulator, def->os.arch, + false, NULL, + &localCaps) < 0) + goto cleanup; + qemuCaps = localCaps; + } + for (i = 0 ; i < def->nnets; i++) { if (def->nets[i]->model && STREQ(def->nets[i]->model, "spapr-vlan")) @@ -864,21 +913,24 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def) rc = qemuAssignSpaprVIOAddress(def, &def->nets[i]->info, 0x1000ul); if (rc) - return rc; + goto cleanup; } 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) + rc = qemuCheckScsiControllerModel(def, qemuCaps, &model); + + if (rc) + goto cleanup; + 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; rc = qemuAssignSpaprVIOAddress(def, &def->controllers[i]->info, 0x2000ul); if (rc) - return rc; + goto cleanup; } for (i = 0 ; i < def->nserials; i++) { @@ -890,12 +942,16 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def) rc = qemuAssignSpaprVIOAddress(def, &def->serials[i]->info, 0x30000000ul); if (rc) - return rc; + goto cleanup; } /* No other devices are currently supported on spapr-vio */ return 0; + +cleanup: + qemuCapsFree(localCaps); + return -1; } #define QEMU_PCI_ADDRESS_LAST_SLOT 31 @@ -1059,7 +1115,7 @@ int qemuDomainAssignAddresses(virDomainDefPtr def, { int rc; - rc = qemuDomainAssignSpaprVIOAddresses(def); + rc = qemuDomainAssignSpaprVIOAddresses(def, qemuCaps); if (rc) return rc; @@ -2433,9 +2489,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) { @@ -2764,10 +2819,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 946a7ac..e999bc7 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -180,7 +180,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, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 3a08c5b..df4a016 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3120,7 +3120,8 @@ qemuProcessReconnect(void *opaque) } if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) - qemuDomainAssignAddresses(obj->def, priv->qemuCaps, obj); + if ((qemuDomainAssignAddresses(obj->def, priv->qemuCaps, obj)) < 0) + goto error; if (virSecurityManagerReserveLabel(driver->securityManager, obj->def, obj->pid) < 0) goto error; @@ -3585,7 +3586,8 @@ int qemuProcessStart(virConnectPtr conn, */ if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { VIR_DEBUG("Assigning domain PCI addresses"); - qemuDomainAssignAddresses(vm->def, priv->qemuCaps, vm); + if ((qemuDomainAssignAddresses(vm->def, priv->qemuCaps, vm)) < 0) + goto cleanup; } VIR_DEBUG("Building emulator command line"); @@ -4267,7 +4269,8 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, */ if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { VIR_DEBUG("Assigning domain PCI addresses"); - qemuDomainAssignAddresses(vm->def, priv->qemuCaps, vm); + if ((qemuDomainAssignAddresses(vm->def, priv->qemuCaps, vm)) < 0) + goto cleanup; } if ((timestamp = virTimeStringNow()) == NULL) { -- 1.7.7.6
On 08/07/2012 11:21 AM, Guannan Ren wrote:
Rename qemuDefaultScsiControllerModel to qemuCheckScsiControllerModel. When scsi model is given explicitly in XML(model > 0) checking if the underlying QEMU supports it or not first, raise an error on checking failure. When the model is not given(mode <= 0), return LSI by default, if the QEMU doesn't support it, raise an error. --- src/qemu/qemu_command.c | 106 +++++++++++++++++++++++++++++++++++----------- src/qemu/qemu_command.h | 3 +- src/qemu/qemu_process.c | 9 +++- 3 files changed, 88 insertions(+), 30 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9999a05..d4791c6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -469,19 +469,58 @@ 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) {
This looks a bit odd; why not just: 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 {
...then fold this else branch into: case 0:
- return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC; + if (STREQ(def->os.arch, "ppc64") && + STREQ(def->os.machine, "pseries")) { + *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI; + } else if (qemuCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI)) { + *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC; + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to determine model for scsi controller")); + return -1; + } } + + return 0; }
But that's just a formatting recommendation, and not a bug in your code, so I'm okay if you leave it as written.
+int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, + virBitmapPtr qemuCaps) { int i, rc; int model; + virBitmapPtr localCaps = NULL;
/* Default values match QEMU. See spapr_(llan|vscsi|vty).c */
+ if (!qemuCaps) { + /* need to get information from real environment */ + if (qemuCapsExtractVersionInfo(def->emulator, def->os.arch, + false, NULL, + &localCaps) < 0) + goto cleanup; + qemuCaps = localCaps; + }
Yet more evidence why we need to rewrite the capabilities collection code to be caching, but that's a project for another day. ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 08/07/2012 03:55 PM, Eric Blake wrote:
On 08/07/2012 11:21 AM, Guannan Ren wrote:
Rename qemuDefaultScsiControllerModel to qemuCheckScsiControllerModel. When scsi model is given explicitly in XML(model > 0) checking if the underlying QEMU supports it or not first, raise an error on checking failure. When the model is not given(mode <= 0), return LSI by default, if the QEMU doesn't support it, raise an error.
ACK.
This patch introduces even more testsuite breaks than 1/3. Please fold the rest of 3/3 into this patch, and when you push, you should have just two patches, both with testsuite improvements, and where both patches in isolation pass 'make check'. And don't forget the /VIRIO/VIRTIO/ spelling fix. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 08/08/2012 07:03 AM, Eric Blake wrote:
On 08/07/2012 11:21 AM, Guannan Ren wrote:
Rename qemuDefaultScsiControllerModel to qemuCheckScsiControllerModel. When scsi model is given explicitly in XML(model > 0) checking if the underlying QEMU supports it or not first, raise an error on checking failure. When the model is not given(mode <= 0), return LSI by default, if the QEMU doesn't support it, raise an error. ACK. This patch introduces even more testsuite breaks than 1/3. Please fold
On 08/07/2012 03:55 PM, Eric Blake wrote: the rest of 3/3 into this patch, and when you push, you should have just two patches, both with testsuite improvements, and where both patches in isolation pass 'make check'.
And don't forget the /VIRIO/VIRTIO/ spelling fix.
Thanks for the review. Finally, it comes to two patches with separate fixed testcase squashed in , each of them could pass 'make check'. The spelling is fixed too. I choose keeping the format of switch statement code. The *model could be value 0 (VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO) or value -1 model = virXMLPropString(node, "model"); if (!model) def->model = -1; Pushed. Guannan
On Wed, Aug 08, 2012 at 01:21:10AM +0800, Guannan Ren wrote:
Rename qemuDefaultScsiControllerModel to qemuCheckScsiControllerModel. When scsi model is given explicitly in XML(model > 0) checking if the underlying QEMU supports it or not first, raise an error on checking failure. When the model is not given(mode <= 0), return LSI by default, if the QEMU doesn't support it, raise an error. --- src/qemu/qemu_command.c | 106 +++++++++++++++++++++++++++++++++++----------- src/qemu/qemu_command.h | 3 +- src/qemu/qemu_process.c | 9 +++- 3 files changed, 88 insertions(+), 30 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9999a05..d4791c6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -469,19 +469,58 @@ 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)
This method is *modifying* the value in the 'model' parameter, so using 'Check' is a horribly misleading name. This should be 'Set' or something similar to make it clear that it is modifying the parameters, not merely checking them.. 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/08/2012 06:28 PM, Daniel P. Berrange wrote:
Rename qemuDefaultScsiControllerModel to qemuCheckScsiControllerModel. When scsi model is given explicitly in XML(model > 0) checking if the underlying QEMU supports it or not first, raise an error on checking failure. When the model is not given(mode <= 0), return LSI by default, if the QEMU doesn't support it, raise an error. --- src/qemu/qemu_command.c | 106 +++++++++++++++++++++++++++++++++++----------- src/qemu/qemu_command.h | 3 +- src/qemu/qemu_process.c | 9 +++- 3 files changed, 88 insertions(+), 30 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9999a05..d4791c6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -469,19 +469,58 @@ 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) This method is *modifying* the value in the 'model' parameter, so using 'Check' is a horribly misleading name. This should be 'Set' or something similar to make it clear that it is modifying the
On Wed, Aug 08, 2012 at 01:21:10AM +0800, Guannan Ren wrote: parameters, not merely checking them..
Daniel
Agreed, I will post a patch to change that. Thanks.
From qemuCheckScsiControllerModel To qemuSetScsiControllerModel
src/qemu/qemu_command.c | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 56943d5..eb3ef26 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -469,7 +469,7 @@ static int qemuAssignDeviceDiskAliasFixed(virDomainDiskDefPtr disk) } static int -qemuCheckScsiControllerModel(virDomainDefPtr def, +qemuSetScsiControllerModel(virDomainDefPtr def, virBitmapPtr qemuCaps, int *model) { @@ -531,7 +531,7 @@ qemuAssignDeviceDiskAliasCustom(virDomainDefPtr def, virDomainDiskFindControllerModel(def, disk, VIR_DOMAIN_CONTROLLER_TYPE_SCSI); - if ((qemuCheckScsiControllerModel(def, qemuCaps, &controllerModel)) < 0) + if ((qemuSetScsiControllerModel(def, qemuCaps, &controllerModel)) < 0) return -1; } @@ -919,7 +919,7 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, for (i = 0 ; i < def->ncontrollers; i++) { model = def->controllers[i]->model; if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { - rc = qemuCheckScsiControllerModel(def, qemuCaps, &model); + rc = qemuSetScsiControllerModel(def, qemuCaps, &model); if (rc) goto cleanup; } @@ -2489,7 +2489,7 @@ qemuBuildDriveDevStr(virDomainDefPtr def, controllerModel = virDomainDiskFindControllerModel(def, disk, VIR_DOMAIN_CONTROLLER_TYPE_SCSI); - if ((qemuCheckScsiControllerModel(def, qemuCaps, &controllerModel)) < 0) + if ((qemuSetScsiControllerModel(def, qemuCaps, &controllerModel)) < 0) goto error; if (controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC) { @@ -2819,7 +2819,7 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, switch (def->type) { case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: model = def->model; - if ((qemuCheckScsiControllerModel(domainDef, qemuCaps, &model)) < 0) + if ((qemuSetScsiControllerModel(domainDef, qemuCaps, &model)) < 0) return NULL; switch (model) { -- 1.7.7.6
On 08/08/2012 08:46 AM, Guannan Ren wrote:
From qemuCheckScsiControllerModel To qemuSetScsiControllerModel
src/qemu/qemu_command.c | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 56943d5..eb3ef26 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -469,7 +469,7 @@ static int qemuAssignDeviceDiskAliasFixed(virDomainDiskDefPtr disk) }
static int -qemuCheckScsiControllerModel(virDomainDefPtr def, +qemuSetScsiControllerModel(virDomainDefPtr def, virBitmapPtr qemuCaps, int *model)
Fix the indentation on the next two lines. ACK with that tweak. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 08/08/2012 10:54 PM, Eric Blake wrote:
On 08/08/2012 08:46 AM, Guannan Ren wrote:
From qemuCheckScsiControllerModel To qemuSetScsiControllerModel
src/qemu/qemu_command.c | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 56943d5..eb3ef26 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -469,7 +469,7 @@ static int qemuAssignDeviceDiskAliasFixed(virDomainDiskDefPtr disk) }
static int -qemuCheckScsiControllerModel(virDomainDefPtr def, +qemuSetScsiControllerModel(virDomainDefPtr def, virBitmapPtr qemuCaps, int *model) Fix the indentation on the next two lines.
ACK with that tweak.
Fixed. Thanks for the review. Pushed. Guannan
--- 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 75c818c..28d742e 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, @@ -755,7 +757,9 @@ mymain(void) QEMU_CAPS_NO_USER_CONFIG, QEMU_CAPS_HDA_MICRO, QEMU_CAPS_NEC_USB_XHCI, - QEMU_CAPS_NETDEV_BRIDGE); + QEMU_CAPS_NETDEV_BRIDGE, + 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 1adba78..2094984 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -458,16 +458,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); @@ -497,7 +500,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); @@ -757,7 +761,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 08/07/2012 11:21 AM, Guannan Ren wrote:
--- tests/qemuhelptest.c | 10 +++++++--- tests/qemuxml2argvtest.c | 16 ++++++++++------ 2 files changed, 17 insertions(+), 9 deletions(-)
@@ -755,7 +757,9 @@ mymain(void) QEMU_CAPS_NO_USER_CONFIG, QEMU_CAPS_HDA_MICRO, QEMU_CAPS_NEC_USB_XHCI, - QEMU_CAPS_NETDEV_BRIDGE); + QEMU_CAPS_NETDEV_BRIDGE, + QEMU_CAPS_SCSI_LSI, + QEMU_CAPS_VIRIO_SCSI_PCI);
I ack'd patch 1 too quickly. Please s/VIRIO/VIRTIO/ throughout your series. ACK with the capability name spelling fixed. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Guannan Ren