[libvirt] [PATCH 6/7] conf: add hypervisor agnostic, domain start-time, validation function for NetDef
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:
>> +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.
> 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