On Thu, 27 Mar 2014 22:55:46 +0100
Hani Benhabiles <kroo...@gmail.com> 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 <h...@linux.com>
> > > Suggested-by: Luiz Capitulino <lcapitul...@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 ?
IMO, yes. If, in the future, it turns out we need a Monitor object we can
just update the callback signature.
