[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 11/27] block: x-blockdev-create QMP command
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH 11/27] block: x-blockdev-create QMP command |
Date: |
Wed, 21 Feb 2018 11:29:54 +0100 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
Am 15.02.2018 um 20:58 hat Eric Blake geschrieben:
> On 02/08/2018 01:23 PM, Kevin Wolf wrote:
> > This adds a synchronous x-blockdev-create QMP command that can create
> > qcow2 images on a given node name.
> >
> > We don't want to block while creating an image, so this is not the final
> > interface in all aspects, but BlockdevCreateOptionsQcow2 and
> > .bdrv_co_create() are what they actually might look like in the end. In
> > any case, this should be good enough to test whether we interpret
> > BlockdevCreateOptions as we should.
> >
> > Signed-off-by: Kevin Wolf <address@hidden>
> > ---
>
> > @@ -3442,6 +3442,18 @@
> > } }
> > ##
> > +# @x-blockdev-create:
> > +#
> > +# Create an image format on a given node.
> > +# TODO Replace with something asynchronous (block job?)
>
> We've learned our lesson - don't commit to the final name on an interface
> that we have not yet experimented with.
>
> > +#
> > +# Since: 2.12
> > +##
> > +{ 'command': 'x-blockdev-create',
> > + 'data': 'BlockdevCreateOptions',
> > + 'boxed': true }
> > +
>
> Lots of code packed in that little description ;)
>
>
> > +++ b/include/block/block_int.h
> > @@ -130,6 +130,8 @@ struct BlockDriver {
> > int (*bdrv_file_open)(BlockDriverState *bs, QDict *options, int flags,
> > Error **errp);
> > void (*bdrv_close)(BlockDriverState *bs);
> > + int coroutine_fn (*bdrv_co_create)(BlockdevCreateOptions *opts,
> > + Error **errp);
>
> I know we haven't been very good in the past, but can you add a comment here
> on the contract that drivers are supposed to obey when implementing this
> callback?
Anything specific you want to see here?
Essentially the meaning of BlockdevCreateOptions depends on the driver
and is documented in the QAPI schema, how Error works is common
knowledge, and I don't see much else to explain here.
I mean, I can add something like "Creates an image. See the QAPI
documentation for BlockdevCreateOptions for details." if you think this
is useful. But is it?
> > + /* Call callback if it exists */
> > + if (!drv->bdrv_co_create) {
> > + error_setg(errp, "Driver does not support blockdev-create");
>
> Should this error message refer to 'x-blockdev-create' in the short term?
Hm, it would be more correct. On the other hand, I'm almost sure we'd
forget to rename it back when we remove the x- prefix...
Kevin
- [Qemu-devel] [PATCH 09/27] qdict: Introduce qdict_rename_keys(), (continued)
- [Qemu-devel] [PATCH 13/27] file-win32: Support .bdrv_co_create, Kevin Wolf, 2018/02/08
- [Qemu-devel] [PATCH 12/27] file-posix: Support .bdrv_co_create, Kevin Wolf, 2018/02/08
- [Qemu-devel] [PATCH 14/27] gluster: Support .bdrv_co_create, Kevin Wolf, 2018/02/08
- [Qemu-devel] [PATCH 15/27] rbd: Support .bdrv_co_create, Kevin Wolf, 2018/02/08
- [Qemu-devel] [PATCH 16/27] nfs: Use QAPI options in nfs_client_open(), Kevin Wolf, 2018/02/08