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

[Luiz Capitulino]

On Sun,  9 Mar 2014 12:16:11 +0100
Hani Benhabiles <address@hidden> wrote:

> Convert device_add, device_del, object_add and object_del commands to use
> the new callback.
> 
> Also fix drive_del id argument type to activate completion.

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.

One more comment below.

> 
> Signed-off-by: Hani Benhabiles <address@hidden>
> Suggested-by: Luiz Capitulino <address@hidden>
> ---
>  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.

>  } mon_cmd_t;
>  
>  /* file descriptors passed via SCM_RIGHTS */
> @@ -4277,13 +4278,17 @@ static const char *next_arg_type(const char *typestr)
>      return (p != NULL ? ++p : typestr);
>  }
>  
> -static void device_add_completion(ReadLineState *rs, const char *str)
> +void device_add_completion(Monitor *mon, int nb_args, const char *str)
>  {
>      GSList *list, *elt;
>      size_t len;
>  
> +    if (nb_args != 2) {
> +        return;
> +    }
> +
>      len = strlen(str);
> -    readline_set_completion_index(rs, len);
> +    readline_set_completion_index(mon->rs, len);
>      list = elt = object_class_get_list(TYPE_DEVICE, false);
>      while (elt) {
>          const char *name;
> @@ -4291,35 +4296,39 @@ static void device_add_completion(ReadLineState *rs, 
> const char *str)
>                                               TYPE_DEVICE);
>          name = object_class_get_name(OBJECT_CLASS(dc));
>          if (!strncmp(name, str, len)) {
> -            readline_add_completion(rs, name);
> +            readline_add_completion(mon->rs, name);
>          }
>          elt = elt->next;
>      }
>      g_slist_free(list);
>  }
>  
> -static void object_add_completion(ReadLineState *rs, const char *str)
> +void object_add_completion(Monitor *mon, int nb_args, const char *str)
>  {
>      GSList *list, *elt;
>      size_t len;
>  
> +    if (nb_args != 2) {
> +        return;
> +    }
> +
>      len = strlen(str);
> -    readline_set_completion_index(rs, len);
> +    readline_set_completion_index(mon->rs, len);
>      list = elt = object_class_get_list(TYPE_USER_CREATABLE, false);
>      while (elt) {
>          const char *name;
>  
>          name = object_class_get_name(OBJECT_CLASS(elt->data));
>          if (!strncmp(name, str, len) && strcmp(name, TYPE_USER_CREATABLE)) {
> -            readline_add_completion(rs, name);
> +            readline_add_completion(mon->rs, name);
>          }
>          elt = elt->next;
>      }
>      g_slist_free(list);
>  }
>  
> -static void device_del_completion(ReadLineState *rs, BusState *bus,
> -                                  const char *str, size_t len)
> +static void device_del_bus_completion(ReadLineState *rs,  BusState *bus,
> +                                      const char *str, size_t len)
>  {
>      BusChild *kid;
>  
> @@ -4332,18 +4341,34 @@ static void device_del_completion(ReadLineState *rs, 
> BusState *bus,
>          }
>  
>          QLIST_FOREACH(dev_child, &dev->child_bus, sibling) {
> -            device_del_completion(rs, dev_child, str, len);
> +            device_del_bus_completion(rs, dev_child, str, len);
>          }
>      }
>  }
>  
> -static void object_del_completion(ReadLineState *rs, const char *str)
> +void device_del_completion(Monitor *mon, int nb_args, const char *str)
> +{
> +    size_t len;
> +
> +    if (nb_args != 2) {
> +        return;
> +    }
> +
> +    len = strlen(str);
> +    readline_set_completion_index(mon->rs, len);
> +    device_del_bus_completion(mon->rs, sysbus_get_default(), str, len);
> +}
> +
> +void object_del_completion(Monitor *mon, int nb_args, const char *str)
>  {
>      ObjectPropertyInfoList *list, *start;
>      size_t len;
>  
> +    if (nb_args != 2) {
> +        return;
> +    }
>      len = strlen(str);
> -    readline_set_completion_index(rs, len);
> +    readline_set_completion_index(mon->rs, len);
>  
>      start = list = qmp_qom_list("/objects", NULL);
>      while (list) {
> @@ -4351,7 +4376,7 @@ static void object_del_completion(ReadLineState *rs, 
> const char *str)
>  
>          if (!strncmp(info->type, "child<", 5)
>              && !strncmp(info->name, str, len)) {
> -            readline_add_completion(rs, info->name);
> +            readline_add_completion(mon->rs, info->name);
>          }
>          list = list->next;
>      }
> @@ -4395,6 +4420,9 @@ static void monitor_find_completion_by_table(Monitor 
> *mon,
>              return monitor_find_completion_by_table(mon, cmd->sub_table,
>                                                      &args[1], nb_args - 1);
>          }
> +        if (cmd->command_completion) {
> +            return cmd->command_completion(mon, nb_args, args[nb_args - 1]);
> +        }
>  
>          ptype = next_arg_type(cmd->args_type);
>          for(i = 0; i < nb_args - 2; i++) {
> @@ -4421,13 +4449,6 @@ static void monitor_find_completion_by_table(Monitor 
> *mon,
>              readline_set_completion_index(mon->rs, strlen(str));
>              bdrv_iterate(block_completion_it, &mbs);
>              break;
> -        case 'O':
> -            if (!strcmp(cmd->name, "device_add") && nb_args == 2) {
> -                device_add_completion(mon->rs, str);
> -            } else if (!strcmp(cmd->name, "object_add") && nb_args == 2) {
> -                object_add_completion(mon->rs, str);
> -            }
> -            break;
>          case 's':
>          case 'S':
>              if (!strcmp(cmd->name, "sendkey")) {
> @@ -4441,12 +4462,6 @@ static void monitor_find_completion_by_table(Monitor 
> *mon,
>              } else if (!strcmp(cmd->name, "help|?")) {
>                  monitor_find_completion_by_table(mon, cmd_table,
>                                                   &args[1], nb_args - 1);
> -            } else if (!strcmp(cmd->name, "device_del") && nb_args == 2) {
> -                size_t len = strlen(str);
> -                readline_set_completion_index(mon->rs, len);
> -                device_del_completion(mon->rs, sysbus_get_default(), str, 
> len);
> -            } else if (!strcmp(cmd->name, "object_del") && nb_args == 2) {
> -                object_del_completion(mon->rs, str);
>              }
>              break;
>          default: