From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45694) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZKf3Z-0001aF-UL for qemu-devel@nongnu.org; Thu, 30 Jul 2015 00:02:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZKf3V-0001po-Jb for qemu-devel@nongnu.org; Thu, 30 Jul 2015 00:02:57 -0400 Received: from szxga01-in.huawei.com ([58.251.152.64]:60076) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZKf3U-0001pb-Gs for qemu-devel@nongnu.org; Thu, 30 Jul 2015 00:02:53 -0400 References: <1438159544-6224-1-git-send-email-zhang.zhanghailiang@huawei.com> <1438159544-6224-24-git-send-email-zhang.zhanghailiang@huawei.com> <55B8958D.8000809@redhat.com> <55B89A2D.9040908@huawei.com> <55B89BBC.1020907@redhat.com> <55B8A045.2030709@huawei.com> <55B99AB0.6080501@redhat.com> From: zhanghailiang Message-ID: <55B9A1D4.2050701@huawei.com> Date: Thu, 30 Jul 2015 12:02:28 +0800 MIME-Version: 1.0 In-Reply-To: <55B99AB0.6080501@redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH COLO-Frame v8 23/34] tap: Make launch_script() public List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Wang , qemu-devel@nongnu.org Cc: lizhijian@cn.fujitsu.com, quintela@redhat.com, yunhong.jiang@intel.com, eddie.dong@intel.com, dgilbert@redhat.com, peter.huangpeng@huawei.com, arei.gonglei@huawei.com, Stefan Hajnoczi , amit.shah@redhat.com, "yanghy@cn.fujitsu.com >> Hongyang Yang" 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 >>>>>> Cc: Jason Wang >>>>>> Signed-off-by: zhanghailiang >>>>>> Signed-off-by: Li Zhijian >>>>>> --- >>>>>> 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); >>>>> >>>>> >>>>> . >>>>> >>>> >>>> >>> >>> >>> . >>> >> >> >> > > > . >