Re: [Qemu-devel] [PATCH 1/2] guest agent: add RPC blacklist command-line option

[Anthony Liguori <aliguori@us.ibm.com>]

On 12/06/2011 10:03 PM, Michael Roth wrote:
> This adds a command-line option, -b/--blacklist, that accepts a
> comma-seperated list of RPCs to disable, or prints a list of
> available RPCs if passed "?".
>
> In consequence this also adds general blacklisting and RPC listing
> facilities to the new QMP dispatch/registry facilities, should the
> QMP monitor ever have a need for such a thing.
>
> Ideally, to avoid support/compatability issues in the future,
> blacklisting guest agent functionality will be the exceptional
> case, but we add the functionality here to handle guest administrators
> with specific requirements.
>
> Signed-off-by: Michael Roth<mdroth@linux.vnet.ibm.com>

Applied.  Thanks.

Regards,

Anthony Liguori

> ---
>   qapi/qmp-core.h     |    3 +++
>   qapi/qmp-dispatch.c |    4 ++++
>   qapi/qmp-registry.c |   43 ++++++++++++++++++++++++++++++++++++++-----
>   qemu-ga.c           |   37 ++++++++++++++++++++++++++++++++++---
>   qerror.c            |    4 ++++
>   qerror.h            |    3 +++
>   6 files changed, 86 insertions(+), 8 deletions(-)
>
> diff --git a/qapi/qmp-core.h b/qapi/qmp-core.h
> index f1c26e4..3cf1781 100644
> --- a/qapi/qmp-core.h
> +++ b/qapi/qmp-core.h
> @@ -31,11 +31,14 @@ typedef struct QmpCommand
>       QmpCommandType type;
>       QmpCommandFunc *fn;
>       QTAILQ_ENTRY(QmpCommand) node;
> +    bool enabled;
>   } QmpCommand;
>
>   void qmp_register_command(const char *name, QmpCommandFunc *fn);
>   QmpCommand *qmp_find_command(const char *name);
>   QObject *qmp_dispatch(QObject *request);
> +void qmp_disable_command(const char *name);
> +char **qmp_get_command_list(void);
>
>   #endif
>
> diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
> index 5584693..43f640a 100644
> --- a/qapi/qmp-dispatch.c
> +++ b/qapi/qmp-dispatch.c
> @@ -79,6 +79,10 @@ static QObject *do_qmp_dispatch(QObject *request, Error **errp)
>           error_set(errp, QERR_COMMAND_NOT_FOUND, command);
>           return NULL;
>       }
> +    if (!cmd->enabled) {
> +        error_set(errp, QERR_COMMAND_DISABLED, command);
> +        return NULL;
> +    }
>
>       if (!qdict_haskey(dict, "arguments")) {
>           args = qdict_new();
> diff --git a/qapi/qmp-registry.c b/qapi/qmp-registry.c
> index 5ff99cf..abafa34 100644
> --- a/qapi/qmp-registry.c
> +++ b/qapi/qmp-registry.c
> @@ -14,7 +14,7 @@
>
>   #include "qapi/qmp-core.h"
>
> -static QTAILQ_HEAD(, QmpCommand) qmp_commands =
> +static QTAILQ_HEAD(QmpCommandList, QmpCommand) qmp_commands =
>       QTAILQ_HEAD_INITIALIZER(qmp_commands);
>
>   void qmp_register_command(const char *name, QmpCommandFunc *fn)
> @@ -24,17 +24,50 @@ void qmp_register_command(const char *name, QmpCommandFunc *fn)
>       cmd->name = name;
>       cmd->type = QCT_NORMAL;
>       cmd->fn = fn;
> +    cmd->enabled = true;
>       QTAILQ_INSERT_TAIL(&qmp_commands, cmd, node);
>   }
>
>   QmpCommand *qmp_find_command(const char *name)
>   {
> -    QmpCommand *i;
> +    QmpCommand *cmd;
>
> -    QTAILQ_FOREACH(i,&qmp_commands, node) {
> -        if (strcmp(i->name, name) == 0) {
> -            return i;
> +    QTAILQ_FOREACH(cmd,&qmp_commands, node) {
> +        if (strcmp(cmd->name, name) == 0) {
> +            return cmd;
>           }
>       }
>       return NULL;
>   }
> +
> +void qmp_disable_command(const char *name)
> +{
> +    QmpCommand *cmd;
> +
> +    QTAILQ_FOREACH(cmd,&qmp_commands, node) {
> +        if (strcmp(cmd->name, name) == 0) {
> +            cmd->enabled = false;
> +            return;
> +        }
> +    }
> +}
> +
> +char **qmp_get_command_list(void)
> +{
> +    QmpCommand *cmd;
> +    int count = 1;
> +    char **list_head, **list;
> +
> +    QTAILQ_FOREACH(cmd,&qmp_commands, node) {
> +        count++;
> +    }
> +
> +    list_head = list = g_malloc0(count * sizeof(char *));
> +
> +    QTAILQ_FOREACH(cmd,&qmp_commands, node) {
> +        *list = strdup(cmd->name);
> +        list++;
> +    }
> +
> +    return list_head;
> +}
> diff --git a/qemu-ga.c b/qemu-ga.c
> index 4932013..200bb15 100644
> --- a/qemu-ga.c
> +++ b/qemu-ga.c
> @@ -27,6 +27,7 @@
>   #include "signal.h"
>   #include "qerror.h"
>   #include "error_int.h"
> +#include "qapi/qmp-core.h"
>
>   #define QGA_VIRTIO_PATH_DEFAULT "/dev/virtio-ports/org.qemu.guest_agent.0"
>   #define QGA_PIDFILE_DEFAULT "/var/run/qemu-ga.pid"
> @@ -91,6 +92,8 @@ static void usage(const char *cmd)
>   "  -v, --verbose     log extra debugging information\n"
>   "  -V, --version     print version information and exit\n"
>   "  -d, --daemonize   become a daemon\n"
> +"  -b, --blacklist   comma-seperated list of RPCs to disable (no spaces, \"?\""
> +"                    to list available RPCs)\n"
>   "  -h, --help        display this help and exit\n"
>   "\n"
>   "Report bugs to<mdroth@linux.vnet.ibm.com>\n"
> @@ -548,7 +551,7 @@ static void init_guest_agent(GAState *s)
>
>   int main(int argc, char **argv)
>   {
> -    const char *sopt = "hVvdm:p:l:f:";
> +    const char *sopt = "hVvdm:p:l:f:b:";
>       const char *method = NULL, *path = NULL, *pidfile = QGA_PIDFILE_DEFAULT;
>       const struct option lopt[] = {
>           { "help", 0, NULL, 'h' },
> @@ -559,13 +562,16 @@ int main(int argc, char **argv)
>           { "method", 0, NULL, 'm' },
>           { "path", 0, NULL, 'p' },
>           { "daemonize", 0, NULL, 'd' },
> +        { "blacklist", 0, NULL, 'b' },
>           { NULL, 0, NULL, 0 }
>       };
> -    int opt_ind = 0, ch, daemonize = 0;
> +    int opt_ind = 0, ch, daemonize = 0, i, j, len;
>       GLogLevelFlags log_level = G_LOG_LEVEL_ERROR | G_LOG_LEVEL_CRITICAL;
>       FILE *log_file = stderr;
>       GAState *s;
>
> +    module_call_init(MODULE_INIT_QAPI);
> +
>       while ((ch = getopt_long(argc, argv, sopt, lopt,&opt_ind)) != -1) {
>           switch (ch) {
>           case 'm':
> @@ -595,6 +601,32 @@ int main(int argc, char **argv)
>           case 'd':
>               daemonize = 1;
>               break;
> +        case 'b': {
> +            char **list_head, **list;
> +            if (*optarg == '?') {
> +                list_head = list = qmp_get_command_list();
> +                while (*list != NULL) {
> +                    printf("%s\n", *list);
> +                    g_free(*list);
> +                    list++;
> +                }
> +                g_free(list_head);
> +                return 0;
> +            }
> +            for (j = 0, i = 0, len = strlen(optarg); i<  len; i++) {
> +                if (optarg[i] == ',') {
> +                    optarg[i] = 0;
> +                    qmp_disable_command(&optarg[j]);
> +                    g_debug("disabling command: %s",&optarg[j]);
> +                    j = i + 1;
> +                }
> +            }
> +            if (j<  i) {
> +                qmp_disable_command(&optarg[j]);
> +                g_debug("disabling command: %s",&optarg[j]);
> +            }
> +            break;
> +        }
>           case 'h':
>               usage(argv[0]);
>               return 0;
> @@ -624,7 +656,6 @@ int main(int argc, char **argv)
>       ga_command_state_init_all(s->command_state);
>       ga_state = s;
>
> -    module_call_init(MODULE_INIT_QAPI);
>       init_guest_agent(ga_state);
>       register_signal_handlers();
>
> diff --git a/qerror.c b/qerror.c
> index 656efc2..a998d2f 100644
> --- a/qerror.c
> +++ b/qerror.c
> @@ -65,6 +65,10 @@ static const QErrorStringTable qerror_table[] = {
>           .desc      = "The command %(name) has not been found",
>       },
>       {
> +        .error_fmt = QERR_COMMAND_DISABLED,
> +        .desc      = "The command %(name) has been disabled for this instance",
> +    },
> +    {
>           .error_fmt = QERR_DEVICE_ENCRYPTED,
>           .desc      = "Device '%(device)' is encrypted",
>       },
> diff --git a/qerror.h b/qerror.h
> index 161d654..0d3d4c5 100644
> --- a/qerror.h
> +++ b/qerror.h
> @@ -66,6 +66,9 @@ QError *qobject_to_qerror(const QObject *obj);
>   #define QERR_COMMAND_NOT_FOUND \
>       "{ 'class': 'CommandNotFound', 'data': { 'name': %s } }"
>
> +#define QERR_COMMAND_DISABLED \
> +    "{ 'class': 'CommandDisabled', 'data': { 'name': %s } }"
> +
>   #define QERR_DEVICE_ENCRYPTED \
>       "{ 'class': 'DeviceEncrypted', 'data': { 'device': %s } }"
>