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