1. Patchew
  2. Libvirt
  3. View series

[libvirt PATCH] qemu: fix setting of scsi-id for ESP SCSI controllers

Daniel P. Berrangé posted 1 patch 3 years, 5 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20201120160821.976578-1-berrange@redhat.com
src/qemu/qemu_command.c                   | 26 +++++++++++++++++++++--
tests/qemuxml2argvdata/sparc-minimal.args |  4 ++--
2 files changed, 26 insertions(+), 4 deletions(-)
[libvirt PATCH] qemu: fix setting of scsi-id for ESP SCSI controllers
Posted by Daniel P. Berrangé 3 years, 5 months ago 
The ESP SCSI controllers have same requirement as the LSI Logic
controller for each disk to be set via the scsi-id=NNN property,
not the lun=NNN property.

Switching the code to use an enum will force authors to pay attention
to this difference when adding future SCSI controllers.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/qemu/qemu_command.c                   | 26 +++++++++++++++++++++--
 tests/qemuxml2argvdata/sparc-minimal.args |  4 ++--
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index fbaacb8dd8..479bcc0b0c 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1625,17 +1625,39 @@ qemuBuildDiskDeviceStr(const virDomainDef *def,
                                                        disk->info.addr.drive.controller)))
            return NULL;
 
-        if (controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC) {
+        switch ((virDomainControllerModelSCSI)controllerModel) {
+        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC:
+        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_NCR53C90:
+        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DC390:
+        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AM53C974:
             virBufferAsprintf(&opt, ",bus=%s.%d,scsi-id=%d",
                               contAlias,
                               disk->info.addr.drive.bus,
                               disk->info.addr.drive.unit);
-        } else {
+            break;
+
+        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO:
+        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_BUSLOGIC:
+        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1068:
+        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VMPVSCSI:
+        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI:
+        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI:
+        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1078:
+        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_TRANSITIONAL:
+        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_NON_TRANSITIONAL:
             virBufferAsprintf(&opt, ",bus=%s.0,channel=%d,scsi-id=%d,lun=%d",
                               contAlias,
                               disk->info.addr.drive.bus,
                               disk->info.addr.drive.target,
                               disk->info.addr.drive.unit);
+            break;
+
+        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT:
+        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST:
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("Unexpected SCSI controller model %d"),
+                           controllerModel);
+            return NULL;
         }
 
         if (scsiVPDDeviceId)
diff --git a/tests/qemuxml2argvdata/sparc-minimal.args b/tests/qemuxml2argvdata/sparc-minimal.args
index 65cf99c895..0bf4c8f825 100644
--- a/tests/qemuxml2argvdata/sparc-minimal.args
+++ b/tests/qemuxml2argvdata/sparc-minimal.args
@@ -26,9 +26,9 @@ path=/tmp/lib/domain--1-redhat62sparc/monitor.sock,server,nowait \
 -usb \
 -drive file=/home/berrange/VirtualMachines/redhat-6.2-sparc.img,format=qcow2,\
 if=none,id=drive-scsi0-0-0-0 \
--device scsi-hd,bus=scsi.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,\
+-device scsi-hd,bus=scsi.0,scsi-id=0,drive=drive-scsi0-0-0-0,\
 id=scsi0-0-0-0,bootindex=1 \
 -drive file=/home/berrange/VirtualMachines/redhat-6.2-sparc.iso,format=raw,\
 if=none,id=drive-scsi0-0-0-1,readonly=on \
--device scsi-cd,bus=scsi.0,channel=0,scsi-id=0,lun=1,drive=drive-scsi0-0-0-1,\
+-device scsi-cd,bus=scsi.0,scsi-id=1,drive=drive-scsi0-0-0-1,\
 id=scsi0-0-0-1
