If the mac address of an interface is changed, we must either create a
new virInterface object in the cache that has the new name/mac, or
modify the existing object in the cache. Because the cache is in a
hash that's indexed by name only, we can't create a new entry having
the same name without destroying the old one first (which isn't a
possibility as someone else is already referencing the old one), so
we're stuck modifying the existing entry. We have to do that without
changing the pointer to the mac though, so we really can only do it if
the length of the new mac is equal to or less than the old
mac. Otherwise, we have to just bail.
Fixing this problem to properly handle this corner case would require
a hash table that used both name and mac as keys, but that would be
inefficient (using the existing hash table implementation, which only
allows a single key), and probably unnecessary, as the length of mac
for any given interface will never change in practice.
---
src/datatypes.c | 33 ++++++++++++++++++++++++++++++++-
1 files changed, 32 insertions(+), 1 deletions(-)
diff --git a/src/datatypes.c b/src/datatypes.c
index 5f90aad..000fa66 100644
--- a/src/datatypes.c
+++ b/src/datatypes.c
@@ -535,7 +535,38 @@ virGetInterface(virConnectPtr conn, const char *name, const char *mac) {
ret = (virInterfacePtr) virHashLookup(conn->interfaces, name);
- if ((ret == NULL) || STRCASENEQ(ret->mac, mac)) {
+ if (ret != NULL) {
+ if (STRCASENEQ(ret->mac, mac)) {
+ /*
+ * If the mac address has changed, try to modify it in
+ * place, which will only work if the new mac is the
+ * same length as, or shorter than, the old mac.
+ */
+ size_t newmaclen = strlen(mac);
+ size_t oldmaclen = strlen(ret->mac);
+ if (newmaclen <= oldmaclen) {
+ strcpy(ret->mac, mac);
+ } else {
+ /*
+ * If it's longer, we're kind of screwed, because we
+ * can't add a new hashtable entry (it would clash
+ * with the existing entry of same name), and we can't
+ * free/re-alloc the existing entry's mac, as some
+ * other thread may already be using the existing mac
+ * pointer. Fortunately, this should never happen,
+ * since the length of the mac address for any
+ * interface is determined by the type of the
+ * interface, and that is unlikely to change.
+ */
+ virMutexUnlock(&conn->lock);
+ virLibConnError(conn, VIR_ERR_INTERNAL_ERROR,
+ "Failed to change interface mac address from %s to %s due to differing lengths.",
+ ret->mac, mac);
+ ret = NULL;
+ goto error;
+ }
+ }
+ } else {
if (VIR_ALLOC(ret) < 0) {
virReportOOMError(conn);
goto error;
--
1.6.0.6
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list