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 <kw...@redhat.com> > > --- > > > @@ -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