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.

[...]
> +char *qemuBuildZPCIDevStr(virDomainDeviceInfoPtr dev);
> +
> +bool qemuCheckDeviceIsZPCI(virDomainDeviceInfoPtr info);

Is this really necessary? Can't these two functions be static?

[...]
> --- 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.

> +            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.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to