Re: [libvirt] [PATCH v5 01/16] qemu: Split qemuDomainSetSCSIControllerModel

On 01/06/2018 12:47 AM, John Ferlan wrote: > Rather than one function serving two purposes, let's split things > up into qemuDomainResetSCSIControllerModel for all current callers > and then add qemuDomainCheckSCSIControllerModel when building the > controller command line to check the capabilities. > > Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; > --- > src/qemu/qemu_alias.c | 4 +-- > src/qemu/qemu_command.c | 62 ++++++++++++++++++++++++++++++++--- > src/qemu/qemu_domain_address.c | 74 +++++++++--------------------------------- > src/qemu/qemu_domain_address.h | 6 ++-- > 4 files changed, 79 insertions(+), 67 deletions(-) > > diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c > index 37fe2aa80..b65276dd9 100644 > --- a/src/qemu/qemu_alias.c > +++ b/src/qemu/qemu_alias.c > @@ -194,8 +194,8 @@ qemuAssignDeviceDiskAlias(virDomainDefPtr def, > virDomainDeviceFindControllerModel(def, &disk->info, > VIR_DOMAIN_CONTROLLER_TYPE_SCSI); > > - if ((qemuDomainSetSCSIControllerModel(def, qemuCaps, > - &controllerModel)) < 0) > + if ((qemuDomainResetSCSIControllerModel(def, qemuCaps, > + &controllerModel)) < 0) > return -1; > } > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index b8aede32d..5c084ae8c 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -1888,6 +1888,57 @@ qemuCheckIOThreads(const virDomainDef *def, > } > > > +static bool > +qemuBuildCheckSCSIControllerModel(virQEMUCapsPtr qemuCaps, > + int model) > +{ > + switch (model) {

> + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC: > + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI)) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("This QEMU doesn't support " > + "the LSI 53C895A SCSI controller")); > + return false; > + } > + break; > + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI: > + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_SCSI)) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("This QEMU doesn't support " > + "virtio scsi controller")); > + return false; > + } > + break; > + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI: > + /*TODO: need checking work here if necessary */ > + break; > + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1068: > + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_MPTSAS1068)) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("This QEMU doesn't support " > + "the LSI SAS1068 (MPT Fusion) controller")); > + return false; > + } > + break; > + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1078: > + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_MEGASAS)) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("This QEMU doesn't support " > + "the LSI SAS1078 (MegaRAID) controller")); > + return false; > + } > + break; > + default:

> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("Unsupported controller model: %s"), > + virDomainControllerModelSCSITypeToString(model)); > + return false; Or, just have this function take virDomainControllerModelSCSI enum and instead of default have VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST and probably VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO; That way, when new model is added compiler identifies this place for adjustment automatically.

> + } > + > + return true; > +} ACK Michal