Commit 54fa1b44afc ("conf: Add loadparm boot option for a boot device")
added the ability to specify a loadparm parameter on a <boot/> tag, while
commit 29ba41c2d40 ("qemu: Add loadparm to qemu command line string")
added that value to the QEMU "-machine" command line parameters.

Unfortunately, the latter commit only looked at disks and network
devices for boot information, even though anything with
VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT could potentially have this tag.
In practice, a <hostdev> tag pointing to a passthrough (SCSI or DASD)
disk device can be used in this way, which means the loadparm is
accepted, but not given to QEMU.

Correct this, and add some XML/argv tests.

Signed-off-by: Eric Farman <farman@linux.ibm.com>
---
 src/qemu/qemu_command.c                       | 24 ++++++++++++++
 ...machine-loadparm-hostdev.s390x-latest.args | 33 +++++++++++++++++++
 .../machine-loadparm-hostdev.xml              | 23 +++++++++++++
 tests/qemuxml2argvtest.c                      |  1 +
 .../machine-loadparm-hostdev.s390x-latest.xml | 33 +++++++++++++++++++
 tests/qemuxml2xmltest.c                       |  1 +
 6 files changed, 115 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/machine-loadparm-hostdev.s390x-latest.args
 create mode 100644 tests/qemuxml2argvdata/machine-loadparm-hostdev.xml
 create mode 100644 tests/qemuxml2xmloutdata/machine-loadparm-hostdev.s390x-latest.xml

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 0f0f72fbea..929bcc0be1 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6595,6 +6595,30 @@ qemuAppendLoadparmMachineParm(virBuffer *buf,
             return;
         }
     }
+
+    for (i = 0; i< def->nhostdevs; i++) {
+        virDomainHostdevDef *hostdev = def->hostdevs[i];
+        virDomainHostdevSubsys *subsys = &hostdev->source.subsys;
+        virDomainHostdevSubsysMediatedDev *mdevsrc = &subsys->u.mdev;
+
+        if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
+            continue;
+
+        /* Only get the load parameter from a bootable disk */
+        if (subsys->type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV &&
+            subsys->type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI)
+            continue;
+
+        /* For MDEV hostdevs, only CCW types are bootable */
+        if (subsys->type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV &&
+            mdevsrc->model != VIR_MDEV_MODEL_TYPE_VFIO_CCW)
+            continue;
+
+        if (hostdev->info->bootIndex == 1 && hostdev->info->loadparm) {
+            virBufferAsprintf(buf, ",loadparm=%s", hostdev->info->loadparm);
+            return;
+        }
+    }
 }
 
 
diff --git a/tests/qemuxml2argvdata/machine-loadparm-hostdev.s390x-latest.args b/tests/qemuxml2argvdata/machine-loadparm-hostdev.s390x-latest.args
new file mode 100644
index 0000000000..0b59d67ea1
--- /dev/null
+++ b/tests/qemuxml2argvdata/machine-loadparm-hostdev.s390x-latest.args
@@ -0,0 +1,33 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1 \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.local/share \
+XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.cache \
+XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \
+/usr/bin/qemu-system-s390x \
+-name guest=QEMUGuest1,debug-threads=on \
+-S \
+-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-QEMUGuest1/master-key.aes"}' \
+-machine s390-ccw-virtio,usb=off,dump-guest-core=off,memory-backend=s390.ram,loadparm=2 \
+-accel tcg \
+-cpu qemu \
+-m 512 \
+-object '{"qom-type":"memory-backend-ram","id":"s390.ram","size":536870912}' \
+-overcommit mem-lock=off \
+-smp 2,sockets=2,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 \
+-audiodev '{"id":"audio1","driver":"none"}' \
+-device vfio-ccw,id=hostdev0,sysfsdev=/sys/bus/mdev/devices/90c6c135-ad44-41d0-b1b7-bae47de48627,bootindex=1,devno=fe.0.0000 \
+-device virtio-balloon-ccw,id=balloon0,devno=fe.0.0001 \
+-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
+-msg timestamp=on
diff --git a/tests/qemuxml2argvdata/machine-loadparm-hostdev.xml b/tests/qemuxml2argvdata/machine-loadparm-hostdev.xml
new file mode 100644
index 0000000000..70ce9789cb
--- /dev/null
+++ b/tests/qemuxml2argvdata/machine-loadparm-hostdev.xml
@@ -0,0 +1,23 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>524288</memory>
+  <currentMemory unit='KiB'>524288</currentMemory>
+  <vcpu placement='static'>2</vcpu>
+  <os>
+    <type arch='s390x' machine='s390-ccw-virtio'>hvm</type>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-s390x</emulator>
+    <hostdev mode='subsystem' type='mdev' model='vfio-ccw'>
+      <source>
+        <address uuid='90c6c135-ad44-41d0-b1b7-bae47de48627'/>
+      </source>
+      <boot order='1' loadparm='2'/>
+    </hostdev>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 60fc69a92c..c879fa90e0 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2629,6 +2629,7 @@ mymain(void)
 
     DO_TEST_NOCAPS("machine-loadparm-s390");
     DO_TEST_NOCAPS("machine-loadparm-net-s390");
