[libvirt] [PATCH 6/7] conf: add hypervisor agnostic, domain start-time, validation function for NetDef

Laine Stump laine at redhat.com
Fri Nov 15 15:43:28 UTC 2019


On 11/14/19 5:20 PM, Cole Robinson wrote:
> On 10/22/19 9:24 PM, Laine Stump wrote:
>> +int
>> +virDomainNetDefRuntimeValidate(const virDomainNetDef *net G_GNUC_UNUSED)
>> +{
>> +    /* Unlike virDomainNetDefValidate(), which is a static function
>> +     * called internally to this file, virDomainActualNetDefValidate()
>> +     * is a public function that can be called from a hypervisor after
>> +     * it has completely setup the NetDef for use by a domain,
>> +     * including possibly allocating a port from the network driver
>> +     * (which could change the effective/"actual" type of the NetDef,
>> +     * thus changing what should/shouldn't be allowed by validation).
>> +     *
> 
> The comment here claims the function is named
> virDomainActualNetDefValidate. I think that naming is more consistent
> rather than invent a new 'runtime' naming scheme, event though runtime
> is more semanticly descriptive. (though TBH I wouldn't mind
> s/actual/runtime/ in the codebase anyways, the 'actual' naming always
> confuses me)

Well, "actual" is describing *what* it is, and "runtime" is describing 
*when* it is. In most cases I've been fine with the "actual" name, but 
this time it didn't seem to adequately convey the usefulness of the 
function, so I decided to try something else out, and missed the 
function name in the comment.

> 
> Comment should be fixed either way, I'll leave it up to you whether to
> actually rename the function.

Thinking about it more, I guess it's better to remain consistent 
everywhere. Maybe someday someone will find some other adjective that 
works better than actual in all circumstances (it might even be 
"runtime", my personal mental jury is still out), and will switch all 
occurrences at once.

> Series:
> 
> Reviewed-by: Cole Robinson <crobinso at redhat.com>
> 
> I think there's a conflict in patch 7, git am gave me some confusing
> results, so make sure the code you are moving is incorporating and
> possible new changes that landed there since this was posted.

Yeah, I had rebased and was going to resend, but you reviewed the 
original before I got around to it.




More information about the libvir-list mailing list