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 <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 ?