[RFC,11/13] hmp: add NBD server commands

At the HMP level there is no nbd_server_add command.  nbd_server_start
automatically exposes all of the VM's block devices, and an option -w
makes them writable.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hmp-commands.hx | 29 +++++++++++++++++++++++++
 hmp.c           | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hmp.h           |  2 ++
 3 file modificati, 97 inserzioni(+)

On Mon, 27 Aug 2012 17:00:24 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> At the HMP level there is no nbd_server_add command.  nbd_server_start
> automatically exposes all of the VM's block devices, and an option -w
> makes them writable.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hmp-commands.hx | 29 +++++++++++++++++++++++++
>  hmp.c           | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  hmp.h           |  2 ++
>  3 file modificati, 97 inserzioni(+)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index f6104b0..cabb886 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1248,6 +1248,35 @@ Remove all matches from the access control list, and set the default
>  policy back to @code{deny}.
>  ETEXI
>  
> +    {
> +        .name       = "nbd_server_start",
> +        .args_type  = "writable:-w,uri:s",
> +        .params     = "nbd_server_start [-w] host:port",
> +        .help       = "serve block devices on the given host and port",
> +        .mhandler.cmd = hmp_nbd_server_start,
> +    },
> +STEXI
> +@item nbd_server_start @var{host}:@var{port}
> +@findex nbd_server_start
> +Start an NBD server on the given host and/or port, and serve all of the
> +virtual machine's block devices that have an inserted media on it.
> +The @option{-w} option makes the devices writable.
> +ETEXI
> +
> +    {
> +        .name       = "nbd_server_stop",
> +        .args_type  = "",
> +        .params     = "nbd_server_stop",
> +        .help       = "stop serving block devices using the NBD protocol",
> +        .mhandler.cmd = hmp_nbd_server_stop,
> +    },
> +STEXI
> +@item nbd_server_stop
> +@findex nbd_server_stop
> +Stop the QEMU embedded NBD server.
> +ETEXI
> +
> +
>  #if defined(TARGET_I386)
>  
>      {
> diff --git a/hmp.c b/hmp.c
> index a9d5675..10ff50d 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -18,6 +18,7 @@
>  #include "qemu-option.h"
>  #include "qemu-timer.h"
>  #include "qmp-commands.h"
> +#include "qemu_socket.h"
>  #include "monitor.h"
>  
>  static void hmp_handle_error(Monitor *mon, Error **errp)
> @@ -1102,3 +1103,68 @@ void hmp_closefd(Monitor *mon, const QDict *qdict)
>      qmp_closefd(fdname, &errp);
>      hmp_handle_error(mon, &errp);
>  }
> +
> +void hmp_nbd_server_start(Monitor *mon, const QDict *qdict)
> +{
> +    const char *uri = qdict_get_str(qdict, "uri");
> +    int writable = qdict_get_try_bool(qdict, "writable", 0);
> +    Error *errp = NULL;
> +    QemuOpts *opts;
> +    BlockDriverState *bs;
> +    IPSocketAddress addr;
> +
> +    /* First check if the address is available and start the server.  */
> +    opts = qemu_opts_create(&socket_opts, NULL, 0, NULL);
> +    if (inet_parse(opts, uri) != 0) {
> +        error_set(&errp, QERR_SOCKET_CREATE_FAILED);
> +	goto exit;
> +    }
> +
> +    memset(&addr, 0, sizeof(addr));
> +    addr.host = (char *) qemu_opt_get(opts, "host");
> +    addr.port = (char *) qemu_opt_get(opts, "port");
> +    addr.ipv4 = qemu_opt_get_bool(opts, "ipv4", 0);
> +    addr.ipv6 = qemu_opt_get_bool(opts, "ipv6", 0);
> +    addr.has_ipv4 = addr.has_ipv6 = true;
> +
> +    if (addr.host == NULL || addr.port == NULL) {
> +        error_set(&errp, QERR_SOCKET_CREATE_FAILED);

You can and should use monitor_printf() directly.

> +        goto exit;
> +    }
> +
> +    qmp_nbd_server_start(&addr, &errp);
> +    if (errp != NULL) {

Please, use error_is_set() instead.

> +        goto exit;
> +    }
> +
> +    /* Then try adding all block devices.  If one fails, close all and
> +     * exit.
> +     */
> +    bs = NULL;
> +    while ((bs = bdrv_next(bs))) {

To keep hmp.c an honest qmp client, you should use qmp_query_block() instead
of looping through BSs directly. You'll find other examples if you find for
qmp_query_block() in this file.

> +        if (!bdrv_is_inserted(bs)) {
> +            continue;
> +        }
> +
> +        qmp_nbd_server_add(bdrv_get_device_name(bs),
> +                           true, !bdrv_is_read_only(bs) && writable,
> +                           &errp);
> +
> +        if (errp != NULL) {
> +            qmp_nbd_server_stop(NULL);
> +            break;
> +        }
> +    }
> +
> +exit:
> +    qemu_opts_del(opts);
> +    hmp_handle_error(mon, &errp);
> +}
> +
> +void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict)
> +{
> +    Error *errp = NULL;
> +
> +    qmp_nbd_server_stop(&errp);
> +    hmp_handle_error(mon, &errp);
> +}
> diff --git a/hmp.h b/hmp.h
> index 7dd93bf..89d3960 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -71,5 +71,7 @@ void hmp_netdev_add(Monitor *mon, const QDict *qdict);
>  void hmp_netdev_del(Monitor *mon, const QDict *qdict);
>  void hmp_getfd(Monitor *mon, const QDict *qdict);
>  void hmp_closefd(Monitor *mon, const QDict *qdict);
> +void hmp_nbd_server_start(Monitor *mon, const QDict *qdict);
> +void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict);
>  
>  #endif

Il 18/09/2012 22:22, Luiz Capitulino ha scritto:
>> > +    if (addr.host == NULL || addr.port == NULL) {
>> > +        error_set(&errp, QERR_SOCKET_CREATE_FAILED);
> You can and should use monitor_printf() directly.

Hmm, why? Then I would have two error paths, one with monitor_printf and
one with hmp_handle_error.  But I will change this to add an Error* to
inet_parse.

>> > +        goto exit;
>> > +    }
>> > +
>> > +    qmp_nbd_server_start(&addr, &errp);
>> > +    if (errp != NULL) {
> Please, use error_is_set() instead.

I'll just rename errp to local_err, which is the usual convention.

>> > +        goto exit;
>> > +    }
>> > +
>> > +    /* Then try adding all block devices.  If one fails, close all and
>> > +     * exit.
>> > +     */
>> > +    bs = NULL;
>> > +    while ((bs = bdrv_next(bs))) {
> To keep hmp.c an honest qmp client, you should use qmp_query_block() instead
> of looping through BSs directly. You'll find other examples if you find for
> qmp_query_block() in this file.

Right, thanks!

Paolo