Re: [libvirt] [PATCH v2 RESEND 09/12] qemu: Generate and use zPCI device in QEMU command line
在 2018/7/24 下午11:09, Andrea Bolognani 写道:
On Tue, 2018-07-10 at 16:02 +0800, Yi Min Zhao wrote:
[...]
> +bool
> +qemuCheckDeviceIsZPCI(virDomainDeviceInfoPtr info)
> +{
> + if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI &&
> + ((info->pciAddressExtFlags & VIR_PCI_ADDRESS_EXTENSION_ZPCI) ||
> + info->addr.pci.zpci))
> + return true;
Missing curly braces.
Also, do you really need to check both the flags and the presence
of the zPCI address bits? It looks like either one or the other
should be enough or, if that's not the case, it should be made so
because having to check for two separate conditions makes me feel
like it would introduce bugs in the long run.
This is actually a problem. I add
info->addr.pci.zpci check in order to
check
if the user specifies zpci address in xml even though it has no zpci
support.
The callers of this function checks zpci capability. If no zpci cap but
the user
specfies zpci address, report an error.
I will change the logic and remove the check on info->addr.pci.zpci.
[...]
> +char *qemuBuildZPCIDevStr(virDomainDeviceInfoPtr dev);
> +
> +bool qemuCheckDeviceIsZPCI(virDomainDeviceInfoPtr info);
Is this really necessary? Can't these two functions be static?
They are also
used in qemu_hotplug.c file.
[...]
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -1014,6 +1014,8 @@ mymain(void)
> QEMU_CAPS_CCW, QEMU_CAPS_VIRTIO_S390);
> DO_TEST("disk-virtio-scsi-ccw", QEMU_CAPS_VIRTIO_SCSI,
> QEMU_CAPS_CCW, QEMU_CAPS_VIRTIO_S390);
> + DO_TEST("disk-virtio-s390-zpci", QEMU_CAPS_DEVICE_ZPCI,
> + QEMU_CAPS_CCW, QEMU_CAPS_VIRTIO_S390);
> DO_TEST("disk-order",
> QEMU_CAPS_DRIVE_BOOT, QEMU_CAPS_VIRTIO_BLK_SCSI);
> DO_TEST("disk-virtio-drive-queues",
> @@ -1574,6 +1576,18 @@ mymain(void)
> QEMU_CAPS_DEVICE_VFIO_PCI);
> DO_TEST_PARSE_ERROR("hostdev-mdev-invalid-target-address",
> QEMU_CAPS_DEVICE_VFIO_PCI);
> + DO_TEST("hostdev-vfio-zpci",
> + QEMU_CAPS_DEVICE_VFIO_PCI, QEMU_CAPS_DEVICE_ZPCI);
> + DO_TEST("hostdev-vfio-zpci-multidomain-many",
> + QEMU_CAPS_DEVICE_VFIO_PCI, X_QEMU_CAPS_HOST_PCI_MULTIDOMAIN,
Capabilities with X_QEMU prefix are no longer used, so you should
not list them here.
Sure.
> + QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_ZPCI);
> + DO_TEST("hostdev-vfio-zpci-autogenerate",
> + QEMU_CAPS_DEVICE_VFIO_PCI, QEMU_CAPS_DEVICE_ZPCI);
> + DO_TEST("hostdev-vfio-zpci-boundaries",
> + QEMU_CAPS_DEVICE_VFIO_PCI, X_QEMU_CAPS_HOST_PCI_MULTIDOMAIN,
> + QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_ZPCI);
> + DO_TEST_FAILURE("hostdev-vfio-zpci",
> + QEMU_CAPS_DEVICE_VFIO_PCI);
Please add these to qemuxml2xmltest at the same time.