Re: [libvirt] [PATCH v2]: set and restore MAC address of a NIC when using PASSTHROUGH mode

On Thu, Jun 16, 2011 at 02:23:07PM +0200, Gerhard Stenzel wrote: > Index: libvirt/src/util/macvtap.c > =================================================================== > --- libvirt.orig/src/util/macvtap.c > +++ libvirt/src/util/macvtap.c > @@ -87,6 +87,7 @@ > > # define LLDPAD_PID_FILE "/var/run/lldpad.pid" > > +#define MACADDRSIZE 6 > > enum virVirtualPortOp { > ASSOCIATE = 0x1, > @@ -191,6 +192,149 @@ err_exit: > > # if WITH_MACVTAP > > +/** > + * getMacaddr: > + * Get the MAC address of a network device > + * > + * @macaddress: Pointer where the MAC address will be stored > + * @srcdev: The interface name of the NIC to get the MAC from > + * > + * Returns zero in case of success, > + * negative value otherwise with error reported. > + * > + */ > +int > +getMacaddr(const unsigned char *macaddress, const char *srcdev ) > +{ > + int sockfd; > + int io; > + struct ifreq ifr; > + > + strcpy(ifr.ifr_name, srcdev); > + > + sockfd = socket(AF_INET, SOCK_STREAM, 0); > + if(sockfd< 0){ > + return -1; > + } > + > + io = ioctl(sockfd, SIOCGIFHWADDR, (char *)&ifr); > + if(io< 0){ > + return -1; > + } > + > + memcpy(macaddress, ifr.ifr_ifru.ifru_hwaddr.sa_data, MACADDRSIZE); > + > + return 0; > +} > + > +/** > + * setMacaddr: > + * Set the MAC address of a network device > + * > + * @macaddress: MAC address to assign to the NIC > + * @srcdev: The interface name of the NIC > + * > + * Returns zero in case of success, > + * negative value otherwise with error reported. > + * > + */ > +int > +setMacaddr(const unsigned char *macaddress, const char *srcdev ) > +{ > + int rc = 0; > + struct nlmsghdr *resp; > + struct nlmsgerr *err; > + struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC }; > + int ifindex; > + unsigned char *recvbuf = NULL; > + unsigned int recvbuflen; > + struct nl_msg *nl_msg; > + > + if (ifaceGetIndex(true, srcdev,&ifindex) != 0) > + return -1; > + > + nl_msg = nlmsg_alloc_simple(RTM_SETLINK, NLM_F_REQUEST); > + > + if (!nl_msg) { > + virReportOOMError(); > + return -1; > + } > + > + if (nlmsg_append(nl_msg,&ifinfo, sizeof(ifinfo), NLMSG_ALIGNTO)< 0) > + goto buffer_too_small; > + > + if (nla_put_u32(nl_msg, IFLA_LINK, ifindex)< 0) > + goto buffer_too_small; > + > + if (nla_put(nl_msg, IFLA_ADDRESS, MACADDRSIZE, macaddress)< 0) > + goto buffer_too_small; > + > + if (srcdev&& > + nla_put(nl_msg, IFLA_IFNAME, strlen(srcdev)+1, srcdev)< 0) > + goto buffer_too_small; > + > + if (nlComm(nl_msg,&recvbuf,&recvbuflen, 0)< 0) { > + rc = -1; > + goto err_exit; > + } > + > + if (recvbuflen< NLMSG_LENGTH(0) || recvbuf == NULL) > + goto malformed_resp; > + > + resp = (struct nlmsghdr *)recvbuf; > + > + switch (resp->nlmsg_type) { > + case NLMSG_ERROR: > + err = (struct nlmsgerr *)NLMSG_DATA(resp); > + if (resp->nlmsg_len< NLMSG_LENGTH(sizeof(*err))) > + goto malformed_resp; > + > + switch (err->error) { > + > + case 0: > + break; > + > + case -EEXIST: > + rc = -1; > + break; > + > + default: > + macvtapError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("error setting device mac address")); > + rc = -1; > + } > + break; > + > + case NLMSG_DONE: > + break; > + > + default: > + goto malformed_resp; > + } > + > +err_exit: > + nlmsg_free(nl_msg); > + > + VIR_FREE(recvbuf); > + > + return rc; > + > +malformed_resp: > + nlmsg_free(nl_msg); > + > + macvtapError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("malformed netlink response message")); > + VIR_FREE(recvbuf); > + return -1; > + > +buffer_too_small: > + nlmsg_free(nl_msg); > + > + macvtapError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("allocated netlink buffer is too small")); > + return -1; > +} There are already (static) functions in src/util/bridge.c, which are able to get and set the MAC address on network devices. So I think we should just make them non-static and call them instead of duplicating it here.