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

On Tue, 2018-08-07 at 17:10 +0800, Yi Min Zhao wrote: [...] > +static int > +virZPCIDeviceAddressIsValid(virZPCIDeviceAddressPtr zpci) > +{ > + if (zpci->uid_assigned && > + (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; > + } > + > + if (zpci->fid_assigned) { > + /* We don't need to check fid because fid covers > + * all range of uint32 type. > + */ > + return 1; > + } This branch is pointless, just drop it (but leave the comment). [...] > @@ -37,6 +37,9 @@ typedef virPCIDeviceAddress *virPCIDeviceAddressPtr; > typedef struct _virPCIDeviceList virPCIDeviceList; > typedef virPCIDeviceList *virPCIDeviceListPtr; > > +# define VIR_DOMAIN_DEVICE_ZPCI_MAX_UID UINT16_MAX > +# define VIR_DOMAIN_DEVICE_ZPCI_MAX_FID UINT32_MAX A single space between the name and the value will do. This should be DO_TEST("disk-virtio-s390-zpci", QEMU_CAPS_DEVICE_ZPCI, QEMU_CAPS_CCW, QEMU_CAPS_VIRTIO_S390); Same later. The rest looks good.