[PATCH] conf: rename virDomainCheckVirtioOptions

Michal Privoznik mprivozn at redhat.com
Fri Jan 29 12:44:12 UTC 2021


On 1/29/21 1:35 PM, Pavel Hrdina wrote:
> On Fri, Jan 29, 2021 at 12:39:22PM +0100, Boris Fiuczynski wrote:
>> Rename virDomainCheckVirtioOptions into
>> virDomainCheckVirtioOptionsAreAbent since it checks if all virtio
>> options are absent. The old name was very misleading.
>>
>> Signed-off-by: Boris Fiuczynski <fiuczy at linux.ibm.com>
>> ---
>>   src/conf/domain_validate.c | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> NACK to this patch. I don't see any point in this rename. The whole file
> mostly checks if something is missing. If anything I would go for
> virDomainVirtioOptionsValidate.

Hm.. meanwhile I pushed it. The problem with the current name is that it 
is very misleading. What the function does is it checks whether user did 
not enable a virtio option for a non-virtio device, e.g. ACS for e1000 
NIC. That explains why callers do very misleading check:

if (model != virtio)
   virDomainCheckVirtioOptions(model->virtio)

Therefore, renaming to AreAbsent() express what is done better. My other 
idea was to push model != virtio check into the function itself, e.g. 
like this:

   virDomainCheckVirtioOptions(model == virtio, model->virtio)

This way we would need to rename the function and still can keep the checks.

Michal




More information about the libvir-list mailing list