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@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list