[libvirt] [PATCH v6 07/13] conf: Introduce parser, formatter for uid and fid

[Yi Min Zhao]

在 2018/10/11 下午7:45, Andrea Bolognani 写道:
> On Fri, 2018-09-28 at 16:46 +0800, Yi Min Zhao wrote:
>> This patch introduces new XML parser/formatter functions. Uid is
>> 16-bit and non-zero. Fid is 32-bit. They are the two attributes of zpci
>> which is introduced as PCI address element. Zpci element is parsed and
>> formatted along with PCI address. And add the related test cases.
>>
>> Signed-off-by: Yi Min Zhao <zyimin at linux.ibm.com>
>> ---
> No internal reviews this time around? :)
Yes, I just want to quickly know if this change is exact.
>
> [...]
>> @@ -227,9 +288,22 @@ virPCIDeviceAddressParseXML(xmlNodePtr node,
>>           goto cleanup;
>>   
>>       }
>> +
>>       if (!virPCIDeviceAddressIsEmpty(addr) && !virPCIDeviceAddressIsValid(addr, true))
>>           goto cleanup;
> Spurious whitespace change.
Oh...good catch.
>
> [...]
>> @@ -6528,7 +6528,19 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
>>           break;
>>       }
>>   
>> -    virBufferAddLit(buf, "/>\n");
>> +    if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI &&
>> +        !virZPCIDeviceAddressIsEmpty(&info->addr.pci.zpci)) {
>> +        virBufferAddLit(buf, ">\n");
>> +        virBufferAdjustIndent(buf, 2);
>> +        virBufferAsprintf(buf,
>> +                          "<zpci uid='0x%.4x' fid='0x%.8x'/>\n",
>> +                          info->addr.pci.zpci.uid,
>> +                          info->addr.pci.zpci.fid);
>> +        virBufferAdjustIndent(buf, -2);
>> +        virBufferAddLit(buf, "</address>\n");
>> +    } else {
>> +        virBufferAddLit(buf, "/>\n");
>> +    }
> This looks like a perfect candidate for virXMLFormatElement(). You
> should probably convert the original code to use that function in a
> separate commit, then start actually using a child buffer here.
Sure.
>
> [...]
>> @@ -2566,6 +2566,7 @@ virPCIHeaderTypeToString;
>>   virPCIIsVirtualFunction;
>>   virPCIStubDriverTypeFromString;
>>   virPCIStubDriverTypeToString;
>> +virZPCIDeviceAddressIsEmpty;
> You can move virZPCIDeviceAddressIsValid() to util/virpci and
> export it too, to be consistent with virPCIDeviceAddress*().
It has been moved to util/virpci. Isn't it?
>
> [...]
>> @@ -272,4 +275,6 @@ VIR_DEFINE_AUTOPTR_FUNC(virPCIDevice, virPCIDeviceFree)
>>   VIR_DEFINE_AUTOPTR_FUNC(virPCIDeviceAddress, virPCIDeviceAddressFree)
>>   VIR_DEFINE_AUTOPTR_FUNC(virPCIEDeviceInfo, virPCIEDeviceInfoFree)
>>   
>> +bool virZPCIDeviceAddressIsEmpty(const virZPCIDeviceAddress *addr);
>> +
> Please place this further up in the file, eg. after
> virPCIDeviceAddressParse().
Sure.
>
>
> Everything else looks good.
>

-- 
Yi Min