[libvirt] [PATCH 3/3] Automatic SCSI controller creation in SCSI disk hotplug broken

[John Ferlan]

On 11/30/2015 06:06 AM, Boris Fiuczynski wrote:
> When a SCSI disk is hotplugged to a domain that does not have the required
> scsi controller already defined the following internal error occurs
> 
> error: Failed to attach device from scsi_disk.xml
> error: internal error: Could not find scsi controller with index 0 required for device
> 
> Commit 0260506c added in method qemuBuildDriveDevStr a lookup of the controller
> alias. The internal error occurs because in method qemuDomainAttachSCSIDisk
> the automatic creation of the potentially missing SCSI controller occurs after
> calling qemuBuildDriveDevStr.
> 
> This patch reverses the calling sequence.
> 
> Signed-off-by: Boris Fiuczynski <fiuczy at linux.vnet.ibm.com>
> Reviewed-by: Bjoern Walk <bwalk at linux.vnet.ibm.com>
> Reviewed-by: Stefan Zimmermann <stzi at linux.vnet.ibm.com>
> ---
>  src/qemu/qemu_hotplug.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 8804d3d..210d485 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -607,6 +607,12 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
>          goto error;
>      }

Would you say the following is an accurate description to add?

    /* Let's make sure our disk has a controller defined and loaded
     * before trying add the disk. The virDomainDiskDefAssignAddress
     * doesn't try to add a controller if one doesn't exist, it just
     * assigns the disk to the calculated spot.
     */

>  
> +    for (i = 0; i <= disk->info.addr.drive.controller; i++) {
> +        cont = qemuDomainFindOrCreateSCSIDiskController(driver, vm, i);
> +        if (!cont)
> +            goto error;
> +    }
> +
>      if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
>          if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0)
>              goto error;
> @@ -617,12 +623,6 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
>      if (!(drivestr = qemuBuildDriveStr(conn, disk, false, priv->qemuCaps)))
>          goto error;
>  
> -    for (i = 0; i <= disk->info.addr.drive.controller; i++) {
> -        cont = qemuDomainFindOrCreateSCSIDiskController(driver, vm, i);
> -        if (!cont)
> -            goto error;
> -    }
> -
>      /* Tell clang that "cont" is non-NULL.
>         This is because disk->info.addr.driver.controller is unsigned,
>         and hence the above loop must iterate at least once.  */
> 

Is there a reason you chose to not grab the clang check too?

John