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

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/

[...] > +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.

[...] > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + "%s", > + _("This QEMU binary doesn't support zPCI.")); No full stop at the end of the error message, please.

[...] > @@ -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.