From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44645) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bQ9q3-0001r6-4h for qemu-devel@nongnu.org; Thu, 21 Jul 2016 05:00:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bQ9py-0007we-TL for qemu-devel@nongnu.org; Thu, 21 Jul 2016 05:00:14 -0400 Received: from mx1.redhat.com ([209.132.183.28]:41681) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bQ9py-0007wL-Lo for qemu-devel@nongnu.org; Thu, 21 Jul 2016 05:00:10 -0400 Date: Thu, 21 Jul 2016 10:00:06 +0100 From: "Daniel P. Berrange" Message-ID: <20160721090006.GB13528@redhat.com> Reply-To: "Daniel P. Berrange" References: <1469089573-20923-1-git-send-email-zhangchen.fnst@cn.fujitsu.com> <1469089573-20923-6-git-send-email-zhangchen.fnst@cn.fujitsu.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1469089573-20923-6-git-send-email-zhangchen.fnst@cn.fujitsu.com> Subject: Re: [Qemu-devel] [RFC PATCH V8 5/7] qemu-char: Add qemu_chr_add_handlers_full() for GMaincontext List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Zhang Chen Cc: qemu devel , Jason Wang , Li Zhijian , Wen Congyang , zhanghailiang , "eddie . dong" , "Dr . David Alan Gilbert" , Paolo Bonzini On Thu, Jul 21, 2016 at 04:26:11PM +0800, Zhang Chen wrote: > Add qemu_chr_add_handlers_full() API, we can use > this API pass in a GMainContext,make handler run > in the context rather than main_loop. > This comments from Daniel P . Berrange. > > Cc: Daniel P . Berrange > Cc: Paolo Bonzini > > Signed-off-by: Zhang Chen > Signed-off-by: Li Zhijian > Signed-off-by: Wen Congyang > --- > include/sysemu/char.h | 10 ++++ > qemu-char.c | 145 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 155 insertions(+) > > diff --git a/include/sysemu/char.h b/include/sysemu/char.h > index 307fd8f..3ab897e 100644 > --- a/include/sysemu/char.h > +++ b/include/sysemu/char.h > @@ -66,6 +66,8 @@ struct CharDriverState { > const uint8_t *buf, int len); > GSource *(*chr_add_watch)(struct CharDriverState *s, GIOCondition cond); > void (*chr_update_read_handler)(struct CharDriverState *s); I'd expect this to be deleted, and all existing impls given a new GMainContext * argument. > + void (*chr_update_read_handler_full)(struct CharDriverState *s, > + GMainContext *context); > int (*chr_ioctl)(struct CharDriverState *s, int cmd, void *arg); > int (*get_msgfds)(struct CharDriverState *s, int* fds, int num); > int (*set_msgfds)(struct CharDriverState *s, int *fds, int num); > @@ -388,6 +390,14 @@ void qemu_chr_add_handlers(CharDriverState *s, > IOEventHandler *fd_event, > void *opaque); > > +/* This API can make handler run in the context what you pass to. */ > +void qemu_chr_add_handlers_full(CharDriverState *s, > + IOCanReadHandler *fd_can_read, > + IOReadHandler *fd_read, > + IOEventHandler *fd_event, > + void *opaque, > + GMainContext *context); > + > void qemu_chr_be_generic_open(CharDriverState *s); > void qemu_chr_accept_input(CharDriverState *s); > int qemu_chr_add_client(CharDriverState *s, int fd); > diff --git a/qemu-char.c b/qemu-char.c > index b597ee1..607b3b8 100644 > --- a/qemu-char.c > +++ b/qemu-char.c > @@ -480,6 +480,40 @@ void qemu_chr_add_handlers(CharDriverState *s, > } > } > > +void qemu_chr_add_handlers_full(CharDriverState *s, > + IOCanReadHandler *fd_can_read, > + IOReadHandler *fd_read, > + IOEventHandler *fd_event, > + void *opaque, > + GMainContext *context) > +{ > + int fe_open; > + > + if (!opaque && !fd_can_read && !fd_read && !fd_event) { > + fe_open = 0; > + remove_fd_in_watch(s); > + } else { > + fe_open = 1; > + } > + s->chr_can_read = fd_can_read; > + s->chr_read = fd_read; > + s->chr_event = fd_event; > + s->handler_opaque = opaque; > + if (fe_open && s->chr_update_read_handler) { You should be checking s->chr_update_read_handler_full > + s->chr_update_read_handler_full(s, context); > + } > + > + if (!s->explicit_fe_open) { > + qemu_chr_fe_set_open(s, fe_open); > + } > + > + /* We're connecting to an already opened device, so let's make sure we > + also get the open event */ > + if (fe_open && s->be_open) { > + qemu_chr_be_generic_open(s); > + } > +} I would expect the entire of the qemu_chr_add_handlers() method body to be deleted and have it simply call qemu_chr_add_handlers_full(), passing in a NULL GMainContext. That way we avoid code duplication. 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 :|