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 *)¶ms[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 :| -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list