Alter the algorithm to return a list of matching names rather than a
list of match virInterfaceObjPtr which are then just dereferenced
extracting the def->name and def->mac. Since the def->mac would be
the same as the passed @mac, just return a list of names and as long
as there's only one, extract the [0] entry from the passed list.
Also alter the error message on failure to include the mac that wasn't
found.
Signed-off-by: John Ferlan <jferlan@redhat.com>
---
src/conf/virinterfaceobj.c | 23 ++++++++++++++---------
src/conf/virinterfaceobj.h | 2 +-
src/test/test_driver.c | 16 ++++++++--------
3 files changed, 23 insertions(+), 18 deletions(-)
diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c
index 3aeeebd..1cc5c92 100644
--- a/src/conf/virinterfaceobj.c
+++ b/src/conf/virinterfaceobj.c
@@ -108,11 +108,11 @@ virInterfaceObjListNew(void)
int
virInterfaceObjListFindByMACString(virInterfaceObjListPtr interfaces,
const char *mac,
- virInterfaceObjPtr *matches,
+ char **const matches,
int maxmatches)
{
size_t i;
- unsigned int matchct = 0;
+ int matchct = 0;
for (i = 0; i < interfaces->count; i++) {
virInterfaceObjPtr obj = interfaces->objs[i];
@@ -121,18 +121,23 @@ virInterfaceObjListFindByMACString(virInterfaceObjListPtr interfaces,
virInterfaceObjLock(obj);
def = obj->def;
if (STRCASEEQ(def->mac, mac)) {
- matchct++;
- if (matchct <= maxmatches) {
- matches[matchct - 1] = obj;
- /* keep the lock if we're returning object to caller */
- /* it is the caller's responsibility to unlock *all* matches */
- continue;
+ if (matchct < maxmatches) {
+ if (VIR_STRDUP(matches[matchct], def->name) < 0) {
+ virInterfaceObjUnlock(obj);
+ goto error;
+ }
+ matchct++;
}
}
virInterfaceObjUnlock(obj);
-
}
return matchct;
+
+ error:
+ while (--matchct >= 0)
+ VIR_FREE(matches[matchct]);
+
+ return -1;
}
diff --git a/src/conf/virinterfaceobj.h b/src/conf/virinterfaceobj.h
index f1bcab9..3934e63 100644
--- a/src/conf/virinterfaceobj.h
+++ b/src/conf/virinterfaceobj.h
@@ -44,7 +44,7 @@ virInterfaceObjListNew(void);
int
virInterfaceObjListFindByMACString(virInterfaceObjListPtr interfaces,
const char *mac,
- virInterfaceObjPtr *matches,
+ char **const matches,
int maxmatches);
virInterfaceObjPtr
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 89a705c..ac16f4f 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3728,17 +3728,18 @@ testInterfaceLookupByMACString(virConnectPtr conn,
const char *mac)
{
testDriverPtr privconn = conn->privateData;
- virInterfaceObjPtr obj;
- virInterfaceDefPtr def;
int ifacect;
+ char *ifacenames[] = { NULL, NULL };
virInterfacePtr ret = NULL;
testDriverLock(privconn);
- ifacect = virInterfaceObjListFindByMACString(privconn->ifaces, mac, &obj, 1);
+ ifacect = virInterfaceObjListFindByMACString(privconn->ifaces, mac,
+ ifacenames, 2);
testDriverUnlock(privconn);
if (ifacect == 0) {
- virReportError(VIR_ERR_NO_INTERFACE, NULL);
+ virReportError(VIR_ERR_NO_INTERFACE,
+ _("no interface with matching mac '%s'"), mac);
goto cleanup;
}
@@ -3747,12 +3748,11 @@ testInterfaceLookupByMACString(virConnectPtr conn,
goto cleanup;
}
- def = virInterfaceObjGetDef(obj);
- ret = virGetInterface(conn, def->name, def->mac);
+ ret = virGetInterface(conn, ifacenames[0], mac);
cleanup:
- if (obj)
- virInterfaceObjUnlock(obj);
+ VIR_FREE(ifacenames[0]);
+ VIR_FREE(ifacenames[1]);
return ret;
}
--
2.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 04/26/2017 12:36 AM, John Ferlan wrote:
> Alter the algorithm to return a list of matching names rather than a
> list of match virInterfaceObjPtr which are then just dereferenced
> extracting the def->name and def->mac. Since the def->mac would be
> the same as the passed @mac, just return a list of names and as long
> as there's only one, extract the [0] entry from the passed list.
> Also alter the error message on failure to include the mac that wasn't
> found.
>
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
> src/conf/virinterfaceobj.c | 23 ++++++++++++++---------
> src/conf/virinterfaceobj.h | 2 +-
> src/test/test_driver.c | 16 ++++++++--------
> 3 files changed, 23 insertions(+), 18 deletions(-)
>
> diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c
> index 3aeeebd..1cc5c92 100644
> --- a/src/conf/virinterfaceobj.c
> +++ b/src/conf/virinterfaceobj.c
> @@ -108,11 +108,11 @@ virInterfaceObjListNew(void)
> int
> virInterfaceObjListFindByMACString(virInterfaceObjListPtr interfaces,
> const char *mac,
> - virInterfaceObjPtr *matches,
> + char **const matches,
> int maxmatches)
> {
> size_t i;
> - unsigned int matchct = 0;
> + int matchct = 0;
>
> for (i = 0; i < interfaces->count; i++) {
> virInterfaceObjPtr obj = interfaces->objs[i];
> @@ -121,18 +121,23 @@ virInterfaceObjListFindByMACString(virInterfaceObjListPtr interfaces,
> virInterfaceObjLock(obj);
> def = obj->def;
> if (STRCASEEQ(def->mac, mac)) {
> - matchct++;
> - if (matchct <= maxmatches) {
> - matches[matchct - 1] = obj;
> - /* keep the lock if we're returning object to caller */
> - /* it is the caller's responsibility to unlock *all* matches */
> - continue;
> + if (matchct < maxmatches) {
> + if (VIR_STRDUP(matches[matchct], def->name) < 0) {
> + virInterfaceObjUnlock(obj);
> + goto error;
> + }
> + matchct++;
> }
> }
> virInterfaceObjUnlock(obj);
> -
> }
> return matchct;
> +
> + error:
> + while (--matchct >= 0)
> + VIR_FREE(matches[matchct]);
> +
> + return -1;
> }
>
>
> diff --git a/src/conf/virinterfaceobj.h b/src/conf/virinterfaceobj.h
> index f1bcab9..3934e63 100644
> --- a/src/conf/virinterfaceobj.h
> +++ b/src/conf/virinterfaceobj.h
> @@ -44,7 +44,7 @@ virInterfaceObjListNew(void);
> int
> virInterfaceObjListFindByMACString(virInterfaceObjListPtr interfaces,
> const char *mac,
> - virInterfaceObjPtr *matches,
> + char **const matches,
> int maxmatches);
>
> virInterfaceObjPtr
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index 89a705c..ac16f4f 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -3728,17 +3728,18 @@ testInterfaceLookupByMACString(virConnectPtr conn,
> const char *mac)
> {
> testDriverPtr privconn = conn->privateData;
> - virInterfaceObjPtr obj;
> - virInterfaceDefPtr def;
> int ifacect;
> + char *ifacenames[] = { NULL, NULL };
> virInterfacePtr ret = NULL;
>
> testDriverLock(privconn);
> - ifacect = virInterfaceObjListFindByMACString(privconn->ifaces, mac, &obj, 1);
> + ifacect = virInterfaceObjListFindByMACString(privconn->ifaces, mac,
> + ifacenames, 2);
ARRAY_CARDINALITY()
> testDriverUnlock(privconn);
>
> if (ifacect == 0) {
> - virReportError(VIR_ERR_NO_INTERFACE, NULL);
> + virReportError(VIR_ERR_NO_INTERFACE,
> + _("no interface with matching mac '%s'"), mac);
> goto cleanup;
> }
>
> @@ -3747,12 +3748,11 @@ testInterfaceLookupByMACString(virConnectPtr conn,
> goto cleanup;
> }
>
> - def = virInterfaceObjGetDef(obj);
> - ret = virGetInterface(conn, def->name, def->mac);
> + ret = virGetInterface(conn, ifacenames[0], mac);
>
> cleanup:
> - if (obj)
> - virInterfaceObjUnlock(obj);
> + VIR_FREE(ifacenames[0]);
> + VIR_FREE(ifacenames[1]);
And this can be probably dynamic then:
for (i = 0; i < ARRAY_CARDINALITY(ifacenames); i++)
VIR_FREE(ifacenames[i]);
> return ret;
> }
>
>
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 05/19/2017 11:29 AM, Michal Privoznik wrote:
> On 04/26/2017 12:36 AM, John Ferlan wrote:
>> Alter the algorithm to return a list of matching names rather than a
>> list of match virInterfaceObjPtr which are then just dereferenced
>> extracting the def->name and def->mac. Since the def->mac would be
>> the same as the passed @mac, just return a list of names and as long
>> as there's only one, extract the [0] entry from the passed list.
>> Also alter the error message on failure to include the mac that wasn't
>> found.
>>
>> Signed-off-by: John Ferlan <jferlan@redhat.com>
>> ---
>> src/conf/virinterfaceobj.c | 23 ++++++++++++++---------
>> src/conf/virinterfaceobj.h | 2 +-
>> src/test/test_driver.c | 16 ++++++++--------
>> 3 files changed, 23 insertions(+), 18 deletions(-)
>>
>> diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c
>> index 3aeeebd..1cc5c92 100644
>> --- a/src/conf/virinterfaceobj.c
>> +++ b/src/conf/virinterfaceobj.c
>> @@ -108,11 +108,11 @@ virInterfaceObjListNew(void)
>> int
>> virInterfaceObjListFindByMACString(virInterfaceObjListPtr interfaces,
>> const char *mac,
>> - virInterfaceObjPtr *matches,
>> + char **const matches,
>> int maxmatches)
>> {
>> size_t i;
>> - unsigned int matchct = 0;
>> + int matchct = 0;
>>
>> for (i = 0; i < interfaces->count; i++) {
>> virInterfaceObjPtr obj = interfaces->objs[i];
>> @@ -121,18 +121,23 @@ virInterfaceObjListFindByMACString(virInterfaceObjListPtr interfaces,
>> virInterfaceObjLock(obj);
>> def = obj->def;
>> if (STRCASEEQ(def->mac, mac)) {
>> - matchct++;
>> - if (matchct <= maxmatches) {
>> - matches[matchct - 1] = obj;
>> - /* keep the lock if we're returning object to caller */
>> - /* it is the caller's responsibility to unlock *all* matches */
>> - continue;
>> + if (matchct < maxmatches) {
>> + if (VIR_STRDUP(matches[matchct], def->name) < 0) {
>> + virInterfaceObjUnlock(obj);
>> + goto error;
>> + }
>> + matchct++;
>> }
>> }
>> virInterfaceObjUnlock(obj);
>> -
>> }
>> return matchct;
>> +
>> + error:
>> + while (--matchct >= 0)
>> + VIR_FREE(matches[matchct]);
>> +
>> + return -1;
>> }
>>
>>
>> diff --git a/src/conf/virinterfaceobj.h b/src/conf/virinterfaceobj.h
>> index f1bcab9..3934e63 100644
>> --- a/src/conf/virinterfaceobj.h
>> +++ b/src/conf/virinterfaceobj.h
>> @@ -44,7 +44,7 @@ virInterfaceObjListNew(void);
>> int
>> virInterfaceObjListFindByMACString(virInterfaceObjListPtr interfaces,
>> const char *mac,
>> - virInterfaceObjPtr *matches,
>> + char **const matches,
>> int maxmatches);
>>
>> virInterfaceObjPtr
>> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
>> index 89a705c..ac16f4f 100644
>> --- a/src/test/test_driver.c
>> +++ b/src/test/test_driver.c
>> @@ -3728,17 +3728,18 @@ testInterfaceLookupByMACString(virConnectPtr conn,
>> const char *mac)
>> {
>> testDriverPtr privconn = conn->privateData;
>> - virInterfaceObjPtr obj;
>> - virInterfaceDefPtr def;
>> int ifacect;
>> + char *ifacenames[] = { NULL, NULL };
>> virInterfacePtr ret = NULL;
>>
>> testDriverLock(privconn);
>> - ifacect = virInterfaceObjListFindByMACString(privconn->ifaces, mac, &obj, 1);
>> + ifacect = virInterfaceObjListFindByMACString(privconn->ifaces, mac,
>> + ifacenames, 2);
>
> ARRAY_CARDINALITY()
>
Don't really see the advantage. All this does is try to ensure the
provided MAC is "unique" by collecting all interfaces with it defined.
Once it reaches 2, FindByMACString won't collect any more since it
reached maxmatches.
John
>> testDriverUnlock(privconn);
>>
>> if (ifacect == 0) {
>> - virReportError(VIR_ERR_NO_INTERFACE, NULL);
>> + virReportError(VIR_ERR_NO_INTERFACE,
>> + _("no interface with matching mac '%s'"), mac);
>> goto cleanup;
>> }
>>
>> @@ -3747,12 +3748,11 @@ testInterfaceLookupByMACString(virConnectPtr conn,
>> goto cleanup;
>> }
>>
>> - def = virInterfaceObjGetDef(obj);
>> - ret = virGetInterface(conn, def->name, def->mac);
>> + ret = virGetInterface(conn, ifacenames[0], mac);
>>
>> cleanup:
>> - if (obj)
>> - virInterfaceObjUnlock(obj);
>> + VIR_FREE(ifacenames[0]);
>> + VIR_FREE(ifacenames[1]);
>
> And this can be probably dynamic then:
>
> for (i = 0; i < ARRAY_CARDINALITY(ifacenames); i++)
> VIR_FREE(ifacenames[i]);
>
>> return ret;
>> }
>>
>>
>
> Michal
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 05/20/2017 01:18 PM, John Ferlan wrote:
>
>
> On 05/19/2017 11:29 AM, Michal Privoznik wrote:
>> On 04/26/2017 12:36 AM, John Ferlan wrote:
>>> Alter the algorithm to return a list of matching names rather than a
>>> list of match virInterfaceObjPtr which are then just dereferenced
>>> extracting the def->name and def->mac. Since the def->mac would be
>>> the same as the passed @mac, just return a list of names and as long
>>> as there's only one, extract the [0] entry from the passed list.
>>> Also alter the error message on failure to include the mac that wasn't
>>> found.
>>>
>>> Signed-off-by: John Ferlan <jferlan@redhat.com>
>>> ---
>>> src/conf/virinterfaceobj.c | 23 ++++++++++++++---------
>>> src/conf/virinterfaceobj.h | 2 +-
>>> src/test/test_driver.c | 16 ++++++++--------
>>> 3 files changed, 23 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c
>>> index 3aeeebd..1cc5c92 100644
>>> --- a/src/conf/virinterfaceobj.c
>>> +++ b/src/conf/virinterfaceobj.c
>>> @@ -108,11 +108,11 @@ virInterfaceObjListNew(void)
>>> int
>>> virInterfaceObjListFindByMACString(virInterfaceObjListPtr interfaces,
>>> const char *mac,
>>> - virInterfaceObjPtr *matches,
>>> + char **const matches,
>>> int maxmatches)
>>> {
>>> size_t i;
>>> - unsigned int matchct = 0;
>>> + int matchct = 0;
>>>
>>> for (i = 0; i < interfaces->count; i++) {
>>> virInterfaceObjPtr obj = interfaces->objs[i];
>>> @@ -121,18 +121,23 @@ virInterfaceObjListFindByMACString(virInterfaceObjListPtr interfaces,
>>> virInterfaceObjLock(obj);
>>> def = obj->def;
>>> if (STRCASEEQ(def->mac, mac)) {
>>> - matchct++;
>>> - if (matchct <= maxmatches) {
>>> - matches[matchct - 1] = obj;
>>> - /* keep the lock if we're returning object to caller */
>>> - /* it is the caller's responsibility to unlock *all* matches */
>>> - continue;
>>> + if (matchct < maxmatches) {
>>> + if (VIR_STRDUP(matches[matchct], def->name) < 0) {
>>> + virInterfaceObjUnlock(obj);
>>> + goto error;
>>> + }
>>> + matchct++;
>>> }
>>> }
>>> virInterfaceObjUnlock(obj);
>>> -
>>> }
>>> return matchct;
>>> +
>>> + error:
>>> + while (--matchct >= 0)
>>> + VIR_FREE(matches[matchct]);
>>> +
>>> + return -1;
>>> }
>>>
>>>
>>> diff --git a/src/conf/virinterfaceobj.h b/src/conf/virinterfaceobj.h
>>> index f1bcab9..3934e63 100644
>>> --- a/src/conf/virinterfaceobj.h
>>> +++ b/src/conf/virinterfaceobj.h
>>> @@ -44,7 +44,7 @@ virInterfaceObjListNew(void);
>>> int
>>> virInterfaceObjListFindByMACString(virInterfaceObjListPtr interfaces,
>>> const char *mac,
>>> - virInterfaceObjPtr *matches,
>>> + char **const matches,
>>> int maxmatches);
>>>
>>> virInterfaceObjPtr
>>> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
>>> index 89a705c..ac16f4f 100644
>>> --- a/src/test/test_driver.c
>>> +++ b/src/test/test_driver.c
>>> @@ -3728,17 +3728,18 @@ testInterfaceLookupByMACString(virConnectPtr conn,
>>> const char *mac)
>>> {
>>> testDriverPtr privconn = conn->privateData;
>>> - virInterfaceObjPtr obj;
>>> - virInterfaceDefPtr def;
>>> int ifacect;
>>> + char *ifacenames[] = { NULL, NULL };
>>> virInterfacePtr ret = NULL;
>>>
>>> testDriverLock(privconn);
>>> - ifacect = virInterfaceObjListFindByMACString(privconn->ifaces, mac, &obj, 1);
>>> + ifacect = virInterfaceObjListFindByMACString(privconn->ifaces, mac,
>>> + ifacenames, 2);
>>
>> ARRAY_CARDINALITY()
>>
>
> Don't really see the advantage. All this does is try to ensure the
> provided MAC is "unique" by collecting all interfaces with it defined.
> Once it reaches 2, FindByMACString won't collect any more since it
> reached maxmatches.
The advantage to me is that we get rid of the magical constant. I can
immediately see what is "2" supposed to mean. But I agree that the test
driver is not our display case. So after all, I don't care that much. I
just thought it would be nice to have it.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.