Re: [Qemu-devel] [PATCH COLO-Frame v8 23/34] tap: Make launch_script() public

[zhanghailiang]

On 2015/7/30 11:32, Jason Wang wrote:

On 07/29/2015 05:43 PM, zhanghailiang wrote:

On 2015/7/29 17:24, Jason Wang wrote:

On 07/29/2015 05:17 PM, zhanghailiang wrote:

On 2015/7/29 16:57, Jason Wang wrote:

On 07/29/2015 04:45 PM, zhanghailiang wrote:

We also change the parameters of launch_script().

A quick question (I don't go through the codes tough). What's the plan
for management(libvirt)? I believe some setup (iptables, fd creation)
should be offloaded to management (libvirt)?

Er, yes, the better way for setup is in libvirt, we didn't look into it
deeply, but it was in our TODO list before, since our first step is to
merge colo's qemu part,
if we realize colo proxy in qemu, it seems to be more simple than this
inconvenient way.

Please consider this as early as possible. The issue is probably not
convenience but security. Running qemu as root is dangerous, that's why
most of the setup was done through management.

Agreed, but if we totally convert proxy to userspace, we will not use
this setup way (Using
iptables command), it will be no problem, is it?

Confused, at least patch 24 has bash script to configure host iptables
and tcs?

This patch series is still based on kernel colo proxy.
Actually, i just want someone helping review the frame part except the net 
related part (patch 21 ~ patch 28).
So please ignore them and it will be dropped after colo proxy been implemented 
in qemu. Sorry for the noise.

Thanks.

Thanks.

Thanks

Cc: Stefan Hajnoczi <stefa...@redhat.com>
Cc: Jason Wang <jasow...@redhat.com>
Signed-off-by: zhanghailiang <zhang.zhanghaili...@huawei.com>
Signed-off-by: Li Zhijian <lizhij...@cn.fujitsu.com>
---
    include/net/tap.h |  2 ++
    net/tap.c         | 31 ++++++++++++++++++-------------
    2 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/include/net/tap.h b/include/net/tap.h
index 5da4edc..ac99b31 100644
--- a/include/net/tap.h
+++ b/include/net/tap.h
@@ -38,4 +38,6 @@ int tap_get_fd(NetClientState *nc);
    struct vhost_net;
    struct vhost_net *tap_get_vhost_net(NetClientState *nc);

+void launch_script(char *const args[], int fd, Error **errp);
+
    #endif /* QEMU_NET_TAP_H */
diff --git a/net/tap.c b/net/tap.c
index c2135cd..a715636 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -60,9 +60,6 @@ typedef struct TAPState {
        unsigned host_vnet_hdr_len;
    } TAPState;

-static void launch_script(const char *setup_script, const char
*ifname,
-                          int fd, Error **errp);
-
    static void tap_send(void *opaque);
    static void tap_writable(void *opaque);

@@ -305,7 +302,14 @@ static void tap_cleanup(NetClientState *nc)
        qemu_purge_queued_packets(nc);

        if (s->down_script[0]) {
-        launch_script(s->down_script, s->down_script_arg, s->fd,
&err);
+        char *args[3];
+        char **parg;
+
+        parg = args;
+        *parg++ = (char *)s->down_script;
+        *parg++ = (char *)s->down_script_arg;
+        *parg = NULL;
+        launch_script(args, s->fd, &err);
            if (err) {
                error_report_err(err);
            }
@@ -382,12 +386,10 @@ static TAPState
*net_tap_fd_init(NetClientState *peer,
        return s;
    }

-static void launch_script(const char *setup_script, const char
*ifname,
-                          int fd, Error **errp)
+void launch_script(char *const args[], int fd, Error **errp)
    {
        int pid, status;
-    char *args[3];
-    char **parg;
+    const char *setup_script = args[0];

        /* try to launch network script */
        pid = fork();
@@ -404,10 +406,6 @@ static void launch_script(const char
*setup_script, const char *ifname,
                    close(i);
                }
            }
-        parg = args;
-        *parg++ = (char *)setup_script;
-        *parg++ = (char *)ifname;
-        *parg = NULL;
            execv(setup_script, args);
            _exit(1);
        } else {
@@ -611,7 +609,14 @@ static int net_tap_init(const NetdevTapOptions
*tap, int *vnet_hdr,
        if (setup_script &&
            setup_script[0] != '\0' &&
            strcmp(setup_script, "no") != 0) {
-        launch_script(setup_script, ifname, fd, &err);
+        char *args[3];
+        char **parg;
+        parg = args;
+        *parg++ = (char *)setup_script;
+        *parg++ = (char *)ifname;
+        *parg = NULL;
+
+        launch_script(args, fd, &err);
            if (err) {
                error_propagate(errp, err);
                close(fd);

.

.

.