+    DO_TEST_CAPS_ARCH_LATEST("machine-loadparm-hostdev", "s390x");
     DO_TEST_NOCAPS("machine-loadparm-multiple-disks-nets-s390");
     DO_TEST_PARSE_ERROR_NOCAPS("machine-loadparm-s390-char-invalid");
     DO_TEST_PARSE_ERROR_NOCAPS("machine-loadparm-s390-len-invalid");
diff --git a/tests/qemuxml2xmloutdata/machine-loadparm-hostdev.s390x-latest.xml b/tests/qemuxml2xmloutdata/machine-loadparm-hostdev.s390x-latest.xml
new file mode 100644
index 0000000000..f564d6deb0
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/machine-loadparm-hostdev.s390x-latest.xml
@@ -0,0 +1,33 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>524288</memory>
+  <currentMemory unit='KiB'>524288</currentMemory>
+  <vcpu placement='static'>2</vcpu>
+  <os>
+    <type arch='s390x' machine='s390-ccw-virtio'>hvm</type>
+  </os>
+  <cpu mode='custom' match='exact' check='none'>
+    <model fallback='forbid'>qemu</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-s390x</emulator>
+    <controller type='pci' index='0' model='pci-root'/>
+    <audio id='1' type='none'/>
+    <hostdev mode='subsystem' type='mdev' managed='no' model='vfio-ccw'>
+      <source>
+        <address uuid='90c6c135-ad44-41d0-b1b7-bae47de48627'/>
+      </source>
+      <boot order='1' loadparm='2'/>
+      <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/>
+    </hostdev>
+    <memballoon model='virtio'>
+      <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0001'/>
+    </memballoon>
+    <panic model='s390'/>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 441bfcbb97..a1be43ab34 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -227,6 +227,7 @@ mymain(void)
     DO_TEST_NOCAPS("machine-core-off");
     DO_TEST_CAPS_LATEST("machine-smm-on");
     DO_TEST_CAPS_LATEST("machine-smm-off");
+    DO_TEST_CAPS_ARCH_LATEST("machine-loadparm-hostdev", "s390x");
     DO_TEST_NOCAPS("machine-loadparm-multiple-disks-nets-s390");
     DO_TEST_NOCAPS("default-kvm-host-arch");
     DO_TEST_NOCAPS("default-qemu-host-arch");
-- 
2.37.2

On 3/10/23 03:38, Eric Farman wrote:
> Commit 54fa1b44afc ("conf: Add loadparm boot option for a boot device")
> added the ability to specify a loadparm parameter on a <boot/> tag, while
> commit 29ba41c2d40 ("qemu: Add loadparm to qemu command line string")
> added that value to the QEMU "-machine" command line parameters.
> 
> Unfortunately, the latter commit only looked at disks and network
> devices for boot information, even though anything with
> VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT could potentially have this tag.
> In practice, a <hostdev> tag pointing to a passthrough (SCSI or DASD)
> disk device can be used in this way, which means the loadparm is
> accepted, but not given to QEMU.
> 
> Correct this, and add some XML/argv tests.
> 
> Signed-off-by: Eric Farman <farman@linux.ibm.com>
> ---
>  src/qemu/qemu_command.c                       | 24 ++++++++++++++
>  ...machine-loadparm-hostdev.s390x-latest.args | 33 +++++++++++++++++++
>  .../machine-loadparm-hostdev.xml              | 23 +++++++++++++
>  tests/qemuxml2argvtest.c                      |  1 +
>  .../machine-loadparm-hostdev.s390x-latest.xml | 33 +++++++++++++++++++
>  tests/qemuxml2xmltest.c                       |  1 +
>  6 files changed, 115 insertions(+)
>  create mode 100644 tests/qemuxml2argvdata/machine-loadparm-hostdev.s390x-latest.args
>  create mode 100644 tests/qemuxml2argvdata/machine-loadparm-hostdev.xml
>  create mode 100644 tests/qemuxml2xmloutdata/machine-loadparm-hostdev.s390x-latest.xml

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

and pushed.

Michal