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

[John Ferlan]

On 01/28/2018 03:48 AM, Michal Privoznik wrote:
> 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 at redhat.com>
>> ---
>>  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) {

Change to:

switch ((virDomainControllerModelSCSI) 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:

Change to:

    case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO:
    case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_BUSLOGIC:
    case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VMPVSCSI:
    case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST:

>> +        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.
> 

NB: Went with cast-ing the switch rather than changing from int model to
virDomainControllerModelSCSI model - mainly because the callers define
parameter as int...

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

Tks -

John