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

[Ján Tomko]

On Wed, Dec 20, 2017 at 01:52:16PM +0100, Ján Tomko wrote:
>On Wed, Dec 06, 2017 at 08:08:05AM -0500, 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.
>>
>
>Yet the change adds 'virtio-scsi'.
>
>>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;
>
>This declaration can be moved inside the if to reduce its scope. (I'm
>with Andrea on this [0])
>

Also, assigning -1 to an unsigned variable won't stick.

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

This condition is always true according to my compiler.

Jan

>>+                newModel = model;
>>         }
>>     }
>>
>
>ACK
>
>Jan
>
>[0] https://www.redhat.com/archives/libvir-list/2017-December/msg00563.html



>--
>libvir-list mailing list
>libvir-list at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180104/819343c5/attachment-0001.sig>