[Qemu-devel] Re: [RFC][PATCH v5 08/21] virtagent: add agent_viewfile qmp/hmp command

[Jes Sorensen]

On 12/03/10 19:03, Michael Roth wrote:
> Utilize the getfile RPC to provide a means to view text files in the
> guest. Getfile can handle binary files as well but we don't advertise
> that here due to the special handling requiring to store it and provide
> it back to the user (base64 encoding it for instance). Hence the
> otherwise confusing "viewfile" as opposed to "getfile".
> 
> Signed-off-by: Michael Roth <address@hidden>
> ---
>  hmp-commands.hx |   16 +++++++++
>  monitor.c       |    1 +
>  qmp-commands.hx |   33 +++++++++++++++++++
>  virtagent.c     |   96 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  virtagent.h     |    3 ++
>  5 files changed, 149 insertions(+), 0 deletions(-)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index e5585ba..423c752 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1212,6 +1212,22 @@ show available trace events and their state
>  ETEXI
>  #endif
>  
> +    {
> +        .name       = "agent_viewfile",
> +        .args_type  = "filepath:s",
> +        .params     = "filepath",
> +        .help       = "Echo a file from the guest filesystem",
> +        .user_print = do_agent_viewfile_print,
> +        .mhandler.cmd_async = do_agent_viewfile,
> +        .flags      = MONITOR_CMD_ASYNC,
> +    },
> +
> +STEXI
> address@hidden agent_viewfile @var{filepath}
> address@hidden agent_viewfile
> +Echo the file identified by @var{filepath} on the guest filesystem
> +ETEXI
> +
>  STEXI
>  @end table
>  ETEXI
> diff --git a/monitor.c b/monitor.c
> index 8cee35d..145895d 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -56,6 +56,7 @@
>  #include "json-parser.h"
>  #include "osdep.h"
>  #include "exec-all.h"
> +#include "virtagent.h"
>  #ifdef CONFIG_SIMPLE_TRACE
>  #include "trace.h"
>  #endif
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 793cf1c..efa2137 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -738,6 +738,39 @@ Example:
>  EQMP
>  
>      {
> +        .name       = "agent_viewfile",
> +        .args_type  = "filepath:s",
> +        .params     = "filepath",
> +        .help       = "Echo a file from the guest filesystem",
> +        .user_print = monitor_user_noop,
> +        .mhandler.cmd_async = do_agent_viewfile,
> +        .flags      = MONITOR_CMD_ASYNC,
> +    },
> +
> +STEXI
> address@hidden agent_viewfile @var{filepath}
> address@hidden agent_viewfile
> +Echo the file identified by @var{filepath} on the guest filesystem
> +ETEXI
> +SQMP
> +agent_viewfile
> +--------
> +
> +Echo the file identified by @var{filepath} from the guest filesystem.
> +
> +Arguments:
> +
> +- "filepath": Full guest path of the desired file
> +
> +Example:
> +
> +-> { "execute": "agent_viewfile",
> +                "arguments": { "filepath": "/sys/kernel/kexec_loaded" } }
> +<- { "return": { "contents": "0" } }
> +
> +EQMP
> +
> +    {
>          .name       = "qmp_capabilities",
>          .args_type  = "",
>          .params     = "",
> diff --git a/virtagent.c b/virtagent.c
> index 34d8545..4a4dc8a 100644
> --- a/virtagent.c
> +++ b/virtagent.c
> @@ -139,3 +139,99 @@ out_free:
>  out:
>      return ret;
>  }
> +
> +/* QMP/HMP RPC client functions */
> +
> +void do_agent_viewfile_print(Monitor *mon, const QObject *data)
> +{
> +    QDict *qdict;
> +    const char *contents = NULL;
> +    int i;
> +
> +    qdict = qobject_to_qdict(data);
> +    if (!qdict_haskey(qdict, "contents")) {
> +        return;
> +    }
> +
> +    contents = qdict_get_str(qdict, "contents");
> +    if (contents != NULL) {
> +         /* monitor_printf truncates so do it in chunks. also, file_contents
> +          * may not be null-termed at proper location so explicitly calc
> +          * last chunk sizes */
> +        for (i = 0; i < strlen(contents); i += 1024) {
> +            monitor_printf(mon, "%.1024s", contents + i);
> +        }
> +    }
> +    monitor_printf(mon, "\n");
> +}
> +
> +static void do_agent_viewfile_cb(const char *resp_data,
> +                                 size_t resp_data_len,
> +                                 MonitorCompletion *mon_cb,
> +                                 void *mon_data)
> +{
> +    xmlrpc_value *resp = NULL;
> +    char *file_contents = NULL;
> +    size_t file_size;
> +    int ret;
> +    xmlrpc_env env;
> +    QDict *qdict = qdict_new();
> +
> +    if (resp_data == NULL) {
> +        LOG("error handling RPC request");
> +        goto out_no_resp;
> +    }
> +
> +    xmlrpc_env_init(&env);
> +    resp = xmlrpc_parse_response(&env, resp_data, resp_data_len);
> +    if (va_rpc_has_error(&env)) {
> +        ret = -1;
> +        goto out_no_resp;
> +    }
> +
> +    xmlrpc_parse_value(&env, resp, "6", &file_contents, &file_size);
> +    if (va_rpc_has_error(&env)) {
> +        ret = -1;
> +        goto out;

I believe this suffers from the same architectural problem I mentioned
in my comment to 07/21 - you don't restrict the file size, so it could
blow up the QEMU process on the host trying to view the wrong file.

I really think it is a bad idea to put this kind of command into the
monitor.

Jes