Patchwork [10/19] QMP: Introduce query commands dispatch table

login
register
mail settings
Submitter Luiz Capitulino
Date Sept. 30, 2010, 8:56 p.m.
Message ID <1285880180-29724-11-git-send-email-lcapitulino@redhat.com>
Download mbox | patch
Permalink /patch/66264/
State New
Headers show

Comments

Luiz Capitulino - Sept. 30, 2010, 8:56 p.m.
The new table is a copy of HMP's table, containing only QObject
handlers.

In the near future HMP will be making QMP calls and then we will
be able to drop QObject handlers from HMP's table.

From now on, QMP and HMP have different query command dispatch
tables.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c |  130 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 128 insertions(+), 2 deletions(-)
Anthony Liguori - Oct. 1, 2010, 1:31 p.m.
On 09/30/2010 03:56 PM, Luiz Capitulino wrote:
> The new table is a copy of HMP's table, containing only QObject
> handlers.
>
> In the near future HMP will be making QMP calls and then we will
> be able to drop QObject handlers from HMP's table.
>
> > From now on, QMP and HMP have different query command dispatch
> tables.
>    

I like this series a lot and I think it's ready to merge.

But I wonder, why have a separate qmp_query_cmds table?  Why not just 
fold the query commands into qmp_cmds?

Regards,

Anthony Liguori

> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
> ---
>   monitor.c |  130 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>   1 files changed, 128 insertions(+), 2 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index 8f58e18..7cb66df 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -190,6 +190,7 @@ static const mon_cmd_t mon_cmds[];
>   static const mon_cmd_t info_cmds[];
>
>   static const mon_cmd_t qmp_cmds[];
> +static const mon_cmd_t qmp_query_cmds[];
>
>   Monitor *cur_mon;
>   Monitor *default_mon;
> @@ -751,7 +752,7 @@ static void do_info_commands(Monitor *mon, QObject **ret_data)
>           }
>       }
>
> -    for (cmd = info_cmds; cmd->name != NULL; cmd++) {
> +    for (cmd = qmp_query_cmds; cmd->name != NULL; cmd++) {
>           if (monitor_handler_ported(cmd)&&  !monitor_cmd_user_only(cmd)) {
>               char buf[128];
>               snprintf(buf, sizeof(buf), "query-%s", cmd->name);
> @@ -2639,6 +2640,131 @@ static const mon_cmd_t qmp_cmds[] = {
>       { /* NULL */ },
>   };
>
> +static const mon_cmd_t qmp_query_cmds[] = {
> +    {
> +        .name       = "version",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "show the version of QEMU",
> +        .user_print = do_info_version_print,
> +        .mhandler.info_new = do_info_version,
> +    },
> +    {
> +        .name       = "commands",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "list QMP available commands",
> +        .user_print = monitor_user_noop,
> +        .mhandler.info_new = do_info_commands,
> +    },
> +    {
> +        .name       = "chardev",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "show the character devices",
> +        .user_print = qemu_chr_info_print,
> +        .mhandler.info_new = qemu_chr_info,
> +    },
> +    {
> +        .name       = "block",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "show the block devices",
> +        .user_print = bdrv_info_print,
> +        .mhandler.info_new = bdrv_info,
> +    },
> +    {
> +        .name       = "blockstats",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "show block device statistics",
> +        .user_print = bdrv_stats_print,
> +        .mhandler.info_new = bdrv_info_stats,
> +    },
> +    {
> +        .name       = "cpus",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "show infos for each CPU",
> +        .user_print = monitor_print_cpus,
> +        .mhandler.info_new = do_info_cpus,
> +    },
> +    {
> +        .name       = "pci",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "show PCI info",
> +        .user_print = do_pci_info_print,
> +        .mhandler.info_new = do_pci_info,
> +    },
> +    {
> +        .name       = "kvm",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "show KVM information",
> +        .user_print = do_info_kvm_print,
> +        .mhandler.info_new = do_info_kvm,
> +    },
> +    {
> +        .name       = "status",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "show the current VM status (running|paused)",
> +        .user_print = do_info_status_print,
> +        .mhandler.info_new = do_info_status,
> +    },
> +    {
> +        .name       = "mice",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "show which guest mouse is receiving events",
> +        .user_print = do_info_mice_print,
> +        .mhandler.info_new = do_info_mice,
> +    },
> +    {
> +        .name       = "vnc",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "show the vnc server status",
> +        .user_print = do_info_vnc_print,
> +        .mhandler.info_new = do_info_vnc,
> +    },
> +    {
> +        .name       = "name",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "show the current VM name",
> +        .user_print = do_info_name_print,
> +        .mhandler.info_new = do_info_name,
> +    },
> +    {
> +        .name       = "uuid",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "show the current VM UUID",
> +        .user_print = do_info_uuid_print,
> +        .mhandler.info_new = do_info_uuid,
> +    },
> +    {
> +        .name       = "migrate",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "show migration status",
> +        .user_print = do_info_migrate_print,
> +        .mhandler.info_new = do_info_migrate,
> +    },
> +    {
> +        .name       = "balloon",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "show balloon information",
> +        .user_print = monitor_print_balloon,
> +        .mhandler.info_async = do_info_balloon,
> +        .flags      = MONITOR_CMD_ASYNC,
> +    },
> +    { /* NULL */ },
> +};
> +
>   /*******************************************************************/
>
>   static const char *pch;
> @@ -3366,7 +3492,7 @@ static const mon_cmd_t *monitor_find_command(const char *cmdname)
>
>   static const mon_cmd_t *qmp_find_query_cmd(const char *info_item)
>   {
> -    return search_dispatch_table(info_cmds, info_item);
> +    return search_dispatch_table(qmp_query_cmds, info_item);
>   }
>
>   static const mon_cmd_t *qmp_find_cmd(const char *cmdname)
>
Luiz Capitulino - Oct. 1, 2010, 1:49 p.m.
On Fri, 01 Oct 2010 08:31:11 -0500
Anthony Liguori <aliguori@linux.vnet.ibm.com> wrote:

> On 09/30/2010 03:56 PM, Luiz Capitulino wrote:
> > The new table is a copy of HMP's table, containing only QObject
> > handlers.
> >
> > In the near future HMP will be making QMP calls and then we will
> > be able to drop QObject handlers from HMP's table.
> >
> > > From now on, QMP and HMP have different query command dispatch
> > tables.
> >    
> 
> I like this series a lot and I think it's ready to merge.
> 
> But I wonder, why have a separate qmp_query_cmds table?  Why not just 
> fold the query commands into qmp_cmds?

Yes, that will be done shortly, but in a different series.

I'm not doing it in this series because it's necessary to change the
signature of all those functions which would make this series too large and
harder to review.

On a related note: I have more monitor patches in my queue. I'm building
and testing them right now. I'm planning to send a pull request of all
pending monitor patches later today. So, if you prefer, you can wait for
that pull request instead of merging this series alone.

It will generate some noise on the list though, as I think it's good practice
to resend patches in a pull request to the list.
Anthony Liguori - Oct. 1, 2010, 2:25 p.m.
On 10/01/2010 08:49 AM, Luiz Capitulino wrote:
> On Fri, 01 Oct 2010 08:31:11 -0500
> Anthony Liguori<aliguori@linux.vnet.ibm.com>  wrote:
>
>    
>> On 09/30/2010 03:56 PM, Luiz Capitulino wrote:
>>      
>>> The new table is a copy of HMP's table, containing only QObject
>>> handlers.
>>>
>>> In the near future HMP will be making QMP calls and then we will
>>> be able to drop QObject handlers from HMP's table.
>>>
>>>        
>>>>  From now on, QMP and HMP have different query command dispatch
>>>>          
>>> tables.
>>>
>>>        
>> I like this series a lot and I think it's ready to merge.
>>
>> But I wonder, why have a separate qmp_query_cmds table?  Why not just
>> fold the query commands into qmp_cmds?
>>      
> Yes, that will be done shortly, but in a different series.
>    

Perfect :-)

> I'm not doing it in this series because it's necessary to change the
> signature of all those functions which would make this series too large and
> harder to review.
>
> On a related note: I have more monitor patches in my queue. I'm building
> and testing them right now. I'm planning to send a pull request of all
> pending monitor patches later today. So, if you prefer, you can wait for
> that pull request instead of merging this series alone.
>    

Yes, let's do that.  This whole series:

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

> It will generate some noise on the list though, as I think it's good practice
> to resend patches in a pull request to the list.
>

Patch

diff --git a/monitor.c b/monitor.c
index 8f58e18..7cb66df 100644
--- a/monitor.c
+++ b/monitor.c
@@ -190,6 +190,7 @@  static const mon_cmd_t mon_cmds[];
 static const mon_cmd_t info_cmds[];
 
 static const mon_cmd_t qmp_cmds[];
+static const mon_cmd_t qmp_query_cmds[];
 
 Monitor *cur_mon;
 Monitor *default_mon;
@@ -751,7 +752,7 @@  static void do_info_commands(Monitor *mon, QObject **ret_data)
         }
     }
 
