| Message ID | 1366187964-14265-5-git-send-email-qemulist@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Apr 17, 2013 at 04:39:13PM +0800, Liu Ping Fan wrote: > From: Liu Ping Fan <pingfank@linux.vnet.ibm.com> > > When vhost net enabled, we should be sure that the user space > fd handler is not in flight > > Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com> > --- > net/tap.c | 5 +++++ > 1 files changed, 5 insertions(+), 0 deletions(-) > > diff --git a/net/tap.c b/net/tap.c > index 35cbb6e..b5629e3 100644 > --- a/net/tap.c > +++ b/net/tap.c > @@ -41,6 +41,7 @@ > #include "qemu/error-report.h" > > #include "net/tap.h" > +#include "util/event_gsource.h" > > #include "hw/vhost_net.h" > > @@ -327,6 +328,10 @@ static void tap_poll(NetClientState *nc, bool enable) > tap_read_poll(s, enable); > tap_write_poll(s, enable); > > + if (!enable) { > + /* need sync so vhost can take over polling */ > + g_source_remove_poll(&s->nsrc->source, &s->nsrc->gfd); > + } We must also re-enable it when .poll(nc, true) is called. Please drop the comments the previous patch added. In fact, this patch can be squashed. I think there was another vhost sync comment which you haven't addressed here. See the previous patch. Stefan
On Thu, Apr 18, 2013 at 10:11 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote: > On Wed, Apr 17, 2013 at 04:39:13PM +0800, Liu Ping Fan wrote: >> From: Liu Ping Fan <pingfank@linux.vnet.ibm.com> >> >> When vhost net enabled, we should be sure that the user space >> fd handler is not in flight >> >> Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com> >> --- >> net/tap.c | 5 +++++ >> 1 files changed, 5 insertions(+), 0 deletions(-) >> >> diff --git a/net/tap.c b/net/tap.c >> index 35cbb6e..b5629e3 100644 >> --- a/net/tap.c >> +++ b/net/tap.c >> @@ -41,6 +41,7 @@ >> #include "qemu/error-report.h" >> >> #include "net/tap.h" >> +#include "util/event_gsource.h" >> >> #include "hw/vhost_net.h" >> >> @@ -327,6 +328,10 @@ static void tap_poll(NetClientState *nc, bool enable) >> tap_read_poll(s, enable); >> tap_write_poll(s, enable); >> >> + if (!enable) { >> + /* need sync so vhost can take over polling */ >> + g_source_remove_poll(&s->nsrc->source, &s->nsrc->gfd); >> + } > > We must also re-enable it when .poll(nc, true) is called. > Yes, will fix it. > Please drop the comments the previous patch added. In fact, this patch > can be squashed. > Ok. > I think there was another vhost sync comment which you haven't addressed > here. See the previous patch. > It is related with multi-queue, and I check the code, it can be async. I will drop the related comment in previous patch Thanks, Pingfan > Stefan
diff --git a/net/tap.c b/net/tap.c index 35cbb6e..b5629e3 100644 --- a/net/tap.c +++ b/net/tap.c @@ -41,6 +41,7 @@ #include "qemu/error-report.h" #include "net/tap.h" +#include "util/event_gsource.h" #include "hw/vhost_net.h" @@ -327,6 +328,10 @@ static void tap_poll(NetClientState *nc, bool enable) tap_read_poll(s, enable); tap_write_poll(s, enable); + if (!enable) { + /* need sync so vhost can take over polling */ + g_source_remove_poll(&s->nsrc->source, &s->nsrc->gfd); + } } int tap_get_fd(NetClientState *nc)