Re: [Qemu-devel] [PATCH 1/7] monitor: Add command_completion callback to mon_cmd_t.

[Hani Benhabiles <kroosec@gmail.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 <hani@linux.com>
> > Suggested-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> >  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 ?