Re: [libvirt] [PATCH v2 RESEND 07/12] conf: Introduce parser, formatter for uid and fid
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