From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57027) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WTWYX-00078W-CI for qemu-devel@nongnu.org; Fri, 28 Mar 2014 09:10:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WTWYS-0006YL-8i for qemu-devel@nongnu.org; Fri, 28 Mar 2014 09:10:45 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39183) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WTWYR-0006YG-TC for qemu-devel@nongnu.org; Fri, 28 Mar 2014 09:10:40 -0400 Date: Fri, 28 Mar 2014 09:10:36 -0400 From: Luiz Capitulino Message-ID: <20140328091036.2c12e3e3@redhat.com> In-Reply-To: <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> <20140327215546.GD22714@Inspiron-3521> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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: Hani Benhabiles Cc: kwolf@redhat.com, imammedo@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com On Thu, 27 Mar 2014 22:55:46 +0100 Hani Benhabiles wrote: > 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. That's not a problem. > > 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 ? IMO, yes. If, in the future, it turns out we need a Monitor object we can just update the callback signature.