Re: [libvirt] [PATCH v5 10/13] qemu: Generate and use zPCI device in QEMU command line
On Tue, 2018-09-18 at 15:40 +0800, Yi Min Zhao wrote: > 在 2018/9/17 下午8:33, Andrea Bolognani 写道: > > On Mon, 2018-09-17 at 13:51 +0800, Yi Min Zhao wrote: > > > How about using switch? I think extension flag should be check and > > > then build command for each case although there's only zpci case. > > > > You can't really sensibly use a switch() for flags. Unless I have > > mistaken what you had in mind... > > Like below simple code, > > if (dev->type != PCI_TYPE || dev->addr.pci.extFlags == NONE) > return 0; > > if (dev->addr.pci.extFlags & ZPCI_FLAG) > return qemuAppendZPCIDevStr(cmd, dev); > > /* There might be new extensions in future. */ Sure, that works. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 10/13] qemu: Generate and use zPCI device in QEMU command line
在 2018/9/17 下午8:33, Andrea Bolognani 写道:
On Mon, 2018-09-17 at 13:51 +0800, Yi Min Zhao wrote:
在 2018/9/11 下午10:31, Andrea Bolognani 写道:
+{
+if (!virZPCIDeviceAddressIsEmpty(>addr.pci.zpci))
+return qemuAppendZPCIDevStr(cmd, dev);
+
+return 0;
I'd rather see this as
if (virZPCIDeviceAddressIsEmpty(>addr.pci.zpci))
return 0;
return qemuAppendZPCIDevStr(cmd, dev);
instead.
How about using switch? I think extension flag should be check and
then build command for each case although there's only zpci case.
You can't really sensibly use a switch() for flags. Unless I have
mistaken what you had in mind...
Like below simple code,
if (dev->type != PCI_TYPE || dev->addr.pci.extFlags == NONE)
return 0;
if (dev->addr.pci.extFlags & ZPCI_FLAG)
return qemuAppendZPCIDevStr(cmd, dev);
/* There might be new extensions in future. */
return 0;
--
Yi Min
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 10/13] qemu: Generate and use zPCI device in QEMU command line
On Mon, 2018-09-17 at 13:51 +0800, Yi Min Zhao wrote:
> 在 2018/9/11 下午10:31, Andrea Bolognani 写道:
> > > +{
> > > +if (!virZPCIDeviceAddressIsEmpty(>addr.pci.zpci))
> > > +return qemuAppendZPCIDevStr(cmd, dev);
> > > +
> > > +return 0;
> >
> > I'd rather see this as
> >
> >if (virZPCIDeviceAddressIsEmpty(>addr.pci.zpci))
> >return 0;
> >
> >return qemuAppendZPCIDevStr(cmd, dev);
> >
> > instead.
>
> How about using switch? I think extension flag should be check and
> then build command for each case although there's only zpci case.
You can't really sensibly use a switch() for flags. Unless I have
mistaken what you had in mind...
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 10/13] qemu: Generate and use zPCI device in QEMU command line
在 2018/9/11 下午10:31, Andrea Bolognani 写道:
On Tue, 2018-09-04 at 16:39 +0800, Yi Min Zhao wrote:
[...]
+char *
+qemuBuildZPCIDevStr(virDomainDeviceInfoPtr dev)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+virBufferAddLit(, "zpci");
+virBufferAsprintf(, ",uid=%u", dev->addr.pci.zpci.uid);
+virBufferAsprintf(, ",fid=%u", dev->addr.pci.zpci.fid);
+virBufferAsprintf(, ",target=%s", dev->alias);
+virBufferAsprintf(, ",id=zpci%u", dev->addr.pci.zpci.uid);
All of the above can be a single virBufferAsprintf() call.
Sure.
[...]
+static int
+qemuAppendZPCIDevStr(virCommandPtr cmd,
+ virDomainDeviceInfoPtr dev)
I'd call this qemuCommandAddZPCIDevice() or something like that.
OK.
+{
+char *devstr = NULL;
+
+virCommandAddArg(cmd, "-device");
Empty line here.
Yeah.
+if (!(devstr = qemuBuildZPCIDevStr(dev)))
+return -1;
[...]
+static int
+qemuBuildExtensionCommandLine(virCommandPtr cmd,
+ virDomainDeviceInfoPtr dev)
Same comment about the naming as above.
OK.
+{
+if (!virZPCIDeviceAddressIsEmpty(>addr.pci.zpci))
+return qemuAppendZPCIDevStr(cmd, dev);
+
+return 0;
I'd rather see this as
if (virZPCIDeviceAddressIsEmpty(>addr.pci.zpci))
return 0;
return qemuAppendZPCIDevStr(cmd, dev);
instead.
How about using switch? I think extension flag should be check and
then build command for each case although there's only zpci case.
[...]
@@ -4327,6 +4390,9 @@ qemuBuildSoundCommandLine(virCommandPtr cmd,
if (sound->model == VIR_DOMAIN_SOUND_MODEL_PCSPK) {
virCommandAddArgList(cmd, "-soundhw", "pcspk", NULL);
} else {
+if (qemuBuildExtensionCommandLine(cmd, >info) < 0)
+return -1;
Do the codecs (handled immediately below) require a zpci device
as well? I figure they don't, but I also don't know much about
sound devices :)
If its address is PCI, we should add a zpci for it. zPCI is not related
to its functionality.
[...]
@@ -9086,6 +9171,9 @@ qemuBuildShmemCommandLine(virLogManagerPtr logManager,
switch ((virDomainShmemModel)shmem->model) {
case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM:
+if (qemuBuildExtensionCommandLine(cmd, >info) < 0)
+return -1;
+
devstr = qemuBuildShmemDevLegacyStr(def, shmem, qemuCaps);
break;
@@ -9104,6 +9192,9 @@ qemuBuildShmemCommandLine(virLogManagerPtr logManager,
ATTRIBUTE_FALLTHROUGH;
case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_DOORBELL:
+if (qemuBuildExtensionCommandLine(cmd, >info) < 0)
+return -1;
+
devstr = qemuBuildShmemDevStr(def, shmem, qemuCaps);
break;
This looks wrong: you should call qemuBuildExtensionCommandLine()
later on, like
if (!devstr)
return -1;
if (qemuBuildExtensionCommandLine(cmd, >info) < 0)
return -1;
virCommandAddArgList(cmd, "-device", devstr, NULL);
instead.
Good catch. Thanks for your comments.
--
Yi Min
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 10/13] qemu: Generate and use zPCI device in QEMU command line
On Tue, 2018-09-04 at 16:39 +0800, Yi Min Zhao wrote:
[...]
> +char *
> +qemuBuildZPCIDevStr(virDomainDeviceInfoPtr dev)
> +{
> +virBuffer buf = VIR_BUFFER_INITIALIZER;
> +
> +virBufferAddLit(, "zpci");
> +virBufferAsprintf(, ",uid=%u", dev->addr.pci.zpci.uid);
> +virBufferAsprintf(, ",fid=%u", dev->addr.pci.zpci.fid);
> +virBufferAsprintf(, ",target=%s", dev->alias);
> +virBufferAsprintf(, ",id=zpci%u", dev->addr.pci.zpci.uid);
All of the above can be a single virBufferAsprintf() call.
[...]
> +static int
> +qemuAppendZPCIDevStr(virCommandPtr cmd,
> + virDomainDeviceInfoPtr dev)
I'd call this qemuCommandAddZPCIDevice() or something like that.
> +{
> +char *devstr = NULL;
> +
> +virCommandAddArg(cmd, "-device");
Empty line here.
> +if (!(devstr = qemuBuildZPCIDevStr(dev)))
> +return -1;
[...]
> +static int
> +qemuBuildExtensionCommandLine(virCommandPtr cmd,
> + virDomainDeviceInfoPtr dev)
Same comment about the naming as above.
> +{
> +if (!virZPCIDeviceAddressIsEmpty(>addr.pci.zpci))
> +return qemuAppendZPCIDevStr(cmd, dev);
> +
> +return 0;
I'd rather see this as
if (virZPCIDeviceAddressIsEmpty(>addr.pci.zpci))
return 0;
return qemuAppendZPCIDevStr(cmd, dev);
instead.
[...]
> @@ -4327,6 +4390,9 @@ qemuBuildSoundCommandLine(virCommandPtr cmd,
> if (sound->model == VIR_DOMAIN_SOUND_MODEL_PCSPK) {
> virCommandAddArgList(cmd, "-soundhw", "pcspk", NULL);
> } else {
> +if (qemuBuildExtensionCommandLine(cmd, >info) < 0)
> +return -1;
Do the codecs (handled immediately below) require a zpci device
as well? I figure they don't, but I also don't know much about
sound devices :)
[...]
> @@ -9086,6 +9171,9 @@ qemuBuildShmemCommandLine(virLogManagerPtr logManager,
>
> switch ((virDomainShmemModel)shmem->model) {
> case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM:
> +if (qemuBuildExtensionCommandLine(cmd, >info) < 0)
> +return -1;
> +
> devstr = qemuBuildShmemDevLegacyStr(def, shmem, qemuCaps);
> break;
>
> @@ -9104,6 +9192,9 @@ qemuBuildShmemCommandLine(virLogManagerPtr logManager,
>
> ATTRIBUTE_FALLTHROUGH;
> case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_DOORBELL:
> +if (qemuBuildExtensionCommandLine(cmd, >info) < 0)
> +return -1;
> +
> devstr = qemuBuildShmemDevStr(def, shmem, qemuCaps);
> break;
This looks wrong: you should call qemuBuildExtensionCommandLine()
later on, like
if (!devstr)
return -1;
if (qemuBuildExtensionCommandLine(cmd, >info) < 0)
return -1;
virCommandAddArgList(cmd, "-device", devstr, NULL);
instead.
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5 10/13] qemu: Generate and use zPCI device in QEMU command line
Add new functions to generate zPCI command string and append it to
QEMU command line. And the related tests are added.
Signed-off-by: Yi Min Zhao
Reviewed-by: Boris Fiuczynski
Reviewed-by: Stefan Zimmermann
Reviewed-by: Bjoern Walk
Reviewed-by: Ján Tomko
---
src/qemu/qemu_command.c| 95 ++
src/qemu/qemu_command.h| 2 +
tests/qemuxml2argvdata/disk-virtio-s390-zpci.args | 1 +
.../hostdev-vfio-zpci-autogenerate.args| 25 ++
.../hostdev-vfio-zpci-autogenerate.xml | 18
.../hostdev-vfio-zpci-boundaries.args | 29 +++
.../hostdev-vfio-zpci-boundaries.xml | 26 ++
.../hostdev-vfio-zpci-multidomain-many.args| 39 +
.../hostdev-vfio-zpci-multidomain-many.xml | 67 +++
tests/qemuxml2argvdata/hostdev-vfio-zpci.args | 2 +
tests/qemuxml2argvtest.c | 13 +++
.../hostdev-vfio-zpci-autogenerate.xml | 30 +++
.../hostdev-vfio-zpci-boundaries.xml | 42 ++
.../hostdev-vfio-zpci-multidomain-many.xml | 79 ++
tests/qemuxml2xmltest.c| 11 +++
15 files changed, 479 insertions(+)
create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-autogenerate.args
create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-autogenerate.xml
create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-boundaries.args
create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-boundaries.xml
create mode 100644
tests/qemuxml2argvdata/hostdev-vfio-zpci-multidomain-many.args
create mode 100644
tests/qemuxml2argvdata/hostdev-vfio-zpci-multidomain-many.xml
create mode 100644 tests/qemuxml2xmloutdata/hostdev-vfio-zpci-autogenerate.xml
create mode 100644 tests/qemuxml2xmloutdata/hostdev-vfio-zpci-boundaries.xml
create mode 100644
tests/qemuxml2xmloutdata/hostdev-vfio-zpci-multidomain-many.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8aa20496bc..afb25b1976 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2155,6 +2155,50 @@ qemuBuildDiskDeviceStr(const virDomainDef *def,
return NULL;
}
+char *
+qemuBuildZPCIDevStr(virDomainDeviceInfoPtr dev)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+virBufferAddLit(, "zpci");
+virBufferAsprintf(, ",uid=%u", dev->addr.pci.zpci.uid);
+virBufferAsprintf(, ",fid=%u", dev->addr.pci.zpci.fid);
+virBufferAsprintf(, ",target=%s", dev->alias);
+virBufferAsprintf(, ",id=zpci%u", dev->addr.pci.zpci.uid);
+
+if (virBufferCheckError() < 0) {
+virBufferFreeAndReset();
+return NULL;
+}
+
+return virBufferContentAndReset();
+}
+
+static int
+qemuAppendZPCIDevStr(virCommandPtr cmd,
+ virDomainDeviceInfoPtr dev)
+{
+char *devstr = NULL;
+
+virCommandAddArg(cmd, "-device");
+if (!(devstr = qemuBuildZPCIDevStr(dev)))
+return -1;
+
+virCommandAddArg(cmd, devstr);
+
+VIR_FREE(devstr);
+return 0;
+}
+
+static int
+qemuBuildExtensionCommandLine(virCommandPtr cmd,
+ virDomainDeviceInfoPtr dev)
+{
+if (!virZPCIDeviceAddressIsEmpty(>addr.pci.zpci))
+return qemuAppendZPCIDevStr(cmd, dev);
+
+return 0;
+}
static int
qemuBuildFloppyCommandLineControllerOptions(virCommandPtr cmd,
@@ -2391,6 +2435,9 @@ qemuBuildDiskCommandLine(virCommandPtr cmd,
if (!qemuDiskBusNeedsDriveArg(disk->bus)) {
if (disk->bus != VIR_DOMAIN_DISK_BUS_FDC ||
virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) {
+if (qemuBuildExtensionCommandLine(cmd, >info) < 0)
+return -1;
+
virCommandAddArg(cmd, "-device");
if (!(optstr = qemuBuildDiskDeviceStr(def, disk, bootindex,
@@ -2591,6 +2638,9 @@ qemuBuildFSDevCommandLine(virCommandPtr cmd,
virCommandAddArg(cmd, optstr);
VIR_FREE(optstr);
+if (qemuBuildExtensionCommandLine(cmd, >info) < 0)
+return -1;
+
virCommandAddArg(cmd, "-device");
if (!(optstr = qemuBuildFSDevStr(def, fs, qemuCaps)))
return -1;
@@ -3076,6 +3126,10 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
goto cleanup;
if (devstr) {
+if (qemuBuildExtensionCommandLine(cmd, >info) < 0) {
+VIR_FREE(devstr);
+goto cleanup;
+}
virCommandAddArg(cmd, "-device");
virCommandAddArg(cmd, devstr);
VIR_FREE(devstr);
@@ -3880,6 +3934,9 @@ qemuBuildWatchdogCommandLine(virCommandPtr cmd,
if (!def->watchdog)
return 0;
+if (qemuBuildExtensionCommandLine(cmd, >watchdog->info) < 0)
+return -1;
+
virCommandAddArg(cmd, "-device");
optstr = qemuBuildWatchdogDevStr(def, watchdog, qemuCaps);
@@ -3964,6 +4021,9 @@
