Re: [Xen-devel] [PATCH v2] libxl: add "xl qemu-monitor-command"

[Dario Faggioli <dario.faggioli@xxxxxxxxxx>]

On Mon, 2016-09-05 at 16:11 +0200, Juergen Gross wrote:
> diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
> @@ -1103,14 +1103,48 @@ int libxl__qmp_x_blockdev_change(libxl__gc
> *gc, int domid, const char *parent,
>      return qmp_run_command(gc, domid, "x-blockdev-change", args,
> NULL, NULL);
>  }
>  
> -int libxl__qmp_hmp(libxl__gc *gc, int domid, const char
> *command_line)
> +static int hmp_callback(libxl__qmp_handler *qmp,
> +                        const libxl__json_object *response,
> +                        void *opaque)
> +{
> +    char **output = opaque;
> +    GC_INIT(qmp->ctx);
> +    int rc = 0;
> +
I'm probably being a bit picky, but libxl's CODING_STYLE says rc should
not be initialized.

> +    if (!output)
> +        goto out;
> +
> +    *output = NULL;
> +
> +    if (libxl__json_object_is_string(response)) {
> +        *output = libxl__strdup(gc,
> libxl__json_object_get_string(response));
> +        goto out;
> +    }
> +
> +    LOG(ERROR, "Response has unexpected format");
> +    rc = ERROR_FAIL;
> +
> +out:
> +    return rc;
> +}
> +
Also, aren't we missing calling GC_FREE on the out path?

> +int libxl__qmp_hmp(libxl__gc *gc, int domid, const char
> *command_line,
> +                   char **output)
>  {
>      libxl__json_object *args = NULL;
>  
>      qmp_parameters_add_string(gc, &args, "command-line",
> command_line);
>  
>      return qmp_run_command(gc, domid, "human-monitor-command", args,
> -                           NULL, NULL);
> +                           hmp_callback, output);
> +}
> +
> +int libxl_qemu_monitor_command(libxl_ctx *ctx, uint32_t domid,
> +                               const char *command_line, char
> **output)
> +{
> +    GC_INIT(ctx);
> +
> +    return libxl__qmp_hmp(gc, domid, command_line, output);
>
 GC_FREE; again ?

>  }
> 

> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> @@ -9536,6 +9536,33 @@ int main_psr_hwinfo(int argc, char **argv)
>  
>  #endif
>  
> +int main_qemu_monitor_command(int argc, char **argv)
> +{
> +    int opt;
> +    uint32_t domid;
> +    char *cmd;
> +    char *output;
> +    int ret;
> +
> +    SWITCH_FOREACH_OPT(opt, "", NULL, "qemu-monitor-command", 2) {
> +        /* No options */
> +    }
> +
> +    domid = find_domain(argv[optind]);
> +    cmd = argv[optind + 1];
> +
> +    if (argc - optind > 2) {
> +        fprintf(stderr, "Invalid arguments.\n");
> +        return 1;
> +    }
> +
> +    ret = libxl_qemu_monitor_command(ctx, domid, cmd, &output);
> +    if (!ret && output)
> +        printf("%s\n", output);
> +
> +    return ret;
>
Things are not fully consistent yet, but there's an ongoing effort for
having xl terminate with either EXIT_SUCCESS or EXIT_FAILURE. Can you
use them when exit()-ing or, like in this case, returning from a
main_foo() function?

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

Attachment: signature.asc
Description: This is a digitally signed message part

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel