[libvirt] [PATCH 4/4] conf: Fix generating addresses for SCSI hostdev

[Ján Tomko]

On Wed, Dec 06, 2017 at 08:08:06AM -0500, John Ferlan wrote:
>https://bugzilla.redhat.com/show_bug.cgi?id=1519130
>
>Commit id 'dc692438' reverted the automagic addition of a SCSI
>controller attempt during virDomainHostdevAssignAddress; however,
>the logic to determine where to place the next_unit depended upon
>the "new" controller being added.  Without the new controller the
>the next time through the call for the next SCSI hostdev found
>would result in the "next_unit" never changing from 0 (zero) and
>as a result the addition of the device will fail due to being a
>duplicate unit number of the first with the error message:
>
>  virDomainDefCheckDuplicateDriveAddresses:$line : unsupported
>      configuration: SCSI host address controller='0' bus='1'
>      target='0' unit='0' in use by another SCSI host device
>
>So instead of walking the controller list looking for SCSI
>controllers, all we can do is "pretend" that they exist and
>allow other code to create them later as necessary.
>
>Signed-off-by: John Ferlan <jferlan at redhat.com>
>---
> src/conf/domain_conf.c | 31 +++++++++++++------------------
> 1 file changed, 13 insertions(+), 18 deletions(-)
>
>diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>index 61b4a0075..73c6708cf 100644
>--- a/src/conf/domain_conf.c
>+++ b/src/conf/domain_conf.c
>@@ -4322,10 +4322,8 @@ virDomainHostdevAssignAddress(virDomainXMLOptionPtr xmlopt,
>                               const virDomainDef *def,
>                               virDomainHostdevDefPtr hostdev)
> {
>-    int next_unit = 0;

Please keep the descriptive 'next_unit' variable name.

>-    unsigned controller = 0;
>+    int controller = 0;
>     unsigned int max_unit;
>-    size_t i;
>     int ret;
>
>     if (xmlopt->config.features & VIR_DOMAIN_DEF_FEATURE_WIDE_SCSI)
>@@ -4333,33 +4331,30 @@ virDomainHostdevAssignAddress(virDomainXMLOptionPtr xmlopt,
>     else
>         max_unit = SCSI_NARROW_BUS_MAX_CONT_UNIT;
>
>-    for (i = 0; i < def->ncontrollers; i++) {
>-        if (def->controllers[i]->type != VIR_DOMAIN_CONTROLLER_TYPE_SCSI)
>-            continue;
>-
>-        controller++;
>-        ret = virDomainControllerSCSINextUnit(def, max_unit,
>-                                              def->controllers[i]->idx);
>-        if (ret >= 0) {
>-            next_unit = ret;
>-            controller = def->controllers[i]->idx;
>-            break;
>-        }
>-    }
>-
>     /* NB: Do not attempt calling virDomainDefMaybeAddController to
>      * automagically add a "new" controller. Doing so will result in
>      * qemuDomainFindOrCreateSCSIDiskController "finding" the controller
>      * in the domain def list and thus not hotplugging the controller as
>      * well as the hostdev in the event that there are either no SCSI
>      * controllers defined or there was no space on an existing one.
>+     *
>+     * Because we cannot add a controller, then we should not walk the
>+     * defined controllers list in order to find empty space. Doing
>+     * so fails to return the valid next unit number for the 2nd
>+     * hostdev being added to the as yet to be created controller.
>      */
>+    do {
>+        ret = virDomainControllerSCSINextUnit(def, max_unit, controller);
>+        if (ret < 0)
>+            controller++;
>+    } while (ret < 0);
>+

Easier to read as:
for (next_unit = -1; next_unit < -1; controller++)
    next_unit = virDomainControllerSCSINextUnit(def, max_unit, controller);

ACK

Jan
>
>     hostdev->info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE;
>     hostdev->info->addr.drive.controller = controller;
>     hostdev->info->addr.drive.bus = 0;
>     hostdev->info->addr.drive.target = 0;
>-    hostdev->info->addr.drive.unit = next_unit;
>+    hostdev->info->addr.drive.unit = ret;
>
>     return 0;
> }
>-- 
>2.13.6
>
>--
>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/20171220/662d77f1/attachment-0001.sig>