[libvirt] [PATCH v5 08/13] qemu: Add zPCI address definition check

Yi Min Zhao zyimin at linux.ibm.com
Thu Sep 13 10:08:57 UTC 2018



在 2018/9/11 下午8:44, Andrea Bolognani 写道:
> On Tue, 2018-09-04 at 16:39 +0800, Yi Min Zhao wrote:
>> We should ensure that the Qemu should support zPCI when zPCI address is
> s/Qemu/QEMU/
Sure.
>
> [...]
>> +static int
>> +qemuDomainZPCIAddressDefValidate(virDomainDeviceDef *dev,
>> +                                     virQEMUCapsPtr qemuCaps)
> The second argument is not aligned properly.
>
> I'd also change the name: qemuDomainDeviceDefValidateZPCIAddress()
> perhaps? It's quite a mouthful, but also more accurate.
ok.
>
> [...]
>> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> +                       "%s",
>> +                       _("This QEMU binary doesn't support zPCI."));
> No full stop at the end of the error message, please.
Could you please explain more?
>
> [...]
>> @@ -5741,6 +5762,10 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
>>                                               def->emulator)))
>>           return -1;
>>   
>> +    ret = qemuDomainZPCIAddressDefValidate((virDomainDeviceDef *)dev, qemuCaps);
> This cast is kinds gross. I realize virDomainDeviceGetInfo()
> requires the pointer to be non-const, but from a semantics point
> of view qemuDomainZPCIAddressDefValidate() should be okay with
> being passed a const pointer - it's only validating the device,
> not changing it, after all.
>
> Please move the cast into the function instead of requiring the
> caller to perform it.
>
> We should also have a generic qemuDomainDeviceDefValidateAddress()
> wrapper that calls the zPCI-specific one only for PCI devices, and
> call that one from here.
>
Got it.

-- 
Yi Min




More information about the libvir-list mailing list