[libvirt] [PATCH] macvtap teardown rework
Daniel P. Berrange
berrange at redhat.com
Thu Feb 18 12:53:28 UTC 2010
On Wed, Feb 17, 2010 at 03:05:53PM -0500, Stefan Berger wrote:
> I have reworked and simplified the teardown of the macvtap device.
> Basically all devices with the same MAC address and link device are kept
> alive and not attempted to be torn down. If a macvtap device linked to a
> physical interface with a certain MAC address 'M' is to be created it
> will automatically fail if the interface is 'up'ed and another macvtap
> with the same properties (MAC addr 'M', link dev) happens to be 'up'.
> This will prevent the VM from starting or the device from being attached
> to a running VM. Stale interfaces are assumed to be there for some
> reason and not stem from libvirt.
>
> In the VM shutdown path I am assuming that an interface name is always
> available so that if the device type is DIRECT it can be torn down using
> its name.
>
> Signed-off-by: Stefan Berger <stefanb at us.ibm.com>
>
> Index: libvirt-macvtap/src/util/macvtap.h
> ===================================================================
> --- libvirt-macvtap.orig/src/util/macvtap.h
> +++ libvirt-macvtap/src/util/macvtap.h
> @@ -35,8 +35,7 @@ int openMacvtapTap(virConnectPtr conn,
> int mode,
> char **res_ifname);
>
> -int delMacvtapByMACAddress(const unsigned char *macaddress,
> - int *hasBusyDev);
> +void delMacvtap(const char *name);
>
> #endif /* WITH_MACVTAP */
>
> Index: libvirt-macvtap/src/util/macvtap.c
> ===================================================================
> --- libvirt-macvtap.orig/src/util/macvtap.c
> +++ libvirt-macvtap/src/util/macvtap.c
> @@ -447,15 +447,14 @@ buffer_too_small:
>
>
> static int
> -link_del(const char *type,
> - const char *name)
> +link_del(const char *name)
> {
> int rc = 0;
> char nlmsgbuf[256];
> struct nlmsghdr *nlm = (struct nlmsghdr *)nlmsgbuf, *resp;
> struct nlmsgerr *err;
> char rtattbuf[64];
> - struct rtattr *rta, *rta1;
> + struct rtattr *rta;
> struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC };
> char *recvbuf = NULL;
> int recvbuflen;
> @@ -467,23 +466,6 @@ link_del(const char *type,
> if (!nlAppend(nlm, sizeof(nlmsgbuf), &ifinfo, sizeof(ifinfo)))
> goto buffer_too_small;
>
> - rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_LINKINFO, NULL, 0);
> - if (!rta)
> - goto buffer_too_small;
> -
> - if (!(rta1 = nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len)))
> - goto buffer_too_small;
> -
> - rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_INFO_KIND,
> - type, strlen(type));
> - if (!rta)
> - goto buffer_too_small;
> -
> - if (!nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))
> - goto buffer_too_small;
> -
> - rta1->rta_len = (char *)nlm + nlm->nlmsg_len - (char *)rta1;
> -
> rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_IFNAME,
> name, strlen(name)+1);
> if (!rta)
> @@ -633,7 +615,8 @@ macvtapModeFromInt(enum virDomainNetdevM
> }
>
>
> -/* openMacvtapTap:
> +/**
> + * openMacvtapTap:
> * Create an instance of a macvtap device and open its tap character
> * device.
> * @conn: Pointer to virConnect object
> @@ -707,14 +690,17 @@ create_name:
> rc = ifUp(cr_ifname, 1);
> if (rc != 0) {
> virReportSystemError(errno,
> - _("cannot 'up' interface %s"), cr_ifname);
> + _("cannot 'up' interface %s -- another "
> + "macvtap device may be 'up' and have the same "
> + "MAC address"),
> + cr_ifname);
> rc = -1;
> goto link_del_exit;
> }
>
> rc = openTap(cr_ifname, 10);
>
> - if (rc > 0)
> + if (rc >= 0)
> *res_ifname = strdup(cr_ifname);
> else
> goto link_del_exit;
> @@ -722,79 +708,22 @@ create_name:
> return rc;
>
> link_del_exit:
> - link_del(type, ifname);
> + link_del(cr_ifname);
>
> return rc;
> }
>
>
> -/* Delete a macvtap type of interface given the MAC address. This
> - * function will delete all macvtap type of interfaces that have the
> - * given MAC address.
> - * @macaddress : Pointer to 6 bytes holding the MAC address of the
> - * macvtap device(s) to destroy
> +/**
> + * delMacvtapByName:
> + * @ifname : The name of the macvtap interface
> *
> - * Returns 0 in case of success, negative value in case of error.
> + * Delete an interface given its name.
> */
> -int
> -delMacvtapByMACAddress(const unsigned char *macaddress,
> - int *hasBusyDev)
> +void
> +delMacvtap(const char *ifname)
> {
> - struct ifreq ifr;
> - FILE *file;
> - char *ifname, *pos;
> - char buffer[1024];
> - off_t oldpos = 0;
> - int tapfd;
> -
> - *hasBusyDev = 0;
> -
> - file = fopen("/proc/net/dev", "r");
> -
> - if (!file) {
> - virReportSystemError(errno, "%s",
> - _("cannot open file to read network interfaces "
> - "from"));
> - return -1;
> - }
> -
> - int sock = socket(AF_INET, SOCK_DGRAM, 0);
> - if (sock < 0) {
> - virReportSystemError(errno, "%s",
> - _("cannot open socket"));
> - goto sock_err;
> - }
> -
> - while (NULL != (ifname = fgets(buffer, sizeof(buffer), file))) {
> - if (c_isspace(ifname[0]))
> - ifname++;
> - if ((pos = strchr(ifname, ':')) != NULL) {
> - pos[0] = 0;
> - if (virStrncpy(ifr.ifr_name, ifname, strlen(ifname),
> - sizeof(ifr.ifr_name)) == NULL)
> - continue;
> - if (ioctl(sock, SIOCGIFHWADDR, (char *)&ifr) >= 0) {
> - if (memcmp(&ifr.ifr_hwaddr.sa_data[0], macaddress, 6) == 0) {
> - tapfd = openTap(ifname, 0);
> - if (tapfd > 0) {
> - close(tapfd);
> - ifUp(ifname, 0);
> - if (link_del("macvtap", ifname) == 0)
> - fseeko(file, oldpos, SEEK_SET);
> - } else {
> - *hasBusyDev = 1;
> - }
> - }
> - }
> - }
> - oldpos = ftello(file);
> - }
> -
> - close(sock);
> -sock_err:
> - fclose(file);
> -
> - return 0;
> + link_del(ifname);
> }
>
> #endif
> Index: libvirt-macvtap/src/qemu/qemu_driver.c
> ===================================================================
> --- libvirt-macvtap.orig/src/qemu/qemu_driver.c
> +++ libvirt-macvtap/src/qemu/qemu_driver.c
> @@ -2942,17 +2942,6 @@ static void qemudShutdownVMDaemon(struct
> }
> }
>
> -#if WITH_MACVTAP
> - def = vm->def;
> - for (i = 0; i < def->nnets; i++) {
> - virDomainNetDefPtr net = def->nets[i];
> - if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
> - int dummy;
> - delMacvtapByMACAddress(net->mac, &dummy);
> - }
> - }
> -#endif
> -
> if (virKillProcess(vm->pid, 0) == 0 &&
> virKillProcess(vm->pid, SIGTERM) < 0)
> virReportSystemError(errno,
> @@ -2999,6 +2988,17 @@ static void qemudShutdownVMDaemon(struct
>
> qemuDomainReAttachHostDevices(driver, vm->def);
>
> +#if WITH_MACVTAP
> + def = vm->def;
> + for (i = 0; i < def->nnets; i++) {
> + virDomainNetDefPtr net = def->nets[i];
> + if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
> + if (net->ifname)
> + delMacvtap(net->ifname);
> + }
> + }
> +#endif
> +
> retry:
> if ((ret = qemuRemoveCgroup(driver, vm, 0)) < 0) {
> if (ret == -EBUSY && (retries++ < 5)) {
> @@ -6347,8 +6347,8 @@ qemudDomainDetachNetDevice(struct qemud_
>
> #if WITH_MACVTAP
> if (detach->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
> - int dummy;
> - delMacvtapByMACAddress(detach->mac, &dummy);
> + if (detach->ifname)
> + delMacvtap(detach->ifname);
> }
> #endif
>
> Index: libvirt-macvtap/src/qemu/qemu_conf.c
> ===================================================================
> --- libvirt-macvtap.orig/src/qemu/qemu_conf.c
> +++ libvirt-macvtap/src/qemu/qemu_conf.c
> @@ -1439,15 +1439,6 @@ qemudPhysIfaceConnect(virConnectPtr conn
> int rc;
> #if WITH_MACVTAP
> char *res_ifname = NULL;
> - int hasBusyDev = 0;
> -
> - delMacvtapByMACAddress(net->mac, &hasBusyDev);
> -
> - if (hasBusyDev) {
> - virReportSystemError(errno, "%s",
> - _("A macvtap with the same MAC address is in use"));
> - return -1;
> - }
>
> rc = openMacvtapTap(conn, net->ifname, net->mac, linkdev, brmode,
> &res_ifname);
> @@ -1460,7 +1451,6 @@ qemudPhysIfaceConnect(virConnectPtr conn
> (void)net;
> (void)linkdev;
> (void)brmode;
> - (void)conn;
> qemuReportError(VIR_ERR_INTERNAL_ERROR,
> "%s", _("No support for macvtap device"));
> rc = -1;
> Index: libvirt-macvtap/src/libvirt_macvtap.syms
> ===================================================================
> --- libvirt-macvtap.orig/src/libvirt_macvtap.syms
> +++ libvirt-macvtap/src/libvirt_macvtap.syms
> @@ -2,4 +2,4 @@
>
> # macvtap.h
> openMacvtapTap;
> -delMacvtapByMACAddress;
> +delMacvtap;
ACK, this looks better
Daniel
--
|: 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 :|
More information about the libvir-list
mailing list