Re: [Qemu-devel] [PATCH 3/7] qcow: Support .bdrv_co_create
On Wed, Mar 14, 2018 at 06:16:18AM -0500, Eric Blake wrote: > On 03/09/2018 03:46 PM, Kevin Wolf wrote: > > This adds the .bdrv_co_create driver callback to qcow, which > > enables image creation over QMP. > > > > Signed-off-by: Kevin Wolf> > --- > > qapi/block-core.json | 21 +- > > block/qcow.c | 196 > > ++- > > 2 files changed, 150 insertions(+), 67 deletions(-) > > > ## > > +# @BlockdevCreateOptionsQcow: > > +# > > +# Driver specific image creation options for qcow. > > +# > > +# @file Node to create the image format on > > +# @size Size of the virtual disk in bytes > > +# @backing-file File name of the backing file if a backing file > > +# should be used > > +# @encrypt Encryption options if the image should be encrypted > > Idea for followup patch - we should strongly document that encryption of > qcow is discouraged as insecure, and/or mention that qcow2 is generally a > better option than qcow when creating images over QMP. Yes to the encryption note, but we should definitely document that 'qcow' is deprecated in general - there's little good reason you would want to use it - it has terrible performance when allocating new clusters. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH 3/7] qcow: Support .bdrv_co_create
On 03/09/2018 03:46 PM, Kevin Wolf wrote: This adds the .bdrv_co_create driver callback to qcow, which enables image creation over QMP. Signed-off-by: Kevin Wolf--- qapi/block-core.json | 21 +- block/qcow.c | 196 ++- 2 files changed, 150 insertions(+), 67 deletions(-) ## +# @BlockdevCreateOptionsQcow: +# +# Driver specific image creation options for qcow. +# +# @file Node to create the image format on +# @size Size of the virtual disk in bytes +# @backing-file File name of the backing file if a backing file +# should be used +# @encrypt Encryption options if the image should be encrypted Idea for followup patch - we should strongly document that encryption of qcow is discouraged as insecure, and/or mention that qcow2 is generally a better option than qcow when creating images over QMP. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH 3/7] qcow: Support .bdrv_co_create
On Fri, Mar 09, 2018 at 10:46:07PM +0100, Kevin Wolf wrote: > This adds the .bdrv_co_create driver callback to qcow, which > enables image creation over QMP. > > Signed-off-by: Kevin Wolf> --- > qapi/block-core.json | 21 +- > block/qcow.c | 196 > ++- > 2 files changed, 150 insertions(+), 67 deletions(-) > > diff --git a/qapi/block-core.json b/qapi/block-core.json > index d38058eeab..c81677c434 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -3497,6 +3497,25 @@ > '*cluster-size':'size' } } > > ## > +# @BlockdevCreateOptionsQcow: > +# > +# Driver specific image creation options for qcow. > +# > +# @file Node to create the image format on > +# @size Size of the virtual disk in bytes > +# @backing-file File name of the backing file if a backing file > +# should be used > +# @encrypt Encryption options if the image should be encrypted > +# > +# Since: 2.12 > +## > +{ 'struct': 'BlockdevCreateOptionsQcow', > + 'data': { 'file': 'BlockdevRef', > +'size': 'size', > +'*backing-file':'str', > +'*encrypt': 'QCryptoBlockCreateOptions' } } > + > +## > # @BlockdevQcow2Version: > # > # @v2: The original QCOW2 format as introduced in qemu 0.10 (version 2) > @@ -3681,8 +3700,8 @@ >'null-co':'BlockdevCreateNotSupported', >'nvme': 'BlockdevCreateNotSupported', >'parallels': 'BlockdevCreateOptionsParallels', > + 'qcow': 'BlockdevCreateOptionsQcow', >'qcow2': 'BlockdevCreateOptionsQcow2', > - 'qcow': 'BlockdevCreateNotSupported', >'qed':'BlockdevCreateNotSupported', >'quorum': 'BlockdevCreateNotSupported', >'raw':'BlockdevCreateNotSupported', > diff --git a/block/qcow.c b/block/qcow.c > index 47a18d9a3a..2e3770ca63 100644 > --- a/block/qcow.c > +++ b/block/qcow.c > @@ -33,6 +33,8 @@ > #include > #include "qapi/qmp/qdict.h" > #include "qapi/qmp/qstring.h" > +#include "qapi/qobject-input-visitor.h" > +#include "qapi/qapi-visit-block-core.h" > #include "crypto/block.h" > #include "migration/blocker.h" > #include "block/crypto.h" > @@ -86,6 +88,8 @@ typedef struct BDRVQcowState { > Error *migration_blocker; > } BDRVQcowState; > > +static QemuOptsList qcow_create_opts; > + > static int decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset); > > static int qcow_probe(const uint8_t *buf, int buf_size, const char *filename) > @@ -810,62 +814,50 @@ static void qcow_close(BlockDriverState *bs) > error_free(s->migration_blocker); > } > > -static int coroutine_fn qcow_co_create_opts(const char *filename, QemuOpts > *opts, > -Error **errp) > +static int coroutine_fn qcow_co_create(BlockdevCreateOptions *opts, > + Error **errp) > { > +BlockdevCreateOptionsQcow *qcow_opts; > int header_size, backing_filename_len, l1_size, shift, i; > QCowHeader header; > uint8_t *tmp; > int64_t total_size = 0; > -char *backing_file = NULL; > -Error *local_err = NULL; > int ret; > +BlockDriverState *bs; > BlockBackend *qcow_blk; > -char *encryptfmt = NULL; > -QDict *options; > -QDict *encryptopts = NULL; > -QCryptoBlockCreateOptions *crypto_opts = NULL; > QCryptoBlock *crypto = NULL; > > -/* Read out options */ > -total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0), > - BDRV_SECTOR_SIZE); > +assert(opts->driver == BLOCKDEV_DRIVER_QCOW); > +qcow_opts = >u.qcow; > + > +/* Sanity checks */ > +total_size = qcow_opts->size; > if (total_size == 0) { > error_setg(errp, "Image size is too small, cannot be zero length"); > -ret = -EINVAL; > -goto cleanup; > +return -EINVAL; > } > > -backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE); > -encryptfmt = qemu_opt_get_del(opts, BLOCK_OPT_ENCRYPT_FORMAT); > -if (encryptfmt) { > -if (qemu_opt_get(opts, BLOCK_OPT_ENCRYPT)) { > -error_setg(errp, "Options " BLOCK_OPT_ENCRYPT " and " > - BLOCK_OPT_ENCRYPT_FORMAT " are mutually exclusive"); > -ret = -EINVAL; > -goto cleanup; > -} > -} else if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) { > -encryptfmt = g_strdup("aes"); > +if (qcow_opts->has_encrypt && > +qcow_opts->encrypt->format != Q_CRYPTO_BLOCK_FORMAT_QCOW) > +{ > +error_setg(errp, "Unsupported encryption format"); > +return -EINVAL; > } > > -ret = bdrv_create_file(filename, opts, _err); > -if (ret < 0) { > -error_propagate(errp, local_err); > -
Re: [Qemu-devel] [PATCH 3/7] qcow: Support .bdrv_co_create
On 2018-03-09 22:46, Kevin Wolf wrote: > This adds the .bdrv_co_create driver callback to qcow, which > enables image creation over QMP. > > Signed-off-by: Kevin Wolf> --- > qapi/block-core.json | 21 +- > block/qcow.c | 196 > ++- > 2 files changed, 150 insertions(+), 67 deletions(-) Reviewed-by: Max Reitz (still not sure about the crumpling) signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 3/7] qcow: Support .bdrv_co_create
Am 09.03.2018 um 22:58 hat Eric Blake geschrieben: > On 03/09/2018 03:46 PM, Kevin Wolf wrote: > > This adds the .bdrv_co_create driver callback to qcow, which > > enables image creation over QMP. > > > > Signed-off-by: Kevin Wolf> > --- > > qapi/block-core.json | 21 +- > > block/qcow.c | 196 > > ++- > > 2 files changed, 150 insertions(+), 67 deletions(-) > > Pre-review question: do we REALLY want to support creation of new qcow > images from QMP? Or are we at the point where we want to declare qcow > a read-only format where we only support it to the extent that you can > convert an existing qcow file into a better supported format like > qcow2? I don't think we want read-only formats if it can be avoided, because we're in a much worse position to run tests then. The other option you mentioned in your reply to the qed patch, just not implementing .bdrv_co_create, but keeping the old callback, would mean that we'd be stuck in a half-converted state forever. My goal is to get rid of .bdrv_co_create_opts in the long run. And actually, qcow and qed were two of the simpler conversions where little remains to be done before the logic in .bdrv_co_create_opts can be generalised in block.c. So I'd just do the conversion. Kevin
Re: [Qemu-devel] [PATCH 3/7] qcow: Support .bdrv_co_create
On 03/09/2018 03:46 PM, Kevin Wolf wrote: This adds the .bdrv_co_create driver callback to qcow, which enables image creation over QMP. Signed-off-by: Kevin Wolf--- qapi/block-core.json | 21 +- block/qcow.c | 196 ++- 2 files changed, 150 insertions(+), 67 deletions(-) Pre-review question: do we REALLY want to support creation of new qcow images from QMP? Or are we at the point where we want to declare qcow a read-only format where we only support it to the extent that you can convert an existing qcow file into a better supported format like qcow2? -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
[Qemu-devel] [PATCH 3/7] qcow: Support .bdrv_co_create
This adds the .bdrv_co_create driver callback to qcow, which enables image creation over QMP. Signed-off-by: Kevin Wolf--- qapi/block-core.json | 21 +- block/qcow.c | 196 ++- 2 files changed, 150 insertions(+), 67 deletions(-) diff --git a/qapi/block-core.json b/qapi/block-core.json index d38058eeab..c81677c434 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -3497,6 +3497,25 @@ '*cluster-size':'size' } } ## +# @BlockdevCreateOptionsQcow: +# +# Driver specific image creation options for qcow. +# +# @file Node to create the image format on +# @size Size of the virtual disk in bytes +# @backing-file File name of the backing file if a backing file +# should be used +# @encrypt Encryption options if the image should be encrypted +# +# Since: 2.12 +## +{ 'struct': 'BlockdevCreateOptionsQcow', + 'data': { 'file': 'BlockdevRef', +'size': 'size', +'*backing-file':'str', +'*encrypt': 'QCryptoBlockCreateOptions' } } + +## # @BlockdevQcow2Version: # # @v2: The original QCOW2 format as introduced in qemu 0.10 (version 2) @@ -3681,8 +3700,8 @@ 'null-co':'BlockdevCreateNotSupported', 'nvme': 'BlockdevCreateNotSupported', 'parallels': 'BlockdevCreateOptionsParallels', + 'qcow': 'BlockdevCreateOptionsQcow', 'qcow2': 'BlockdevCreateOptionsQcow2', - 'qcow': 'BlockdevCreateNotSupported', 'qed':'BlockdevCreateNotSupported', 'quorum': 'BlockdevCreateNotSupported', 'raw':'BlockdevCreateNotSupported', diff --git a/block/qcow.c b/block/qcow.c index 47a18d9a3a..2e3770ca63 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -33,6 +33,8 @@ #include #include "qapi/qmp/qdict.h" #include "qapi/qmp/qstring.h" +#include "qapi/qobject-input-visitor.h" +#include "qapi/qapi-visit-block-core.h" #include "crypto/block.h" #include "migration/blocker.h" #include "block/crypto.h" @@ -86,6 +88,8 @@ typedef struct BDRVQcowState { Error *migration_blocker; } BDRVQcowState; +static QemuOptsList qcow_create_opts; + static int decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset); static int qcow_probe(const uint8_t *buf, int buf_size, const char *filename) @@ -810,62 +814,50 @@ static void qcow_close(BlockDriverState *bs) error_free(s->migration_blocker); } -static int coroutine_fn qcow_co_create_opts(const char *filename, QemuOpts *opts, -Error **errp) +static int coroutine_fn qcow_co_create(BlockdevCreateOptions *opts, + Error **errp) { +BlockdevCreateOptionsQcow *qcow_opts; int header_size, backing_filename_len, l1_size, shift, i; QCowHeader header; uint8_t *tmp; int64_t total_size = 0; -char *backing_file = NULL; -Error *local_err = NULL; int ret; +BlockDriverState *bs; BlockBackend *qcow_blk; -char *encryptfmt = NULL; -QDict *options; -QDict *encryptopts = NULL; -QCryptoBlockCreateOptions *crypto_opts = NULL; QCryptoBlock *crypto = NULL; -/* Read out options */ -total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0), - BDRV_SECTOR_SIZE); +assert(opts->driver == BLOCKDEV_DRIVER_QCOW); +qcow_opts = >u.qcow; + +/* Sanity checks */ +total_size = qcow_opts->size; if (total_size == 0) { error_setg(errp, "Image size is too small, cannot be zero length"); -ret = -EINVAL; -goto cleanup; +return -EINVAL; } -backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE); -encryptfmt = qemu_opt_get_del(opts, BLOCK_OPT_ENCRYPT_FORMAT); -if (encryptfmt) { -if (qemu_opt_get(opts, BLOCK_OPT_ENCRYPT)) { -error_setg(errp, "Options " BLOCK_OPT_ENCRYPT " and " - BLOCK_OPT_ENCRYPT_FORMAT " are mutually exclusive"); -ret = -EINVAL; -goto cleanup; -} -} else if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) { -encryptfmt = g_strdup("aes"); +if (qcow_opts->has_encrypt && +qcow_opts->encrypt->format != Q_CRYPTO_BLOCK_FORMAT_QCOW) +{ +error_setg(errp, "Unsupported encryption format"); +return -EINVAL; } -ret = bdrv_create_file(filename, opts, _err); -if (ret < 0) { -error_propagate(errp, local_err); -goto cleanup; +/* Create BlockBackend to write to the image */ +bs = bdrv_open_blockdev_ref(qcow_opts->file, errp); +if (bs == NULL) { +return -EIO; } -qcow_blk = blk_new_open(filename, NULL, NULL, -BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, -