[v2,04/17] blockdev: 'blockdev-add' QMP command

For examples see the changes to qmp-commands.hx.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c       |  57 ++++++++++++++
 qapi-schema.json | 236 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qmp-commands.hx  |  59 ++++++++++++++
 3 files changed, 352 insertions(+)

Le Tuesday 01 Oct 2013 à 15:20:06 (+0200), Kevin Wolf a écrit :
> For examples see the changes to qmp-commands.hx.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  blockdev.c       |  57 ++++++++++++++
>  qapi-schema.json | 236 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  qmp-commands.hx  |  59 ++++++++++++++
>  3 files changed, 352 insertions(+)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 29a5b70..3059987 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -38,6 +38,8 @@
>  #include "qemu/option.h"
>  #include "qemu/config-file.h"
>  #include "qapi/qmp/types.h"
> +#include "qapi-visit.h"
> +#include "qapi/qmp-output-visitor.h"
>  #include "sysemu/sysemu.h"
>  #include "block/block_int.h"
>  #include "qmp-commands.h"
> @@ -2066,6 +2068,61 @@ void qmp_block_job_complete(const char *device, Error **errp)
>      block_job_complete(job, errp);
>  }
>  
> +void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
> +{
> +    QmpOutputVisitor *ov = qmp_output_visitor_new();
> +    QObject *obj;
> +    QDict *qdict;
> +    DriveInfo *dinfo;
> +    Error *local_err = NULL;
> +
> +    /* Require an ID in the top level */
> +    if (!options->has_id) {
> +        error_setg(errp, "Block device needs an ID");
Here we leak ov.
Maybe goto fail;

> +        return;
> +    }
> +
> +    /* TODO Sort it out in raw-posix and drive_init: Reject aio=native with
> +     * cache.direct=false instead of silently switching to aio=threads, except
> +     * if called from drive_init.
> +     *
> +     * For now, simply forbidding the combination for all drivers will do. */
> +    if (options->has_aio && options->aio == BLOCKDEV_AIO_OPTIONS_NATIVE) {
> +        bool direct = options->cache->has_direct && options->cache->direct;
> +        if (!options->has_cache && !direct) {
> +            error_setg(errp, "aio=native requires cache.direct=true");
> +            goto fail;
> +        }
> +    }
> +
> +    visit_type_BlockdevOptions(qmp_output_get_visitor(ov),
> +                               &options, NULL, &local_err);
> +    if (error_is_set(&local_err)) {
> +        error_propagate(errp, local_err);
> +        goto fail;
> +    }
> +
> +    obj = qmp_output_get_qobject(ov);
> +    qdict = qobject_to_qdict(obj);
> +
> +    qdict_flatten(qdict);
> +
> +    QemuOpts *opts = qemu_opts_from_qdict(&qemu_drive_opts, qdict, &local_err);
> +    if (error_is_set(&local_err)) {
> +        error_propagate(errp, local_err);
> +        goto fail;
> +    }
> +
> +    dinfo = blockdev_init(opts, IF_NONE);
> +    if (!dinfo) {
> +        error_setg(errp, "Could not open image");
> +        goto fail;
> +    }
> +
> +fail:
> +    qmp_output_visitor_cleanup(ov);
> +}
> +
>  static void do_qmp_query_block_jobs_one(void *opaque, BlockDriverState *bs)
>  {
>      BlockJobInfoList **prev = opaque;
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 145eca8..7192119 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3902,3 +3902,239 @@
>  ##
>  { 'command': 'query-rx-filter', 'data': { '*name': 'str' },
>    'returns': ['RxFilterInfo'] }
> +
> +
> +##
> +# @BlockdevDiscardOptions
> +#
> +# Determines how to handle discard requests.
> +#
> +# @ignore:      Ignore the request
> +# @unmap:       Forward as an unmap request
> +#
> +# Since: 1.7
> +##
> +{ 'enum': 'BlockdevDiscardOptions',
> +  'data': [ 'ignore', 'unmap' ] }
> +
> +##
> +# @BlockdevAioOptions
> +#
> +# Selects the AIO backend to handle I/O requests
> +#
> +# @threads:     Use qemu's thread pool
> +# @native:      Use native AIO backend (only Linux and Windows)
> +#
> +# Since: 1.7
> +##
> +{ 'enum': 'BlockdevAioOptions',
> +  'data': [ 'threads', 'native' ] }
> +
> +##
> +# @BlockdevCacheOptions
> +#
> +# Includes cache-related options for block devices
> +#
> +# @writeback:   #optional enables writeback mode for any caches (default: true)
> +# @direct:      #optional enables use of O_DIRECT (bypass the host page cache;
> +#               default: false)
> +# @no-flush:    #optional ignore any flush requests for the device (default:
> +#               false)
> +#
> +# Since: 1.7
> +##
> +{ 'type': 'BlockdevCacheOptions',
> +  'data': { '*writeback': 'bool',
> +            '*direct': 'bool',
> +            '*no-flush': 'bool' } }
> +
> +##
> +# @BlockdevOptionsBase
> +#
> +# Options that are available for all block devices, independent of the block
> +# driver.
> +#
> +# @driver:      block driver name
> +# @id:          #optional id by which the new block device can be referred to.
> +#               This is a required option on the top level of blockdev-add, and
> +#               currently not allowed on any other level.
> +# @discard:     #optional discard-related options (default: ignore)
> +# @cache:       #optional cache-related options
> +# @aio:         #optional AIO backend (default: threads)
> +# @rerror:      #optional how to handle read errors on the device
> +#               (default: report)
> +# @werror:      #optional how to handle write errors on the device
> +#               (default: enospc)
> +# @read-only:   #optional whether the block device should be read-only
> +#               (default: false)
> +#
> +# Since: 1.7
> +##
> +{ 'type': 'BlockdevOptionsBase',
> +  'data': { 'driver': 'str',
> +            '*id': 'str',
> +            '*discard': 'BlockdevDiscardOptions',
> +            '*cache': 'BlockdevCacheOptions',
> +            '*aio': 'BlockdevAioOptions',
> +            '*rerror': 'BlockdevOnError',
> +            '*werror': 'BlockdevOnError',
> +            '*read-only': 'bool' } }
> +
> +##
> +# @BlockdevOptionsFile
> +#
> +# Driver specific block device options for the file backend and similar
> +# protocols.
> +#
> +# @filename:    path to the image file
> +#
> +# Since: 1.7
> +##
> +{ 'type': 'BlockdevOptionsFile',
> +  'data': { 'filename': 'str' } }
> +
> +##
> +# @BlockdevOptionsVVFAT
> +#
> +# Driver specific block device options for the vvfat protocol.
> +#
> +# @dir:         directory to be exported as FAT image
> +# @fat-type:    #optional FAT type: 12, 16 or 32
> +# @floppy:      #optional whether to export a floppy image (true) or
> +#               partitioned hard disk (false; default)
> +# @rw:          #optional whether to allow write operations (default: false)
> +#
> +# Since: 1.7
> +##
> +{ 'type': 'BlockdevOptionsVVFAT',
> +  'data': { 'dir': 'str', '*fat-type': 'int', '*floppy': 'bool',
> +            '*rw': 'bool' } }
> +
> +##
> +# @BlockdevOptionsGenericFormat
> +#
> +# Driver specific block device options for image format that have no option
> +# besides their data source.
> +#
> +# @file:        reference to or definition of the data source block device
> +#
> +# Since: 1.7
> +##
> +{ 'type': 'BlockdevOptionsGenericFormat',
> +  'data': { 'file': 'BlockdevRef' } }
> +
> +##
> +# @BlockdevOptionsGenericCOWFormat
> +#
> +# Driver specific block device options for image format that have no option
> +# besides their data source and an optional backing file.
> +#
> +# @backing:     #optional reference to or definition of the backing file block
> +#               device (if missing, taken from the image file content). It is
> +#               allowed to pass an empty string here in order to disable the
> +#               default backing file.
> +#
> +# Since: 1.7
> +##
> +{ 'type': 'BlockdevOptionsGenericCOWFormat',
> +  'base': 'BlockdevOptionsGenericFormat',
> +  'data': { '*backing': 'BlockdevRef' } }
> +
> +##
> +# @BlockdevOptionsQcow2
> +#
> +# Driver specific block device options for qcow2.
> +#
> +# @lazy-refcounts: #optional whether to enable the lazy refcounts feature
> +#                  (default is taken from the image file)
> +#
> +# @pass-discard-request: #optional whether discard requests to the qcow2 device
> +#                        should be forwarded to the data source
> +#
> +# @pass-discard-snapshot: #optional whether discard requests for the data source
> +#                         should be issued when a snapshot operation (e.g.
> +#                         deleting a snapshot) frees clusters in the qcow2 file
> +#
> +# @pass-discard-other: #optional whether discard requests for the data source
> +#                      should be issued on other occasions where a cluster gets
> +#                      freed
> +#
> +# Since: 1.7
> +##
> +{ 'type': 'BlockdevOptionsQcow2',
> +  'base': 'BlockdevOptionsGenericCOWFormat',
> +  'data': { '*lazy-refcounts': 'bool',
> +            '*pass-discard-request': 'bool',
> +            '*pass-discard-snapshot': 'bool',
> +            '*pass-discard-other': 'bool' } }
> +
> +##
> +# @BlockdevOptions
> +#
> +# Options for creating a block device.
> +#
> +# Since: 1.7
> +##
> +{ 'union': 'BlockdevOptions',
> +  'base': 'BlockdevOptionsBase',
> +  'discriminator': 'driver',
> +  'data': {
> +      'file':       'BlockdevOptionsFile',
> +      'http':       'BlockdevOptionsFile',
> +      'https':      'BlockdevOptionsFile',
> +      'ftp':        'BlockdevOptionsFile',
> +      'ftps':       'BlockdevOptionsFile',
> +      'tftp':       'BlockdevOptionsFile',
> +# TODO gluster: Wait for structured options
> +# TODO iscsi: Wait for structured options
> +# TODO nbd: Should take InetSocketAddress for 'host'?
> +# TODO rbd: Wait for structured options
> +# TODO sheepdog: Wait for structured options
> +# TODO ssh: Should take InetSocketAddress for 'host'?
> +      'vvfat':      'BlockdevOptionsVVFAT',
> +
> +# TODO blkdebug: Wait for structured options
> +# TODO blkverify: Wait for structured options
> +
> +      'bochs':      'BlockdevOptionsGenericFormat',
> +      'cloop':      'BlockdevOptionsGenericFormat',
> +      'cow':        'BlockdevOptionsGenericCOWFormat',
> +      'dmg':        'BlockdevOptionsGenericFormat',
> +      'parallels':  'BlockdevOptionsGenericFormat',
> +      'qcow':       'BlockdevOptionsGenericCOWFormat',
> +      'qcow2':      'BlockdevOptionsQcow2',
> +      'qed':        'BlockdevOptionsGenericCOWFormat',
> +      'raw':        'BlockdevOptionsGenericFormat',
> +      'vdi':        'BlockdevOptionsGenericFormat',
> +      'vhdx':       'BlockdevOptionsGenericFormat',
> +      'vmdk':       'BlockdevOptionsGenericCOWFormat',
> +      'vpc':        'BlockdevOptionsGenericFormat'
> +  } }
> +
> +##
> +# @BlockdevRef
> +#
> +# Reference to a block device.
> +#
> +# @definition:      defines a new block device inline
> +# @reference:       references the ID of an existing block device. An
> +#                   empty string means that no block device should be
> +#                   referenced.
> +#
> +# Since: 1.7
> +##
> +{ 'union': 'BlockdevRef',
> +  'discriminator': {},
> +  'data': { 'definition': 'BlockdevOptions',
> +            'reference': 'str' } }
> +
> +##
> +# @blockdev-add:
> +#
> +# Creates a new block device.
> +#
> +# @options: block device options for the new device
> +#
> +# Since: 1.7
> +##
> +{ 'command': 'blockdev-add', 'data': { 'options': 'BlockdevOptions' } }
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index b17c46e..449ecea 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -3240,3 +3240,62 @@ Example:
>     }
>  
>  EQMP
> +
> +    {
> +        .name       = "blockdev-add",
> +        .args_type  = "options:q",
> +        .mhandler.cmd_new = qmp_marshal_input_blockdev_add,
> +    },
> +
> +SQMP
> +blockdev-add
> +------------
> +
> +Add a block device.
> +
> +Arguments:
> +
> +- "options": block driver options
> +
> +Example (1):
> +
> +-> { "execute": "blockdev-add",
> +    "arguments": { "options" : { "driver": "qcow2",
> +                                 "file": { "driver": "file",
> +                                           "filename": "test.qcow2" } } } }
> +<- { "return": {} }
> +
> +Example (2):
> +
> +-> { "execute": "blockdev-add",
> +     "arguments": {
> +         "options": {
> +           "driver": "qcow2",
> +           "id": "my_disk",
> +           "discard": "unmap",
> +           "throttling": {
> +               "bps-total": 1234567,
> +               "iops-write": 100
> +           },
> +           "cache": {
> +               "direct": true,
> +               "writeback": true
> +           },
> +           "file": {
> +               "driver": "file",
> +               "filename": "/tmp/test.qcow2"
> +           },
> +           "backing": {
> +               "driver": "raw",
> +               "file": {
> +                   "driver": "file",
> +                   "filename": "/dev/fdset/4"
> +               }
> +           }
> +         }
> +       }
> +     }
> +
> +<- { "return": {} }
> +
> +EQMP
> -- 
> 1.8.1.4
>

