[libvirt] [PATCH] conf: duplicate interface name instead of MAC provided to lookup the interface

Introduced by 6094d6ec7fc9ea3e28c18c880b76858f06a8b129. Found by running libvirt-perl tests. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/virinterfaceobj.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c index a6814a6aee..21d76e7507 100644 --- a/src/conf/virinterfaceobj.c +++ b/src/conf/virinterfaceobj.c @@ -182,7 +182,7 @@ virInterfaceObjListFindByMACStringCb(void *payload, virObjectLock(obj); if (STRCASEEQ(obj->def->mac, data->matchStr)) { - if (VIR_STRDUP(data->macs[data->nmacs], data->matchStr) < 0) { + if (VIR_STRDUP(data->macs[data->nmacs], obj->def->name) < 0) { data->error = true; goto cleanup; } -- 2.13.6

On Fri, 2017-10-20 at 16:07 +0200, Pavel Hrdina wrote:
Introduced by 6094d6ec7fc9ea3e28c18c880b76858f06a8b129. Found by running libvirt-perl tests.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/virinterfaceobj.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c index a6814a6aee..21d76e7507 100644 --- a/src/conf/virinterfaceobj.c +++ b/src/conf/virinterfaceobj.c @@ -182,7 +182,7 @@ virInterfaceObjListFindByMACStringCb(void *payload, virObjectLock(obj);
if (STRCASEEQ(obj->def->mac, data->matchStr)) { - if (VIR_STRDUP(data->macs[data->nmacs], data->matchStr) < 0) { + if (VIR_STRDUP(data->macs[data->nmacs], obj->def->name) < 0) { data->error = true; goto cleanup; }
Reviewed-by: Andrea Bolognani <abologna@redhat.com> As an aside, I think 'macs' is a pretty bad name for the array, since we're not storing MAC addresses but rather interface names, so using either 'matches' or 'names' would be much better IMHO. Do you want me to send a follow-up patch performing the rename? -- Andrea Bolognani / Red Hat / Virtualization

On Fri, Oct 20, 2017 at 04:25:13PM +0200, Andrea Bolognani wrote:
On Fri, 2017-10-20 at 16:07 +0200, Pavel Hrdina wrote:
Introduced by 6094d6ec7fc9ea3e28c18c880b76858f06a8b129. Found by running libvirt-perl tests.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/virinterfaceobj.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c index a6814a6aee..21d76e7507 100644 --- a/src/conf/virinterfaceobj.c +++ b/src/conf/virinterfaceobj.c @@ -182,7 +182,7 @@ virInterfaceObjListFindByMACStringCb(void *payload, virObjectLock(obj);
if (STRCASEEQ(obj->def->mac, data->matchStr)) { - if (VIR_STRDUP(data->macs[data->nmacs], data->matchStr) < 0) { + if (VIR_STRDUP(data->macs[data->nmacs], obj->def->name) < 0) { data->error = true; goto cleanup; }
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Thanks
As an aside, I think 'macs' is a pretty bad name for the array, since we're not storing MAC addresses but rather interface names, so using either 'matches' or 'names' would be much better IMHO. Do you want me to send a follow-up patch performing the rename?
That would be good idea, feel free to send it and probably push it as trivial. Pavel

On 10/20/2017 10:28 AM, Pavel Hrdina wrote:
On Fri, Oct 20, 2017 at 04:25:13PM +0200, Andrea Bolognani wrote:
On Fri, 2017-10-20 at 16:07 +0200, Pavel Hrdina wrote:
Introduced by 6094d6ec7fc9ea3e28c18c880b76858f06a8b129. Found by running libvirt-perl tests.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/virinterfaceobj.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c index a6814a6aee..21d76e7507 100644 --- a/src/conf/virinterfaceobj.c +++ b/src/conf/virinterfaceobj.c @@ -182,7 +182,7 @@ virInterfaceObjListFindByMACStringCb(void *payload, virObjectLock(obj);
if (STRCASEEQ(obj->def->mac, data->matchStr)) { - if (VIR_STRDUP(data->macs[data->nmacs], data->matchStr) < 0) { + if (VIR_STRDUP(data->macs[data->nmacs], obj->def->name) < 0) { data->error = true; goto cleanup; }
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Thanks
As an aside, I think 'macs' is a pretty bad name for the array, since we're not storing MAC addresses but rather interface names, so using either 'matches' or 'names' would be much better IMHO. Do you want me to send a follow-up patch performing the rename?
That would be good idea, feel free to send it and probably push it as trivial.
Pavel
Doh - thanks for catching this... Who knew the perl tests would do better than the libvirt tests. As for [n]macs and naming - that's why I preferred the non-descript nElems, elems, and maxelems.... <sigh>. John
participants (3)
-
Andrea Bolognani
-
John Ferlan
-
Pavel Hrdina