Re: [libvirt] [PATCH 3/3] common-impl: make virDomainObjGetPersistentDef return only persistent config

On 02/02/2016 08:18 AM, Nikolay Shirokovskiy wrote: Again sorry for the delay... I have a suspicion that there's a "hidden" meaning/side-effect to calling this... The whole usage of "newDef" vs "def" needs better nomenclature to stick in my long term memory. To refresh my short term though... The 'newDef' is the "Next" definition to activate at shutdown (of a running domain), while 'def' is currently active definition (whether it's running or not). Perhaps the function is less than adequately named considering that Persistent usually refers to on disk, although in this case, I'm wondering if it means whatever is active for the domain without having to have the caller know whether it's a "running" and whether it's taking it from disk. In the case of a transient domain, the persistent domain is only valid when "running" and there's no need for 'newDef', hence why virDomainObjSetDefTransient will make that check (and why virDomainObjGetPersistentDef will call it). I didn't follow in depth the instances you pointed out, but perhaps they are all that's left using the Get function. Maybe in the time since the Get function was created and now there's been code that's remove calls to the Get function and thus we've reached a point where we could do what you propose, but I haven't spent cycles researching all that. My point is - it's not only the current callers - what's the recent history on this? Has anything that used to call it now no longer call it and what was the reason? A caller in a more general code path that would want the 'def' returned for a transient domain - what would it call? I think that answer is virDomainObjGetDefs. If so, then the comment could be modified to indicate code paths that need to get the transient definition should call virDomainObjGetDefs (yet another aptly named method). John