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