-    for (cmd = info_cmds; cmd->name != NULL; cmd++) {
+    for (cmd = qmp_query_cmds; cmd->name != NULL; cmd++) {
         if (monitor_handler_ported(cmd) && !monitor_cmd_user_only(cmd)) {
             char buf[128];
             snprintf(buf, sizeof(buf), "query-%s", cmd->name);
@@ -2639,6 +2640,131 @@  static const mon_cmd_t qmp_cmds[] = {
     { /* NULL */ },
 };
 
+static const mon_cmd_t qmp_query_cmds[] = {
+    {
+        .name       = "version",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show the version of QEMU",
+        .user_print = do_info_version_print,
+        .mhandler.info_new = do_info_version,
+    },
+    {
+        .name       = "commands",
+        .args_type  = "",
+        .params     = "",
+        .help       = "list QMP available commands",
+        .user_print = monitor_user_noop,
+        .mhandler.info_new = do_info_commands,
+    },
+    {
+        .name       = "chardev",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show the character devices",
+        .user_print = qemu_chr_info_print,
+        .mhandler.info_new = qemu_chr_info,
+    },
+    {
+        .name       = "block",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show the block devices",
+        .user_print = bdrv_info_print,
+        .mhandler.info_new = bdrv_info,
+    },
+    {
+        .name       = "blockstats",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show block device statistics",
+        .user_print = bdrv_stats_print,
+        .mhandler.info_new = bdrv_info_stats,
+    },
+    {
+        .name       = "cpus",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show infos for each CPU",
+        .user_print = monitor_print_cpus,
+        .mhandler.info_new = do_info_cpus,
+    },
+    {
+        .name       = "pci",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show PCI info",
+        .user_print = do_pci_info_print,
+        .mhandler.info_new = do_pci_info,
+    },
+    {
+        .name       = "kvm",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show KVM information",
+        .user_print = do_info_kvm_print,
+        .mhandler.info_new = do_info_kvm,
+    },
+    {
+        .name       = "status",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show the current VM status (running|paused)",
+        .user_print = do_info_status_print,
+        .mhandler.info_new = do_info_status,
+    },
+    {
+        .name       = "mice",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show which guest mouse is receiving events",
+        .user_print = do_info_mice_print,
+        .mhandler.info_new = do_info_mice,
+    },
+    {
+        .name       = "vnc",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show the vnc server status",
+        .user_print = do_info_vnc_print,
+        .mhandler.info_new = do_info_vnc,
+    },
+    {
+        .name       = "name",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show the current VM name",
+        .user_print = do_info_name_print,
+        .mhandler.info_new = do_info_name,
+    },
+    {
+        .name       = "uuid",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show the current VM UUID",
+        .user_print = do_info_uuid_print,
+        .mhandler.info_new = do_info_uuid,
+    },
+    {
+        .name       = "migrate",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show migration status",
+        .user_print = do_info_migrate_print,
+        .mhandler.info_new = do_info_migrate,
+    },
+    {
+        .name       = "balloon",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show balloon information",
+        .user_print = monitor_print_balloon,
+        .mhandler.info_async = do_info_balloon,
+        .flags      = MONITOR_CMD_ASYNC,
+    },
+    { /* NULL */ },
+};
+
 /*******************************************************************/
 
 static const char *pch;
@@ -3366,7 +3492,7 @@  static const mon_cmd_t *monitor_find_command(const char *cmdname)
 
 static const mon_cmd_t *qmp_find_query_cmd(const char *info_item)
 {
-    return search_dispatch_table(info_cmds, info_item);
+    return search_dispatch_table(qmp_query_cmds, info_item);
 }
 
 static const mon_cmd_t *qmp_find_cmd(const char *cmdname)