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

[Jason Wang]

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?

>
> Thanks.
>
>>>> Thanks
>>>>
>>>>> Cc: Stefan Hajnoczi <address@hidden>
>>>>> Cc: Jason Wang <address@hidden>
>>>>> Signed-off-by: zhanghailiang <address@hidden>
>>>>> Signed-off-by: Li Zhijian <address@hidden>
>>>>> ---
>>>>>    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);
>>>>
>>>>
>>>> .
>>>>
>>>
>>>
>>
>>
>> .
>>
>
>
>