[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] PATCH 1/4: More generic MAC address handling



This patch improves the MAC address handling.

Currently our XML parser auto-generates a MAC addres using the KVM vendor
prefix. This isn't much use for other drivers. This patch addresses this:

 - Stores each driver's vendor prefix in the capability object
 - Changes domain parser to use the per-driver vendor prefix for
   generating mac addresses
 - Adds more utility methods to util.c for parsing/generating/formatting
   MAC addresses
 - Updates each driver to record its vendor prefix for MAC address.


NB, for LXC driver we're 'borrowing' KVM's vendor prefix. 

 capabilities.c  |   16 +++++++++++++++-
 capabilities.h  |   11 +++++++++++
 domain_conf.c   |   34 +++++++---------------------------
 domain_conf.h   |    8 ++------
 lxc_conf.c      |    3 +++
 lxc_driver.c    |    4 +++-
 openvz_conf.c   |    2 +-
 qemu_conf.c     |    3 +++
 qemu_driver.c   |    6 ++++--
 util.c          |   24 +++++++++++++++++++++++-
 util.h          |   12 +++++++++++-
 xen_internal.c  |    3 +++
 xend_internal.c |    8 ++++++--
 xm_internal.c   |   34 +++++++++++-----------------------
 14 files changed, 103 insertions(+), 65 deletions(-)


Daniel

diff -r c928fca9ce2b src/capabilities.c
--- a/src/capabilities.c	Tue Oct 14 11:16:52 2008 +0100
+++ b/src/capabilities.c	Tue Oct 14 12:24:44 2008 +0100
@@ -26,7 +26,7 @@
 #include "capabilities.h"
 #include "buf.h"
 #include "memory.h"
-
+#include "util.h"
 
 /**
  * virCapabilitiesNew:
@@ -647,3 +647,17 @@ virCapabilitiesFormatXML(virCapsPtr caps
 
     return virBufferContentAndReset(&xml);
 }
+
+extern void
+virCapabilitiesSetMacPrefix(virCapsPtr caps,
+                            unsigned char *prefix)
+{
+    memcpy(caps->macPrefix, prefix, sizeof(caps->macPrefix));
+}
+
+extern void
+virCapabilitiesGenerateMac(virCapsPtr caps,
+                           unsigned char *mac)
+{
+    virGenerateMacAddr(caps->macPrefix, mac);
+}
diff -r c928fca9ce2b src/capabilities.h
--- a/src/capabilities.h	Tue Oct 14 11:16:52 2008 +0100
+++ b/src/capabilities.h	Tue Oct 14 12:24:13 2008 +0100
@@ -23,6 +23,9 @@
 
 #ifndef __VIR_CAPABILITIES_H
 #define __VIR_CAPABILITIES_H
+
+#include "internal.h"
+#include "util.h"
 
 typedef struct _virCapsGuestFeature virCapsGuestFeature;
 typedef virCapsGuestFeature *virCapsGuestFeaturePtr;
@@ -95,6 +98,7 @@ struct _virCaps {
     virCapsHost host;
     int nguests;
     virCapsGuestPtr *guests;
+    unsigned char macPrefix[VIR_MAC_PREFIX_BUFLEN];
 };
 
 
@@ -106,6 +110,13 @@ extern void
 extern void
 virCapabilitiesFree(virCapsPtr caps);
 
+extern void
+virCapabilitiesSetMacPrefix(virCapsPtr caps,
+                            unsigned char *prefix);
+
+extern void
+virCapabilitiesGenerateMac(virCapsPtr caps,
+                           unsigned char *mac);
 
 extern int
 virCapabilitiesAddHostFeature(virCapsPtr caps,
diff -r c928fca9ce2b src/domain_conf.c
--- a/src/domain_conf.c	Tue Oct 14 11:16:52 2008 +0100
+++ b/src/domain_conf.c	Tue Oct 14 12:26:36 2008 +0100
@@ -762,23 +762,14 @@ cleanup:
 }
 
 
-static void virDomainNetRandomMAC(virDomainNetDefPtr def) {
-    /* XXX there different vendor prefixes per hypervisor */
-    def->mac[0] = 0x52;
-    def->mac[1] = 0x54;
-    def->mac[2] = 0x00;
-    def->mac[3] = 1 + (int)(256*(rand()/(RAND_MAX+1.0)));
-    def->mac[4] = 1 + (int)(256*(rand()/(RAND_MAX+1.0)));
-    def->mac[5] = 1 + (int)(256*(rand()/(RAND_MAX+1.0)));
-}
-
 
 /* Parse the XML definition for a network interface
  * @param node XML nodeset to parse for net definition
  * @return 0 on success, -1 on failure
  */
-virDomainNetDefPtr
+static virDomainNetDefPtr
 virDomainNetDefParseXML(virConnectPtr conn,
+                        virCapsPtr caps,
                         xmlNodePtr node) {
     virDomainNetDefPtr def;
     xmlNodePtr cur;
@@ -857,22 +848,9 @@ virDomainNetDefParseXML(virConnectPtr co
     }
 
     if (macaddr) {
-        unsigned int mac[6];
-        sscanf((const char *)macaddr, "%02x:%02x:%02x:%02x:%02x:%02x",
-               (unsigned int*)&mac[0],
-               (unsigned int*)&mac[1],
-               (unsigned int*)&mac[2],
-               (unsigned int*)&mac[3],
-               (unsigned int*)&mac[4],
-               (unsigned int*)&mac[5]);
-        def->mac[0] = mac[0];
-        def->mac[1] = mac[1];
-        def->mac[2] = mac[2];
-        def->mac[3] = mac[3];
-        def->mac[4] = mac[4];
-        def->mac[5] = mac[5];
+        virParseMacAddr((const char *)macaddr, def->mac);
     } else {
-        virDomainNetRandomMAC(def);
+        virCapabilitiesGenerateMac(caps, def->mac);
     }
 
     switch (def->type) {
@@ -1630,6 +1608,7 @@ static int virDomainLifecycleParseXML(vi
 
 
 virDomainDeviceDefPtr virDomainDeviceDefParse(virConnectPtr conn,
+                                              virCapsPtr caps,
                                               const virDomainDefPtr def,
                                               const char *xmlStr)
 {
@@ -1666,7 +1645,7 @@ virDomainDeviceDefPtr virDomainDeviceDef
             goto error;
     } else if (xmlStrEqual(node->name, BAD_CAST "interface")) {
         dev->type = VIR_DOMAIN_DEVICE_NET;
-        if (!(dev->data.net = virDomainNetDefParseXML(conn, node)))
+        if (!(dev->data.net = virDomainNetDefParseXML(conn, caps, node)))
             goto error;
     } else if (xmlStrEqual(node->name, BAD_CAST "input")) {
         dev->type = VIR_DOMAIN_DEVICE_INPUT;
@@ -1988,6 +1967,7 @@ static virDomainDefPtr virDomainDefParse
         goto no_memory;
     for (i = 0 ; i < n ; i++) {
         virDomainNetDefPtr net = virDomainNetDefParseXML(conn,
+                                                         caps,
                                                          nodes[i]);
         if (!net)
             goto error;
diff -r c928fca9ce2b src/domain_conf.h
--- a/src/domain_conf.h	Tue Oct 14 11:16:52 2008 +0100
+++ b/src/domain_conf.h	Tue Oct 14 12:26:56 2008 +0100
@@ -127,14 +127,12 @@ enum virDomainNetType {
 };
 
 
-#define VIR_DOMAIN_NET_MAC_SIZE 6
-
 /* Stores the virtual network interface configuration */
 typedef struct _virDomainNetDef virDomainNetDef;
 typedef virDomainNetDef *virDomainNetDefPtr;
 struct _virDomainNetDef {
     int type;
-    unsigned char mac[VIR_DOMAIN_NET_MAC_SIZE];
+    unsigned char mac[VIR_MAC_BUFLEN];
     char *model;
     union {
         struct {
@@ -513,6 +511,7 @@ void virDomainRemoveInactive(virDomainOb
 
 #ifndef PROXY
 virDomainDeviceDefPtr virDomainDeviceDefParse(virConnectPtr conn,
+                                              virCapsPtr caps,
                                               const virDomainDefPtr def,
                                               const char *xmlStr);
 virDomainDefPtr virDomainDefParseString(virConnectPtr conn,
@@ -573,9 +572,6 @@ int virDiskNameToBusDeviceIndex(virDomai
                                 int *busIdx,
                                 int *devIdx);
 
-virDomainNetDefPtr virDomainNetDefParseXML(virConnectPtr conn,
-                        xmlNodePtr node);
-
 const char *virDomainDefDefaultEmulator(virConnectPtr conn,
                                         virDomainDefPtr def,
                                         virCapsPtr caps);
diff -r c928fca9ce2b src/lxc_conf.c
--- a/src/lxc_conf.c	Tue Oct 14 11:16:52 2008 +0100
+++ b/src/lxc_conf.c	Tue Oct 14 12:33:39 2008 +0100
@@ -42,6 +42,9 @@ virCapsPtr lxcCapsInit(void)
                                    0, 0)) == NULL)
         goto no_memory;
 
+    /* XXX shouldn't 'borrow' KVM's prefix */
+    virCapabilitiesSetMacPrefix(caps, (unsigned char []){ 0x52, 0x54, 0x00 });
+
     if ((guest = virCapabilitiesAddGuest(caps,
                                          "exe",
                                          utsname.machine,
diff -r c928fca9ce2b src/lxc_driver.c
--- a/src/lxc_driver.c	Tue Oct 14 11:16:52 2008 +0100
+++ b/src/lxc_driver.c	Tue Oct 14 12:34:45 2008 +0100
@@ -1187,6 +1187,7 @@ static int lxcGetSchedulerParameters(vir
     int rc = 0;
     virCgroupPtr group;
     virDomainObjPtr domain;
+    unsigned long val;
 
     if (virCgroupHaveSupport() != 0)
         return 0;
@@ -1208,7 +1209,8 @@ static int lxcGetSchedulerParameters(vir
     if (rc != 0)
         return rc;
 
-    rc = virCgroupGetCpuShares(group, (unsigned long *)&params[0].value.ul);
+    rc = virCgroupGetCpuShares(group, &val);
+    params[0].value.ul = val;
     strncpy(params[0].field, "cpu_shares", sizeof(params[0].field));
     params[0].type = VIR_DOMAIN_SCHED_FIELD_ULLONG;
 
diff -r c928fca9ce2b src/openvz_conf.c
--- a/src/openvz_conf.c	Tue Oct 14 11:16:52 2008 +0100
+++ b/src/openvz_conf.c	Tue Oct 14 14:41:29 2008 +0100
@@ -108,7 +108,7 @@ int openvzCheckEmptyMac(const unsigned c
 int openvzCheckEmptyMac(const unsigned char *mac)
 {
     int i;
-    for (i = 0; i < VIR_DOMAIN_NET_MAC_SIZE; i++)
+    for (i = 0; i < VIR_MAC_BUFLEN; i++)
         if (mac[i] != 0x00)
             return 1;
 
diff -r c928fca9ce2b src/qemu_conf.c
--- a/src/qemu_conf.c	Tue Oct 14 11:16:52 2008 +0100
+++ b/src/qemu_conf.c	Tue Oct 14 12:32:22 2008 +0100
@@ -364,6 +364,9 @@ virCapsPtr qemudCapsInit(void) {
     if ((caps = virCapabilitiesNew(utsname.machine,
                                    0, 0)) == NULL)
         goto no_memory;
+
+    /* Using KVM's mac prefix for QEMU too */
+    virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x52, 0x54, 0x00 });
 
     if (qemudCapsInitNUMA(caps) < 0)
         goto no_memory;
diff -r c928fca9ce2b src/qemu_driver.c
--- a/src/qemu_driver.c	Tue Oct 14 11:16:52 2008 +0100
+++ b/src/qemu_driver.c	Tue Oct 14 12:33:23 2008 +0100
@@ -2389,7 +2389,7 @@ static int qemudDomainChangeEjectableMed
 {
     struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData;
     virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid);
-    virDomainDiskDefPtr origdisk, newdisk;
+    virDomainDiskDefPtr origdisk = NULL, newdisk;
     char *cmd, *reply, *safe_path;
     char *devname = NULL;
     unsigned int qemuCmdFlags;
@@ -2631,7 +2631,9 @@ static int qemudDomainAttachDevice(virDo
         return -1;
     }
 
-    dev = virDomainDeviceDefParse(dom->conn, vm->def, xml);
+    dev = virDomainDeviceDefParse(dom->conn,
+                                  driver->caps,
+                                  vm->def, xml);
     if (dev == NULL) {
         return -1;
     }
diff -r c928fca9ce2b src/util.c
--- a/src/util.c	Tue Oct 14 11:16:52 2008 +0100
+++ b/src/util.c	Tue Oct 14 12:23:15 2008 +0100
@@ -983,7 +983,7 @@ virParseMacAddr(const char* str, unsigne
     int i;
 
     errno = 0;
-    for (i = 0; i < 6; i++) {
+    for (i = 0; i < VIR_MAC_BUFLEN; i++) {
         char *end_ptr;
         unsigned long result;
 
@@ -1012,6 +1012,28 @@ virParseMacAddr(const char* str, unsigne
 
     return -1;
 }
+
+void virFormatMacAddr(const unsigned char *addr,
+                      char *str)
+{
+    snprintf(str, VIR_MAC_STRING_BUFLEN,
+             "%02X:%02X:%02X:%02X:%02X:%02X",
+             addr[0], addr[1], addr[2],
+             addr[3], addr[4], addr[5]);
+    str[VIR_MAC_STRING_BUFLEN-1] = '\0';
+}
+
+void virGenerateMacAddr(const unsigned char *prefix,
+                        unsigned char *addr)
+{
+    addr[0] = prefix[0];
+    addr[1] = prefix[1];
+    addr[2] = prefix[2];
+    addr[3] = 1 + (int)(256*(rand()/(RAND_MAX+1.0)));
+    addr[4] = 1 + (int)(256*(rand()/(RAND_MAX+1.0)));
+    addr[5] = 1 + (int)(256*(rand()/(RAND_MAX+1.0)));
+}
+
 
 int virEnumFromString(const char *const*types,
                       unsigned int ntypes,
diff -r c928fca9ce2b src/util.h
--- a/src/util.h	Tue Oct 14 11:16:52 2008 +0100
+++ b/src/util.h	Tue Oct 14 12:21:58 2008 +0100
@@ -1,3 +1,4 @@
+
 /*
  * utils.h: common, generic utility functions
  *
@@ -113,7 +114,16 @@ void virSkipSpaces(const char **str);
 void virSkipSpaces(const char **str);
 int virParseNumber(const char **str);
 
-int virParseMacAddr(const char* str, unsigned char *addr);
+#define VIR_MAC_BUFLEN 6
+#define VIR_MAC_PREFIX_BUFLEN 3
+#define VIR_MAC_STRING_BUFLEN VIR_MAC_BUFLEN * 3
+
+int virParseMacAddr(const char* str,
+                    unsigned char *addr);
+void virFormatMacAddr(const unsigned char *addr,
+                      char *str);
+void virGenerateMacAddr(const unsigned char *prefix,
+                        unsigned char *addr);
 
 int virDiskNameToIndex(const char* str);
 
diff -r c928fca9ce2b src/xen_internal.c
--- a/src/xen_internal.c	Tue Oct 14 11:16:52 2008 +0100
+++ b/src/xen_internal.c	Tue Oct 14 12:27:38 2008 +0100
@@ -2132,6 +2132,9 @@ xenHypervisorBuildCapabilities(virConnec
 
     if ((caps = virCapabilitiesNew(hostmachine, 1, 1)) == NULL)
         goto no_memory;
+
+    virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x00, 0x16, 0x3e });
+
     if (hvm_type && STRNEQ(hvm_type, "") &&
         virCapabilitiesAddHostFeature(caps, hvm_type) < 0)
         goto no_memory;
diff -r c928fca9ce2b src/xend_internal.c
--- a/src/xend_internal.c	Tue Oct 14 11:16:52 2008 +0100
+++ b/src/xend_internal.c	Tue Oct 14 12:29:07 2008 +0100
@@ -3852,7 +3852,9 @@ xenDaemonAttachDevice(virDomainPtr domai
                                      NULL)))
         goto cleanup;
 
-    if (!(dev = virDomainDeviceDefParse(domain->conn, def, xml)))
+    if (!(dev = virDomainDeviceDefParse(domain->conn,
+                                        priv->caps,
+                                        def, xml)))
         goto cleanup;
 
 
@@ -3940,7 +3942,9 @@ xenDaemonDetachDevice(virDomainPtr domai
                                      NULL)))
         goto cleanup;
 
-    if (!(dev = virDomainDeviceDefParse(domain->conn, def, xml)))
+    if (!(dev = virDomainDeviceDefParse(domain->conn,
+                                        priv->caps,
+                                        def, xml)))
         goto cleanup;
 
     if (virDomainXMLDevID(domain, dev, class, ref, sizeof(ref)))
diff -r c928fca9ce2b src/xm_internal.c
--- a/src/xm_internal.c	Tue Oct 14 11:16:52 2008 +0100
+++ b/src/xm_internal.c	Tue Oct 14 12:30:37 2008 +0100
@@ -2422,12 +2422,16 @@ xenXMDomainAttachDevice(virDomainPtr dom
     int ret = -1;
     virDomainDeviceDefPtr dev = NULL;
     virDomainDefPtr def;
+    xenUnifiedPrivatePtr priv;
 
     if ((!domain) || (!domain->conn) || (!domain->name) || (!xml)) {
         xenXMError((domain ? domain->conn : NULL), VIR_ERR_INVALID_ARG,
                    __FUNCTION__);
         return -1;
     }
+
+    priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
+
     if (domain->conn->flags & VIR_CONNECT_RO)
         return -1;
     if (domain->id != -1)
@@ -2440,6 +2444,7 @@ xenXMDomainAttachDevice(virDomainPtr dom
     def = entry->def;
 
     if (!(dev = virDomainDeviceDefParse(domain->conn,
+                                        priv->caps,
                                         entry->def,
                                         xml)))
         return -1;
@@ -2489,28 +2494,6 @@ xenXMDomainAttachDevice(virDomainPtr dom
     return ret;
 }
 
-
-/**
- * xenXMAutoAssignMac:
- * @mac: pointer to Mac String
- *
- * a mac is assigned automatically.
- *
- * Returns 0 in case of success, -1 in case of failure.
- */
-char *
-xenXMAutoAssignMac() {
-    char *buf;
-
-    if (VIR_ALLOC_N(buf, 18) < 0)
-        return 0;
-    srand((unsigned)time(NULL));
-    sprintf(buf, "00:16:3e:%02x:%02x:%02x"
-            ,1 + (int)(256*(rand()/(RAND_MAX+1.0)))
-            ,1 + (int)(256*(rand()/(RAND_MAX+1.0)))
-            ,1 + (int)(256*(rand()/(RAND_MAX+1.0))));
-    return buf;
-}
 
 /**
  * xenXMDomainDetachDevice:
@@ -2529,12 +2512,16 @@ xenXMDomainDetachDevice(virDomainPtr dom
     virDomainDefPtr def;
     int ret = -1;
     int i;
+    xenUnifiedPrivatePtr priv;
 
     if ((!domain) || (!domain->conn) || (!domain->name) || (!xml)) {
         xenXMError((domain ? domain->conn : NULL), VIR_ERR_INVALID_ARG,
                    __FUNCTION__);
         return -1;
     }
+
+    priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
+
     if (domain->conn->flags & VIR_CONNECT_RO)
         return -1;
     if (domain->id != -1)
@@ -2546,6 +2533,7 @@ xenXMDomainDetachDevice(virDomainPtr dom
     def = entry->def;
 
     if (!(dev = virDomainDeviceDefParse(domain->conn,
+                                        priv->caps,
                                         entry->def,
                                         xml)))
         return -1;
@@ -2573,7 +2561,7 @@ xenXMDomainDetachDevice(virDomainPtr dom
         for (i = 0 ; i < def->nnets ; i++) {
             if (!memcmp(def->nets[i]->mac,
                         dev->data.net->mac,
-                        VIR_DOMAIN_NET_MAC_SIZE)) {
+                        sizeof(def->nets[i]->mac))) {
                 virDomainNetDefFree(def->nets[i]);
                 if (i < (def->nnets - 1))
                     memmove(def->nets + i,

-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]