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é <berra...@redhat.com>
Reviewed-by: Laine Stump <la...@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
