On 03/11/2017 09:41 PM, Laine Stump wrote:
> Ah, but wait! I ACKed this too soon! *This* is the patch that's renaming the functions. It should be changing the arguments in some cases too (and at least one of the names seems wrong).
>
>
> On 03/10/2017 04:10 PM, John Ferlan wrote:
>> Rename the API's to remove the storage pool source pieces
>
> Yeah, if it's consistent in all cases, I can agree with that...
>
>>
>> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
>> ---
>> src/conf/storage_adapter_conf.c | 14 +++++++-------
>> src/conf/storage_adapter_conf.h | 14 +++++++-------
>> src/conf/storage_conf.c | 8 ++++----
>> src/libvirt_private.syms | 8 ++++----
>> 4 files changed, 22 insertions(+), 22 deletions(-)
>>
>> diff --git a/src/conf/storage_adapter_conf.c b/src/conf/storage_adapter_conf.c
>> index 3a16bcc..4f5b665 100644
>> --- a/src/conf/storage_adapter_conf.c
>> +++ b/src/conf/storage_adapter_conf.c
>> @@ -38,7 +38,7 @@ VIR_ENUM_IMPL(virStoragePoolSourceAdapter,
>>
>>
>> void
>> -virStoragePoolSourceAdapterClear(virStoragePoolSourceAdapterPtr adapter)
>> +virStorageAdapterClear(virStoragePoolSourceAdapterPtr adapter)
>> {
>> if (adapter->type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) {
>> VIR_FREE(adapter->data.fchost.wwnn);
>> @@ -55,9 +55,9 @@ virStoragePoolSourceAdapterClear(virStoragePoolSourceAdapterPtr adapter)
>>
>>
>> int
>> -virStoragePoolDefParseSourceAdapter(virStoragePoolSourcePtr source,
>> - xmlNodePtr node,
>> - xmlXPathContextPtr ctxt)
>> +virStorageAdapterParseXML(virStoragePoolSourcePtr source,
>> + xmlNodePtr node,
>> + xmlXPathContextPtr ctxt)
>
> This function should take a virStoragePoolSourceAdapterPtr rather than a virStoragePoolSourcePtr.
>
>> {
>> int ret = -1;
>> xmlNodePtr relnode = ctxt->node;
>> @@ -177,7 +177,7 @@ virStoragePoolDefParseSourceAdapter(virStoragePoolSourcePtr source,
>>
>>
>> int
>> -virStoragePoolSourceAdapterParseValidate(virStoragePoolDefPtr ret)
>> +virStorageAdapterParseValidate(virStoragePoolDefPtr ret)
>
>
> This function should take a virStoragePoolSourceAdapterPtr rather than virStoragePoolDefPtr, and the name should just be "virStorageAdapterValidate(), since the parsing is already finished, and this function just validates.
>
I'd prefer to use virStorageAdapterValidateParse() - as that what it's
doing validating that the parse was correct. So is this is a case where
a verb can turn into an adverb? (it's a grammar question!)
I don't think there's any "order" I could choose other than a really
painful use a long name now only to change to a shorter name approach in
later patches. I'd like to reduce the amount of churn though just for
the sake of "name sanity" between the steps that no one will care about
except when they have to backport something... In which case, gitk is
your friend because it really makes the what changed when lookup simple.
John
>
>> {
>> if (!ret->source.adapter.type) {
>> virReportError(VIR_ERR_XML_ERROR, "%s",
>> @@ -245,8 +245,8 @@ virStoragePoolSourceAdapterParseValidate(virStoragePoolDefPtr ret)
>>
>>
>> void
>> -virStoragePoolSourceAdapterFormat(virBufferPtr buf,
>> - virStoragePoolSourcePtr src)
>> +virStorageAdapterFormat(virBufferPtr buf,
>> + virStoragePoolSourcePtr src)
>
> Again - the arg should be virStoragePoolSourceAdapterPtr.
>
>> {
>> virBufferAsprintf(buf, "<adapter type='%s'",
>> virStoragePoolSourceAdapterTypeToString(src->adapter.type));
>> diff --git a/src/conf/storage_adapter_conf.h b/src/conf/storage_adapter_conf.h
>> index dec2f18..ec812a1 100644
>> --- a/src/conf/storage_adapter_conf.h
>> +++ b/src/conf/storage_adapter_conf.h
>> @@ -26,18 +26,18 @@
>> # include "storage_conf.h"
>>
>> void
>> -virStoragePoolSourceAdapterClear(virStoragePoolSourceAdapterPtr adapter);
>> +virStorageAdapterClear(virStoragePoolSourceAdapterPtr adapter);
>>
>> int
>> -virStoragePoolDefParseSourceAdapter(virStoragePoolSourcePtr source,
>> - xmlNodePtr node,
>> - xmlXPathContextPtr ctxt);
>> +virStorageAdapterParseXML(virStoragePoolSourcePtr source,
>> + xmlNodePtr node,
>> + xmlXPathContextPtr ctxt);
>
> same
>
>>
>> int
>> -virStoragePoolSourceAdapterParseValidate(virStoragePoolDefPtr ret);
>> +virStorageAdapterParseValidate(virStoragePoolDefPtr ret);
>>
>> void
>> -virStoragePoolSourceAdapterFormat(virBufferPtr buf,
>> - virStoragePoolSourcePtr src);
>> +virStorageAdapterFormat(virBufferPtr buf,
>> + virStoragePoolSourcePtr src);
>>
>> #endif /* __VIR_STORAGE_ADAPTER_CONF_H__ */
>> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
>> index 9314504..8709101 100644
>> --- a/src/conf/storage_conf.c
>> +++ b/src/conf/storage_conf.c
>> @@ -363,7 +363,7 @@ virStoragePoolSourceClear(virStoragePoolSourcePtr source)
>> VIR_FREE(source->devices);
>> VIR_FREE(source->dir);
>> VIR_FREE(source->name);
>> - virStoragePoolSourceAdapterClear(&source->adapter);
>> + virStorageAdapterClear(&source->adapter);
>> VIR_FREE(source->initiator.iqn);
>> virStorageAuthDefFree(source->auth);
>> VIR_FREE(source->vendor);
>> @@ -565,7 +565,7 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
>> goto cleanup;
>>
>> if ((adapternode = virXPathNode("./adapter", ctxt))) {
>> - if (virStoragePoolDefParseSourceAdapter(source, adapternode, ctxt) < 0)
>> + if (virStorageAdapterParseXML(source, adapternode, ctxt) < 0)
>> goto cleanup;
>> }
>>
>> @@ -802,7 +802,7 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
>> }
>>
>> if ((options->flags & VIR_STORAGE_POOL_SOURCE_ADAPTER) &&
>> - (virStoragePoolSourceAdapterParseValidate(ret)) < 0)
>> + (virStorageAdapterParseValidate(ret)) < 0)
>> goto error;
>>
>> /* If DEVICE is the only source type, then its required */
>> @@ -960,7 +960,7 @@ virStoragePoolSourceFormat(virBufferPtr buf,
>> if ((options->flags & VIR_STORAGE_POOL_SOURCE_ADAPTER) &&
>> (src->adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST ||
>> src->adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST))
>> - virStoragePoolSourceAdapterFormat(buf, src);
>> + virStorageAdapterFormat(buf, src);
>>
>> if (options->flags & VIR_STORAGE_POOL_SOURCE_NAME)
>> virBufferEscapeString(buf, "<name>%s</name>\n", src->name);
>> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
>> index 76cf2ae..6a2bdf2 100644
>> --- a/src/libvirt_private.syms
>> +++ b/src/libvirt_private.syms
>> @@ -848,10 +848,10 @@ virDomainSnapshotUpdateRelations;
>>
>>
>> # conf/storage_adapter_conf.h
>> -virStoragePoolDefParseSourceAdapter;
>> -virStoragePoolSourceAdapterClear;
>> -virStoragePoolSourceAdapterFormat;
>> -virStoragePoolSourceAdapterParseValidate;
>> +virStorageAdapterClear;
>> +virStorageAdapterFormat;
>> +virStorageAdapterParseValidate;
>> +virStorageAdapterParseXML;
>>
>>
>> # conf/storage_conf.h
>
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
