| Message ID | 1438159544-6224-24-git-send-email-zhang.zhanghailiang@huawei.com |
|---|---|
| State | New |
| Headers | show |
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)? 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);
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. > 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); > > > . >
On Wed, Jul 29, 2015 at 04:57:49PM +0800, 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)? Yep, libvirt will run QEMU unprivileged, so this effectively blocks any use of QEMU executed scripts for managing host network setup, making this launch_script facility effectively useless. Regards, Daniel
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. 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); >> >> >> . >> > >
* Daniel P. Berrange (berrange@redhat.com) wrote: > On Wed, Jul 29, 2015 at 04:57:49PM +0800, 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)? > > Yep, libvirt will run QEMU unprivileged, so this effectively blocks any > use of QEMU executed scripts for managing host network setup, making > this launch_script facility effectively useless. Not useless; it's very useful to those getting it going without libvirt. Obviously it won't get used when libvirt is setup with it, but that's a separate step. Dave > > Regards, > Daniel > -- > |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| > |: http://libvirt.org -o- http://virt-manager.org :| > |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| > |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
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? 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); >>> >>> >>> . >>> >> >> > > > . >
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 <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); >>>> >>>> >>>> . >>>> >>> >>> >> >> >> . >> > > >
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); >>>>> >>>>> >>>>> . >>>>> >>>> >>>> >>> >>> >>> . >>> >> >> >> > > > . >
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);