On 10/01/2013 07:20 AM, Kevin Wolf wrote:
> For examples see the changes to qmp-commands.hx.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  blockdev.c       |  57 ++++++++++++++
>  qapi-schema.json | 236 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  qmp-commands.hx  |  59 ++++++++++++++
>  3 files changed, 352 insertions(+)
> 

> +# @BlockdevOptionsVVFAT
> +#
> +# Driver specific block device options for the vvfat protocol.
> +#
> +# @dir:         directory to be exported as FAT image
> +# @fat-type:    #optional FAT type: 12, 16 or 32
> +# @floppy:      #optional whether to export a floppy image (true) or
> +#               partitioned hard disk (false; default)
> +# @rw:          #optional whether to allow write operations (default: false)

Aligned across multiple options...

> +# @BlockdevOptionsGenericFormat
> +#
> +# Driver specific block device options for image format that have no option
> +# besides their data source.
> +#
> +# @file:        reference to or definition of the data source block device

...nothing to compare to here (but aligned with other commands)...

> +# @BlockdevOptionsGenericCOWFormat
> +#
> +# Driver specific block device options for image format that have no option
> +# besides their data source and an optional backing file.
> +#
> +# @backing:     #optional reference to or definition of the backing file block
> +#               device (if missing, taken from the image file content). It is
> +#               allowed to pass an empty string here in order to disable the
> +#               default backing file.

