[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 10/19] storage: Create accessor API's for virStoragePoolObj




On 07/21/2017 04:22 AM, Pavel Hrdina wrote:
> On Thu, Jul 20, 2017 at 05:38:53PM -0400, John Ferlan wrote:
>> On 07/14/2017 11:37 AM, Pavel Hrdina wrote:
>>> On Tue, May 09, 2017 at 11:30:17AM -0400, John Ferlan wrote:
>>>> In preparation for making a private object, create accessor API's for
>>>> consumer storage functions to use:
>>>>
>>>>     virStoragePoolObjGetDef
>>>>     virStoragePoolObjGetNewDef
>>>>     virStoragePoolObjStealNewDef
>>>>     virStoragePoolObjGetConfigFile
>>>>     virStoragePoolObjSetConfigFile
>>>>     virStoragePoolObjIsActive
>>>>     virStoragePoolObjSetActive
>>>>     virStoragePoolObjGetAsyncjobs
>>>>     virStoragePoolObjIncrAsyncjobs
>>>>     virStoragePoolObjDecrAsyncjobs
>>>>
>>>> Signed-off-by: John Ferlan <jferlan redhat com>
>>>> ---
>>>>  src/conf/virstorageobj.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  src/conf/virstorageobj.h | 36 +++++++++++++++++++----
>>>>  src/libvirt_private.syms | 10 +++++++
>>>>  3 files changed, 115 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
>>>> index 23346f3..7d6b311 100644
>>>> --- a/src/conf/virstorageobj.c
>>>> +++ b/src/conf/virstorageobj.c
>>>> @@ -37,6 +37,80 @@
>>>>  VIR_LOG_INIT("conf.virstorageobj");
>>>
>>> [...]
>>>
>>>> +void
>>>> +virStoragePoolObjStealNewDef(virStoragePoolObjPtr obj)
>>>> +{
>>>> +    virStoragePoolDefFree(obj->def);
>>>> +    obj->def = obj->newDef;
>>>> +    obj->newDef = NULL;
>>>> +}
>>>
>>> I didn't notice this until the usage in following patches, the "Steal"
>>> part of the name is confusing.  We have a macro "VIR_STEAL_PTR" which
>>> returns pointer and sets the original one to NULL.  This function
>>> doesn't return the pointer, it replaces @def with @newDef.
>>>
>>> How about virStoragePoolObjUseNewDef() or
>>> virStoragePoolObjDefUseNewDef() or feel free to come up with another
>>> name which would be better than "Steel".
>>>
>>> Pavel
>>>
>>
>> It's theft by deception.
>>
>> How about virStoragePoolObjDefSwapNewDef
> 
> "Swap" is not a good choice as well, it replaces and discards the
> original @def by @newDef, "Swap" can mean that the @def will be @newDef.
> 
> Pavel
> 

I'll go with virStoragePoolObjDefUseNewDef

Although it's more like :

  virStoragePoolObjDefFreeAndAssignNewDefToDef

at some point the name of the function can be too descriptive and too
long;-). Not sure Use is the best either, but I did waste some time
looking at synonyms with nothing that made me think - yeah *that's* it
although Transmogrify came close (google transmogrify+calvin+hobbes -
when I worked at HP that's what we called the layer that "converted" or
"virtualized" privileged machine instructions into instructions that the
guest could run).

John


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]