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@xxxxxxxxxx>
>> ---
>> 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@xxxxxxxxxxxxxxxxxx>
>
>> }
>> }
>>
>> @@ -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'/>
>>
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
