On Tue, 2018-07-10 at 16:02 +0800, Yi Min Zhao wrote:
> + <define name="uint32">
> + <choice>
> + <data type="string">
> + <param name="pattern">(0x)?[0-9a-fA-F]{1,8}</param>
> + </data>
> + <data type="int">
This should probably be unignedInt instead of int, but other uint*
types defined in the file also use int so if anything changing all
of them would be the job for a follow-up patch.
[...]
> + <define name="zpciaddress">
> + <optional>
> + <attribute name="uid">
> + <choice>
> + <data type="string">
> + <param name="pattern">(0x)?[0-9a-fA-F]{1,4}</param>
> + </data>
> + <data type="unsignedInt">
> + <param name="minInclusive">1</param>
> + <param name="maxInclusive">65535</param>
> + </data>
> + </choice>
> + </attribute>
I don't see why you wouldn't just use the existing uint16 type
here. Is that because uid can't be zero? Making sure that's
actually the case is a job for the PostParse() or Validate()
callback anyway, because schema validation is entirely opt-in
and thus can't be relied upon.
[...]
> +static int
> +virZPCIDeviceAddressIsValid(virZPCIDeviceAddressPtr zpci)
> +{
> + if (!zpci->uid_assigned)
> + return 1;
> +
> + if (zpci->zpci_uid > VIR_DOMAIN_DEVICE_ZPCI_MAX_UID ||
> + zpci->zpci_uid == 0) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("Invalid PCI address uid='0x%x', "
> + "must be > 0x0 and <= 0x%x"),
> + zpci->zpci_uid,
> + VIR_DOMAIN_DEVICE_ZPCI_MAX_UID);
> + return 0;
> + }
fid should be validated as well.
[...]
> +<domain type='qemu'>
> + <name>QEMUGuest1</name>
> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> + <memory>219136</memory>
> + <vcpu>1</vcpu>
> + <os>
> + <type arch='s390x' machine='s390-ccw'>hvm</type>
The correct machine type would be s390-ccw-virtio.
There are a bunch of existing test files that incorrectly use
s390-ccw, feel free to clean that up as well ;)
[...]
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index bbb995656e..e3282e2b98 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -397,6 +397,8 @@ mymain(void)
> QEMU_CAPS_VIRTIO_SCSI);
> DO_TEST("disk-virtio-scsi-ioeventfd",
> QEMU_CAPS_VIRTIO_SCSI);
> + DO_TEST("disk-virtio-s390-zpci",
> + QEMU_CAPS_DEVICE_ZPCI, QEMU_CAPS_CCW);
> DO_TEST("disk-scsi-megasas",
> QEMU_CAPS_SCSI_MEGASAS);
> DO_TEST("disk-scsi-mptsas1068",
> @@ -476,6 +478,7 @@ mymain(void)
> DO_TEST("hostdev-usb-address", NONE);
> DO_TEST("hostdev-pci-address", NONE);
> DO_TEST("hostdev-vfio", NONE);
> + DO_TEST("hostdev-vfio-zpci", QEMU_CAPS_DEVICE_ZPCI, QEMU_CAPS_CCW);
I haven't actually tried that, but you should be able to add the
test cases to qemuxml2argvtest at the same time as you add them
here, for consistency's sake.
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
