Re: [libvirt] [PATCH] macvtap teardown rework
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 > Okay, applied, I just removed an unused header pointed out by syntax-check :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] macvtap teardown rework
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
>
> 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) {
> -virReport
Re: [libvirt] [PATCH] macvtap teardown rework
On Wed, Feb 17, 2010 at 12:05 PM, 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. This is a huge improvement. It makes the normal shutdown path nice and simple, and returns a clear error message instead of making an elaborate attempt to work around a prior unclean shutdown. Just one cosmetic nit: the "name" argument in the delMacvtap() declaration in macvtap.h should be "ifname" for consistency with the definition in macvtap.c. --Ed -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] macvtap teardown rework
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
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_