...and again...

> +# Driver specific block device options for qcow2.
> +#
> +# @lazy-refcounts: #optional whether to enable the lazy refcounts feature
> +#                  (default is taken from the image file)
> +#
> +# @pass-discard-request: #optional whether discard requests to the qcow2 device
> +#                        should be forwarded to the data source

...but here, using exactly one space (different alignment per option).

I honestly don't care which style you use.  And while it looks odd
having two different styles in the same patch, it really doesn't matter
to me, so I'm not asking for a respin just for this (then again, you
already have a fix needed in the C code).

> +SQMP
> +blockdev-add
> +------------
> +
> +Add a block device.
> +
> +Arguments:
> +
> +- "options": block driver options
> +
> +Example (1):
> +
> +-> { "execute": "blockdev-add",
> +    "arguments": { "options" : { "driver": "qcow2",
> +                                 "file": { "driver": "file",
> +                                           "filename": "test.qcow2" } } } }
> +<- { "return": {} }
> +
> +Example (2):
> +
> +-> { "execute": "blockdev-add",
> +     "arguments": {
> +         "options": {
> +           "driver": "qcow2",
> +           "id": "my_disk",
> +           "discard": "unmap",
> +           "throttling": {
> +               "bps-total": 1234567,
> +               "iops-write": 100
> +           },

Drop throttling, as it is no longer in the qapi.