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

[zhanghailiang <zhang.zhanghailiang@huawei.com>]

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 <stefanha@redhat.com>
>>>>>> Cc: Jason Wang <jasowang@redhat.com>
>>>>>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>>>>>> Signed-off-by: Li Zhijian <lizhijian@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);
>>>>>
>>>>>
>>>>> .
>>>>>
>>>>
>>>>
>>>
>>>
>>> .
>>>
>>
>>
>>
>
>
> .
>