-- 
2.28.0
Re: [libvirt PATCH] qemu: fix setting of scsi-id for ESP SCSI controllers
Posted by Laine Stump 3 years, 5 months ago 
On 11/20/20 11:08 AM, Daniel P. Berrangé wrote:
> The ESP SCSI controllers have same requirement as the LSI Logic
> controller for each disk to be set via the scsi-id=NNN property,
> not the lun=NNN property.


Maybe you could list the 3 controllers that are "ESP controllers" in the 
commit log message (it confused me for a minute because I didn't see the 
original commit where you added the support, so I didn't realize you'd 
added 3 separate new models, and not just a single new model) (I 
suspected it after seeing the code, of course, and a look with git blame 
verified my suspicion, so it was only momentary :-)


>
> Switching the code to use an enum will force authors to pay attention
> to this difference when adding future SCSI controllers.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>


Reviewed-by: Laine Stump <laine@redhat.com>


(with extra points for keeping even the thought of last-century hardware 
alive :-)


> ---
>   src/qemu/qemu_command.c                   | 26 +++++++++++++++++++++--
>   tests/qemuxml2argvdata/sparc-minimal.args |  4 ++--
>   2 files changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index fbaacb8dd8..479bcc0b0c 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -1625,17 +1625,39 @@ qemuBuildDiskDeviceStr(const virDomainDef *def,
>                                                          disk->info.addr.drive.controller)))
>              return NULL;
>   
> -        if (controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC) {
> +        switch ((virDomainControllerModelSCSI)controllerModel) {
> +        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC:
> +        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_NCR53C90:
> +        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DC390:
> +        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AM53C974:
>               virBufferAsprintf(&opt, ",bus=%s.%d,scsi-id=%d",
>                                 contAlias,
>                                 disk->info.addr.drive.bus,
>                                 disk->info.addr.drive.unit);
> -        } else {
> +            break;
> +
> +        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO:
> +        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_BUSLOGIC:
> +        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1068:
> +        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VMPVSCSI:
> +        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI:
> +        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI:
> +        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1078:
> +        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_TRANSITIONAL:
> +        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_NON_TRANSITIONAL:
>               virBufferAsprintf(&opt, ",bus=%s.0,channel=%d,scsi-id=%d,lun=%d",
>                                 contAlias,
>                                 disk->info.addr.drive.bus,
>                                 disk->info.addr.drive.target,
>                                 disk->info.addr.drive.unit);
> +            break;
> +
> +        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT:
> +        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST:
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Unexpected SCSI controller model %d"),
> +                           controllerModel);
> +            return NULL;
>           }
>   
>           if (scsiVPDDeviceId)
> diff --git a/tests/qemuxml2argvdata/sparc-minimal.args b/tests/qemuxml2argvdata/sparc-minimal.args
> index 65cf99c895..0bf4c8f825 100644
> --- a/tests/qemuxml2argvdata/sparc-minimal.args
> +++ b/tests/qemuxml2argvdata/sparc-minimal.args
> @@ -26,9 +26,9 @@ path=/tmp/lib/domain--1-redhat62sparc/monitor.sock,server,nowait \
>   -usb \
>   -drive file=/home/berrange/VirtualMachines/redhat-6.2-sparc.img,format=qcow2,\
>   if=none,id=drive-scsi0-0-0-0 \
> --device scsi-hd,bus=scsi.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,\
> +-device scsi-hd,bus=scsi.0,scsi-id=0,drive=drive-scsi0-0-0-0,\
>   id=scsi0-0-0-0,bootindex=1 \
>   -drive file=/home/berrange/VirtualMachines/redhat-6.2-sparc.iso,format=raw,\
>   if=none,id=drive-scsi0-0-0-1,readonly=on \
> --device scsi-cd,bus=scsi.0,channel=0,scsi-id=0,lun=1,drive=drive-scsi0-0-0-1,\
> +-device scsi-cd,bus=scsi.0,scsi-id=1,drive=drive-scsi0-0-0-1,\
>   id=scsi0-0-0-1

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.