[libvirt] [PATCH 3/4] conf: Use existing SCSI hostdev model to create new

[John Ferlan]

On 12/15/2017 11:32 AM, Eric Farman wrote:
> 
> 
> On 12/06/2017 08:08 AM, John Ferlan wrote:
>> In virDomainDefMaybeAddHostdevSCSIcontroller when we add a new
>> controller because someone neglected to add one or we're adding
>> one because the existing one is full, we should copy over the
>> model number from the existing controller since whatever we
>> create should at least have the same characteristics as the one
>> we cannot use because it's full.
>>
>> NB: This affects the existing hostdev-scsi-autogen-address test
>> which would add a default ('lsi') SCSI controller for the various
>> scsi_host's that would create a controller for the hostdev.
>>
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>>   src/conf/domain_conf.c                                    | 13
>> ++++++++++++-
>>   tests/qemuxml2xmloutdata/hostdev-scsi-autogen-address.xml |  2 +-
>>   2 files changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 66e21c4bd..61b4a0075 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -17689,12 +17689,22 @@
>> virDomainDefMaybeAddHostdevSCSIcontroller(virDomainDefPtr def)
>>       size_t i;
>>       int maxController = -1;
>>       virDomainHostdevDefPtr hostdev;
>> +    virDomainControllerModelSCSI model = -1;
>> +    virDomainControllerModelSCSI newModel = -1;
>>
>>       for (i = 0; i < def->nhostdevs; i++) {
>>           hostdev = def->hostdevs[i];
>>           if (virHostdevIsSCSIDevice(hostdev) &&
>>               (int)hostdev->info->addr.drive.controller >
>> maxController) {
>>               maxController = hostdev->info->addr.drive.controller;
>> +            /* We may be creating a new controller because this one
>> is full.
>> +             * So let's grab the model from it and update the model
>> we're
>> +             * going to add as long as this one isn't undefined. The
>> premise
>> +             * being keeping the same controller model for all SCSI
>> hostdevs. */
>> +            model = virDomainDeviceFindControllerModel(def,
>> hostdev->info,
>> +                                                      
>> VIR_DOMAIN_CONTROLLER_TYPE_SCSI);
>> +            if (model != -1)
>> +                newModel = model;
> 
> What's the harm if the last controller were undefined?  Wouldn't it get
> populated down the road anyway if one were not set at this point (we
> send -1 today, after all).  That could eliminate one of the two
> virDomainControllerModelSCSI variables here, I would think.

Yes, a -1 is passed along which (for now) means a SCSI LSI controller
would be created except for pseries which would generate something
different.

John

> 
> But, either way I think it's fine.
> 
> Reviewed-by: Eric Farman <farman at linux.vnet.ibm.com>
> 
>>           }
>>       }
>>
>> @@ -17702,7 +17712,8 @@
>> virDomainDefMaybeAddHostdevSCSIcontroller(virDomainDefPtr def)
>>           return 0;
>>
>>       for (i = 0; i <= maxController; i++) {
>> -        if (virDomainDefMaybeAddController(def,
>> VIR_DOMAIN_CONTROLLER_TYPE_SCSI, i, -1) < 0)
>> +        if (virDomainDefMaybeAddController(def,
>> VIR_DOMAIN_CONTROLLER_TYPE_SCSI,
>> +                                           i, newModel) < 0)
>>               return -1;
>>       }
>>
>> diff --git a/tests/qemuxml2xmloutdata/hostdev-scsi-autogen-address.xml
>> b/tests/qemuxml2xmloutdata/hostdev-scsi-autogen-address.xml
>> index 8e93056ee..cea212b64 100644
>> --- a/tests/qemuxml2xmloutdata/hostdev-scsi-autogen-address.xml
>> +++ b/tests/qemuxml2xmloutdata/hostdev-scsi-autogen-address.xml
>> @@ -29,7 +29,7 @@
>>         <address type='pci' domain='0x0000' bus='0x00' slot='0x01'
>> function='0x1'/>
>>       </controller>
>>       <controller type='pci' index='0' model='pci-root'/>
>> -    <controller type='scsi' index='1'>
>> +    <controller type='scsi' index='1' model='virtio-scsi'>
>>         <address type='pci' domain='0x0000' bus='0x00' slot='0x04'
>> function='0x0'/>
>>       </controller>
>>       <input type='mouse' bus='ps2'/>
>>
>