10:57 a.m.
Andrea Bolognani (3):
tests: Add controller-scsi-auto
qemu: Handle MODEL_SCSI_{AUTO,DEFAULT} appropriately
qemu: Use virDomainControllerDefNew() more
src/qemu/qemu_command.c | 4 +--
src/qemu/qemu_domain_address.c | 4 +--
src/qemu/qemu_hotplug.c | 4 +--
src/qemu/qemu_validate.c | 2 +-
.../controller-scsi-auto.x86_64-latest.args | 32 +++++++++++++++++++
.../controller-scsi-auto.x86_64-latest.xml | 30 +++++++++++++++++
.../qemuxmlconfdata/controller-scsi-auto.xml | 15 +++++++++
tests/qemuxmlconftest.c | 1 +
8 files changed, 85 insertions(+), 7 deletions(-)
create mode 100644 tests/qemuxmlconfdata/controller-scsi-auto.x86_64-latest.args
create mode 100644 tests/qemuxmlconfdata/controller-scsi-auto.x86_64-latest.xml
create mode 100644 tests/qemuxmlconfdata/controller-scsi-auto.xml
--
2.43.0
10:57 a.m.
New subject: [PATCH 1/3] tests: Add controller-scsi-auto
The "auto" SCSI controller model was introduced for use in the
ESX driver, but the QEMU driver doesn't reject the value.
Add a test case showing the behavior when such a configuration
is encountered.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
.../controller-scsi-auto.x86_64-latest.args | 32 +++++++++++++++++++
.../controller-scsi-auto.x86_64-latest.xml | 30 +++++++++++++++++
.../qemuxmlconfdata/controller-scsi-auto.xml | 15 +++++++++
tests/qemuxmlconftest.c | 1 +
4 files changed, 78 insertions(+)
create mode 100644 tests/qemuxmlconfdata/controller-scsi-auto.x86_64-latest.args
create mode 100644 tests/qemuxmlconfdata/controller-scsi-auto.x86_64-latest.xml
create mode 100644 tests/qemuxmlconfdata/controller-scsi-auto.xml
diff --git a/tests/qemuxmlconfdata/controller-scsi-auto.x86_64-latest.args
b/tests/qemuxmlconfdata/controller-scsi-auto.x86_64-latest.args
new file mode 100644
index 0000000000..9283d9ab19
--- /dev/null
+++ b/tests/qemuxmlconfdata/controller-scsi-auto.x86_64-latest.args
@@ -0,0 +1,32 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/var/lib/libvirt/qemu/domain--1-guest \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-guest/.local/share \
+XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-guest/.cache \
+XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-guest/.config \
+/usr/bin/qemu-system-x86_64 \
+-name guest=guest,debug-threads=on \
+-S \
+-object
'{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-guest/master-key.aes"}'
\
+-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram,acpi=off \
+-accel tcg \
+-cpu qemu64 \
+-m size=219136k \
+-object
'{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}'
\
+-overcommit mem-lock=off \
+-smp 8,sockets=8,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-boot strict=on \
+-device
'{"driver":"lsi","id":"scsi0","bus":"pci.0","addr":"0x2"}'
\
+-audiodev
'{"id":"audio1","driver":"none"}' \
+-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
+-msg timestamp=on
diff --git a/tests/qemuxmlconfdata/controller-scsi-auto.x86_64-latest.xml
b/tests/qemuxmlconfdata/controller-scsi-auto.x86_64-latest.xml
new file mode 100644
index 0000000000..311f9ae8db
--- /dev/null
+++ b/tests/qemuxmlconfdata/controller-scsi-auto.x86_64-latest.xml
@@ -0,0 +1,30 @@
+<domain type='qemu'>
+ <name>guest</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>8</vcpu>
+ <os>
+ <type arch='x86_64' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <cpu mode='custom' match='exact' check='none'>
+ <model fallback='forbid'>qemu64</model>
+ </cpu>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <controller type='scsi' index='0' model='lsilogic'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x02' function='0x0'/>
+ </controller>
+ <controller type='usb' index='0' model='none'/>
+ <controller type='pci' index='0' model='pci-root'/>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <audio id='1' type='none'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxmlconfdata/controller-scsi-auto.xml
b/tests/qemuxmlconfdata/controller-scsi-auto.xml
new file mode 100644
index 0000000000..38699b8c82
--- /dev/null
+++ b/tests/qemuxmlconfdata/controller-scsi-auto.xml
@@ -0,0 +1,15 @@
+<domain type='qemu'>
+ <name>guest</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <vcpu placement='static'>8</vcpu>
+ <os>
+ <type arch='x86_64' machine='pc'>hvm</type>
+ </os>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <controller type='scsi' index='0' model='auto'/>
+ <controller type='usb' index='0' model='none'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c
index 0188337209..0a27be77a8 100644
--- a/tests/qemuxmlconftest.c
+++ b/tests/qemuxmlconftest.c
@@ -1586,6 +1586,7 @@ mymain(void)
DO_TEST_CAPS_LATEST("disk-scsi-disk-vpd");
DO_TEST_CAPS_LATEST_PARSE_ERROR("disk-scsi-disk-vpd-build-error");
DO_TEST_CAPS_LATEST("controller-virtio-scsi");
+ DO_TEST_CAPS_LATEST("controller-scsi-auto");
DO_TEST_CAPS_LATEST("disk-sata-device");
DO_TEST_CAPS_LATEST("disk-aio");
DO_TEST_CAPS_LATEST("disk-aio-io_uring");
--
2.43.0
10:57 a.m.
New subject: [PATCH 2/3] qemu: Handle MODEL_SCSI_{AUTO,DEFAULT} appropriately
The qemuDomainGetSCSIControllerModel() function, which is
responsible for choosing a model for a SCSI controller that
didn't have one provided by the user, considers values >0 to
mean "model has been set".
Since MODEL_SCSI_AUTO == 0, this means that such a value is
considered the same as MODEL_SCSI_DEFAULT (-1). This makes
sense, as not specifying a model name or explicitly asking for
one to be automatically chosen intuitively should result in
the same behavior.
Specifically, there is no case in which a value of
MODEL_SCSI_AUTO or MODEL_SCSI_DEFAULT is encountered after the
initial controller creation: it is either replaced with an
actual model, or an error is raised.
Despite this, there are a few places in the QEMU driver where
we incorrectly treat these values as if they were actual
model names. To reduce confusion, make sure that no longer
happens.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_command.c | 4 ++--
src/qemu/qemu_domain_address.c | 4 ++--
src/qemu/qemu_validate.c | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 31d42495f4..aa77821331 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -505,7 +505,6 @@ qemuBuildDeviceAddresDriveProps(virJSONValue *props,
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:
@@ -527,6 +526,7 @@ qemuBuildDeviceAddresDriveProps(virJSONValue *props,
break;
case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO:
case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST:
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unexpected SCSI controller model %1$d"),
@@ -2685,7 +2685,6 @@ qemuBuildControllerSCSIDevProps(virDomainControllerDef *def,
case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DC390:
driver = "dc-390";
break;
- case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO:
case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_BUSLOGIC:
case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_NCR53C90: /* It is built-in dev */
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -2693,6 +2692,7 @@ qemuBuildControllerSCSIDevProps(virDomainControllerDef *def,
virDomainControllerModelSCSITypeToString(def->model));
return NULL;
case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO:
case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST:
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unexpected SCSI controller model %1$d"),
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 49fd4d454d..af486aea97 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -567,7 +567,6 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDef *dev,
case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
switch ((virDomainControllerModelSCSI) cont->model) {
- case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT:
case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_NCR53C90:
return 0;
@@ -577,7 +576,6 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDef *dev,
/* Transitional devices only work in conventional PCI slots */
case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_TRANSITIONAL:
- case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO:
case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_BUSLOGIC:
case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC:
case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1068:
@@ -588,6 +586,8 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDef *dev,
case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AM53C974:
return pciFlags;
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO:
case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST:
return 0;
}
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index d3bea65139..73afd094a9 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -3364,7 +3364,6 @@ qemuValidateCheckSCSIControllerModel(virQEMUCaps *qemuCaps,
return false;
}
break;
- case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO:
case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_BUSLOGIC:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Unsupported controller model: %1$s"),
@@ -3389,6 +3388,7 @@ qemuValidateCheckSCSIControllerModel(virQEMUCaps *qemuCaps,
}
return true;
case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO:
case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST:
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unexpected SCSI controller model %1$d"),
--
2.43.0
10:57 a.m.
New subject: [PATCH 3/3] qemu: Use virDomainControllerDefNew() more
Instead of open-coding a partial version of it.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 137904034d..0f7ffb6506 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -873,9 +873,9 @@ qemuDomainFindOrCreateSCSIDiskController(virDomainObj *vm,
/* No SCSI controller present, for backward compatibility we
* now hotplug a controller */
- cont = g_new0(virDomainControllerDef, 1);
- cont->type = VIR_DOMAIN_CONTROLLER_TYPE_SCSI;
+ cont = virDomainControllerDefNew(VIR_DOMAIN_CONTROLLER_TYPE_SCSI);
cont->idx = controller;
+
if (model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT)
cont->model = qemuDomainGetSCSIControllerModel(vm->def, cont,
priv->qemuCaps);
else
--
2.43.0
9:34 a.m.
On 1/30/24 10:57 AM, Andrea Bolognani wrote:
Andrea Bolognani (3):
tests: Add controller-scsi-auto
qemu: Handle MODEL_SCSI_{AUTO,DEFAULT} appropriately
qemu: Use virDomainControllerDefNew() more
src/qemu/qemu_command.c | 4 +--
src/qemu/qemu_domain_address.c | 4 +--
src/qemu/qemu_hotplug.c | 4 +--
src/qemu/qemu_validate.c | 2 +-
.../controller-scsi-auto.x86_64-latest.args | 32 +++++++++++++++++++
.../controller-scsi-auto.x86_64-latest.xml | 30 +++++++++++++++++
.../qemuxmlconfdata/controller-scsi-auto.xml | 15 +++++++++
tests/qemuxmlconftest.c | 1 +
8 files changed, 85 insertions(+), 7 deletions(-)
create mode 100644 tests/qemuxmlconfdata/controller-scsi-auto.x86_64-latest.args
create mode 100644 tests/qemuxmlconfdata/controller-scsi-auto.x86_64-latest.xml
create mode 100644 tests/qemuxmlconfdata/controller-scsi-auto.xml
Reviewed-by: Jonathon Jongsma <jjongsma(a)redhat.com>
295
days inactive
296
days old
4 comments
2 participants
participants (2)
-
Andrea Bolognani -
Jonathon Jongsma