Rename the API's to remove the storage pool source pieces
Signed-off-by: John Ferlan <jferlan@redhat.com>
---
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)
{
int ret = -1;
xmlNodePtr relnode = ctxt->node;
@@ -177,7 +177,7 @@ virStoragePoolDefParseSourceAdapter(virStoragePoolSourcePtr source,
int
-virStoragePoolSourceAdapterParseValidate(virStoragePoolDefPtr ret)
+virStorageAdapterParseValidate(virStoragePoolDefPtr ret)
{
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)
{
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);
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
--
2.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 03/10/2017 04:10 PM, John Ferlan wrote: > Rename the API's to remove the storage pool source pieces > > Signed-off-by: John Ferlan <jferlan@redhat.com> > --- > 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(-) > ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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@redhat.com>
> ---
> 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.
> {
> 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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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@redhat.com>
>> ---
>> 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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 03/14/2017 07:05 PM, John Ferlan wrote:
>
> 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@redhat.com>
>>> ---
>>> 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!)
Yeah, that name makes sense once you explain it. Maybe. Is it really validating that the parse was done correctly? Or is it just validating that the data in the object meets various criteria? Seems like it's the latter. Would you really want to validate the object any differently if it had just been parsed from XML vs. if the object was generated in some other manner? (e.g. some chunk of C code that created the object and filled in attributes programmatically)
>
> 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.
Yeah, I didn't look ahead when I started reviewing the series, so I didn't realize what the end product was going to be. *A lot* of my comments are moot in light of that.
> 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.
I've stared straight at it for multiple years now, but just last week thought to try out for the first time the "old Version" and "new Version" buttons in gitk - combine those with a very large number for context lines, and it is truly very useful.
Another nice way to look at diffs is with "git difftool -d -t meld revA..revB". I haven't found a simple way to move from one commit to the next, but once it's displayed it is a very nice format.
(while on the topic of tools, what I would *really* like is a tool like gitk where you could select a chunk and drag it from one commit to another, or drag entire commits around to change the order on a branch, or copy a commit or group of commits from one branch to another (essentially something that would do an entire "git rebase -i master" or "git cherry-pick" at the drag of a mouse). Any suggestions?
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[...] >>>> 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!) > > Yeah, that name makes sense once you explain it. Maybe. Is it really validating that the parse was done correctly? Or is it just validating that the data in the object meets various criteria? Seems like it's the latter. Would you really want to validate the object any differently if it had just been parsed from XML vs. if the object was generated in some other manner? (e.g. some chunk of C code that created the object and filled in attributes programmatically) > Fair enough - I'll just change to Validate, but while working through merge conflicts in my branch I ran into virDomainDiskDefParseValidate It's a change that wasn't sent with this series, but I think I know now where I got the name originally. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.