From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56123) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WTIHK-0006iw-2t for qemu-devel@nongnu.org; Thu, 27 Mar 2014 17:56:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WTIHE-0002AU-4M for qemu-devel@nongnu.org; Thu, 27 Mar 2014 17:56:02 -0400 Received: from mail-ee0-x235.google.com ([2a00:1450:4013:c00::235]:49633) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WTIHD-0002AO-PV for qemu-devel@nongnu.org; Thu, 27 Mar 2014 17:55:56 -0400 Received: by mail-ee0-f53.google.com with SMTP id b57so3374544eek.26 for ; Thu, 27 Mar 2014 14:55:54 -0700 (PDT) Date: Thu, 27 Mar 2014 22:55:46 +0100 From: Hani Benhabiles Message-ID: <20140327215546.GD22714@Inspiron-3521> References: <1394363777-14132-1-git-send-email-kroosec@gmail.com> <1394363777-14132-2-git-send-email-kroosec@gmail.com> <20140327162151.5aa28487@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20140327162151.5aa28487@redhat.com> Subject: Re: [Qemu-devel] [PATCH 1/7] monitor: Add command_completion callback to mon_cmd_t. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: kwolf@redhat.com, imammedo@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com On Thu, Mar 27, 2014 at 04:21:51PM -0400, Luiz Capitulino wrote: > > I think this patch has to be split into at least three patches. One for > the drive_del change (which is unrelated with the other stuff), one > converting device_add & device_del and the third one converting > object_add and object_del. > > Also, please, take the simplest conversion for the first patch, which > introduces the callback. > Ok, I will split them. Just felt that the number of patches was growing-up too fast. > One more comment below. > > > > > Signed-off-by: Hani Benhabiles > > Suggested-by: Luiz Capitulino > > --- > > hmp-commands.hx | 6 +++++- > > hmp.h | 4 ++++ > > monitor.c | 65 +++++++++++++++++++++++++++++++++++---------------------- > > 3 files changed, 49 insertions(+), 26 deletions(-) > > > > diff --git a/hmp-commands.hx b/hmp-commands.hx > > index f3fc514..4c4d261 100644 > > --- a/hmp-commands.hx > > +++ b/hmp-commands.hx > > @@ -176,7 +176,7 @@ ETEXI > > > > { > > .name = "drive_del", > > - .args_type = "id:s", > > + .args_type = "id:B", > > .params = "device", > > .help = "remove host block device", > > .user_print = monitor_user_noop, > > @@ -658,6 +658,7 @@ ETEXI > > .help = "add device, like -device on the command line", > > .user_print = monitor_user_noop, > > .mhandler.cmd_new = do_device_add, > > + .command_completion = device_add_completion, > > }, > > > > STEXI > > @@ -673,6 +674,7 @@ ETEXI > > .params = "device", > > .help = "remove device", > > .mhandler.cmd = hmp_device_del, > > + .command_completion = device_del_completion, > > }, > > > > STEXI > > @@ -1254,6 +1256,7 @@ ETEXI > > .params = "[qom-type=]type,id=str[,prop=value][,...]", > > .help = "create QOM object", > > .mhandler.cmd = hmp_object_add, > > + .command_completion = object_add_completion, > > }, > > > > STEXI > > @@ -1268,6 +1271,7 @@ ETEXI > > .params = "id", > > .help = "destroy QOM object", > > .mhandler.cmd = hmp_object_del, > > + .command_completion = object_del_completion, > > }, > > > > STEXI > > diff --git a/hmp.h b/hmp.h > > index ed58f0e..558658f 100644 > > --- a/hmp.h > > +++ b/hmp.h > > @@ -92,5 +92,9 @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict); > > void hmp_cpu_add(Monitor *mon, const QDict *qdict); > > void hmp_object_add(Monitor *mon, const QDict *qdict); > > void hmp_object_del(Monitor *mon, const QDict *qdict); > > +void device_add_completion(Monitor *mon, int nb_args, const char *str); > > +void device_del_completion(Monitor *mon, int nb_args, const char *str); > > +void object_add_completion(Monitor *mon, int nb_args, const char *str); > > +void object_del_completion(Monitor *mon, int nb_args, const char *str); > > > > #endif > > diff --git a/monitor.c b/monitor.c > > index 342e83b..2c8528c 100644 > > --- a/monitor.c > > +++ b/monitor.c > > @@ -137,6 +137,7 @@ typedef struct mon_cmd_t { > > * used, and mhandler of 1st level plays the role of help function. > > */ > > struct mon_cmd_t *sub_table; > > + void (*command_completion)(Monitor *mon, int nb_args, const char *str); > > Why does command_completion need 'mon'? It seems to me that a ReadLineState > suffices. No particular reason right now. I just tried to keep the prototype general enough in order to not have to make a mass conversion in case some need shows up in the future. Should I just use ReadLineState in the next series version ?