Re: [PATCH 3/3] qemu: command: add multi boot device support on s390x

On Wed, Nov 06, 2024 at 15:31:57 +0100, Boris Fiuczynski wrote: > If QEMU supports multi boot device make use of it instead of using the > single boot device machine parameter. > > Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com&gt; > --- > src/qemu/qemu_command.c | 40 ++++++++++++--- > src/qemu/qemu_command.h | 6 ++- > src/qemu/qemu_hotplug.c | 6 ++- > .../machine-loadparm-hostdev.s390x-9.1.0.args | 33 ++++++++++++ > .../machine-loadparm-hostdev.s390x-9.1.0.xml | 33 ++++++++++++ > ...machine-loadparm-hostdev.s390x-latest.args | 4 +- > ...-multiple-disks-nets-s390.s390x-9.1.0.args | 40 +++++++++++++++ > ...m-multiple-disks-nets-s390.s390x-9.1.0.xml | 51 +++++++++++++++++++ > ...multiple-disks-nets-s390.s390x-latest.args | 8 +-- > ...machine-loadparm-net-s390.s390x-9.1.0.args | 34 +++++++++++++ > .../machine-loadparm-net-s390.s390x-9.1.0.xml | 32 ++++++++++++ > ...achine-loadparm-net-s390.s390x-latest.args | 4 +- > .../machine-loadparm-s390.s390x-9.1.0.args | 34 +++++++++++++ > .../machine-loadparm-s390.s390x-9.1.0.xml | 33 ++++++++++++ > .../machine-loadparm-s390.s390x-latest.args | 4 +- > tests/qemuxmlconftest.c | 4 ++ > 16 files changed, 346 insertions(+), 20 deletions(-) > create mode 100644 tests/qemuxmlconfdata/machine-loadparm-hostdev.s390x-9.1.0.args > create mode 100644 tests/qemuxmlconfdata/machine-loadparm-hostdev.s390x-9.1.0.xml > create mode 100644 tests/qemuxmlconfdata/machine-loadparm-multiple-disks-nets-s390.s390x-9.1.0.args > create mode 100644 tests/qemuxmlconfdata/machine-loadparm-multiple-disks-nets-s390.s390x-9.1.0.xml > create mode 100644 tests/qemuxmlconfdata/machine-loadparm-net-s390.s390x-9.1.0.args > create mode 100644 tests/qemuxmlconfdata/machine-loadparm-net-s390.s390x-9.1.0.xml > create mode 100644 tests/qemuxmlconfdata/machine-loadparm-s390.s390x-9.1.0.args > create mode 100644 tests/qemuxmlconfdata/machine-loadparm-s390.s390x-9.1.0.xml > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index a5ff7695c3..cdc0ea18d7 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -1608,6 +1608,7 @@ qemuBuildDiskDeviceProps(const virDomainDef *def, > g_autofree char *chardev = NULL; > g_autofree char *drive = NULL; > unsigned int bootindex = 0; > + g_autofree char *bootLoadparm = NULL; > unsigned int logical_block_size = disk->blockio.logical_block_size; > unsigned int physical_block_size = disk->blockio.physical_block_size; > unsigned int discard_granularity = disk->blockio.discard_granularity; > @@ -1746,9 +1747,14 @@ qemuBuildDiskDeviceProps(const virDomainDef *def, > } > > /* bootindex for floppies is configured via the fdc controller */ > - if (disk->device != VIR_DOMAIN_DISK_DEVICE_FLOPPY) > + if (disk->device != VIR_DOMAIN_DISK_DEVICE_FLOPPY) { > bootindex = disk->info.effectiveBootIndex; > > + if (disk->info.loadparm && > + virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_CCW_DEVICE_LOADPARM)) > + bootLoadparm = g_strdup(disk->info.loadparm); You don't need to copy this .... > + } > + > if (disk->wwn) { > unsigned long long w = 0; > > @@ -1791,6 +1797,7 @@ qemuBuildDiskDeviceProps(const virDomainDef *def, > "S:chardev", chardev, > "s:id", disk->info.alias, > "p:bootindex", bootindex, > + "S:loadparm", bootLoadparm, ... to pass it here. This takes const strings and copies them internally into the JSON buffer regardless. Just assign the pointer without copy. You also don't need to check that the string is non-NULL before passing it to g_strdup, or once you remove g_strdup to the temporary pointer. Looking at how the parsing of loadparm is handled I guess we still need to keep the capability (it' can't be moved into post-parse).