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

[Yi Min Zhao]

在 2018/7/26 下午4:21, Andrea Bolognani 写道:
> On Thu, 2018-07-26 at 15:15 +0800, Yi Min Zhao wrote:
>> 在 2018/7/24 下午10:25, Andrea Bolognani 写道:
>>> [...]
>>>> +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.
>> FID has been defined in schema. It is impossible to overflow uint32 range.
>> So...is it required to validate FID here?
> Mh, fair enough. Not for the schema part (as I mentioned earlier,
> that's entirely optional so it can't be relied upon when it comes
> to validating data) but for the 32-bit fid fitting into a 32-bit
> datatype by definition. Perhaps add a quick comment pointing out
> why validating fid is not necessary...
>
> Additional thoughts: should you check fid_assigned up there as
> well? Would it be a good idea to use the more specific uint16_t
> and uint32_t datatypes, and define VIR_DOMAIN_DEVICE_ZPCI_MAX_*ID
> in terms of the standard UINT*_MAX macros?
I thought again bout fid_assigned check. You're right. We should check it.
For your comments on macros, good idea.
>
> [...]
>>>> +    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.
>> The qemu cmd generator is introduced in latter patch.
>> I add the test cases in the corresponding patch.
> You should be able to add them to the xml2argv test even before
> you teach libvirt how to generate a QEMU command line for the new
> feature; then, when you add the missing pieces, it will be very
> clear from the diff what exactly changed.
>
> But as long as you make sure test end up in both xml2argv and
> xml2xml by the end of the series, it doesn't really matter.
>
OK. Thanks!