On 14.04.2015 18:21, Lubomir Rintel wrote:

...and provide a bogus implementation for non-Linux. No functional change. We'd like to use it from within virNetDevSetNamespace() to determine if we need to move a 802.11 PHY to a namespace when moving the interface. --- src/util/virnetdev.c | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 9a6d4e7..ee60f09 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -114,6 +114,28 @@ struct _virNetDevMcastList { virNetDevMcastEntryPtr *entries; };

+#ifdef __linux__ +# define NET_SYSFS "/sys/class/net/" + +static int +virNetDevSysfsFile(char **pf_sysfs_device_link, const char *ifname, + const char *file) +{ + + if (virAsprintf(pf_sysfs_device_link, NET_SYSFS "%s/%s", ifname, file) < 0) + return -1; + return 0; +} +#else +static int +virNetDevSysfsFile(char **pf_sysfs_device_link, const char *ifname, + const char *file)

Not that I reviewed the whole patch set (maybe tomorrow), but these arguments are missing ATTRIBUTE_UNUSED. Just a small nit that can be fixed before pushing. No need for v2. ACK Michal

On 14.04.2015 18:21, Lubomir Rintel wrote:

The 802.11 interfaces can not be moved by themselves, their Phy has to move too.

If there are other interfaces, they have to move too -- hopefully it's not too confusing. This is a less-invasive alternative to defining a new hostdev type for PHYs. --- src/util/virnetdev.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)

diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index ee60f09..df48763 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -584,6 +584,8 @@ int virNetDevSetNamespace(const char *ifname, pid_t pidInNs) { int rc; char *pid = NULL; + char *phy_path = NULL; + const char *argv[] = { "ip", "link", "set", ifname, "netns", NULL, NULL }; @@ -591,6 +593,35 @@ int virNetDevSetNamespace(const char *ifname, pid_t pidInNs) if (virAsprintf(&pid, "%lld", (long long) pidInNs) == -1) return -1;

+ /* The 802.11 wireless devices only move together with their PHY. */ + if (virNetDevSysfsFile(&phy_path, ifname, "phy80211/name") >= 0) { + int len; + char *phy = NULL; + + len = virFileReadAllQuiet(phy_path, 1024, &phy); + VIR_FREE(phy_path); + + /* Remove a line break. */ + if (len > 0) + phy[len - 1] = '\0'; + + if (len >= 0) { + const char *iwargv[] = { + "iw", "phy", phy, "set", "netns", NULL, NULL + }; + + iwargv[5] = pid; + rc = virRun(iwargv, NULL); + + VIR_FREE(phy); + + if (rc == 0) { + VIR_FREE(pid); + return rc; + } + } + } + argv[5] = pid; rc = virRun(argv, NULL);

I understand what you mean, but the code style could be better. I'll polish and resend the patch in your name. Michal

There was a couple of problems with the style fixes applied to the original patch: 1.) virFileReadAllQuiet comparison was incorrectly parenthesized when moved into a condition, causing the len to be set to the result of comparison. This, together with the removed underflow check would underflow the phy buffer. 2.) The logic was broken. Failure to call "ip" would abort the function, thus the "iw" branch would never be reached. This aims to fix the issues and work around possible style complains :) Signed-off-by: Lubomir Rintel <lkundrak@v3.sk> --- src/util/virnetdev.c | 46 +++++++++++++++++++++------------------------- 1 file changed, 21 insertions(+), 25 deletions(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 98ce152..e4fcd81 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -478,40 +478,36 @@ int virNetDevSetNamespace(const char *ifname, pid_t pidInNs) char *phy_path = NULL; int len; - const char *argv[] = { - "ip", "link", "set", ifname, "netns", NULL, NULL - }; - - const char *iwargv[] = { - "iw", "phy", NULL, "set", "netns", NULL, NULL - }; - if (virAsprintf(&pid, "%lld", (long long) pidInNs) == -1) return -1; - argv[5] = pid; - if (virRun(argv, NULL) < 0) - goto cleanup; - /* The 802.11 wireless devices only move together with their PHY. */ if (virNetDevSysfsFile(&phy_path, ifname, "phy80211/name") < 0) goto cleanup; - if ((len = virFileReadAllQuiet(phy_path, 1024, &phy) < 0)) { - if (errno == ENOENT) { - /* Okay, this is not a wireless card. Claim success. */ - ret = 0; - } - goto cleanup; - } + if ((len = virFileReadAllQuiet(phy_path, 1024, &phy)) <= 0) { + /* Not a wireless device. */ + const char *argv[] = { + "ip", "link", "set", ifname, "netns", NULL, NULL + }; - /* Remove a line break. */ - phy[len - 1] = '\0'; + argv[5] = pid; + if (virRun(argv, NULL) < 0) + goto cleanup; - iwargv[2] = phy; - iwargv[5] = pid; - if (virRun(iwargv, NULL) < 0) - goto cleanup; + } else { + const char *argv[] = { + "iw", "phy", NULL, "set", "netns", NULL, NULL + }; + + /* Remove a line break. */ + phy[len - 1] = '\0'; + + argv[2] = phy; + argv[5] = pid; + if (virRun(argv, NULL) < 0) + goto cleanup; + } ret = 0; cleanup: -- 2.4.2