Re: [PATCHv3 4/5] netdevveth: Simplify virNetDevVethCreate by using virNetDevGenerateName

Simplify virNetDevVethCreate by using common GenerateName/ReserveName functions. Signed-off-by: Shi Lei <shi_lei(a)massclouds.com&gt; --- src/lxc/lxc_process.c | 3 + src/util/virnetdevveth.c | 140 +++++++++------------------------------ 2 files changed, 36 insertions(+), 107 deletions(-) diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index eb29431e..2e8ae706 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -307,6 +307,9 @@ virLXCProcessSetupInterfaceTap(virDomainDefPtr vm, VIR_DEBUG("calling vethCreate()"); parentVeth = net->ifname; + if (parentVeth) + virNetDevReserveName(parentVeth); +

if (virNetDevVethCreate(&parentVeth, &containerVeth) < 0) return NULL; VIR_DEBUG("parentVeth: %s, containerVeth: %s", parentVeth, containerVeth); diff --git a/src/util/virnetdevveth.c b/src/util/virnetdevveth.c index b3eee1af..d6932a2e 100644 --- a/src/util/virnetdevveth.c +++ b/src/util/virnetdevveth.c @@ -32,48 +32,6 @@ VIR_LOG_INIT("util.netdevveth"); -/* Functions */ - -virMutex virNetDevVethCreateMutex = VIR_MUTEX_INITIALIZER; - -static int virNetDevVethExists(int devNum) -{ - int ret; - g_autofree char *path = NULL; - - path = g_strdup_printf(SYSFS_NET_DIR "vnet%d/", devNum); - ret = virFileExists(path) ? 1 : 0; - VIR_DEBUG("Checked dev vnet%d usage: %d", devNum, ret); - return ret; -} - -/** - * virNetDevVethGetFreeNum: - * @startDev: device number to start at (x in vethx) - * - * Looks in /sys/class/net/ to find the first available veth device - * name. - * - * Returns non-negative device number on success or -1 in case of error - */ -static int virNetDevVethGetFreeNum(int startDev) -{ - int devNum; - -#define MAX_DEV_NUM 65536 - - for (devNum = startDev; devNum < MAX_DEV_NUM; devNum++) { - int ret = virNetDevVethExists(devNum); - if (ret < 0) - return -1; - if (ret == 0) - return devNum; - } - - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("No free veth devices available")); - return -1; -} /** * virNetDevVethCreate: @@ -102,77 +60,45 @@ static int virNetDevVethGetFreeNum(int startDev) */ int virNetDevVethCreate(char** veth1, char** veth2) { - int ret = -1; - int vethNum = 0; - size_t i; - - /* - * We might race with other containers, but this is reasonably - * unlikely, so don't do too many retries for device creation - */ - virMutexLock(&virNetDevVethCreateMutex); -#define MAX_VETH_RETRIES 10 - - for (i = 0; i < MAX_VETH_RETRIES; i++) { - g_autofree char *veth1auto = NULL; - g_autofree char *veth2auto = NULL; - g_autoptr(virCommand) cmd = NULL; - - int status; - if (!*veth1) { - int veth1num; - if ((veth1num = virNetDevVethGetFreeNum(vethNum)) < 0) - goto cleanup; - - veth1auto = g_strdup_printf("vnet%d", veth1num); - vethNum = veth1num + 1; - } - if (!*veth2) { - int veth2num; - if ((veth2num = virNetDevVethGetFreeNum(vethNum)) < 0) - goto cleanup; + int status; + g_autofree char *veth1auto = NULL; + g_autofree char *veth2auto = NULL; + g_autoptr(virCommand) cmd = NULL; - veth2auto = g_strdup_printf("vnet%d", veth2num); - vethNum = veth2num + 1; - } > + if (!*veth1) { + if (virNetDevGenerateName(&veth1auto, VIR_NET_DEV_GEN_NAME_VNET) < 0) + return -1; + } > + if (!*veth2) { + if (virNetDevGenerateName(&veth2auto, VIR_NET_DEV_GEN_NAME_VNET) < 0) + return -1; + }

- cmd = virCommandNew("ip"); - virCommandAddArgList(cmd, "link", "add", - *veth1 ? *veth1 : veth1auto, - "type", "veth", "peer", "name", - *veth2 ? *veth2 : veth2auto, - NULL); - - if (virCommandRun(cmd, &status) < 0) - goto cleanup; - - if (status == 0) { - if (veth1auto) { - *veth1 = veth1auto; - veth1auto = NULL; - } - if (veth2auto) { - *veth2 = veth2auto; - veth2auto = NULL; - } - VIR_DEBUG("Create Host: %s guest: %s", *veth1, *veth2); - ret = 0; - goto cleanup; - } + cmd = virCommandNew("ip"); + virCommandAddArgList(cmd, "link", "add", + *veth1 ? *veth1 : veth1auto, + "type", "veth", "peer", "name", + *veth2 ? *veth2 : veth2auto, + NULL); - VIR_DEBUG("Failed to create veth host: %s guest: %s: %d", - *veth1 ? *veth1 : veth1auto, - *veth2 ? *veth2 : veth2auto, - status); + if (virCommandRun(cmd, &status) < 0 || status) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Failed to allocate free veth pair")); + return -1; } - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to allocate free veth pair after %d attempts"), - MAX_VETH_RETRIES); + VIR_DEBUG("create veth host: %s guest: %s: %d", + *veth1 ? *veth1 : veth1auto, + *veth2 ? *veth2 : veth2auto, + status); + + if (veth1auto) + *veth1 = g_steal_pointer(&veth1auto); + if (veth2auto) + *veth2 = g_steal_pointer(&veth2auto); - cleanup: - virMutexUnlock(&virNetDevVethCreateMutex); - return ret; + VIR_DEBUG("Create Host: %s guest: %s", *veth1, *veth2); + return 0; } /**