[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] Add interface to traverse the qmp command li
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v2] Add interface to traverse the qmp command list by QmpCommand |
Date: |
Tue, 08 Oct 2013 07:15:32 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130923 Thunderbird/17.0.9 |
On 10/08/2013 12:23 AM, Mark Wu wrote:
> In the original code, qmp_get_command_list is used to construct
> a list of all commands' name. To get the information of all qga
> commands, it traverses the name list and search the command info
> with its name. So it can cause O(n^2) in the number of commands.
>
> This patch adds an interface to traverse the qmp command list by
> QmpCommand to replace qmp_get_command_list. It can decrease the
> complexity from O(n^2) to O(n).
>
> Signed-off-by: Mark Wu <address@hidden>
> ---
> Changes:
> v2:
> 1. Keep the signature of qmp_command_is_enabled (per Eric and Michael)
> 2. Remove the unnecessary pointer castings (per Eric)
>
> include/qapi/qmp/dispatch.h | 5 ++--
> qapi/qmp-registry.c | 27 +++---------------
> qga/commands.c | 38 ++++++++++---------------
> qga/main.c | 68
> +++++++++++++++++----------------------------
> 4 files changed, 48 insertions(+), 90 deletions(-)
Reviewed-by: Eric Blake <address@hidden>
> +++ b/qga/main.c
> @@ -347,48 +347,34 @@ static gint ga_strcmp(gconstpointer str1, gconstpointer
> str2)
> }
>
> /* disable commands that aren't safe for fsfreeze */
> -static void ga_disable_non_whitelisted(void)
> +static void ga_disable_non_whitelisted(QmpCommand *cmd, void *opaque)
> {
> - char **list_head, **list;
> bool whitelisted;
> int i;
>
> - list_head = list = qmp_get_command_list();
> - while (*list != NULL) {
> - whitelisted = false;
> - i = 0;
> - while (ga_freeze_whitelist[i] != NULL) {
> - if (strcmp(*list, ga_freeze_whitelist[i]) == 0) {
> - whitelisted = true;
> - }
> - i++;
> - }
> - if (!whitelisted) {
> - g_debug("disabling command: %s", *list);
> - qmp_disable_command(*list);
> + whitelisted = false;
> + i = 0;
> + while (ga_freeze_whitelist[i] != NULL) {
> + if (strcmp(cmd->name, ga_freeze_whitelist[i]) == 0) {
This (and several other instances) accesses cmd->name directly where we
were formally using the string from the returned list. Should these
spots be modified to go through an accessor method instead, similar to
qmp_command_is_enabled, so that QmpCommand can be treated as an opaque
type from this file?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature