Re: [PATCH] Reordering how tap is initialized
Stephane Bakhos wrote: Stephane Bakhos wrote: This is my first patch, so I apoligize for breaking any convention. This patch modifies the order used in net.c for tap initialization. It runs the script before the device is opened. This will break existing scripts that do not rely on explicitly setting ifname= and instead rely on tap_open() to allocate a tap device. In fact, this is one of the most common usages. What about adding create and destroy scripts that are executed before tap_open / tap_close? Right now, the scripts serve an important purpose. The run after we allocate a tap device but before the guest runs. They serve as a hook in a very specific place in time. The semantics you describe are basically, run a script some time before we open the tap device. Well, that's effectively equivalent to just running the script before running QEMU. So I don't really see any compelling reason to introduce such a hook in QEMU today since you can already achieve this functionality. Regards, Anthony Liguori -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Reordering how tap is initialized
Stephane Bakhos wrote: This is my first patch, so I apoligize for breaking any convention. This patch modifies the order used in net.c for tap initialization. It runs the script before the device is opened. This will break existing scripts that do not rely on explicitly setting ifname= and instead rely on tap_open() to allocate a tap device. In fact, this is one of the most common usages. What about adding create and destroy scripts that are executed before tap_open / tap_close? It would achieve my goals of having 2 taps per vm and knowing where the tap have to connect to and it wouldn't break any existing functionality. The 2 problems that I have with the current auto allocation is that I cannot have a certain way of connecting the right tap to the right bridge. The other problem is that it requires qemu to start as root, which I'd rather not do. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Reordering how tap is initialized
Stephane Bakhos wrote: This is my first patch, so I apoligize for breaking any convention. This patch modifies the order used in net.c for tap initialization. It runs the script before the device is opened. This will break existing scripts that do not rely on explicitly setting ifname= and instead rely on tap_open() to allocate a tap device. In fact, this is one of the most common usages. Regards, Anthony Liguori -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Reordering how tap is initialized
This is my first patch, so I apoligize for breaking any convention.
This patch modifies the order used in net.c for tap initialization.
It runs the script before the device is opened. This permits the creation
of the tap device via a script instead of having to do it in advance.
It also waits for the tap to be closed before running the down script, as
this permits the destruction of the tap device afterwards.
It applies to git and kvm-88diff --git a/net.c b/net.c
index 1e845cf..afbb0f2 100644
--- a/net.c
+++ b/net.c
@@ -1295,7 +1295,7 @@ typedef struct TAPState {
unsigned int using_vnet_hdr : 1;
} TAPState;
-static int launch_script(const char *setup_script, const char *ifname, int fd);
+static int launch_script(const char *setup_script, const char *ifname);
static int tap_can_send(void *opaque);
static void tap_send(void *opaque);
@@ -1557,12 +1557,13 @@ static void tap_cleanup(VLANClientState *vc)
qemu_purge_queued_packets(vc);
-if (s->down_script[0])
-launch_script(s->down_script, s->down_script_arg, s->fd);
-
tap_read_poll(s, 0);
tap_write_poll(s, 0);
close(s->fd);
+
+if (s->down_script[0])
+launch_script(s->down_script, s->down_script_arg);
+
qemu_free(s);
}
@@ -1794,7 +1795,7 @@ static int tap_open(char *ifname, int ifname_size, int
*vnet_hdr)
}
#endif
-static int launch_script(const char *setup_script, const char *ifname, int fd)
+static int launch_script(const char *setup_script, const char *ifname)
{
sigset_t oldmask, mask;
int pid, status;
@@ -1813,8 +1814,7 @@ static int launch_script(const char *setup_script, const
char *ifname, int fd)
for (i = 0; i < open_max; i++) {
if (i != STDIN_FILENO &&
i != STDOUT_FILENO &&
-i != STDERR_FILENO &&
-i != fd) {
+i != STDERR_FILENO) {
close(i);
}
}
@@ -1852,16 +1852,18 @@ static TAPState *net_tap_init(VLANState *vlan, const
char *model,
else
ifname[0] = '\0';
vnet_hdr = 0;
-TFR(fd = tap_open(ifname, sizeof(ifname), &vnet_hdr));
-if (fd < 0)
-return NULL;
if (!setup_script || !strcmp(setup_script, "no"))
setup_script = "";
if (setup_script[0] != '\0' &&
-launch_script(setup_script, ifname, fd)) {
+launch_script(setup_script, ifname)) {
return NULL;
}
+
+TFR(fd = tap_open(ifname, sizeof(ifname), &vnet_hdr));
+if (fd < 0)
+return NULL;
+
s = net_tap_fd_init(vlan, model, name, fd, vnet_hdr);
snprintf(s->vc->info_str, sizeof(s->vc->info_str),
"ifname=%s,script=%s,downscript=%s",
