[libvirt] [PATCH v2 RESEND 07/12] conf: Introduce parser, formatter for uid and fid

[Andrea Bolognani]

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