The following changes since commit 609ef9f451759151d0bfe7c3843410ab94d68f18: Merge remote-tracking branch 'remotes/berrange/tags/qio-next-pull-request' into staging (2018-06-28 17:53:31 +0100) are available in the git repository at: git://repo.or.cz/qemu/kevin.git tags/for-upstream for you to fetch changes up to 583c99d39368526dfb57a715b04a6ceea27dbe1e: block: Remove unused sector-based vectored I/O (2018-06-29 14:20:56 +0200) ---------------------------------------------------------------- Block layer patches: - Make truncate operations asynchronous (so that preallocation in blockdev-create doesn't block the main loop any more) - usb-storage: Add rerror/werror properties - nvme: Add num_queues property - qemu-img convert: Copy offloading fixes (including data corruption fix) - qcow2: Fix cluster leak on temporary write error - Use byte-based functions instead of bdrv_co_readv/writev() - Various small fixes and cleanups ---------------------------------------------------------------- Eric Blake (8): parallels: Switch to byte-based calls qcow: Switch get_cluster_offset to be byte-based qcow: Switch qcow_co_readv to byte-based calls qcow: Switch qcow_co_writev to byte-based calls qcow: Switch to a byte-based driver replication: Switch to byte-based calls vhdx: Switch to byte-based calls block: Remove unused sector-based vectored I/O Fam Zheng (5): qcow2: Remove dead check on !ret block: Move request tracking to children in copy offloading qcow2: Fix src_offset in copy offloading iscsi: Don't blindly use designator length in response for memcpy file-posix: Fix EINTR handling Kevin Wolf (12): qapi/job: The next release will be 3.0 usb-storage: Add rerror/werror properties qcow2: Fix qcow2_truncate() error return value block: Convert .bdrv_truncate callback to coroutine_fn qcow2: Remove coroutine trampoline for preallocate_co() block: Move bdrv_truncate() implementation to io.c block: Use tracked request for truncate file-posix: Make .bdrv_co_truncate asynchronous qemu-iotests: Update 026.out.nocache reference output qcow2: Free allocated clusters on write error qemu-iotests: Test qcow2 not leaking clusters on write error file-posix: Implement co versions of discard/flush Markus Armbruster (3): block-qdict: Pacify Coverity after commit f1b34a248e9 block/crypto: Pacify Coverity after commit f853465aacb block/crypto: Simplify block_crypto_{open,create}_opts_init() Weiping Zhang (1): hw/block/nvme: add optional parameter num_queues for nvme device qapi/job.json | 18 +- block/crypto.h | 8 +- block/qcow2.h | 1 + include/block/block.h | 8 +- include/block/block_int.h | 7 +- include/block/raw-aio.h | 4 +- include/hw/scsi/scsi.h | 2 + block.c | 64 +------ block/copy-on-read.c | 8 +- block/crypto.c | 112 +++-------- block/file-posix.c | 367 +++++++++++++++++++------------------ block/file-win32.c | 6 +- block/gluster.c | 14 +- block/io.c | 219 +++++++++++++++------- block/iscsi.c | 10 +- block/nfs.c | 7 +- block/parallels.c | 16 +- block/qcow.c | 135 +++++++------- block/qcow2-cluster.c | 11 ++ block/qcow2.c | 140 ++++++-------- block/qed.c | 8 +- block/raw-format.c | 8 +- block/rbd.c | 8 +- block/replication.c | 14 +- block/sheepdog.c | 12 +- block/ssh.c | 6 +- block/vhdx.c | 12 +- hw/block/nvme.c | 5 +- hw/scsi/scsi-bus.c | 11 +- hw/usb/dev-storage.c | 2 + qobject/block-qdict.c | 16 +- tests/qemu-iotests/026 | 17 ++ tests/qemu-iotests/026.out | 8 + tests/qemu-iotests/026.out.nocache | 14 +- tests/qemu-iotests/063 | 9 + tests/qemu-iotests/063.out | 12 ++ 36 files changed, 685 insertions(+), 634 deletions(-) From: Markus Armbruster <armbru@redhat.com> Commit f1b34a248e9 replaced less-than-obvious test in qdict_flatten_qdict() by the obvious one. Sadly, it made something else non-obvious: the fact that @new_key passed to qdict_put_obj() can't be null, because that depends on the function's precondition (target == qdict) == !prefix. Tweak the function some more to help Coverity and human readers alike. Fixes: CID 1393620 Signed-off-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- qobject/block-qdict.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/qobject/block-qdict.c b/qobject/block-qdict.c index XXXXXXX..XXXXXXX 100644 --- a/qobject/block-qdict.c +++ b/qobject/block-qdict.c @@ -XXX,XX +XXX,XX @@ static void qdict_flatten_qdict(QDict *qdict, QDict *target, const char *prefix) const QDictEntry *entry, *next; QDict *dict_val; QList *list_val; - char *new_key; + char *key, *new_key; entry = qdict_first(qdict); @@ -XXX,XX +XXX,XX @@ static void qdict_flatten_qdict(QDict *qdict, QDict *target, const char *prefix) value = qdict_entry_value(entry); dict_val = qobject_to(QDict, value); list_val = qobject_to(QList, value); - new_key = NULL; if (prefix) { - new_key = g_strdup_printf("%s.%s", prefix, entry->key); + key = new_key = g_strdup_printf("%s.%s", prefix, entry->key); + } else { + key = entry->key; + new_key = NULL; } /* @@ -XXX,XX +XXX,XX @@ static void qdict_flatten_qdict(QDict *qdict, QDict *target, const char *prefix) * well advised not to modify them altogether.) */ if (dict_val && qdict_size(dict_val)) { - qdict_flatten_qdict(dict_val, target, - new_key ? new_key : entry->key); + qdict_flatten_qdict(dict_val, target, key); if (target == qdict) { qdict_del(qdict, entry->key); } } else if (list_val && !qlist_empty(list_val)) { - qdict_flatten_qlist(list_val, target, - new_key ? new_key : entry->key); + qdict_flatten_qlist(list_val, target, key); if (target == qdict) { qdict_del(qdict, entry->key); } } else if (target != qdict) { - qdict_put_obj(target, new_key, qobject_ref(value)); + qdict_put_obj(target, key, qobject_ref(value)); } g_free(new_key); -- 2.13.6 From: Markus Armbruster <armbru@redhat.com> Coverity can't see that qobject_input_visitor_new_flat_confused() returns non-null when it doesn't set @local_err. Check the return value instead, like all the other callers do. Fixes: CID 1393615 Fixes: CID 1393616 Signed-off-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block/crypto.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/block/crypto.c b/block/crypto.c index XXXXXXX..XXXXXXX 100644 --- a/block/crypto.c +++ b/block/crypto.c @@ -XXX,XX +XXX,XX @@ block_crypto_open_opts_init(QCryptoBlockFormat format, ret->format = format; v = qobject_input_visitor_new_flat_confused(opts, &local_err); - if (local_err) { + if (!v) { goto out; } @@ -XXX,XX +XXX,XX @@ block_crypto_create_opts_init(QCryptoBlockFormat format, ret->format = format; v = qobject_input_visitor_new_flat_confused(opts, &local_err); - if (local_err) { + if (!v) { goto out; } -- 2.13.6 Commit 51f63ec7d tried to change all references to 2.13 into 3.0, but it failed to achieve this because it was not properly rebased on top of the series introducing qapi/job.json. Change the references now. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- qapi/job.json | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/qapi/job.json b/qapi/job.json index XXXXXXX..XXXXXXX 100644 --- a/qapi/job.json +++ b/qapi/job.json @@ -XXX,XX +XXX,XX @@ # @id: The job identifier # @status: The new job status # -# Since: 2.13 +# Since: 3.0 ## { 'event': 'JOB_STATUS_CHANGE', 'data': { 'id': 'str', @@ -XXX,XX +XXX,XX @@ # # @id: The job identifier. # -# Since: 2.13 +# Since: 3.0 ## { 'command': 'job-pause', 'data': { 'id': 'str' } } @@ -XXX,XX +XXX,XX @@ # # @id : The job identifier. # -# Since: 2.13 +# Since: 3.0 ## { 'command': 'job-resume', 'data': { 'id': 'str' } } @@ -XXX,XX +XXX,XX @@ # # @id: The job identifier. # -# Since: 2.13 +# Since: 3.0 ## { 'command': 'job-cancel', 'data': { 'id': 'str' } } @@ -XXX,XX +XXX,XX @@ # # @id: The job identifier. # -# Since: 2.13 +# Since: 3.0 ## { 'command': 'job-complete', 'data': { 'id': 'str' } } @@ -XXX,XX +XXX,XX @@ # # @id: The job identifier. # -# Since: 2.13 +# Since: 3.0 ## { 'command': 'job-dismiss', 'data': { 'id': 'str' } } @@ -XXX,XX +XXX,XX @@ # @id: The identifier of any job in the transaction, or of a job that is not # part of any transaction. # -# Since: 2.13 +# Since: 3.0 ## { 'command': 'job-finalize', 'data': { 'id': 'str' } } @@ -XXX,XX +XXX,XX @@ # the reason for the job failure. It should not be parsed # by applications. # -# Since: 2.13 +# Since: 3.0 ## { 'struct': 'JobInfo', 'data': { 'id': 'str', 'type': 'JobType', 'status': 'JobStatus', @@ -XXX,XX +XXX,XX @@ # # Returns: a list with a @JobInfo for each active job # -# Since: 2.13 +# Since: 3.0 ## { 'command': 'query-jobs', 'returns': ['JobInfo'] } -- 2.13.6 The error handling policy was traditionally set with -drive, but with -blockdev it is no longer possible to set frontend options. scsi-disk (and other block devices) have long supported qdev properties to configure the error handling policy, so let's add these options to usb-storage as well and just forward them to the internal scsi-disk instance. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> --- include/hw/scsi/scsi.h | 2 ++ hw/scsi/scsi-bus.c | 11 ++++++++++- hw/usb/dev-storage.c | 2 ++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/scsi/scsi.h +++ b/include/hw/scsi/scsi.h @@ -XXX,XX +XXX,XX @@ static inline SCSIBus *scsi_bus_from_device(SCSIDevice *d) SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockBackend *blk, int unit, bool removable, int bootindex, bool share_rw, + BlockdevOnError rerror, + BlockdevOnError werror, const char *serial, Error **errp); void scsi_bus_legacy_handle_cmdline(SCSIBus *bus); void scsi_legacy_handle_cmdline(void); diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c index XXXXXXX..XXXXXXX 100644 --- a/hw/scsi/scsi-bus.c +++ b/hw/scsi/scsi-bus.c @@ -XXX,XX +XXX,XX @@ static void scsi_qdev_unrealize(DeviceState *qdev, Error **errp) SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockBackend *blk, int unit, bool removable, int bootindex, bool share_rw, + BlockdevOnError rerror, + BlockdevOnError werror, const char *serial, Error **errp) { const char *driver; @@ -XXX,XX +XXX,XX @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockBackend *blk, object_unparent(OBJECT(dev)); return NULL; } + + qdev_prop_set_enum(dev, "rerror", rerror); + qdev_prop_set_enum(dev, "werror", werror); + object_property_set_bool(OBJECT(dev), true, "realized", &err); if (err != NULL) { error_propagate(errp, err); @@ -XXX,XX +XXX,XX @@ void scsi_bus_legacy_handle_cmdline(SCSIBus *bus) } qemu_opts_loc_restore(dinfo->opts); scsi_bus_legacy_add_drive(bus, blk_by_legacy_dinfo(dinfo), - unit, false, -1, false, NULL, &error_fatal); + unit, false, -1, false, + BLOCKDEV_ON_ERROR_AUTO, + BLOCKDEV_ON_ERROR_AUTO, + NULL, &error_fatal); } loc_pop(&loc); } diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c index XXXXXXX..XXXXXXX 100644 --- a/hw/usb/dev-storage.c +++ b/hw/usb/dev-storage.c @@ -XXX,XX +XXX,XX @@ static void usb_msd_storage_realize(USBDevice *dev, Error **errp) &usb_msd_scsi_info_storage, NULL); scsi_dev = scsi_bus_legacy_add_drive(&s->bus, blk, 0, !!s->removable, s->conf.bootindex, s->conf.share_rw, + s->conf.rerror, s->conf.werror, dev->serial, errp); blk_unref(blk); @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_usb_msd = { static Property msd_properties[] = { DEFINE_BLOCK_PROPERTIES(MSDState, conf), + DEFINE_BLOCK_ERROR_PROPERTIES(MSDState, conf), DEFINE_PROP_BIT("removable", MSDState, removable, 0, false), DEFINE_PROP_END_OF_LIST(), }; -- 2.13.6 From: Weiping Zhang <zhangweiping@didichuxing.com> Add an optional paramter num_queues for device, and set it to 64 by default. Signed-off-by: Weiping Zhang <zhangweiping@didichuxing.com> Acked-by: Keith Busch <keith.busch@intel.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- hw/block/nvme.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index XXXXXXX..XXXXXXX 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -XXX,XX +XXX,XX @@ * Usage: add options: * -drive file=<file>,if=none,id=<drive_id> * -device nvme,drive=<drive_id>,serial=<serial>,id=<id[optional]>, \ - * cmb_size_mb=<cmb_size_mb[optional]> + * cmb_size_mb=<cmb_size_mb[optional]>, \ + * num_queues=<N[optional]> * * Note cmb_size_mb denotes size of CMB in MB. CMB is assumed to be at * offset 0 in BAR2 and supports only WDS, RDS and SQS for now. @@ -XXX,XX +XXX,XX @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp) pcie_endpoint_cap_init(&n->parent_obj, 0x80); n->num_namespaces = 1; - n->num_queues = 64; n->reg_size = pow2ceil(0x1004 + 2 * (n->num_queues + 1) * 4); n->ns_size = bs_size / (uint64_t)n->num_namespaces; @@ -XXX,XX +XXX,XX @@ static Property nvme_props[] = { DEFINE_BLOCK_PROPERTIES(NvmeCtrl, conf), DEFINE_PROP_STRING("serial", NvmeCtrl, serial), DEFINE_PROP_UINT32("cmb_size_mb", NvmeCtrl, cmb_size_mb, 0), + DEFINE_PROP_UINT32("num_queues", NvmeCtrl, num_queues, 64), DEFINE_PROP_END_OF_LIST(), }; -- 2.13.6 If qcow2_alloc_clusters_at() returns an error, we do need to negate it to get back the positive errno code for error_setg_errno(), but we still need to return the negative error code. Fixes: 772d1f973f87269f6a4a4ea4b880680f3779bbdf Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> --- block/qcow2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/qcow2.c b/block/qcow2.c index XXXXXXX..XXXXXXX 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -XXX,XX +XXX,XX @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset, if (clusters_allocated < 0) { error_setg_errno(errp, -clusters_allocated, "Failed to allocate data clusters"); - return -clusters_allocated; + return clusters_allocated; } assert(clusters_allocated == nb_new_data_clusters); -- 2.13.6 bdrv_truncate() is an operation that can block (even for a quite long time, depending on the PreallocMode) in I/O paths that shouldn't block. Convert it to a coroutine_fn so that we have the infrastructure for drivers to make their .bdrv_co_truncate implementation asynchronous. This change could potentially introduce new race conditions because bdrv_truncate() isn't necessarily executed atomically any more. Whether this is a problem needs to be evaluated for each block driver that supports truncate: * file-posix/win32, gluster, iscsi, nfs, rbd, ssh, sheepdog: The protocol drivers are trivially safe because they don't actually yield yet, so there is no change in behaviour. * copy-on-read, crypto, raw-format: Essentially just filter drivers that pass the request to a child node, no problem. * qcow2: The implementation modifies metadata, so it needs to hold s->lock to be safe with concurrent I/O requests. In order to avoid double locking, this requires pulling the locking out into preallocate_co() and using qcow2_write_caches() instead of bdrv_flush(). * qed: Does a single header update, this is fine without locking. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> --- include/block/block.h | 4 +++ include/block/block_int.h | 4 +-- block.c | 63 +++++++++++++++++++++++++++++++++++----- block/copy-on-read.c | 8 ++--- block/crypto.c | 9 +++--- block/file-posix.c | 12 ++++---- block/file-win32.c | 6 ++-- block/gluster.c | 14 +++++---- block/iscsi.c | 8 ++--- block/nfs.c | 7 +++-- block/qcow2.c | 74 ++++++++++++++++++++++++++++------------------- block/qed.c | 8 +++-- block/raw-format.c | 8 ++--- block/rbd.c | 8 +++-- block/sheepdog.c | 12 ++++---- block/ssh.c | 6 ++-- 16 files changed, 162 insertions(+), 89 deletions(-) diff --git a/include/block/block.h b/include/block/block.h index XXXXXXX..XXXXXXX 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_pwrite_zeroes(BdrvChild *child, int64_t offset, BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs, const char *backing_file); void bdrv_refresh_filename(BlockDriverState *bs); + +int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, + PreallocMode prealloc, Error **errp); int bdrv_truncate(BdrvChild *child, int64_t offset, PreallocMode prealloc, Error **errp); + int64_t bdrv_nb_sectors(BlockDriverState *bs); int64_t bdrv_getlength(BlockDriverState *bs); int64_t bdrv_get_allocated_file_size(BlockDriverState *bs); diff --git a/include/block/block_int.h b/include/block/block_int.h index XXXXXXX..XXXXXXX 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -XXX,XX +XXX,XX @@ struct BlockDriver { * bdrv_parse_filename. */ const char *protocol_name; - int (*bdrv_truncate)(BlockDriverState *bs, int64_t offset, - PreallocMode prealloc, Error **errp); + int coroutine_fn (*bdrv_co_truncate)(BlockDriverState *bs, int64_t offset, + PreallocMode prealloc, Error **errp); int64_t (*bdrv_getlength)(BlockDriverState *bs); bool has_variable_length; diff --git a/block.c b/block.c index XXXXXXX..XXXXXXX 100644 --- a/block.c +++ b/block.c @@ -XXX,XX +XXX,XX @@ exit: /** * Truncate file to 'offset' bytes (needed only for file protocols) */ -int bdrv_truncate(BdrvChild *child, int64_t offset, PreallocMode prealloc, - Error **errp) +int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, + PreallocMode prealloc, Error **errp) { BlockDriverState *bs = child->bs; BlockDriver *drv = bs->drv; @@ -XXX,XX +XXX,XX @@ int bdrv_truncate(BdrvChild *child, int64_t offset, PreallocMode prealloc, return -EINVAL; } - if (!drv->bdrv_truncate) { + bdrv_inc_in_flight(bs); + + if (!drv->bdrv_co_truncate) { if (bs->file && drv->is_filter) { - return bdrv_truncate(bs->file, offset, prealloc, errp); + ret = bdrv_co_truncate(bs->file, offset, prealloc, errp); + goto out; } error_setg(errp, "Image format driver does not support resize"); - return -ENOTSUP; + ret = -ENOTSUP; + goto out; } if (bs->read_only) { error_setg(errp, "Image is read-only"); - return -EACCES; + ret = -EACCES; + goto out; } assert(!(bs->open_flags & BDRV_O_INACTIVE)); - ret = drv->bdrv_truncate(bs, offset, prealloc, errp); + ret = drv->bdrv_co_truncate(bs, offset, prealloc, errp); if (ret < 0) { - return ret; + goto out; } ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS); if (ret < 0) { @@ -XXX,XX +XXX,XX @@ int bdrv_truncate(BdrvChild *child, int64_t offset, PreallocMode prealloc, bdrv_dirty_bitmap_truncate(bs, offset); bdrv_parent_cb_resize(bs); atomic_inc(&bs->write_gen); + +out: + bdrv_dec_in_flight(bs); return ret; } +typedef struct TruncateCo { + BdrvChild *child; + int64_t offset; + PreallocMode prealloc; + Error **errp; + int ret; +} TruncateCo; + +static void coroutine_fn bdrv_truncate_co_entry(void *opaque) +{ + TruncateCo *tco = opaque; + tco->ret = bdrv_co_truncate(tco->child, tco->offset, tco->prealloc, + tco->errp); +} + +int bdrv_truncate(BdrvChild *child, int64_t offset, PreallocMode prealloc, + Error **errp) +{ + Coroutine *co; + TruncateCo tco = { + .child = child, + .offset = offset, + .prealloc = prealloc, + .errp = errp, + .ret = NOT_DONE, + }; + + if (qemu_in_coroutine()) { + /* Fast-path if already in coroutine context */ + bdrv_truncate_co_entry(&tco); + } else { + co = qemu_coroutine_create(bdrv_truncate_co_entry, &tco); + qemu_coroutine_enter(co); + BDRV_POLL_WHILE(child->bs, tco.ret == NOT_DONE); + } + + return tco.ret; +} + /** * Length of a allocated file in bytes. Sparse files are counted by actual * allocated space. Return < 0 if error or unknown. diff --git a/block/copy-on-read.c b/block/copy-on-read.c index XXXXXXX..XXXXXXX 100644 --- a/block/copy-on-read.c +++ b/block/copy-on-read.c @@ -XXX,XX +XXX,XX @@ static int64_t cor_getlength(BlockDriverState *bs) } -static int cor_truncate(BlockDriverState *bs, int64_t offset, - PreallocMode prealloc, Error **errp) +static int coroutine_fn cor_co_truncate(BlockDriverState *bs, int64_t offset, + PreallocMode prealloc, Error **errp) { - return bdrv_truncate(bs->file, offset, prealloc, errp); + return bdrv_co_truncate(bs->file, offset, prealloc, errp); } @@ -XXX,XX +XXX,XX @@ BlockDriver bdrv_copy_on_read = { .bdrv_child_perm = cor_child_perm, .bdrv_getlength = cor_getlength, - .bdrv_truncate = cor_truncate, + .bdrv_co_truncate = cor_co_truncate, .bdrv_co_preadv = cor_co_preadv, .bdrv_co_pwritev = cor_co_pwritev, diff --git a/block/crypto.c b/block/crypto.c index XXXXXXX..XXXXXXX 100644 --- a/block/crypto.c +++ b/block/crypto.c @@ -XXX,XX +XXX,XX @@ static int block_crypto_co_create_generic(BlockDriverState *bs, return ret; } -static int block_crypto_truncate(BlockDriverState *bs, int64_t offset, - PreallocMode prealloc, Error **errp) +static int coroutine_fn +block_crypto_co_truncate(BlockDriverState *bs, int64_t offset, + PreallocMode prealloc, Error **errp) { BlockCrypto *crypto = bs->opaque; uint64_t payload_offset = @@ -XXX,XX +XXX,XX @@ static int block_crypto_truncate(BlockDriverState *bs, int64_t offset, offset += payload_offset; - return bdrv_truncate(bs->file, offset, prealloc, errp); + return bdrv_co_truncate(bs->file, offset, prealloc, errp); } static void block_crypto_close(BlockDriverState *bs) @@ -XXX,XX +XXX,XX @@ BlockDriver bdrv_crypto_luks = { .bdrv_child_perm = bdrv_format_default_perms, .bdrv_co_create = block_crypto_co_create_luks, .bdrv_co_create_opts = block_crypto_co_create_opts_luks, - .bdrv_truncate = block_crypto_truncate, + .bdrv_co_truncate = block_crypto_co_truncate, .create_opts = &block_crypto_create_opts_luks, .bdrv_reopen_prepare = block_crypto_reopen_prepare, diff --git a/block/file-posix.c b/block/file-posix.c index XXXXXXX..XXXXXXX 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -XXX,XX +XXX,XX @@ out: return result; } -static int raw_truncate(BlockDriverState *bs, int64_t offset, - PreallocMode prealloc, Error **errp) +static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset, + PreallocMode prealloc, Error **errp) { BDRVRawState *s = bs->opaque; struct stat st; @@ -XXX,XX +XXX,XX @@ BlockDriver bdrv_file = { .bdrv_io_unplug = raw_aio_unplug, .bdrv_attach_aio_context = raw_aio_attach_aio_context, - .bdrv_truncate = raw_truncate, + .bdrv_co_truncate = raw_co_truncate, .bdrv_getlength = raw_getlength, .bdrv_get_info = raw_get_info, .bdrv_get_allocated_file_size @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_host_device = { .bdrv_io_plug = raw_aio_plug, .bdrv_io_unplug = raw_aio_unplug, - .bdrv_truncate = raw_truncate, + .bdrv_co_truncate = raw_co_truncate, .bdrv_getlength = raw_getlength, .bdrv_get_info = raw_get_info, .bdrv_get_allocated_file_size @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_host_cdrom = { .bdrv_io_plug = raw_aio_plug, .bdrv_io_unplug = raw_aio_unplug, - .bdrv_truncate = raw_truncate, + .bdrv_co_truncate = raw_co_truncate, .bdrv_getlength = raw_getlength, .has_variable_length = true, .bdrv_get_allocated_file_size @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_host_cdrom = { .bdrv_io_plug = raw_aio_plug, .bdrv_io_unplug = raw_aio_unplug, - .bdrv_truncate = raw_truncate, + .bdrv_co_truncate = raw_co_truncate, .bdrv_getlength = raw_getlength, .has_variable_length = true, .bdrv_get_allocated_file_size diff --git a/block/file-win32.c b/block/file-win32.c index XXXXXXX..XXXXXXX 100644 --- a/block/file-win32.c +++ b/block/file-win32.c @@ -XXX,XX +XXX,XX @@ static void raw_close(BlockDriverState *bs) } } -static int raw_truncate(BlockDriverState *bs, int64_t offset, - PreallocMode prealloc, Error **errp) +static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset, + PreallocMode prealloc, Error **errp) { BDRVRawState *s = bs->opaque; LONG low, high; @@ -XXX,XX +XXX,XX @@ BlockDriver bdrv_file = { .bdrv_aio_pwritev = raw_aio_pwritev, .bdrv_aio_flush = raw_aio_flush, - .bdrv_truncate = raw_truncate, + .bdrv_co_truncate = raw_co_truncate, .bdrv_getlength = raw_getlength, .bdrv_get_allocated_file_size = raw_get_allocated_file_size, diff --git a/block/gluster.c b/block/gluster.c index XXXXXXX..XXXXXXX 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -XXX,XX +XXX,XX @@ static coroutine_fn int qemu_gluster_co_rw(BlockDriverState *bs, return acb.ret; } -static int qemu_gluster_truncate(BlockDriverState *bs, int64_t offset, - PreallocMode prealloc, Error **errp) +static coroutine_fn int qemu_gluster_co_truncate(BlockDriverState *bs, + int64_t offset, + PreallocMode prealloc, + Error **errp) { BDRVGlusterState *s = bs->opaque; return qemu_gluster_do_truncate(s->fd, offset, prealloc, errp); @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_gluster = { .bdrv_co_create_opts = qemu_gluster_co_create_opts, .bdrv_getlength = qemu_gluster_getlength, .bdrv_get_allocated_file_size = qemu_gluster_allocated_file_size, - .bdrv_truncate = qemu_gluster_truncate, + .bdrv_co_truncate = qemu_gluster_co_truncate, .bdrv_co_readv = qemu_gluster_co_readv, .bdrv_co_writev = qemu_gluster_co_writev, .bdrv_co_flush_to_disk = qemu_gluster_co_flush_to_disk, @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_gluster_tcp = { .bdrv_co_create_opts = qemu_gluster_co_create_opts, .bdrv_getlength = qemu_gluster_getlength, .bdrv_get_allocated_file_size = qemu_gluster_allocated_file_size, - .bdrv_truncate = qemu_gluster_truncate, + .bdrv_co_truncate = qemu_gluster_co_truncate, .bdrv_co_readv = qemu_gluster_co_readv, .bdrv_co_writev = qemu_gluster_co_writev, .bdrv_co_flush_to_disk = qemu_gluster_co_flush_to_disk, @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_gluster_unix = { .bdrv_co_create_opts = qemu_gluster_co_create_opts, .bdrv_getlength = qemu_gluster_getlength, .bdrv_get_allocated_file_size = qemu_gluster_allocated_file_size, - .bdrv_truncate = qemu_gluster_truncate, + .bdrv_co_truncate = qemu_gluster_co_truncate, .bdrv_co_readv = qemu_gluster_co_readv, .bdrv_co_writev = qemu_gluster_co_writev, .bdrv_co_flush_to_disk = qemu_gluster_co_flush_to_disk, @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_gluster_rdma = { .bdrv_co_create_opts = qemu_gluster_co_create_opts, .bdrv_getlength = qemu_gluster_getlength, .bdrv_get_allocated_file_size = qemu_gluster_allocated_file_size, - .bdrv_truncate = qemu_gluster_truncate, + .bdrv_co_truncate = qemu_gluster_co_truncate, .bdrv_co_readv = qemu_gluster_co_readv, .bdrv_co_writev = qemu_gluster_co_writev, .bdrv_co_flush_to_disk = qemu_gluster_co_flush_to_disk, diff --git a/block/iscsi.c b/block/iscsi.c index XXXXXXX..XXXXXXX 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -XXX,XX +XXX,XX @@ static void iscsi_reopen_commit(BDRVReopenState *reopen_state) } } -static int iscsi_truncate(BlockDriverState *bs, int64_t offset, - PreallocMode prealloc, Error **errp) +static int coroutine_fn iscsi_co_truncate(BlockDriverState *bs, int64_t offset, + PreallocMode prealloc, Error **errp) { IscsiLun *iscsilun = bs->opaque; Error *local_err = NULL; @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_iscsi = { .bdrv_getlength = iscsi_getlength, .bdrv_get_info = iscsi_get_info, - .bdrv_truncate = iscsi_truncate, + .bdrv_co_truncate = iscsi_co_truncate, .bdrv_refresh_limits = iscsi_refresh_limits, .bdrv_co_block_status = iscsi_co_block_status, @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_iser = { .bdrv_getlength = iscsi_getlength, .bdrv_get_info = iscsi_get_info, - .bdrv_truncate = iscsi_truncate, + .bdrv_co_truncate = iscsi_co_truncate, .bdrv_refresh_limits = iscsi_refresh_limits, .bdrv_co_block_status = iscsi_co_block_status, diff --git a/block/nfs.c b/block/nfs.c index XXXXXXX..XXXXXXX 100644 --- a/block/nfs.c +++ b/block/nfs.c @@ -XXX,XX +XXX,XX @@ static int64_t nfs_get_allocated_file_size(BlockDriverState *bs) return (task.ret < 0 ? task.ret : st.st_blocks * 512); } -static int nfs_file_truncate(BlockDriverState *bs, int64_t offset, - PreallocMode prealloc, Error **errp) +static int coroutine_fn +nfs_file_co_truncate(BlockDriverState *bs, int64_t offset, + PreallocMode prealloc, Error **errp) { NFSClient *client = bs->opaque; int ret; @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_nfs = { .bdrv_has_zero_init = nfs_has_zero_init, .bdrv_get_allocated_file_size = nfs_get_allocated_file_size, - .bdrv_truncate = nfs_file_truncate, + .bdrv_co_truncate = nfs_file_co_truncate, .bdrv_file_open = nfs_file_open, .bdrv_close = nfs_file_close, diff --git a/block/qcow2.c b/block/qcow2.c index XXXXXXX..XXXXXXX 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -XXX,XX +XXX,XX @@ static void coroutine_fn preallocate_co(void *opaque) BlockDriverState *bs = params->bs; uint64_t offset = params->offset; uint64_t new_length = params->new_length; - BDRVQcow2State *s = bs->opaque; uint64_t bytes; uint64_t host_offset = 0; unsigned int cur_bytes; int ret; QCowL2Meta *meta; - qemu_co_mutex_lock(&s->lock); - assert(offset <= new_length); bytes = new_length - offset; @@ -XXX,XX +XXX,XX @@ static void coroutine_fn preallocate_co(void *opaque) ret = 0; done: - qemu_co_mutex_unlock(&s->lock); params->ret = ret; } @@ -XXX,XX +XXX,XX @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp) /* And if we're supposed to preallocate metadata, do that now */ if (qcow2_opts->preallocation != PREALLOC_MODE_OFF) { + BDRVQcow2State *s = blk_bs(blk)->opaque; + qemu_co_mutex_lock(&s->lock); ret = preallocate(blk_bs(blk), 0, qcow2_opts->size); + qemu_co_mutex_unlock(&s->lock); + if (ret < 0) { error_setg_errno(errp, -ret, "Could not preallocate metadata"); goto out; @@ -XXX,XX +XXX,XX @@ fail: return ret; } -static int qcow2_truncate(BlockDriverState *bs, int64_t offset, - PreallocMode prealloc, Error **errp) +static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset, + PreallocMode prealloc, Error **errp) { BDRVQcow2State *s = bs->opaque; uint64_t old_length; @@ -XXX,XX +XXX,XX @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset, return -EINVAL; } + qemu_co_mutex_lock(&s->lock); + /* cannot proceed if image has snapshots */ if (s->nb_snapshots) { error_setg(errp, "Can't resize an image which has snapshots"); - return -ENOTSUP; + ret = -ENOTSUP; + goto fail; } /* cannot proceed if image has bitmaps */ if (s->nb_bitmaps) { /* TODO: resize bitmaps in the image */ error_setg(errp, "Can't resize an image which has bitmaps"); - return -ENOTSUP; + ret = -ENOTSUP; + goto fail; } old_length = bs->total_sectors * 512; @@ -XXX,XX +XXX,XX @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset, if (prealloc != PREALLOC_MODE_OFF) { error_setg(errp, "Preallocation can't be used for shrinking an image"); - return -EINVAL; + ret = -EINVAL; + goto fail; } ret = qcow2_cluster_discard(bs, ROUND_UP(offset, s->cluster_size), @@ -XXX,XX +XXX,XX @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset, QCOW2_DISCARD_ALWAYS, true); if (ret < 0) { error_setg_errno(errp, -ret, "Failed to discard cropped clusters"); - return ret; + goto fail; } ret = qcow2_shrink_l1_table(bs, new_l1_size); if (ret < 0) { error_setg_errno(errp, -ret, "Failed to reduce the number of L2 tables"); - return ret; + goto fail; } ret = qcow2_shrink_reftable(bs); if (ret < 0) { error_setg_errno(errp, -ret, "Failed to discard unused refblocks"); - return ret; + goto fail; } old_file_size = bdrv_getlength(bs->file->bs); if (old_file_size < 0) { error_setg_errno(errp, -old_file_size, "Failed to inquire current file length"); - return old_file_size; + ret = old_file_size; + goto fail; } last_cluster = qcow2_get_last_cluster(bs, old_file_size); if (last_cluster < 0) { error_setg_errno(errp, -last_cluster, "Failed to find the last cluster"); - return last_cluster; + ret = last_cluster; + goto fail; } if ((last_cluster + 1) * s->cluster_size < old_file_size) { Error *local_err = NULL; - bdrv_truncate(bs->file, (last_cluster + 1) * s->cluster_size, - PREALLOC_MODE_OFF, &local_err); + bdrv_co_truncate(bs->file, (last_cluster + 1) * s->cluster_size, + PREALLOC_MODE_OFF, &local_err); if (local_err) { warn_reportf_err(local_err, "Failed to truncate the tail of the image: "); @@ -XXX,XX +XXX,XX @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset, ret = qcow2_grow_l1_table(bs, new_l1_size, true); if (ret < 0) { error_setg_errno(errp, -ret, "Failed to grow the L1 table"); - return ret; + goto fail; } } @@ -XXX,XX +XXX,XX @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset, ret = preallocate(bs, old_length, offset); if (ret < 0) { error_setg_errno(errp, -ret, "Preallocation failed"); - return ret; + goto fail; } break; @@ -XXX,XX +XXX,XX @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset, if (old_file_size < 0) { error_setg_errno(errp, -old_file_size, "Failed to inquire current file length"); - return old_file_size; + ret = old_file_size; + goto fail; } old_file_size = ROUND_UP(old_file_size, s->cluster_size); @@ -XXX,XX +XXX,XX @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset, if (allocation_start < 0) { error_setg_errno(errp, -allocation_start, "Failed to resize refcount structures"); - return allocation_start; + ret = allocation_start; + goto fail; } clusters_allocated = qcow2_alloc_clusters_at(bs, allocation_start, @@ -XXX,XX +XXX,XX @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset, if (clusters_allocated < 0) { error_setg_errno(errp, -clusters_allocated, "Failed to allocate data clusters"); - return clusters_allocated; + ret = clusters_allocated; + goto fail; } assert(clusters_allocated == nb_new_data_clusters); @@ -XXX,XX +XXX,XX @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset, /* Allocate the data area */ new_file_size = allocation_start + nb_new_data_clusters * s->cluster_size; - ret = bdrv_truncate(bs->file, new_file_size, prealloc, errp); + ret = bdrv_co_truncate(bs->file, new_file_size, prealloc, errp); if (ret < 0) { error_prepend(errp, "Failed to resize underlying file: "); qcow2_free_clusters(bs, allocation_start, nb_new_data_clusters * s->cluster_size, QCOW2_DISCARD_OTHER); - return ret; + goto fail; } /* Create the necessary L2 entries */ @@ -XXX,XX +XXX,XX @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset, qcow2_free_clusters(bs, host_offset, nb_new_data_clusters * s->cluster_size, QCOW2_DISCARD_OTHER); - return ret; + goto fail; } guest_offset += nb_clusters * s->cluster_size; @@ -XXX,XX +XXX,XX @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset, if (prealloc != PREALLOC_MODE_OFF) { /* Flush metadata before actually changing the image size */ - ret = bdrv_flush(bs); + ret = qcow2_write_caches(bs); if (ret < 0) { error_setg_errno(errp, -ret, "Failed to flush the preallocated area to disk"); - return ret; + goto fail; } } @@ -XXX,XX +XXX,XX @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset, &offset, sizeof(uint64_t)); if (ret < 0) { error_setg_errno(errp, -ret, "Failed to update the image size"); - return ret; + goto fail; } s->l1_vm_state_index = new_l1_size; - return 0; + ret = 0; +fail: + qemu_co_mutex_unlock(&s->lock); + return ret; } /* XXX: put compressed sectors first, then all the cluster aligned @@ -XXX,XX +XXX,XX @@ qcow2_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset, if (cluster_offset < 0) { return cluster_offset; } - return bdrv_truncate(bs->file, cluster_offset, PREALLOC_MODE_OFF, NULL); + return bdrv_co_truncate(bs->file, cluster_offset, PREALLOC_MODE_OFF, + NULL); } if (offset_into_cluster(s, offset)) { @@ -XXX,XX +XXX,XX @@ BlockDriver bdrv_qcow2 = { .bdrv_co_pdiscard = qcow2_co_pdiscard, .bdrv_co_copy_range_from = qcow2_co_copy_range_from, .bdrv_co_copy_range_to = qcow2_co_copy_range_to, - .bdrv_truncate = qcow2_truncate, + .bdrv_co_truncate = qcow2_co_truncate, .bdrv_co_pwritev_compressed = qcow2_co_pwritev_compressed, .bdrv_make_empty = qcow2_make_empty, diff --git a/block/qed.c b/block/qed.c index XXXXXXX..XXXXXXX 100644 --- a/block/qed.c +++ b/block/qed.c @@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_qed_co_pwrite_zeroes(BlockDriverState *bs, QED_AIOCB_WRITE | QED_AIOCB_ZERO); } -static int bdrv_qed_truncate(BlockDriverState *bs, int64_t offset, - PreallocMode prealloc, Error **errp) +static int coroutine_fn bdrv_qed_co_truncate(BlockDriverState *bs, + int64_t offset, + PreallocMode prealloc, + Error **errp) { BDRVQEDState *s = bs->opaque; uint64_t old_image_size; @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_qed = { .bdrv_co_readv = bdrv_qed_co_readv, .bdrv_co_writev = bdrv_qed_co_writev, .bdrv_co_pwrite_zeroes = bdrv_qed_co_pwrite_zeroes, - .bdrv_truncate = bdrv_qed_truncate, + .bdrv_co_truncate = bdrv_qed_co_truncate, .bdrv_getlength = bdrv_qed_getlength, .bdrv_get_info = bdrv_qed_get_info, .bdrv_refresh_limits = bdrv_qed_refresh_limits, diff --git a/block/raw-format.c b/block/raw-format.c index XXXXXXX..XXXXXXX 100644 --- a/block/raw-format.c +++ b/block/raw-format.c @@ -XXX,XX +XXX,XX @@ static void raw_refresh_limits(BlockDriverState *bs, Error **errp) } } -static int raw_truncate(BlockDriverState *bs, int64_t offset, - PreallocMode prealloc, Error **errp) +static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset, + PreallocMode prealloc, Error **errp) { BDRVRawState *s = bs->opaque; @@ -XXX,XX +XXX,XX @@ static int raw_truncate(BlockDriverState *bs, int64_t offset, s->size = offset; offset += s->offset; - return bdrv_truncate(bs->file, offset, prealloc, errp); + return bdrv_co_truncate(bs->file, offset, prealloc, errp); } static void raw_eject(BlockDriverState *bs, bool eject_flag) @@ -XXX,XX +XXX,XX @@ BlockDriver bdrv_raw = { .bdrv_co_block_status = &raw_co_block_status, .bdrv_co_copy_range_from = &raw_co_copy_range_from, .bdrv_co_copy_range_to = &raw_co_copy_range_to, - .bdrv_truncate = &raw_truncate, + .bdrv_co_truncate = &raw_co_truncate, .bdrv_getlength = &raw_getlength, .has_variable_length = true, .bdrv_measure = &raw_measure, diff --git a/block/rbd.c b/block/rbd.c index XXXXXXX..XXXXXXX 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -XXX,XX +XXX,XX @@ static int64_t qemu_rbd_getlength(BlockDriverState *bs) return info.size; } -static int qemu_rbd_truncate(BlockDriverState *bs, int64_t offset, - PreallocMode prealloc, Error **errp) +static int coroutine_fn qemu_rbd_co_truncate(BlockDriverState *bs, + int64_t offset, + PreallocMode prealloc, + Error **errp) { BDRVRBDState *s = bs->opaque; int r; @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_rbd = { .bdrv_get_info = qemu_rbd_getinfo, .create_opts = &qemu_rbd_create_opts, .bdrv_getlength = qemu_rbd_getlength, - .bdrv_truncate = qemu_rbd_truncate, + .bdrv_co_truncate = qemu_rbd_co_truncate, .protocol_name = "rbd", .bdrv_aio_preadv = qemu_rbd_aio_preadv, diff --git a/block/sheepdog.c b/block/sheepdog.c index XXXXXXX..XXXXXXX 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -XXX,XX +XXX,XX @@ static int64_t sd_getlength(BlockDriverState *bs) return s->inode.vdi_size; } -static int sd_truncate(BlockDriverState *bs, int64_t offset, - PreallocMode prealloc, Error **errp) +static int coroutine_fn sd_co_truncate(BlockDriverState *bs, int64_t offset, + PreallocMode prealloc, Error **errp) { BDRVSheepdogState *s = bs->opaque; int ret, fd; @@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num, assert(!flags); if (offset > s->inode.vdi_size) { - ret = sd_truncate(bs, offset, PREALLOC_MODE_OFF, NULL); + ret = sd_co_truncate(bs, offset, PREALLOC_MODE_OFF, NULL); if (ret < 0) { return ret; } @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_sheepdog = { .bdrv_has_zero_init = bdrv_has_zero_init_1, .bdrv_getlength = sd_getlength, .bdrv_get_allocated_file_size = sd_get_allocated_file_size, - .bdrv_truncate = sd_truncate, + .bdrv_co_truncate = sd_co_truncate, .bdrv_co_readv = sd_co_readv, .bdrv_co_writev = sd_co_writev, @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_sheepdog_tcp = { .bdrv_has_zero_init = bdrv_has_zero_init_1, .bdrv_getlength = sd_getlength, .bdrv_get_allocated_file_size = sd_get_allocated_file_size, - .bdrv_truncate = sd_truncate, + .bdrv_co_truncate = sd_co_truncate, .bdrv_co_readv = sd_co_readv, .bdrv_co_writev = sd_co_writev, @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_sheepdog_unix = { .bdrv_has_zero_init = bdrv_has_zero_init_1, .bdrv_getlength = sd_getlength, .bdrv_get_allocated_file_size = sd_get_allocated_file_size, - .bdrv_truncate = sd_truncate, + .bdrv_co_truncate = sd_co_truncate, .bdrv_co_readv = sd_co_readv, .bdrv_co_writev = sd_co_writev, diff --git a/block/ssh.c b/block/ssh.c index XXXXXXX..XXXXXXX 100644 --- a/block/ssh.c +++ b/block/ssh.c @@ -XXX,XX +XXX,XX @@ static int64_t ssh_getlength(BlockDriverState *bs) return length; } -static int ssh_truncate(BlockDriverState *bs, int64_t offset, - PreallocMode prealloc, Error **errp) +static int coroutine_fn ssh_co_truncate(BlockDriverState *bs, int64_t offset, + PreallocMode prealloc, Error **errp) { BDRVSSHState *s = bs->opaque; @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_ssh = { .bdrv_co_readv = ssh_co_readv, .bdrv_co_writev = ssh_co_writev, .bdrv_getlength = ssh_getlength, - .bdrv_truncate = ssh_truncate, + .bdrv_co_truncate = ssh_co_truncate, .bdrv_co_flush_to_disk = ssh_co_flush, .create_opts = &ssh_create_opts, }; -- 2.13.6 All callers are coroutine_fns now, so we can just directly call preallocate_co(). Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> --- block/qcow2.c | 51 ++++++++------------------------------------------- 1 file changed, 8 insertions(+), 43 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index XXXXXXX..XXXXXXX 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -XXX,XX +XXX,XX @@ static int qcow2_set_up_encryption(BlockDriverState *bs, return ret; } - -typedef struct PreallocCo { - BlockDriverState *bs; - uint64_t offset; - uint64_t new_length; - - int ret; -} PreallocCo; - /** * Preallocates metadata structures for data clusters between @offset (in the * guest disk) and @new_length (which is thus generally the new guest disk @@ -XXX,XX +XXX,XX @@ typedef struct PreallocCo { * * Returns: 0 on success, -errno on failure. */ -static void coroutine_fn preallocate_co(void *opaque) +static int coroutine_fn preallocate_co(BlockDriverState *bs, uint64_t offset, + uint64_t new_length) { - PreallocCo *params = opaque; - BlockDriverState *bs = params->bs; - uint64_t offset = params->offset; - uint64_t new_length = params->new_length; uint64_t bytes; uint64_t host_offset = 0; unsigned int cur_bytes; @@ -XXX,XX +XXX,XX @@ static void coroutine_fn preallocate_co(void *opaque) ret = qcow2_alloc_cluster_offset(bs, offset, &cur_bytes, &host_offset, &meta); if (ret < 0) { - goto done; + return ret; } while (meta) { @@ -XXX,XX +XXX,XX @@ static void coroutine_fn preallocate_co(void *opaque) if (ret < 0) { qcow2_free_any_clusters(bs, meta->alloc_offset, meta->nb_clusters, QCOW2_DISCARD_NEVER); - goto done; + return ret; } /* There are no dependent requests, but we need to remove our @@ -XXX,XX +XXX,XX @@ static void coroutine_fn preallocate_co(void *opaque) ret = bdrv_pwrite(bs->file, (host_offset + cur_bytes) - 1, &data, 1); if (ret < 0) { - goto done; + return ret; } } - ret = 0; - -done: - params->ret = ret; -} - -static int preallocate(BlockDriverState *bs, - uint64_t offset, uint64_t new_length) -{ - PreallocCo params = { - .bs = bs, - .offset = offset, - .new_length = new_length, - .ret = -EINPROGRESS, - }; - - if (qemu_in_coroutine()) { - preallocate_co(&params); - } else { - Coroutine *co = qemu_coroutine_create(preallocate_co, &params); - bdrv_coroutine_enter(bs, co); - BDRV_POLL_WHILE(bs, params.ret == -EINPROGRESS); - } - return params.ret; + return 0; } /* qcow2_refcount_metadata_size: @@ -XXX,XX +XXX,XX @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp) if (qcow2_opts->preallocation != PREALLOC_MODE_OFF) { BDRVQcow2State *s = blk_bs(blk)->opaque; qemu_co_mutex_lock(&s->lock); - ret = preallocate(blk_bs(blk), 0, qcow2_opts->size); + ret = preallocate_co(blk_bs(blk), 0, qcow2_opts->size); qemu_co_mutex_unlock(&s->lock); if (ret < 0) { @@ -XXX,XX +XXX,XX @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset, break; case PREALLOC_MODE_METADATA: - ret = preallocate(bs, old_length, offset); + ret = preallocate_co(bs, old_length, offset); if (ret < 0) { error_setg_errno(errp, -ret, "Preallocation failed"); goto fail; -- 2.13.6 This moves the bdrv_truncate() implementation from block.c to block/io.c so it can have access to the tracked requests infrastructure. This involves making refresh_total_sectors() public (in block_int.h). Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> --- include/block/block_int.h | 2 + block.c | 111 +--------------------------------------------- block/io.c | 109 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 112 insertions(+), 110 deletions(-) diff --git a/include/block/block_int.h b/include/block/block_int.h index XXXXXXX..XXXXXXX 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_copy_range_to(BdrvChild *src, uint64_t src_offset, BdrvChild *dst, uint64_t dst_offset, uint64_t bytes, BdrvRequestFlags flags); +int refresh_total_sectors(BlockDriverState *bs, int64_t hint); + #endif /* BLOCK_INT_H */ diff --git a/block.c b/block.c index XXXXXXX..XXXXXXX 100644 --- a/block.c +++ b/block.c @@ -XXX,XX +XXX,XX @@ static int find_image_format(BlockBackend *file, const char *filename, * Set the current 'total_sectors' value * Return 0 on success, -errno on error. */ -static int refresh_total_sectors(BlockDriverState *bs, int64_t hint) +int refresh_total_sectors(BlockDriverState *bs, int64_t hint) { BlockDriver *drv = bs->drv; @@ -XXX,XX +XXX,XX @@ static void bdrv_parent_cb_change_media(BlockDriverState *bs, bool load) } } -static void bdrv_parent_cb_resize(BlockDriverState *bs) -{ - BdrvChild *c; - QLIST_FOREACH(c, &bs->parents, next_parent) { - if (c->role->resize) { - c->role->resize(c); - } - } -} - /* * Sets the backing file link of a BDS. A new reference is created; callers * which don't need their own reference any more must call bdrv_unref(). @@ -XXX,XX +XXX,XX @@ exit: } /** - * Truncate file to 'offset' bytes (needed only for file protocols) - */ -int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, - PreallocMode prealloc, Error **errp) -{ - BlockDriverState *bs = child->bs; - BlockDriver *drv = bs->drv; - int ret; - - assert(child->perm & BLK_PERM_RESIZE); - - /* if bs->drv == NULL, bs is closed, so there's nothing to do here */ - if (!drv) { - error_setg(errp, "No medium inserted"); - return -ENOMEDIUM; - } - if (offset < 0) { - error_setg(errp, "Image size cannot be negative"); - return -EINVAL; - } - - bdrv_inc_in_flight(bs); - - if (!drv->bdrv_co_truncate) { - if (bs->file && drv->is_filter) { - ret = bdrv_co_truncate(bs->file, offset, prealloc, errp); - goto out; - } - error_setg(errp, "Image format driver does not support resize"); - ret = -ENOTSUP; - goto out; - } - if (bs->read_only) { - error_setg(errp, "Image is read-only"); - ret = -EACCES; - goto out; - } - - assert(!(bs->open_flags & BDRV_O_INACTIVE)); - - ret = drv->bdrv_co_truncate(bs, offset, prealloc, errp); - if (ret < 0) { - goto out; - } - ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS); - if (ret < 0) { - error_setg_errno(errp, -ret, "Could not refresh total sector count"); - } else { - offset = bs->total_sectors * BDRV_SECTOR_SIZE; - } - bdrv_dirty_bitmap_truncate(bs, offset); - bdrv_parent_cb_resize(bs); - atomic_inc(&bs->write_gen); - -out: - bdrv_dec_in_flight(bs); - return ret; -} - -typedef struct TruncateCo { - BdrvChild *child; - int64_t offset; - PreallocMode prealloc; - Error **errp; - int ret; -} TruncateCo; - -static void coroutine_fn bdrv_truncate_co_entry(void *opaque) -{ - TruncateCo *tco = opaque; - tco->ret = bdrv_co_truncate(tco->child, tco->offset, tco->prealloc, - tco->errp); -} - -int bdrv_truncate(BdrvChild *child, int64_t offset, PreallocMode prealloc, - Error **errp) -{ - Coroutine *co; - TruncateCo tco = { - .child = child, - .offset = offset, - .prealloc = prealloc, - .errp = errp, - .ret = NOT_DONE, - }; - - if (qemu_in_coroutine()) { - /* Fast-path if already in coroutine context */ - bdrv_truncate_co_entry(&tco); - } else { - co = qemu_coroutine_create(bdrv_truncate_co_entry, &tco); - qemu_coroutine_enter(co); - BDRV_POLL_WHILE(child->bs, tco.ret == NOT_DONE); - } - - return tco.ret; -} - -/** * Length of a allocated file in bytes. Sparse files are counted by actual * allocated space. Return < 0 if error or unknown. */ diff --git a/block/io.c b/block/io.c index XXXXXXX..XXXXXXX 100644 --- a/block/io.c +++ b/block/io.c @@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_copy_range(BdrvChild *src, uint64_t src_offset, bdrv_dec_in_flight(dst_bs); return ret; } + +static void bdrv_parent_cb_resize(BlockDriverState *bs) +{ + BdrvChild *c; + QLIST_FOREACH(c, &bs->parents, next_parent) { + if (c->role->resize) { + c->role->resize(c); + } + } +} + +/** + * Truncate file to 'offset' bytes (needed only for file protocols) + */ +int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, + PreallocMode prealloc, Error **errp) +{ + BlockDriverState *bs = child->bs; + BlockDriver *drv = bs->drv; + int ret; + + assert(child->perm & BLK_PERM_RESIZE); + + /* if bs->drv == NULL, bs is closed, so there's nothing to do here */ + if (!drv) { + error_setg(errp, "No medium inserted"); + return -ENOMEDIUM; + } + if (offset < 0) { + error_setg(errp, "Image size cannot be negative"); + return -EINVAL; + } + + bdrv_inc_in_flight(bs); + + if (!drv->bdrv_co_truncate) { + if (bs->file && drv->is_filter) { + ret = bdrv_co_truncate(bs->file, offset, prealloc, errp); + goto out; + } + error_setg(errp, "Image format driver does not support resize"); + ret = -ENOTSUP; + goto out; + } + if (bs->read_only) { + error_setg(errp, "Image is read-only"); + ret = -EACCES; + goto out; + } + + assert(!(bs->open_flags & BDRV_O_INACTIVE)); + + ret = drv->bdrv_co_truncate(bs, offset, prealloc, errp); + if (ret < 0) { + goto out; + } + ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS); + if (ret < 0) { + error_setg_errno(errp, -ret, "Could not refresh total sector count"); + } else { + offset = bs->total_sectors * BDRV_SECTOR_SIZE; + } + bdrv_dirty_bitmap_truncate(bs, offset); + bdrv_parent_cb_resize(bs); + atomic_inc(&bs->write_gen); + +out: + bdrv_dec_in_flight(bs); + return ret; +} + +typedef struct TruncateCo { + BdrvChild *child; + int64_t offset; + PreallocMode prealloc; + Error **errp; + int ret; +} TruncateCo; + +static void coroutine_fn bdrv_truncate_co_entry(void *opaque) +{ + TruncateCo *tco = opaque; + tco->ret = bdrv_co_truncate(tco->child, tco->offset, tco->prealloc, + tco->errp); +} + +int bdrv_truncate(BdrvChild *child, int64_t offset, PreallocMode prealloc, + Error **errp) +{ + Coroutine *co; + TruncateCo tco = { + .child = child, + .offset = offset, + .prealloc = prealloc, + .errp = errp, + .ret = NOT_DONE, + }; + + if (qemu_in_coroutine()) { + /* Fast-path if already in coroutine context */ + bdrv_truncate_co_entry(&tco); + } else { + co = qemu_coroutine_create(bdrv_truncate_co_entry, &tco); + qemu_coroutine_enter(co); + BDRV_POLL_WHILE(child->bs, tco.ret == NOT_DONE); + } + + return tco.ret; +} -- 2.13.6 When growing an image, block drivers (especially protocol drivers) may initialise the newly added area. I/O requests to the same area need to wait for this initialisation to be completed so that data writes don't get overwritten and reads don't read uninitialised data. To avoid overhead in the fast I/O path by adding new locking in the protocol drivers and to restrict the impact to requests that actually touch the new area, reuse the existing tracked request infrastructure in block/io.c and mark all discard requests as serialising. With this change, it is safe for protocol drivers to make .bdrv_co_truncate actually asynchronous. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> --- include/block/block_int.h | 1 + block/io.c | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/include/block/block_int.h b/include/block/block_int.h index XXXXXXX..XXXXXXX 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -XXX,XX +XXX,XX @@ enum BdrvTrackedRequestType { BDRV_TRACKED_READ, BDRV_TRACKED_WRITE, BDRV_TRACKED_DISCARD, + BDRV_TRACKED_TRUNCATE, }; typedef struct BdrvTrackedRequest { diff --git a/block/io.c b/block/io.c index XXXXXXX..XXXXXXX 100644 --- a/block/io.c +++ b/block/io.c @@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, { BlockDriverState *bs = child->bs; BlockDriver *drv = bs->drv; + BdrvTrackedRequest req; + int64_t old_size, new_bytes; int ret; assert(child->perm & BLK_PERM_RESIZE); @@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, return -EINVAL; } + old_size = bdrv_getlength(bs); + if (old_size < 0) { + error_setg_errno(errp, -old_size, "Failed to get old image size"); + return old_size; + } + + if (offset > old_size) { + new_bytes = offset - old_size; + } else { + new_bytes = 0; + } + bdrv_inc_in_flight(bs); + tracked_request_begin(&req, bs, offset, new_bytes, BDRV_TRACKED_TRUNCATE); + + /* If we are growing the image and potentially using preallocation for the + * new area, we need to make sure that no write requests are made to it + * concurrently or they might be overwritten by preallocation. */ + if (new_bytes) { + mark_request_serialising(&req, 1); + wait_serialising_requests(&req); + } if (!drv->bdrv_co_truncate) { if (bs->file && drv->is_filter) { @@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, atomic_inc(&bs->write_gen); out: + tracked_request_end(&req); bdrv_dec_in_flight(bs); + return ret; } -- 2.13.6 This moves the code to resize an image file to the thread pool to avoid blocking. Creating large images with preallocation with blockdev-create is now actually a background job instead of blocking the monitor (and most other things) until the preallocation has completed. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> --- include/block/raw-aio.h | 4 +- block/file-posix.c | 266 +++++++++++++++++++++++++++--------------------- 2 files changed, 154 insertions(+), 116 deletions(-) diff --git a/include/block/raw-aio.h b/include/block/raw-aio.h index XXXXXXX..XXXXXXX 100644 --- a/include/block/raw-aio.h +++ b/include/block/raw-aio.h @@ -XXX,XX +XXX,XX @@ #define QEMU_AIO_DISCARD 0x0010 #define QEMU_AIO_WRITE_ZEROES 0x0020 #define QEMU_AIO_COPY_RANGE 0x0040 +#define QEMU_AIO_TRUNCATE 0x0080 #define QEMU_AIO_TYPE_MASK \ (QEMU_AIO_READ | \ QEMU_AIO_WRITE | \ @@ -XXX,XX +XXX,XX @@ QEMU_AIO_FLUSH | \ QEMU_AIO_DISCARD | \ QEMU_AIO_WRITE_ZEROES | \ - QEMU_AIO_COPY_RANGE) + QEMU_AIO_COPY_RANGE | \ + QEMU_AIO_TRUNCATE) /* AIO flags */ #define QEMU_AIO_MISALIGNED 0x1000 diff --git a/block/file-posix.c b/block/file-posix.c index XXXXXXX..XXXXXXX 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -XXX,XX +XXX,XX @@ typedef struct RawPosixAIOData { #define aio_ioctl_cmd aio_nbytes /* for QEMU_AIO_IOCTL */ off_t aio_offset; int aio_type; - int aio_fd2; - off_t aio_offset2; + union { + struct { + int aio_fd2; + off_t aio_offset2; + }; + struct { + PreallocMode prealloc; + Error **errp; + }; + }; } RawPosixAIOData; #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) @@ -XXX,XX +XXX,XX @@ static ssize_t handle_aiocb_discard(RawPosixAIOData *aiocb) return ret; } +static int handle_aiocb_truncate(RawPosixAIOData *aiocb) +{ + int result = 0; + int64_t current_length = 0; + char *buf = NULL; + struct stat st; + int fd = aiocb->aio_fildes; + int64_t offset = aiocb->aio_offset; + Error **errp = aiocb->errp; + + if (fstat(fd, &st) < 0) { + result = -errno; + error_setg_errno(errp, -result, "Could not stat file"); + return result; + } + + current_length = st.st_size; + if (current_length > offset && aiocb->prealloc != PREALLOC_MODE_OFF) { + error_setg(errp, "Cannot use preallocation for shrinking files"); + return -ENOTSUP; + } + + switch (aiocb->prealloc) { +#ifdef CONFIG_POSIX_FALLOCATE + case PREALLOC_MODE_FALLOC: + /* + * Truncating before posix_fallocate() makes it about twice slower on + * file systems that do not support fallocate(), trying to check if a + * block is allocated before allocating it, so don't do that here. + */ + if (offset != current_length) { + result = -posix_fallocate(fd, current_length, + offset - current_length); + if (result != 0) { + /* posix_fallocate() doesn't set errno. */ + error_setg_errno(errp, -result, + "Could not preallocate new data"); + } + } else { + result = 0; + } + goto out; +#endif + case PREALLOC_MODE_FULL: + { + int64_t num = 0, left = offset - current_length; + off_t seek_result; + + /* + * Knowing the final size from the beginning could allow the file + * system driver to do less allocations and possibly avoid + * fragmentation of the file. + */ + if (ftruncate(fd, offset) != 0) { + result = -errno; + error_setg_errno(errp, -result, "Could not resize file"); + goto out; + } + + buf = g_malloc0(65536); + + seek_result = lseek(fd, current_length, SEEK_SET); + if (seek_result < 0) { + result = -errno; + error_setg_errno(errp, -result, + "Failed to seek to the old end of file"); + goto out; + } + + while (left > 0) { + num = MIN(left, 65536); + result = write(fd, buf, num); + if (result < 0) { + result = -errno; + error_setg_errno(errp, -result, + "Could not write zeros for preallocation"); + goto out; + } + left -= result; + } + if (result >= 0) { + result = fsync(fd); + if (result < 0) { + result = -errno; + error_setg_errno(errp, -result, + "Could not flush file to disk"); + goto out; + } + } + goto out; + } + case PREALLOC_MODE_OFF: + if (ftruncate(fd, offset) != 0) { + result = -errno; + error_setg_errno(errp, -result, "Could not resize file"); + } + return result; + default: + result = -ENOTSUP; + error_setg(errp, "Unsupported preallocation mode: %s", + PreallocMode_str(aiocb->prealloc)); + return result; + } + +out: + if (result < 0) { + if (ftruncate(fd, current_length) < 0) { + error_report("Failed to restore old file length: %s", + strerror(errno)); + } + } + + g_free(buf); + return result; +} + static int aio_worker(void *arg) { RawPosixAIOData *aiocb = arg; @@ -XXX,XX +XXX,XX @@ static int aio_worker(void *arg) case QEMU_AIO_COPY_RANGE: ret = handle_aiocb_copy_range(aiocb); break; + case QEMU_AIO_TRUNCATE: + ret = handle_aiocb_truncate(aiocb); + break; default: fprintf(stderr, "invalid aio request (0x%x)\n", aiocb->aio_type); ret = -EINVAL; @@ -XXX,XX +XXX,XX @@ static void raw_close(BlockDriverState *bs) * * Returns: 0 on success, -errno on failure. */ -static int raw_regular_truncate(int fd, int64_t offset, PreallocMode prealloc, - Error **errp) +static int coroutine_fn +raw_regular_truncate(BlockDriverState *bs, int fd, int64_t offset, + PreallocMode prealloc, Error **errp) { - int result = 0; - int64_t current_length = 0; - char *buf = NULL; - struct stat st; - - if (fstat(fd, &st) < 0) { - result = -errno; - error_setg_errno(errp, -result, "Could not stat file"); - return result; - } - - current_length = st.st_size; - if (current_length > offset && prealloc != PREALLOC_MODE_OFF) { - error_setg(errp, "Cannot use preallocation for shrinking files"); - return -ENOTSUP; - } - - switch (prealloc) { -#ifdef CONFIG_POSIX_FALLOCATE - case PREALLOC_MODE_FALLOC: - /* - * Truncating before posix_fallocate() makes it about twice slower on - * file systems that do not support fallocate(), trying to check if a - * block is allocated before allocating it, so don't do that here. - */ - if (offset != current_length) { - result = -posix_fallocate(fd, current_length, offset - current_length); - if (result != 0) { - /* posix_fallocate() doesn't set errno. */ - error_setg_errno(errp, -result, - "Could not preallocate new data"); - } - } else { - result = 0; - } - goto out; -#endif - case PREALLOC_MODE_FULL: - { - int64_t num = 0, left = offset - current_length; - off_t seek_result; - - /* - * Knowing the final size from the beginning could allow the file - * system driver to do less allocations and possibly avoid - * fragmentation of the file. - */ - if (ftruncate(fd, offset) != 0) { - result = -errno; - error_setg_errno(errp, -result, "Could not resize file"); - goto out; - } - - buf = g_malloc0(65536); - - seek_result = lseek(fd, current_length, SEEK_SET); - if (seek_result < 0) { - result = -errno; - error_setg_errno(errp, -result, - "Failed to seek to the old end of file"); - goto out; - } - - while (left > 0) { - num = MIN(left, 65536); - result = write(fd, buf, num); - if (result < 0) { - result = -errno; - error_setg_errno(errp, -result, - "Could not write zeros for preallocation"); - goto out; - } - left -= result; - } - if (result >= 0) { - result = fsync(fd); - if (result < 0) { - result = -errno; - error_setg_errno(errp, -result, - "Could not flush file to disk"); - goto out; - } - } - goto out; - } - case PREALLOC_MODE_OFF: - if (ftruncate(fd, offset) != 0) { - result = -errno; - error_setg_errno(errp, -result, "Could not resize file"); - } - return result; - default: - result = -ENOTSUP; - error_setg(errp, "Unsupported preallocation mode: %s", - PreallocMode_str(prealloc)); - return result; - } + RawPosixAIOData *acb = g_new(RawPosixAIOData, 1); + ThreadPool *pool; -out: - if (result < 0) { - if (ftruncate(fd, current_length) < 0) { - error_report("Failed to restore old file length: %s", - strerror(errno)); - } - } + *acb = (RawPosixAIOData) { + .bs = bs, + .aio_fildes = fd, + .aio_type = QEMU_AIO_TRUNCATE, + .aio_offset = offset, + .prealloc = prealloc, + .errp = errp, + }; - g_free(buf); - return result; + /* @bs can be NULL, bdrv_get_aio_context() returns the main context then */ + pool = aio_get_thread_pool(bdrv_get_aio_context(bs)); + return thread_pool_submit_co(pool, aio_worker, acb); } static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset, @@ -XXX,XX +XXX,XX @@ static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset, } if (S_ISREG(st.st_mode)) { - return raw_regular_truncate(s->fd, offset, prealloc, errp); + return raw_regular_truncate(bs, s->fd, offset, prealloc, errp); } if (prealloc != PREALLOC_MODE_OFF) { @@ -XXX,XX +XXX,XX @@ static int64_t raw_get_allocated_file_size(BlockDriverState *bs) return (int64_t)st.st_blocks * 512; } -static int raw_co_create(BlockdevCreateOptions *options, Error **errp) +static int coroutine_fn +raw_co_create(BlockdevCreateOptions *options, Error **errp) { BlockdevCreateOptionsFile *file_opts; int fd; @@ -XXX,XX +XXX,XX @@ static int raw_co_create(BlockdevCreateOptions *options, Error **errp) } /* Clear the file by truncating it to 0 */ - result = raw_regular_truncate(fd, 0, PREALLOC_MODE_OFF, errp); + result = raw_regular_truncate(NULL, fd, 0, PREALLOC_MODE_OFF, errp); if (result < 0) { goto out_close; } @@ -XXX,XX +XXX,XX @@ static int raw_co_create(BlockdevCreateOptions *options, Error **errp) /* Resize and potentially preallocate the file to the desired * final size */ - result = raw_regular_truncate(fd, file_opts->size, file_opts->preallocation, - errp); + result = raw_regular_truncate(NULL, fd, file_opts->size, + file_opts->preallocation, errp); if (result < 0) { goto out_close; } -- 2.13.6 From: Fam Zheng <famz@redhat.com> In the beginning of the function, we initialize the local variable to 0, and in the body of the function, we check the assigned values and exit the loop immediately. So here it can never be non-zero. Reported-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block/qcow2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/qcow2.c b/block/qcow2.c index XXXXXXX..XXXXXXX 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_handle_l2meta(BlockDriverState *bs, while (l2meta != NULL) { QCowL2Meta *next; - if (!ret && link_l2) { + if (link_l2) { ret = qcow2_alloc_cluster_link_l2(bs, l2meta); if (ret) { goto out; -- 2.13.6 From: Fam Zheng <famz@redhat.com> in_flight and tracked requests need to be tracked in every layer during recursion. For now the only user is qemu-img convert where overlapping requests and IOThreads don't exist, therefore this change doesn't make much difference form user point of view, but it is incorrect as part of the API. Fix it. Reported-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block/io.c | 59 ++++++++++++++++++++++++++++------------------------------- 1 file changed, 28 insertions(+), 31 deletions(-) diff --git a/block/io.c b/block/io.c index XXXXXXX..XXXXXXX 100644 --- a/block/io.c +++ b/block/io.c @@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_co_copy_range_internal(BdrvChild *src, BdrvRequestFlags flags, bool recurse_src) { + BdrvTrackedRequest src_req, dst_req; + BlockDriverState *src_bs = src->bs; + BlockDriverState *dst_bs = dst->bs; int ret; if (!src || !dst || !src->bs || !dst->bs) { @@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_co_copy_range_internal(BdrvChild *src, || src->bs->encrypted || dst->bs->encrypted) { return -ENOTSUP; } + bdrv_inc_in_flight(src_bs); + bdrv_inc_in_flight(dst_bs); + tracked_request_begin(&src_req, src_bs, src_offset, + bytes, BDRV_TRACKED_READ); + tracked_request_begin(&dst_req, dst_bs, dst_offset, + bytes, BDRV_TRACKED_WRITE); + + wait_serialising_requests(&src_req); + wait_serialising_requests(&dst_req); if (recurse_src) { - return src->bs->drv->bdrv_co_copy_range_from(src->bs, - src, src_offset, - dst, dst_offset, - bytes, flags); + ret = src->bs->drv->bdrv_co_copy_range_from(src->bs, + src, src_offset, + dst, dst_offset, + bytes, flags); } else { - return dst->bs->drv->bdrv_co_copy_range_to(dst->bs, - src, src_offset, - dst, dst_offset, - bytes, flags); + ret = dst->bs->drv->bdrv_co_copy_range_to(dst->bs, + src, src_offset, + dst, dst_offset, + bytes, flags); } + tracked_request_end(&src_req); + tracked_request_end(&dst_req); + bdrv_dec_in_flight(src_bs); + bdrv_dec_in_flight(dst_bs); + return ret; } /* Copy range from @src to @dst. @@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_copy_range(BdrvChild *src, uint64_t src_offset, BdrvChild *dst, uint64_t dst_offset, uint64_t bytes, BdrvRequestFlags flags) { - BdrvTrackedRequest src_req, dst_req; - BlockDriverState *src_bs = src->bs; - BlockDriverState *dst_bs = dst->bs; - int ret; - - bdrv_inc_in_flight(src_bs); - bdrv_inc_in_flight(dst_bs); - tracked_request_begin(&src_req, src_bs, src_offset, - bytes, BDRV_TRACKED_READ); - tracked_request_begin(&dst_req, dst_bs, dst_offset, - bytes, BDRV_TRACKED_WRITE); - - wait_serialising_requests(&src_req); - wait_serialising_requests(&dst_req); - ret = bdrv_co_copy_range_from(src, src_offset, - dst, dst_offset, - bytes, flags); - - tracked_request_end(&src_req); - tracked_request_end(&dst_req); - bdrv_dec_in_flight(src_bs); - bdrv_dec_in_flight(dst_bs); - return ret; + return bdrv_co_copy_range_from(src, src_offset, + dst, dst_offset, + bytes, flags); } static void bdrv_parent_cb_resize(BlockDriverState *bs) -- 2.13.6 From: Markus Armbruster <armbru@redhat.com> block_crypto_open_opts_init() and block_crypto_create_opts_init() contain a virtual visit of QCryptoBlockOptions and QCryptoBlockCreateOptions less member "format", respectively. Change their callers to put member "format" in the QDict, so they can use the generated visitors for these types instead. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block/crypto.h | 8 ++--- block/crypto.c | 99 +++++++++------------------------------------------------- block/qcow.c | 5 ++- block/qcow2.c | 10 +++--- 4 files changed, 22 insertions(+), 100 deletions(-) diff --git a/block/crypto.h b/block/crypto.h index XXXXXXX..XXXXXXX 100644 --- a/block/crypto.h +++ b/block/crypto.h @@ -XXX,XX +XXX,XX @@ } QCryptoBlockCreateOptions * -block_crypto_create_opts_init(QCryptoBlockFormat format, - QDict *opts, - Error **errp); +block_crypto_create_opts_init(QDict *opts, Error **errp); QCryptoBlockOpenOptions * -block_crypto_open_opts_init(QCryptoBlockFormat format, - QDict *opts, - Error **errp); +block_crypto_open_opts_init(QDict *opts, Error **errp); #endif /* BLOCK_CRYPTO_H__ */ diff --git a/block/crypto.c b/block/crypto.c index XXXXXXX..XXXXXXX 100644 --- a/block/crypto.c +++ b/block/crypto.c @@ -XXX,XX +XXX,XX @@ static QemuOptsList block_crypto_create_opts_luks = { QCryptoBlockOpenOptions * -block_crypto_open_opts_init(QCryptoBlockFormat format, - QDict *opts, - Error **errp) +block_crypto_open_opts_init(QDict *opts, Error **errp) { Visitor *v; - QCryptoBlockOpenOptions *ret = NULL; - Error *local_err = NULL; - - ret = g_new0(QCryptoBlockOpenOptions, 1); - ret->format = format; + QCryptoBlockOpenOptions *ret; - v = qobject_input_visitor_new_flat_confused(opts, &local_err); + v = qobject_input_visitor_new_flat_confused(opts, errp); if (!v) { - goto out; - } - - visit_start_struct(v, NULL, NULL, 0, &local_err); - if (local_err) { - goto out; - } - - switch (format) { - case Q_CRYPTO_BLOCK_FORMAT_LUKS: - visit_type_QCryptoBlockOptionsLUKS_members( - v, &ret->u.luks, &local_err); - break; - - case Q_CRYPTO_BLOCK_FORMAT_QCOW: - visit_type_QCryptoBlockOptionsQCow_members( - v, &ret->u.qcow, &local_err); - break; - - default: - error_setg(&local_err, "Unsupported block format %d", format); - break; - } - if (!local_err) { - visit_check_struct(v, &local_err); + return NULL; } - visit_end_struct(v, NULL); + visit_type_QCryptoBlockOpenOptions(v, NULL, &ret, errp); - out: - if (local_err) { - error_propagate(errp, local_err); - qapi_free_QCryptoBlockOpenOptions(ret); - ret = NULL; - } visit_free(v); return ret; } QCryptoBlockCreateOptions * -block_crypto_create_opts_init(QCryptoBlockFormat format, - QDict *opts, - Error **errp) +block_crypto_create_opts_init(QDict *opts, Error **errp) { Visitor *v; - QCryptoBlockCreateOptions *ret = NULL; - Error *local_err = NULL; - - ret = g_new0(QCryptoBlockCreateOptions, 1); - ret->format = format; + QCryptoBlockCreateOptions *ret; - v = qobject_input_visitor_new_flat_confused(opts, &local_err); + v = qobject_input_visitor_new_flat_confused(opts, errp); if (!v) { - goto out; - } - - visit_start_struct(v, NULL, NULL, 0, &local_err); - if (local_err) { - goto out; - } - - switch (format) { - case Q_CRYPTO_BLOCK_FORMAT_LUKS: - visit_type_QCryptoBlockCreateOptionsLUKS_members( - v, &ret->u.luks, &local_err); - break; - - case Q_CRYPTO_BLOCK_FORMAT_QCOW: - visit_type_QCryptoBlockOptionsQCow_members( - v, &ret->u.qcow, &local_err); - break; - - default: - error_setg(&local_err, "Unsupported block format %d", format); - break; - } - if (!local_err) { - visit_check_struct(v, &local_err); + return NULL; } - visit_end_struct(v, NULL); + visit_type_QCryptoBlockCreateOptions(v, NULL, &ret, errp); - out: - if (local_err) { - error_propagate(errp, local_err); - qapi_free_QCryptoBlockCreateOptions(ret); - ret = NULL; - } visit_free(v); return ret; } @@ -XXX,XX +XXX,XX @@ static int block_crypto_open_generic(QCryptoBlockFormat format, } cryptoopts = qemu_opts_to_qdict(opts, NULL); + qdict_put_str(cryptoopts, "format", QCryptoBlockFormat_str(format)); - open_opts = block_crypto_open_opts_init(format, cryptoopts, errp); + open_opts = block_crypto_open_opts_init(cryptoopts, errp); if (!open_opts) { goto cleanup; } @@ -XXX,XX +XXX,XX @@ static int coroutine_fn block_crypto_co_create_opts_luks(const char *filename, &block_crypto_create_opts_luks, true); - create_opts = block_crypto_create_opts_init(Q_CRYPTO_BLOCK_FORMAT_LUKS, - cryptoopts, errp); + qdict_put_str(cryptoopts, "format", "luks"); + create_opts = block_crypto_create_opts_init(cryptoopts, errp); if (!create_opts) { ret = -EINVAL; goto fail; diff --git a/block/qcow.c b/block/qcow.c index XXXXXXX..XXXXXXX 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -XXX,XX +XXX,XX @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags, ret = -EINVAL; goto fail; } - qdict_del(encryptopts, "format"); - crypto_opts = block_crypto_open_opts_init( - Q_CRYPTO_BLOCK_FORMAT_QCOW, encryptopts, errp); + qdict_put_str(encryptopts, "format", "qcow"); + crypto_opts = block_crypto_open_opts_init(encryptopts, errp); if (!crypto_opts) { ret = -EINVAL; goto fail; diff --git a/block/qcow2.c b/block/qcow2.c index XXXXXXX..XXXXXXX 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -XXX,XX +XXX,XX @@ static int qcow2_update_options_prepare(BlockDriverState *bs, ret = -EINVAL; goto fail; } - qdict_del(encryptopts, "format"); - r->crypto_opts = block_crypto_open_opts_init( - Q_CRYPTO_BLOCK_FORMAT_QCOW, encryptopts, errp); + qdict_put_str(encryptopts, "format", "qcow"); + r->crypto_opts = block_crypto_open_opts_init(encryptopts, errp); break; case QCOW_CRYPT_LUKS: @@ -XXX,XX +XXX,XX @@ static int qcow2_update_options_prepare(BlockDriverState *bs, ret = -EINVAL; goto fail; } - qdict_del(encryptopts, "format"); - r->crypto_opts = block_crypto_open_opts_init( - Q_CRYPTO_BLOCK_FORMAT_LUKS, encryptopts, errp); + qdict_put_str(encryptopts, "format", "luks"); + r->crypto_opts = block_crypto_open_opts_init(encryptopts, errp); break; default: -- 2.13.6 Commit abf754fe406 updated 026.out, but forgot to also update 026.out.nocache. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> --- tests/qemu-iotests/026.out.nocache | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/qemu-iotests/026.out.nocache b/tests/qemu-iotests/026.out.nocache index XXXXXXX..XXXXXXX 100644 --- a/tests/qemu-iotests/026.out.nocache +++ b/tests/qemu-iotests/026.out.nocache @@ -XXX,XX +XXX,XX @@ Failed to flush the L2 table cache: No space left on device Failed to flush the refcount block cache: No space left on device write failed: No space left on device -11 leaked clusters were found on the image. +10 leaked clusters were found on the image. This means waste of disk space, but no harm to data. Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 @@ -XXX,XX +XXX,XX @@ Failed to flush the L2 table cache: No space left on device Failed to flush the refcount block cache: No space left on device write failed: No space left on device -11 leaked clusters were found on the image. +10 leaked clusters were found on the image. This means waste of disk space, but no harm to data. Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 @@ -XXX,XX +XXX,XX @@ Failed to flush the L2 table cache: No space left on device Failed to flush the refcount block cache: No space left on device write failed: No space left on device -11 leaked clusters were found on the image. +10 leaked clusters were found on the image. This means waste of disk space, but no harm to data. Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 -- 2.13.6 If we managed to allocate the clusters, but then failed to write the data, there's a good chance that we'll still be able to free the clusters again in order to avoid cluster leaks (the refcounts are cached, so even if we can't write them out right now, we may be able to do so when the VM is resumed after a werror=stop/enospc pause). Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Tested-by: Eric Blake <eblake@redhat.com> --- block/qcow2.h | 1 + block/qcow2-cluster.c | 11 +++++++++++ block/qcow2.c | 2 ++ 3 files changed, 14 insertions(+) diff --git a/block/qcow2.h b/block/qcow2.h index XXXXXXX..XXXXXXX 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -XXX,XX +XXX,XX @@ uint64_t qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs, int compressed_size); int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m); +void qcow2_alloc_cluster_abort(BlockDriverState *bs, QCowL2Meta *m); int qcow2_cluster_discard(BlockDriverState *bs, uint64_t offset, uint64_t bytes, enum qcow2_discard_type type, bool full_discard); diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index XXXXXXX..XXXXXXX 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -XXX,XX +XXX,XX @@ err: return ret; } +/** + * Frees the allocated clusters because the request failed and they won't + * actually be linked. + */ +void qcow2_alloc_cluster_abort(BlockDriverState *bs, QCowL2Meta *m) +{ + BDRVQcow2State *s = bs->opaque; + qcow2_free_clusters(bs, m->alloc_offset, m->nb_clusters << s->cluster_bits, + QCOW2_DISCARD_NEVER); +} + /* * Returns the number of contiguous clusters that can be used for an allocating * write, but require COW to be performed (this includes yet unallocated space, diff --git a/block/qcow2.c b/block/qcow2.c index XXXXXXX..XXXXXXX 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_handle_l2meta(BlockDriverState *bs, if (ret) { goto out; } + } else { + qcow2_alloc_cluster_abort(bs, l2meta); } /* Take the request off the list of running requests */ -- 2.13.6 This adds a test for a temporary write failure, which simulates the situation after werror=stop/enospc has stopped the VM. We shouldn't leave leaked clusters behind in such cases. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> --- tests/qemu-iotests/026 | 17 +++++++++++++++++ tests/qemu-iotests/026.out | 8 ++++++++ tests/qemu-iotests/026.out.nocache | 8 ++++++++ 3 files changed, 33 insertions(+) diff --git a/tests/qemu-iotests/026 b/tests/qemu-iotests/026 index XXXXXXX..XXXXXXX 100755 --- a/tests/qemu-iotests/026 +++ b/tests/qemu-iotests/026 @@ -XXX,XX +XXX,XX @@ done done done +echo +echo === Avoid cluster leaks after temporary failure === +echo + +cat > "$TEST_DIR/blkdebug.conf" <<EOF +[inject-error] +event = "write_aio" +errno = "5" +once = "on" +EOF + +# After the failed first write, do a second write so that the updated refcount +# block is actually written back +_make_test_img 64M +$QEMU_IO -c "write 0 1M" -c "write 0 1M" "$BLKDBG_TEST_IMG" | _filter_qemu_io +_check_test_img + # success, all done echo "*** done" rm -f $seq.full diff --git a/tests/qemu-iotests/026.out b/tests/qemu-iotests/026.out index XXXXXXX..XXXXXXX 100644 --- a/tests/qemu-iotests/026.out +++ b/tests/qemu-iotests/026.out @@ -XXX,XX +XXX,XX @@ write failed: No space left on device 96 leaked clusters were found on the image. This means waste of disk space, but no harm to data. + +=== Avoid cluster leaks after temporary failure === + +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 +write failed: Input/output error +wrote 1048576/1048576 bytes at offset 0 +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +No errors were found on the image. *** done diff --git a/tests/qemu-iotests/026.out.nocache b/tests/qemu-iotests/026.out.nocache index XXXXXXX..XXXXXXX 100644 --- a/tests/qemu-iotests/026.out.nocache +++ b/tests/qemu-iotests/026.out.nocache @@ -XXX,XX +XXX,XX @@ write failed: No space left on device 96 leaked clusters were found on the image. This means waste of disk space, but no harm to data. + +=== Avoid cluster leaks after temporary failure === + +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 +write failed: Input/output error +wrote 1048576/1048576 bytes at offset 0 +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +No errors were found on the image. *** done -- 2.13.6 This simplifies file-posix by implementing the coroutine variants of the discard and flush BlockDriver callbacks. These were the last remaining users of paio_submit(), which can be removed now. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- block/file-posix.c | 72 ++++++++++++++++++------------------------------------ 1 file changed, 24 insertions(+), 48 deletions(-) diff --git a/block/file-posix.c b/block/file-posix.c index XXXXXXX..XXXXXXX 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -XXX,XX +XXX,XX @@ static inline int paio_submit_co(BlockDriverState *bs, int fd, return paio_submit_co_full(bs, fd, offset, -1, 0, qiov, bytes, type); } -static BlockAIOCB *paio_submit(BlockDriverState *bs, int fd, - int64_t offset, QEMUIOVector *qiov, int bytes, - BlockCompletionFunc *cb, void *opaque, int type) -{ - RawPosixAIOData *acb = g_new(RawPosixAIOData, 1); - ThreadPool *pool; - - acb->bs = bs; - acb->aio_type = type; - acb->aio_fildes = fd; - - acb->aio_nbytes = bytes; - acb->aio_offset = offset; - - if (qiov) { - acb->aio_iov = qiov->iov; - acb->aio_niov = qiov->niov; - assert(qiov->size == acb->aio_nbytes); - } - - trace_paio_submit(acb, opaque, offset, bytes, type); - pool = aio_get_thread_pool(bdrv_get_aio_context(bs)); - return thread_pool_submit_aio(pool, aio_worker, acb, cb, opaque); -} - static int coroutine_fn raw_co_prw(BlockDriverState *bs, uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int type) { @@ -XXX,XX +XXX,XX @@ static void raw_aio_unplug(BlockDriverState *bs) #endif } -static BlockAIOCB *raw_aio_flush(BlockDriverState *bs, - BlockCompletionFunc *cb, void *opaque) +static int raw_co_flush_to_disk(BlockDriverState *bs) { BDRVRawState *s = bs->opaque; + int ret; - if (fd_open(bs) < 0) - return NULL; + ret = fd_open(bs); + if (ret < 0) { + return ret; + } - return paio_submit(bs, s->fd, 0, NULL, 0, cb, opaque, QEMU_AIO_FLUSH); + return paio_submit_co(bs, s->fd, 0, NULL, 0, QEMU_AIO_FLUSH); } static void raw_aio_attach_aio_context(BlockDriverState *bs, @@ -XXX,XX +XXX,XX @@ static void coroutine_fn raw_co_invalidate_cache(BlockDriverState *bs, #endif /* !__linux__ */ } -static coroutine_fn BlockAIOCB *raw_aio_pdiscard(BlockDriverState *bs, - int64_t offset, int bytes, - BlockCompletionFunc *cb, void *opaque) +static coroutine_fn int +raw_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes) { BDRVRawState *s = bs->opaque; - return paio_submit(bs, s->fd, offset, NULL, bytes, - cb, opaque, QEMU_AIO_DISCARD); + return paio_submit_co(bs, s->fd, offset, NULL, bytes, QEMU_AIO_DISCARD); } static int coroutine_fn raw_co_pwrite_zeroes( @@ -XXX,XX +XXX,XX @@ BlockDriver bdrv_file = { .bdrv_co_preadv = raw_co_preadv, .bdrv_co_pwritev = raw_co_pwritev, - .bdrv_aio_flush = raw_aio_flush, - .bdrv_aio_pdiscard = raw_aio_pdiscard, + .bdrv_co_flush_to_disk = raw_co_flush_to_disk, + .bdrv_co_pdiscard = raw_co_pdiscard, .bdrv_co_copy_range_from = raw_co_copy_range_from, .bdrv_co_copy_range_to = raw_co_copy_range_to, .bdrv_refresh_limits = raw_refresh_limits, @@ -XXX,XX +XXX,XX @@ static int fd_open(BlockDriverState *bs) return -EIO; } -static coroutine_fn BlockAIOCB *hdev_aio_pdiscard(BlockDriverState *bs, - int64_t offset, int bytes, - BlockCompletionFunc *cb, void *opaque) +static coroutine_fn int +hdev_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes) { BDRVRawState *s = bs->opaque; + int ret; - if (fd_open(bs) < 0) { - return NULL; + ret = fd_open(bs); + if (ret < 0) { + return ret; } - return paio_submit(bs, s->fd, offset, NULL, bytes, - cb, opaque, QEMU_AIO_DISCARD|QEMU_AIO_BLKDEV); + return paio_submit_co(bs, s->fd, offset, NULL, bytes, + QEMU_AIO_DISCARD | QEMU_AIO_BLKDEV); } static coroutine_fn int hdev_co_pwrite_zeroes(BlockDriverState *bs, @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_host_device = { .bdrv_co_preadv = raw_co_preadv, .bdrv_co_pwritev = raw_co_pwritev, - .bdrv_aio_flush = raw_aio_flush, - .bdrv_aio_pdiscard = hdev_aio_pdiscard, + .bdrv_co_flush_to_disk = raw_co_flush_to_disk, + .bdrv_co_pdiscard = hdev_co_pdiscard, .bdrv_co_copy_range_from = raw_co_copy_range_from, .bdrv_co_copy_range_to = raw_co_copy_range_to, .bdrv_refresh_limits = raw_refresh_limits, @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_host_cdrom = { .bdrv_co_preadv = raw_co_preadv, .bdrv_co_pwritev = raw_co_pwritev, - .bdrv_aio_flush = raw_aio_flush, + .bdrv_co_flush_to_disk = raw_co_flush_to_disk, .bdrv_refresh_limits = raw_refresh_limits, .bdrv_io_plug = raw_aio_plug, .bdrv_io_unplug = raw_aio_unplug, @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_host_cdrom = { .bdrv_co_preadv = raw_co_preadv, .bdrv_co_pwritev = raw_co_pwritev, - .bdrv_aio_flush = raw_aio_flush, + .bdrv_co_flush_to_disk = raw_co_flush_to_disk, .bdrv_refresh_limits = raw_refresh_limits, .bdrv_io_plug = raw_aio_plug, .bdrv_io_unplug = raw_aio_unplug, -- 2.13.6 From: Fam Zheng <famz@redhat.com> Not updating src_offset will result in wrong data being written to dst image. Reported-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block/qcow2.c | 1 + tests/qemu-iotests/063 | 9 +++++++++ tests/qemu-iotests/063.out | 12 ++++++++++++ 3 files changed, 22 insertions(+) diff --git a/block/qcow2.c b/block/qcow2.c index XXXXXXX..XXXXXXX 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -XXX,XX +XXX,XX @@ qcow2_co_copy_range_to(BlockDriverState *bs, } bytes -= cur_bytes; + src_offset += cur_bytes; dst_offset += cur_bytes; } ret = 0; diff --git a/tests/qemu-iotests/063 b/tests/qemu-iotests/063 index XXXXXXX..XXXXXXX 100755 --- a/tests/qemu-iotests/063 +++ b/tests/qemu-iotests/063 @@ -XXX,XX +XXX,XX @@ if $QEMU_IMG convert -f $IMGFMT -O $IMGFMT -n "$TEST_IMG.orig" "$TEST_IMG" >/dev exit 1 fi +echo "== Regression testing for copy offloading bug ==" + +_make_test_img 1M +TEST_IMG="$TEST_IMG.target" _make_test_img 1M +$QEMU_IO -c 'write -P 1 0 512k' -c 'write -P 2 512k 512k' "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c 'write -P 4 512k 512k' -c 'write -P 3 0 512k' "$TEST_IMG.target" | _filter_qemu_io +$QEMU_IMG convert -n -O $IMGFMT "$TEST_IMG" "$TEST_IMG.target" +$QEMU_IMG compare "$TEST_IMG" "$TEST_IMG.target" + echo "*** done" rm -f $seq.full status=0 diff --git a/tests/qemu-iotests/063.out b/tests/qemu-iotests/063.out index XXXXXXX..XXXXXXX 100644 --- a/tests/qemu-iotests/063.out +++ b/tests/qemu-iotests/063.out @@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4194304 No errors were found on the image. == Testing conversion to a smaller file fails == Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2097152 +== Regression testing for copy offloading bug == +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 +Formatting 'TEST_DIR/t.IMGFMT.target', fmt=IMGFMT size=1048576 +wrote 524288/524288 bytes at offset 0 +512 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +wrote 524288/524288 bytes at offset 524288 +512 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +wrote 524288/524288 bytes at offset 524288 +512 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +wrote 524288/524288 bytes at offset 0 +512 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +Images are identical. *** done -- 2.13.6 From: Fam Zheng <famz@redhat.com> Per SCSI definition the designator_length we receive from INQUIRY is 8, 12 or at most 16, but we should be careful because the remote iscsi target may misbehave, otherwise we could have a buffer overflow. Reported-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block/iscsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/iscsi.c b/block/iscsi.c index XXXXXXX..XXXXXXX 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -XXX,XX +XXX,XX @@ static void iscsi_populate_target_desc(unsigned char *desc, IscsiLun *lun) desc[5] = (dd->designator_type & 0xF) | ((dd->association & 3) << 4); desc[7] = dd->designator_length; - memcpy(desc + 8, dd->designator, dd->designator_length); + memcpy(desc + 8, dd->designator, MIN(dd->designator_length, 20)); desc[28] = 0; desc[29] = (lun->block_size >> 16) & 0xFF; -- 2.13.6 From: Fam Zheng <famz@redhat.com> EINTR should be checked against errno, not ret. While fixing the bug, collect the branches with a switch block. Also, change the return value from -ENOSTUP to -ENOSPC when the actual issue is request range passes EOF, which should be distinguishable from the case of error == ENOSYS by the caller, so that it could still retry with other byte ranges, whereas it shouldn't retry anymore upon ENOSYS. Signed-off-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block/file-posix.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/block/file-posix.c b/block/file-posix.c index XXXXXXX..XXXXXXX 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -XXX,XX +XXX,XX @@ static ssize_t handle_aiocb_copy_range(RawPosixAIOData *aiocb) ssize_t ret = copy_file_range(aiocb->aio_fildes, &in_off, aiocb->aio_fd2, &out_off, bytes, 0); - if (ret == -EINTR) { - continue; + if (ret == 0) { + /* No progress (e.g. when beyond EOF), let the caller fall back to + * buffer I/O. */ + return -ENOSPC; } if (ret < 0) { - if (errno == ENOSYS) { + switch (errno) { + case ENOSYS: return -ENOTSUP; - } else { + case EINTR: + continue; + default: return -errno; } } - if (!ret) { - /* No progress (e.g. when beyond EOF), fall back to buffer I/O. */ - return -ENOTSUP; - } bytes -= ret; } return 0; -- 2.13.6 From: Eric Blake <eblake@redhat.com> We are gradually moving away from sector-based interfaces, towards byte-based. Make the change for the last few sector-based calls into the block layer from the parallels driver. Ideally, the parallels driver should switch to doing everything byte-based, but that's a more invasive change that requires a bit more auditing. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Denis V. Lunev <den@openvz.org> Reviewed-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block/parallels.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/block/parallels.c b/block/parallels.c index XXXXXXX..XXXXXXX 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -XXX,XX +XXX,XX @@ static int64_t allocate_clusters(BlockDriverState *bs, int64_t sector_num, }; qemu_iovec_init_external(&qiov, &iov, 1); - ret = bdrv_co_readv(bs->backing, idx * s->tracks, nb_cow_sectors, - &qiov); + ret = bdrv_co_preadv(bs->backing, idx * s->tracks * BDRV_SECTOR_SIZE, + nb_cow_bytes, &qiov, 0); if (ret < 0) { qemu_vfree(iov.iov_base); return ret; } - ret = bdrv_co_writev(bs->file, s->data_end, nb_cow_sectors, &qiov); + ret = bdrv_co_pwritev(bs->file, s->data_end * BDRV_SECTOR_SIZE, + nb_cow_bytes, &qiov, 0); qemu_vfree(iov.iov_base); if (ret < 0) { return ret; @@ -XXX,XX +XXX,XX @@ static coroutine_fn int parallels_co_writev(BlockDriverState *bs, qemu_iovec_reset(&hd_qiov); qemu_iovec_concat(&hd_qiov, qiov, bytes_done, nbytes); - ret = bdrv_co_writev(bs->file, position, n, &hd_qiov); + ret = bdrv_co_pwritev(bs->file, position * BDRV_SECTOR_SIZE, nbytes, + &hd_qiov, 0); if (ret < 0) { break; } @@ -XXX,XX +XXX,XX @@ static coroutine_fn int parallels_co_readv(BlockDriverState *bs, if (position < 0) { if (bs->backing) { - ret = bdrv_co_readv(bs->backing, sector_num, n, &hd_qiov); + ret = bdrv_co_preadv(bs->backing, sector_num * BDRV_SECTOR_SIZE, + nbytes, &hd_qiov, 0); if (ret < 0) { break; } @@ -XXX,XX +XXX,XX @@ static coroutine_fn int parallels_co_readv(BlockDriverState *bs, qemu_iovec_memset(&hd_qiov, 0, 0, nbytes); } } else { - ret = bdrv_co_readv(bs->file, position, n, &hd_qiov); + ret = bdrv_co_preadv(bs->file, position * BDRV_SECTOR_SIZE, nbytes, + &hd_qiov, 0); if (ret < 0) { break; } -- 2.13.6 From: Eric Blake <eblake@redhat.com> We are gradually moving away from sector-based interfaces, towards byte-based. Make the change for the internal helper function get_cluster_offset(), by changing n_start and n_end to be byte offsets rather than sector indices within the cluster being allocated. However, assert that these values are still sector-aligned (at least qcrypto_block_encrypt() still wants that). For now we get that alignment for free because we still use sector-based driver callbacks. A later patch will then switch the qcow driver as a whole over to byte-based operation; but will still leave things at sector alignments as it is not worth auditing the qcow image format to worry about sub-sector requests. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block/qcow.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/block/qcow.c b/block/qcow.c index XXXXXXX..XXXXXXX 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -XXX,XX +XXX,XX @@ static int qcow_reopen_prepare(BDRVReopenState *state, * * 0 to not allocate. * - * 1 to allocate a normal cluster (for sector indexes 'n_start' to - * 'n_end') + * 1 to allocate a normal cluster (for sector-aligned byte offsets 'n_start' + * to 'n_end' within the cluster) * * 2 to allocate a compressed cluster of size * 'compressed_size'. 'compressed_size' must be > 0 and < @@ -XXX,XX +XXX,XX @@ static int get_cluster_offset(BlockDriverState *bs, if (!allocate) return 0; BLKDBG_EVENT(bs->file, BLKDBG_CLUSTER_ALLOC); + assert(QEMU_IS_ALIGNED(n_start | n_end, BDRV_SECTOR_SIZE)); /* allocate a new cluster */ if ((cluster_offset & QCOW_OFLAG_COMPRESSED) && - (n_end - n_start) < s->cluster_sectors) { + (n_end - n_start) < s->cluster_size) { /* if the cluster is already compressed, we must decompress it in the case it is not completely overwritten */ @@ -XXX,XX +XXX,XX @@ static int get_cluster_offset(BlockDriverState *bs, /* if encrypted, we must initialize the cluster content which won't be written */ if (bs->encrypted && - (n_end - n_start) < s->cluster_sectors) { - uint64_t start_sect; + (n_end - n_start) < s->cluster_size) { + uint64_t start_offset; assert(s->crypto); - start_sect = (offset & ~(s->cluster_size - 1)) >> 9; - for(i = 0; i < s->cluster_sectors; i++) { + start_offset = offset & ~(s->cluster_size - 1); + for (i = 0; i < s->cluster_size; i += BDRV_SECTOR_SIZE) { if (i < n_start || i >= n_end) { - memset(s->cluster_data, 0x00, 512); + memset(s->cluster_data, 0x00, BDRV_SECTOR_SIZE); if (qcrypto_block_encrypt(s->crypto, - (start_sect + i) * - BDRV_SECTOR_SIZE, + start_offset + i, s->cluster_data, BDRV_SECTOR_SIZE, NULL) < 0) { @@ -XXX,XX +XXX,XX @@ static int get_cluster_offset(BlockDriverState *bs, } BLKDBG_EVENT(bs->file, BLKDBG_WRITE_AIO); ret = bdrv_pwrite(bs->file, - cluster_offset + i * 512, - s->cluster_data, 512); + cluster_offset + i, + s->cluster_data, + BDRV_SECTOR_SIZE); if (ret < 0) { return ret; } @@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow_co_writev(BlockDriverState *bs, int64_t sector_num, n = nb_sectors; } ret = get_cluster_offset(bs, sector_num << 9, 1, 0, - index_in_cluster, - index_in_cluster + n, &cluster_offset); + index_in_cluster << 9, + (index_in_cluster + n) << 9, &cluster_offset); if (ret < 0) { break; } -- 2.13.6 From: Eric Blake <eblake@redhat.com> We are gradually moving away from sector-based interfaces, towards byte-based. Make the change for the internals of the qcow driver read function, by iterating over offset/bytes instead of sector_num/nb_sectors, and with a rename of index_in_cluster and repurposing of n to track bytes instead of sectors. A later patch will then switch the qcow driver as a whole over to byte-based operation. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block/qcow.c | 42 ++++++++++++++++++++---------------------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/block/qcow.c b/block/qcow.c index XXXXXXX..XXXXXXX 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num, int nb_sectors, QEMUIOVector *qiov) { BDRVQcowState *s = bs->opaque; - int index_in_cluster; + int offset_in_cluster; int ret = 0, n; uint64_t cluster_offset; struct iovec hd_iov; QEMUIOVector hd_qiov; uint8_t *buf; void *orig_buf; + int64_t offset = sector_num * BDRV_SECTOR_SIZE; + int64_t bytes = nb_sectors * BDRV_SECTOR_SIZE; if (qiov->niov > 1) { buf = orig_buf = qemu_try_blockalign(bs, qiov->size); @@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num, qemu_co_mutex_lock(&s->lock); - while (nb_sectors != 0) { + while (bytes != 0) { /* prepare next request */ - ret = get_cluster_offset(bs, sector_num << 9, - 0, 0, 0, 0, &cluster_offset); + ret = get_cluster_offset(bs, offset, 0, 0, 0, 0, &cluster_offset); if (ret < 0) { break; } - index_in_cluster = sector_num & (s->cluster_sectors - 1); - n = s->cluster_sectors - index_in_cluster; - if (n > nb_sectors) { - n = nb_sectors; + offset_in_cluster = offset & (s->cluster_size - 1); + n = s->cluster_size - offset_in_cluster; + if (n > bytes) { + n = bytes; } if (!cluster_offset) { if (bs->backing) { /* read from the base image */ hd_iov.iov_base = (void *)buf; - hd_iov.iov_len = n * 512; + hd_iov.iov_len = n; qemu_iovec_init_external(&hd_qiov, &hd_iov, 1); qemu_co_mutex_unlock(&s->lock); /* qcow2 emits this on bs->file instead of bs->backing */ BLKDBG_EVENT(bs->file, BLKDBG_READ_BACKING_AIO); - ret = bdrv_co_readv(bs->backing, sector_num, n, &hd_qiov); + ret = bdrv_co_preadv(bs->backing, offset, n, &hd_qiov, 0); qemu_co_mutex_lock(&s->lock); if (ret < 0) { break; } } else { /* Note: in this case, no need to wait */ - memset(buf, 0, 512 * n); + memset(buf, 0, n); } } else if (cluster_offset & QCOW_OFLAG_COMPRESSED) { /* add AIO support for compressed blocks ? */ @@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num, ret = -EIO; break; } - memcpy(buf, - s->cluster_cache + index_in_cluster * 512, 512 * n); + memcpy(buf, s->cluster_cache + offset_in_cluster, n); } else { if ((cluster_offset & 511) != 0) { ret = -EIO; break; } hd_iov.iov_base = (void *)buf; - hd_iov.iov_len = n * 512; + hd_iov.iov_len = n; qemu_iovec_init_external(&hd_qiov, &hd_iov, 1); qemu_co_mutex_unlock(&s->lock); BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO); - ret = bdrv_co_readv(bs->file, - (cluster_offset >> 9) + index_in_cluster, - n, &hd_qiov); + ret = bdrv_co_preadv(bs->file, cluster_offset + offset_in_cluster, + n, &hd_qiov, 0); qemu_co_mutex_lock(&s->lock); if (ret < 0) { break; @@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num, if (bs->encrypted) { assert(s->crypto); if (qcrypto_block_decrypt(s->crypto, - sector_num * BDRV_SECTOR_SIZE, buf, - n * BDRV_SECTOR_SIZE, NULL) < 0) { + offset, buf, n, NULL) < 0) { ret = -EIO; break; } @@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num, } ret = 0; - nb_sectors -= n; - sector_num += n; - buf += n * 512; + bytes -= n; + offset += n; + buf += n; } qemu_co_mutex_unlock(&s->lock); -- 2.13.6 From: Eric Blake <eblake@redhat.com> We are gradually moving away from sector-based interfaces, towards byte-based. Make the change for the internals of the qcow driver write function, by iterating over offset/bytes instead of sector_num/nb_sectors, and with a rename of index_in_cluster and repurposing of n to track bytes instead of sectors. A later patch will then switch the qcow driver as a whole over to byte-based operation. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block/qcow.c | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/block/qcow.c b/block/qcow.c index XXXXXXX..XXXXXXX 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow_co_writev(BlockDriverState *bs, int64_t sector_num, int flags) { BDRVQcowState *s = bs->opaque; - int index_in_cluster; + int offset_in_cluster; uint64_t cluster_offset; int ret = 0, n; struct iovec hd_iov; QEMUIOVector hd_qiov; uint8_t *buf; void *orig_buf; + int64_t offset = sector_num * BDRV_SECTOR_SIZE; + int64_t bytes = nb_sectors * BDRV_SECTOR_SIZE; assert(!flags); s->cluster_cache_offset = -1; /* disable compressed cache */ @@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow_co_writev(BlockDriverState *bs, int64_t sector_num, qemu_co_mutex_lock(&s->lock); - while (nb_sectors != 0) { - - index_in_cluster = sector_num & (s->cluster_sectors - 1); - n = s->cluster_sectors - index_in_cluster; - if (n > nb_sectors) { - n = nb_sectors; + while (bytes != 0) { + offset_in_cluster = offset & (s->cluster_size - 1); + n = s->cluster_size - offset_in_cluster; + if (n > bytes) { + n = bytes; } - ret = get_cluster_offset(bs, sector_num << 9, 1, 0, - index_in_cluster << 9, - (index_in_cluster + n) << 9, &cluster_offset); + ret = get_cluster_offset(bs, offset, 1, 0, offset_in_cluster, + offset_in_cluster + n, &cluster_offset); if (ret < 0) { break; } @@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow_co_writev(BlockDriverState *bs, int64_t sector_num, } if (bs->encrypted) { assert(s->crypto); - if (qcrypto_block_encrypt(s->crypto, sector_num * BDRV_SECTOR_SIZE, - buf, n * BDRV_SECTOR_SIZE, NULL) < 0) { + if (qcrypto_block_encrypt(s->crypto, offset, buf, n, NULL) < 0) { ret = -EIO; break; } } hd_iov.iov_base = (void *)buf; - hd_iov.iov_len = n * 512; + hd_iov.iov_len = n; qemu_iovec_init_external(&hd_qiov, &hd_iov, 1); qemu_co_mutex_unlock(&s->lock); BLKDBG_EVENT(bs->file, BLKDBG_WRITE_AIO); - ret = bdrv_co_writev(bs->file, - (cluster_offset >> 9) + index_in_cluster, - n, &hd_qiov); + ret = bdrv_co_pwritev(bs->file, cluster_offset + offset_in_cluster, + n, &hd_qiov, 0); qemu_co_mutex_lock(&s->lock); if (ret < 0) { break; } ret = 0; - nb_sectors -= n; - sector_num += n; - buf += n * 512; + bytes -= n; + offset += n; + buf += n; } qemu_co_mutex_unlock(&s->lock); -- 2.13.6 From: Eric Blake <eblake@redhat.com> We are gradually moving away from sector-based interfaces, towards byte-based. The qcow driver is now ready to fully utilize the byte-based callback interface, as long as we override the default alignment to still be 512 (needed at least for asserts present because of encryption, but easier to do everywhere than to audit which sub-sector requests are handled correctly, especially since we no longer recommend qcow for new disk images). Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block/qcow.c | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/block/qcow.c b/block/qcow.c index XXXXXXX..XXXXXXX 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -XXX,XX +XXX,XX @@ typedef struct QCowHeader { typedef struct BDRVQcowState { int cluster_bits; int cluster_size; - int cluster_sectors; int l2_bits; int l2_size; unsigned int l1_size; @@ -XXX,XX +XXX,XX @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags, } s->cluster_bits = header.cluster_bits; s->cluster_size = 1 << s->cluster_bits; - s->cluster_sectors = 1 << (s->cluster_bits - 9); s->l2_bits = header.l2_bits; s->l2_size = 1 << s->l2_bits; bs->total_sectors = header.size / 512; @@ -XXX,XX +XXX,XX @@ static int decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset) return 0; } -static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num, - int nb_sectors, QEMUIOVector *qiov) +static void qcow_refresh_limits(BlockDriverState *bs, Error **errp) +{ + /* At least encrypted images require 512-byte alignment. Apply the + * limit universally, rather than just on encrypted images, as + * it's easier to let the block layer handle rounding than to + * audit this code further. */ + bs->bl.request_alignment = BDRV_SECTOR_SIZE; +} + +static coroutine_fn int qcow_co_preadv(BlockDriverState *bs, uint64_t offset, + uint64_t bytes, QEMUIOVector *qiov, + int flags) { BDRVQcowState *s = bs->opaque; int offset_in_cluster; @@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num, QEMUIOVector hd_qiov; uint8_t *buf; void *orig_buf; - int64_t offset = sector_num * BDRV_SECTOR_SIZE; - int64_t bytes = nb_sectors * BDRV_SECTOR_SIZE; + assert(!flags); if (qiov->niov > 1) { buf = orig_buf = qemu_try_blockalign(bs, qiov->size); if (buf == NULL) { @@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num, return ret; } -static coroutine_fn int qcow_co_writev(BlockDriverState *bs, int64_t sector_num, - int nb_sectors, QEMUIOVector *qiov, - int flags) +static coroutine_fn int qcow_co_pwritev(BlockDriverState *bs, uint64_t offset, + uint64_t bytes, QEMUIOVector *qiov, + int flags) { BDRVQcowState *s = bs->opaque; int offset_in_cluster; @@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow_co_writev(BlockDriverState *bs, int64_t sector_num, QEMUIOVector hd_qiov; uint8_t *buf; void *orig_buf; - int64_t offset = sector_num * BDRV_SECTOR_SIZE; - int64_t bytes = nb_sectors * BDRV_SECTOR_SIZE; assert(!flags); s->cluster_cache_offset = -1; /* disable compressed cache */ @@ -XXX,XX +XXX,XX @@ qcow_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset, if (ret != Z_STREAM_END || out_len >= s->cluster_size) { /* could not compress: write normal cluster */ - ret = qcow_co_writev(bs, offset >> BDRV_SECTOR_BITS, - bytes >> BDRV_SECTOR_BITS, qiov, 0); + ret = qcow_co_pwritev(bs, offset, bytes, qiov, 0); if (ret < 0) { goto fail; } @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_qcow = { .bdrv_co_create_opts = qcow_co_create_opts, .bdrv_has_zero_init = bdrv_has_zero_init_1, .supports_backing = true, + .bdrv_refresh_limits = qcow_refresh_limits, - .bdrv_co_readv = qcow_co_readv, - .bdrv_co_writev = qcow_co_writev, + .bdrv_co_preadv = qcow_co_preadv, + .bdrv_co_pwritev = qcow_co_pwritev, .bdrv_co_block_status = qcow_co_block_status, .bdrv_make_empty = qcow_make_empty, -- 2.13.6 From: Eric Blake <eblake@redhat.com> We are gradually moving away from sector-based interfaces, towards byte-based. Make the change for the last few sector-based calls into the block layer from the replication driver. Ideally, the replication driver should switch to doing everything byte-based, but that's a more invasive change that requires a bit more auditing. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block/replication.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/block/replication.c b/block/replication.c index XXXXXXX..XXXXXXX 100644 --- a/block/replication.c +++ b/block/replication.c @@ -XXX,XX +XXX,XX @@ static coroutine_fn int replication_co_readv(BlockDriverState *bs, backup_cow_request_begin(&req, child->bs->job, sector_num * BDRV_SECTOR_SIZE, remaining_bytes); - ret = bdrv_co_readv(bs->file, sector_num, remaining_sectors, - qiov); + ret = bdrv_co_preadv(bs->file, sector_num * BDRV_SECTOR_SIZE, + remaining_bytes, qiov, 0); backup_cow_request_end(&req); goto out; } - ret = bdrv_co_readv(bs->file, sector_num, remaining_sectors, qiov); + ret = bdrv_co_preadv(bs->file, sector_num * BDRV_SECTOR_SIZE, + remaining_sectors * BDRV_SECTOR_SIZE, qiov, 0); out: return replication_return_value(s, ret); } @@ -XXX,XX +XXX,XX @@ static coroutine_fn int replication_co_writev(BlockDriverState *bs, } if (ret == 0) { - ret = bdrv_co_writev(top, sector_num, - remaining_sectors, qiov); + ret = bdrv_co_pwritev(top, sector_num * BDRV_SECTOR_SIZE, + remaining_sectors * BDRV_SECTOR_SIZE, qiov, 0); return replication_return_value(s, ret); } @@ -XXX,XX +XXX,XX @@ static coroutine_fn int replication_co_writev(BlockDriverState *bs, qemu_iovec_concat(&hd_qiov, qiov, bytes_done, count); target = ret ? top : base; - ret = bdrv_co_writev(target, sector_num, n, &hd_qiov); + ret = bdrv_co_pwritev(target, sector_num * BDRV_SECTOR_SIZE, + n * BDRV_SECTOR_SIZE, &hd_qiov, 0); if (ret < 0) { goto out1; } -- 2.13.6 From: Eric Blake <eblake@redhat.com> We are gradually moving away from sector-based interfaces, towards byte-based. Make the change for the last few sector-based calls into the block layer from the vhdx driver. Ideally, the vhdx driver should switch to doing everything byte-based, but that's a more invasive change that requires a bit more auditing. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block/vhdx.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/block/vhdx.c b/block/vhdx.c index XXXXXXX..XXXXXXX 100644 --- a/block/vhdx.c +++ b/block/vhdx.c @@ -XXX,XX +XXX,XX @@ static coroutine_fn int vhdx_co_readv(BlockDriverState *bs, int64_t sector_num, break; case PAYLOAD_BLOCK_FULLY_PRESENT: qemu_co_mutex_unlock(&s->lock); - ret = bdrv_co_readv(bs->file, - sinfo.file_offset >> BDRV_SECTOR_BITS, - sinfo.sectors_avail, &hd_qiov); + ret = bdrv_co_preadv(bs->file, sinfo.file_offset, + sinfo.sectors_avail * BDRV_SECTOR_SIZE, + &hd_qiov, 0); qemu_co_mutex_lock(&s->lock); if (ret < 0) { goto exit; @@ -XXX,XX +XXX,XX @@ static coroutine_fn int vhdx_co_writev(BlockDriverState *bs, int64_t sector_num, } /* block exists, so we can just overwrite it */ qemu_co_mutex_unlock(&s->lock); - ret = bdrv_co_writev(bs->file, - sinfo.file_offset >> BDRV_SECTOR_BITS, - sectors_to_write, &hd_qiov); + ret = bdrv_co_pwritev(bs->file, sinfo.file_offset, + sectors_to_write * BDRV_SECTOR_SIZE, + &hd_qiov, 0); qemu_co_mutex_lock(&s->lock); if (ret < 0) { goto error_bat_restore; -- 2.13.6 From: Eric Blake <eblake@redhat.com> We are gradually moving away from sector-based interfaces, towards byte-based. Now that all callers of vectored I/O have been converted to use our preferred byte-based bdrv_co_p{read,write}v(), we can delete the unused bdrv_co_{read,write}v(). Furthermore, this gets rid of the signature difference between the public bdrv_co_writev() and the callback .bdrv_co_writev (the latter still exists, because some drivers still need more work before they are fully byte-based). Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- include/block/block.h | 4 ---- block/io.c | 36 ------------------------------------ 2 files changed, 40 deletions(-) diff --git a/include/block/block.h b/include/block/block.h index XXXXXXX..XXXXXXX 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -XXX,XX +XXX,XX @@ int bdrv_pwrite(BdrvChild *child, int64_t offset, const void *buf, int bytes); int bdrv_pwritev(BdrvChild *child, int64_t offset, QEMUIOVector *qiov); int bdrv_pwrite_sync(BdrvChild *child, int64_t offset, const void *buf, int count); -int coroutine_fn bdrv_co_readv(BdrvChild *child, int64_t sector_num, - int nb_sectors, QEMUIOVector *qiov); -int coroutine_fn bdrv_co_writev(BdrvChild *child, int64_t sector_num, - int nb_sectors, QEMUIOVector *qiov); /* * Efficiently zero a region of the disk image. Note that this is a regular * I/O request like read or write and should have a reasonable size. This diff --git a/block/io.c b/block/io.c index XXXXXXX..XXXXXXX 100644 --- a/block/io.c +++ b/block/io.c @@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_preadv(BdrvChild *child, return ret; } -static int coroutine_fn bdrv_co_do_readv(BdrvChild *child, - int64_t sector_num, int nb_sectors, QEMUIOVector *qiov, - BdrvRequestFlags flags) -{ - if (nb_sectors < 0 || nb_sectors > BDRV_REQUEST_MAX_SECTORS) { - return -EINVAL; - } - - return bdrv_co_preadv(child, sector_num << BDRV_SECTOR_BITS, - nb_sectors << BDRV_SECTOR_BITS, qiov, flags); -} - -int coroutine_fn bdrv_co_readv(BdrvChild *child, int64_t sector_num, - int nb_sectors, QEMUIOVector *qiov) -{ - return bdrv_co_do_readv(child, sector_num, nb_sectors, qiov, 0); -} - static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs, int64_t offset, int bytes, BdrvRequestFlags flags) { @@ -XXX,XX +XXX,XX @@ out: return ret; } -static int coroutine_fn bdrv_co_do_writev(BdrvChild *child, - int64_t sector_num, int nb_sectors, QEMUIOVector *qiov, - BdrvRequestFlags flags) -{ - if (nb_sectors < 0 || nb_sectors > BDRV_REQUEST_MAX_SECTORS) { - return -EINVAL; - } - - return bdrv_co_pwritev(child, sector_num << BDRV_SECTOR_BITS, - nb_sectors << BDRV_SECTOR_BITS, qiov, flags); -} - -int coroutine_fn bdrv_co_writev(BdrvChild *child, int64_t sector_num, - int nb_sectors, QEMUIOVector *qiov) -{ - return bdrv_co_do_writev(child, sector_num, nb_sectors, qiov, 0); -} - int coroutine_fn bdrv_co_pwrite_zeroes(BdrvChild *child, int64_t offset, int bytes, BdrvRequestFlags flags) { -- 2.13.6 The following changes since commit 16aaacb307ed607b9780c12702c44f0fe52edc7e: Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20200430' into staging (2020-04-30 14:00:36 +0100) are available in the Git repository at: git://repo.or.cz/qemu/kevin.git tags/for-upstream for you to fetch changes up to eaae29ef89d498d0eac553c77b554f310a47f809: qemu-storage-daemon: Fix non-string --object properties (2020-04-30 17:51:07 +0200) ---------------------------------------------------------------- Block layer patches: - Fix resize (extending) of short overlays - nvme: introduce PMR support from NVMe 1.4 spec - qemu-storage-daemon: Fix non-string --object properties ---------------------------------------------------------------- Alberto Garcia (1): qcow2: Add incompatibility note between backing files and raw external data files Andrzej Jakowski (1): nvme: introduce PMR support from NVMe 1.4 spec Kevin Wolf (12): block: Add flags to BlockDriver.bdrv_co_truncate() block: Add flags to bdrv(_co)_truncate() block-backend: Add flags to blk_truncate() qcow2: Support BDRV_REQ_ZERO_WRITE for truncate raw-format: Support BDRV_REQ_ZERO_WRITE for truncate file-posix: Support BDRV_REQ_ZERO_WRITE for truncate block: truncate: Don't make backing file data visible iotests: Filter testfiles out in filter_img_info() iotests: Test committing to short backing file qcow2: Forward ZERO_WRITE flag for full preallocation qom: Factor out user_creatable_add_dict() qemu-storage-daemon: Fix non-string --object properties Paolo Bonzini (1): qemu-iotests: allow qcow2 external discarded clusters to contain stale data docs/interop/qcow2.txt | 3 + hw/block/nvme.h | 2 + include/block/block.h | 5 +- include/block/block_int.h | 10 +- include/block/nvme.h | 172 ++++++++++++++++++++++++++ include/qom/object_interfaces.h | 16 +++ include/sysemu/block-backend.h | 2 +- block.c | 3 +- block/block-backend.c | 4 +- block/commit.c | 4 +- block/crypto.c | 7 +- block/file-posix.c | 6 +- block/file-win32.c | 2 +- block/gluster.c | 1 + block/io.c | 43 ++++++- block/iscsi.c | 2 +- block/mirror.c | 2 +- block/nfs.c | 3 +- block/parallels.c | 6 +- block/qcow.c | 4 +- block/qcow2-cluster.c | 2 +- block/qcow2-refcount.c | 2 +- block/qcow2.c | 73 +++++++++-- block/qed.c | 3 +- block/raw-format.c | 6 +- block/rbd.c | 1 + block/sheepdog.c | 4 +- block/ssh.c | 2 +- block/vdi.c | 2 +- block/vhdx-log.c | 2 +- block/vhdx.c | 6 +- block/vmdk.c | 8 +- block/vpc.c | 2 +- blockdev.c | 2 +- hw/block/nvme.c | 109 ++++++++++++++++ qemu-img.c | 2 +- qemu-io-cmds.c | 2 +- qemu-storage-daemon.c | 4 +- qom/object_interfaces.c | 31 +++++ qom/qom-qmp-cmds.c | 24 +--- tests/test-block-iothread.c | 9 +- tests/qemu-iotests/iotests.py | 5 +- hw/block/Makefile.objs | 2 +- hw/block/trace-events | 4 + tests/qemu-iotests/244 | 10 +- tests/qemu-iotests/244.out | 9 +- tests/qemu-iotests/274 | 155 +++++++++++++++++++++++ tests/qemu-iotests/274.out | 268 ++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/group | 1 + 49 files changed, 951 insertions(+), 96 deletions(-) create mode 100755 tests/qemu-iotests/274 create mode 100644 tests/qemu-iotests/274.out From: Alberto Garcia <berto@igalia.com> Backing files and raw external data files are mutually exclusive. The documentation of the raw external data bit (in autoclear_features) already indicates that, but we should also mention it on the other side. Suggested-by: Eric Blake <eblake@redhat.com> Signed-off-by: Alberto Garcia <berto@igalia.com> Message-Id: <20200410121816.8334-1-berto@igalia.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- docs/interop/qcow2.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt index XXXXXXX..XXXXXXX 100644 --- a/docs/interop/qcow2.txt +++ b/docs/interop/qcow2.txt @@ -XXX,XX +XXX,XX @@ The first cluster of a qcow2 image contains the file header: is stored (NB: The string is not null terminated). 0 if the image doesn't have a backing file. + Note: backing files are incompatible with raw external data + files (auto-clear feature bit 1). + 16 - 19: backing_file_size Length of the backing file name in bytes. Must not be longer than 1023 bytes. Undefined if the image doesn't have -- 2.25.3 From: Paolo Bonzini <pbonzini@redhat.com> Test 244 checks the expected behavior of qcow2 external data files with respect to zero and discarded clusters. Filesystems however are free to ignore discard requests, and this seems to be the case for overlayfs. Relax the tests to skip checks on the external data file for discarded areas, which implies not using qemu-img compare in the data_file_raw=on case. This fixes docker tests on RHEL8. Cc: Kevin Wolf <kwolf@redhat.com> Cc: qemu-block@nongnu.org Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20200409191006.24429-1-pbonzini@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- tests/qemu-iotests/244 | 10 ++++++++-- tests/qemu-iotests/244.out | 9 ++++++--- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/tests/qemu-iotests/244 b/tests/qemu-iotests/244 index XXXXXXX..XXXXXXX 100755 --- a/tests/qemu-iotests/244 +++ b/tests/qemu-iotests/244 @@ -XXX,XX +XXX,XX @@ $QEMU_IO -c 'read -P 0 0 1M' \ echo $QEMU_IO -c 'read -P 0 0 1M' \ -c 'read -P 0x11 1M 1M' \ - -c 'read -P 0 2M 2M' \ -c 'read -P 0x11 4M 1M' \ -c 'read -P 0 5M 1M' \ -f raw "$TEST_IMG.data" | @@ -XXX,XX +XXX,XX @@ $QEMU_IO -c 'read -P 0 0 1M' \ -f $IMGFMT "$TEST_IMG" | _filter_qemu_io +# Discarded clusters are only marked as such in the qcow2 metadata, but +# they can contain stale data in the external data file. Instead, zero +# clusters must be zeroed in the external data file too. echo -$QEMU_IMG compare "$TEST_IMG" "$TEST_IMG.data" +$QEMU_IO -c 'read -P 0 0 1M' \ + -c 'read -P 0x11 1M 1M' \ + -c 'read -P 0 3M 3M' \ + -f raw "$TEST_IMG".data | + _filter_qemu_io echo -n "qcow2 file size after I/O: " du -b $TEST_IMG | cut -f1 diff --git a/tests/qemu-iotests/244.out b/tests/qemu-iotests/244.out index XXXXXXX..XXXXXXX 100644 --- a/tests/qemu-iotests/244.out +++ b/tests/qemu-iotests/244.out @@ -XXX,XX +XXX,XX @@ read 1048576/1048576 bytes at offset 0 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) read 1048576/1048576 bytes at offset 1048576 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -read 2097152/2097152 bytes at offset 2097152 -2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) read 1048576/1048576 bytes at offset 4194304 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) read 1048576/1048576 bytes at offset 5242880 @@ -XXX,XX +XXX,XX @@ read 1048576/1048576 bytes at offset 1048576 read 4194304/4194304 bytes at offset 2097152 4 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -Images are identical. +read 1048576/1048576 bytes at offset 0 +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 1048576/1048576 bytes at offset 1048576 +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 3145728/3145728 bytes at offset 3145728 +3 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) qcow2 file size after I/O: 327680 === bdrv_co_block_status test for file and offset=0 === -- 2.25.3 This adds a new BdrvRequestFlags parameter to the .bdrv_co_truncate() driver callbacks, and a supported_truncate_flags field in BlockDriverState that allows drivers to advertise support for request flags in the context of truncate. For now, we always pass 0 and no drivers declare support for any flag. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-Id: <20200424125448.63318-2-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- include/block/block_int.h | 10 +++++++++- block/crypto.c | 3 ++- block/file-posix.c | 2 +- block/file-win32.c | 2 +- block/gluster.c | 1 + block/io.c | 8 +++++++- block/iscsi.c | 2 +- block/nfs.c | 3 ++- block/qcow2.c | 2 +- block/qed.c | 1 + block/raw-format.c | 2 +- block/rbd.c | 1 + block/sheepdog.c | 4 ++-- block/ssh.c | 2 +- tests/test-block-iothread.c | 3 ++- 15 files changed, 33 insertions(+), 13 deletions(-) diff --git a/include/block/block_int.h b/include/block/block_int.h index XXXXXXX..XXXXXXX 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -XXX,XX +XXX,XX @@ struct BlockDriver { */ int coroutine_fn (*bdrv_co_truncate)(BlockDriverState *bs, int64_t offset, bool exact, PreallocMode prealloc, - Error **errp); + BdrvRequestFlags flags, Error **errp); int64_t (*bdrv_getlength)(BlockDriverState *bs); bool has_variable_length; @@ -XXX,XX +XXX,XX @@ struct BlockDriverState { /* Flags honored during pwrite_zeroes (so far: BDRV_REQ_FUA, * BDRV_REQ_MAY_UNMAP, BDRV_REQ_WRITE_UNCHANGED) */ unsigned int supported_zero_flags; + /* + * Flags honoured during truncate (so far: BDRV_REQ_ZERO_WRITE). + * + * If BDRV_REQ_ZERO_WRITE is given, the truncate operation must make sure + * that any added space reads as all zeros. If this can't be guaranteed, + * the operation must fail. + */ + unsigned int supported_truncate_flags; /* the following member gives a name to every node on the bs graph. */ char node_name[32]; diff --git a/block/crypto.c b/block/crypto.c index XXXXXXX..XXXXXXX 100644 --- a/block/crypto.c +++ b/block/crypto.c @@ -XXX,XX +XXX,XX @@ static int block_crypto_co_create_generic(BlockDriverState *bs, static int coroutine_fn block_crypto_co_truncate(BlockDriverState *bs, int64_t offset, bool exact, - PreallocMode prealloc, Error **errp) + PreallocMode prealloc, BdrvRequestFlags flags, + Error **errp) { BlockCrypto *crypto = bs->opaque; uint64_t payload_offset = diff --git a/block/file-posix.c b/block/file-posix.c index XXXXXXX..XXXXXXX 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -XXX,XX +XXX,XX @@ raw_regular_truncate(BlockDriverState *bs, int fd, int64_t offset, static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset, bool exact, PreallocMode prealloc, - Error **errp) + BdrvRequestFlags flags, Error **errp) { BDRVRawState *s = bs->opaque; struct stat st; diff --git a/block/file-win32.c b/block/file-win32.c index XXXXXXX..XXXXXXX 100644 --- a/block/file-win32.c +++ b/block/file-win32.c @@ -XXX,XX +XXX,XX @@ static void raw_close(BlockDriverState *bs) static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset, bool exact, PreallocMode prealloc, - Error **errp) + BdrvRequestFlags flags, Error **errp) { BDRVRawState *s = bs->opaque; LONG low, high; diff --git a/block/gluster.c b/block/gluster.c index XXXXXXX..XXXXXXX 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -XXX,XX +XXX,XX @@ static coroutine_fn int qemu_gluster_co_truncate(BlockDriverState *bs, int64_t offset, bool exact, PreallocMode prealloc, + BdrvRequestFlags flags, Error **errp) { BDRVGlusterState *s = bs->opaque; diff --git a/block/io.c b/block/io.c index XXXXXXX..XXXXXXX 100644 --- a/block/io.c +++ b/block/io.c @@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, bool exact, BlockDriverState *bs = child->bs; BlockDriver *drv = bs->drv; BdrvTrackedRequest req; + BdrvRequestFlags flags = 0; int64_t old_size, new_bytes; int ret; @@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, bool exact, } if (drv->bdrv_co_truncate) { - ret = drv->bdrv_co_truncate(bs, offset, exact, prealloc, errp); + if (flags & ~bs->supported_truncate_flags) { + error_setg(errp, "Block driver does not support requested flags"); + ret = -ENOTSUP; + goto out; + } + ret = drv->bdrv_co_truncate(bs, offset, exact, prealloc, flags, errp); } else if (bs->file && drv->is_filter) { ret = bdrv_co_truncate(bs->file, offset, exact, prealloc, errp); } else { diff --git a/block/iscsi.c b/block/iscsi.c index XXXXXXX..XXXXXXX 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -XXX,XX +XXX,XX @@ static void iscsi_reopen_commit(BDRVReopenState *reopen_state) static int coroutine_fn iscsi_co_truncate(BlockDriverState *bs, int64_t offset, bool exact, PreallocMode prealloc, - Error **errp) + BdrvRequestFlags flags, Error **errp) { IscsiLun *iscsilun = bs->opaque; int64_t cur_length; diff --git a/block/nfs.c b/block/nfs.c index XXXXXXX..XXXXXXX 100644 --- a/block/nfs.c +++ b/block/nfs.c @@ -XXX,XX +XXX,XX @@ static int64_t nfs_get_allocated_file_size(BlockDriverState *bs) static int coroutine_fn nfs_file_co_truncate(BlockDriverState *bs, int64_t offset, bool exact, - PreallocMode prealloc, Error **errp) + PreallocMode prealloc, BdrvRequestFlags flags, + Error **errp) { NFSClient *client = bs->opaque; int ret; diff --git a/block/qcow2.c b/block/qcow2.c index XXXXXXX..XXXXXXX 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -XXX,XX +XXX,XX @@ fail: static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset, bool exact, PreallocMode prealloc, - Error **errp) + BdrvRequestFlags flags, Error **errp) { BDRVQcow2State *s = bs->opaque; uint64_t old_length; diff --git a/block/qed.c b/block/qed.c index XXXXXXX..XXXXXXX 100644 --- a/block/qed.c +++ b/block/qed.c @@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_qed_co_truncate(BlockDriverState *bs, int64_t offset, bool exact, PreallocMode prealloc, + BdrvRequestFlags flags, Error **errp) { BDRVQEDState *s = bs->opaque; diff --git a/block/raw-format.c b/block/raw-format.c index XXXXXXX..XXXXXXX 100644 --- a/block/raw-format.c +++ b/block/raw-format.c @@ -XXX,XX +XXX,XX @@ static void raw_refresh_limits(BlockDriverState *bs, Error **errp) static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset, bool exact, PreallocMode prealloc, - Error **errp) + BdrvRequestFlags flags, Error **errp) { BDRVRawState *s = bs->opaque; diff --git a/block/rbd.c b/block/rbd.c index XXXXXXX..XXXXXXX 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -XXX,XX +XXX,XX @@ static int coroutine_fn qemu_rbd_co_truncate(BlockDriverState *bs, int64_t offset, bool exact, PreallocMode prealloc, + BdrvRequestFlags flags, Error **errp) { int r; diff --git a/block/sheepdog.c b/block/sheepdog.c index XXXXXXX..XXXXXXX 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -XXX,XX +XXX,XX @@ static int64_t sd_getlength(BlockDriverState *bs) static int coroutine_fn sd_co_truncate(BlockDriverState *bs, int64_t offset, bool exact, PreallocMode prealloc, - Error **errp) + BdrvRequestFlags flags, Error **errp) { BDRVSheepdogState *s = bs->opaque; int ret, fd; @@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num, assert(!flags); if (offset > s->inode.vdi_size) { - ret = sd_co_truncate(bs, offset, false, PREALLOC_MODE_OFF, NULL); + ret = sd_co_truncate(bs, offset, false, PREALLOC_MODE_OFF, 0, NULL); if (ret < 0) { return ret; } diff --git a/block/ssh.c b/block/ssh.c index XXXXXXX..XXXXXXX 100644 --- a/block/ssh.c +++ b/block/ssh.c @@ -XXX,XX +XXX,XX @@ static int64_t ssh_getlength(BlockDriverState *bs) static int coroutine_fn ssh_co_truncate(BlockDriverState *bs, int64_t offset, bool exact, PreallocMode prealloc, - Error **errp) + BdrvRequestFlags flags, Error **errp) { BDRVSSHState *s = bs->opaque; diff --git a/tests/test-block-iothread.c b/tests/test-block-iothread.c index XXXXXXX..XXXXXXX 100644 --- a/tests/test-block-iothread.c +++ b/tests/test-block-iothread.c @@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_test_co_pdiscard(BlockDriverState *bs, static int coroutine_fn bdrv_test_co_truncate(BlockDriverState *bs, int64_t offset, bool exact, - PreallocMode prealloc, Error **errp) + PreallocMode prealloc, BdrvRequestFlags flags, + Error **errp) { return 0; } -- 2.25.3 Now that block drivers can support flags for .bdrv_co_truncate, expose the parameter in the node level interfaces bdrv_co_truncate() and bdrv_truncate(). Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-Id: <20200424125448.63318-3-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- include/block/block.h | 5 +++-- block/block-backend.c | 2 +- block/crypto.c | 2 +- block/io.c | 12 +++++++----- block/parallels.c | 6 +++--- block/qcow.c | 4 ++-- block/qcow2-refcount.c | 2 +- block/qcow2.c | 15 +++++++++------ block/raw-format.c | 2 +- block/vhdx-log.c | 2 +- block/vhdx.c | 2 +- block/vmdk.c | 2 +- tests/test-block-iothread.c | 6 +++--- 13 files changed, 34 insertions(+), 28 deletions(-) diff --git a/include/block/block.h b/include/block/block.h index XXXXXXX..XXXXXXX 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -XXX,XX +XXX,XX @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs, void bdrv_refresh_filename(BlockDriverState *bs); int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, bool exact, - PreallocMode prealloc, Error **errp); + PreallocMode prealloc, BdrvRequestFlags flags, + Error **errp); int bdrv_truncate(BdrvChild *child, int64_t offset, bool exact, - PreallocMode prealloc, Error **errp); + PreallocMode prealloc, BdrvRequestFlags flags, Error **errp); int64_t bdrv_nb_sectors(BlockDriverState *bs); int64_t bdrv_getlength(BlockDriverState *bs); diff --git a/block/block-backend.c b/block/block-backend.c index XXXXXXX..XXXXXXX 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -XXX,XX +XXX,XX @@ int blk_truncate(BlockBackend *blk, int64_t offset, bool exact, return -ENOMEDIUM; } - return bdrv_truncate(blk->root, offset, exact, prealloc, errp); + return bdrv_truncate(blk->root, offset, exact, prealloc, 0, errp); } int blk_save_vmstate(BlockBackend *blk, const uint8_t *buf, diff --git a/block/crypto.c b/block/crypto.c index XXXXXXX..XXXXXXX 100644 --- a/block/crypto.c +++ b/block/crypto.c @@ -XXX,XX +XXX,XX @@ block_crypto_co_truncate(BlockDriverState *bs, int64_t offset, bool exact, offset += payload_offset; - return bdrv_co_truncate(bs->file, offset, exact, prealloc, errp); + return bdrv_co_truncate(bs->file, offset, exact, prealloc, 0, errp); } static void block_crypto_close(BlockDriverState *bs) diff --git a/block/io.c b/block/io.c index XXXXXXX..XXXXXXX 100644 --- a/block/io.c +++ b/block/io.c @@ -XXX,XX +XXX,XX @@ static void bdrv_parent_cb_resize(BlockDriverState *bs) * 'offset' bytes in length. */ int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, bool exact, - PreallocMode prealloc, Error **errp) + PreallocMode prealloc, BdrvRequestFlags flags, + Error **errp) { BlockDriverState *bs = child->bs; BlockDriver *drv = bs->drv; BdrvTrackedRequest req; - BdrvRequestFlags flags = 0; int64_t old_size, new_bytes; int ret; @@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, bool exact, } ret = drv->bdrv_co_truncate(bs, offset, exact, prealloc, flags, errp); } else if (bs->file && drv->is_filter) { - ret = bdrv_co_truncate(bs->file, offset, exact, prealloc, errp); + ret = bdrv_co_truncate(bs->file, offset, exact, prealloc, flags, errp); } else { error_setg(errp, "Image format driver does not support resize"); ret = -ENOTSUP; @@ -XXX,XX +XXX,XX @@ typedef struct TruncateCo { int64_t offset; bool exact; PreallocMode prealloc; + BdrvRequestFlags flags; Error **errp; int ret; } TruncateCo; @@ -XXX,XX +XXX,XX @@ static void coroutine_fn bdrv_truncate_co_entry(void *opaque) { TruncateCo *tco = opaque; tco->ret = bdrv_co_truncate(tco->child, tco->offset, tco->exact, - tco->prealloc, tco->errp); + tco->prealloc, tco->flags, tco->errp); aio_wait_kick(); } int bdrv_truncate(BdrvChild *child, int64_t offset, bool exact, - PreallocMode prealloc, Error **errp) + PreallocMode prealloc, BdrvRequestFlags flags, Error **errp) { Coroutine *co; TruncateCo tco = { @@ -XXX,XX +XXX,XX @@ int bdrv_truncate(BdrvChild *child, int64_t offset, bool exact, .offset = offset, .exact = exact, .prealloc = prealloc, + .flags = flags, .errp = errp, .ret = NOT_DONE, }; diff --git a/block/parallels.c b/block/parallels.c index XXXXXXX..XXXXXXX 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -XXX,XX +XXX,XX @@ static int64_t allocate_clusters(BlockDriverState *bs, int64_t sector_num, } else { ret = bdrv_truncate(bs->file, (s->data_end + space) << BDRV_SECTOR_BITS, - false, PREALLOC_MODE_OFF, NULL); + false, PREALLOC_MODE_OFF, 0, NULL); } if (ret < 0) { return ret; @@ -XXX,XX +XXX,XX @@ static int coroutine_fn parallels_co_check(BlockDriverState *bs, * That means we have to pass exact=true. */ ret = bdrv_truncate(bs->file, res->image_end_offset, true, - PREALLOC_MODE_OFF, &local_err); + PREALLOC_MODE_OFF, 0, &local_err); if (ret < 0) { error_report_err(local_err); res->check_errors++; @@ -XXX,XX +XXX,XX @@ static void parallels_close(BlockDriverState *bs) /* errors are ignored, so we might as well pass exact=true */ bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS, true, - PREALLOC_MODE_OFF, NULL); + PREALLOC_MODE_OFF, 0, NULL); } g_free(s->bat_dirty_bmap); diff --git a/block/qcow.c b/block/qcow.c index XXXXXXX..XXXXXXX 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -XXX,XX +XXX,XX @@ static int get_cluster_offset(BlockDriverState *bs, return -E2BIG; } ret = bdrv_truncate(bs->file, cluster_offset + s->cluster_size, - false, PREALLOC_MODE_OFF, NULL); + false, PREALLOC_MODE_OFF, 0, NULL); if (ret < 0) { return ret; } @@ -XXX,XX +XXX,XX @@ static int qcow_make_empty(BlockDriverState *bs) l1_length) < 0) return -1; ret = bdrv_truncate(bs->file, s->l1_table_offset + l1_length, false, - PREALLOC_MODE_OFF, NULL); + PREALLOC_MODE_OFF, 0, NULL); if (ret < 0) return ret; diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index XXXXXXX..XXXXXXX 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -XXX,XX +XXX,XX @@ static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res, } ret = bdrv_truncate(bs->file, offset + s->cluster_size, false, - PREALLOC_MODE_OFF, &local_err); + PREALLOC_MODE_OFF, 0, &local_err); if (ret < 0) { error_report_err(local_err); goto resize_fail; diff --git a/block/qcow2.c b/block/qcow2.c index XXXXXXX..XXXXXXX 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -XXX,XX +XXX,XX @@ static int coroutine_fn preallocate_co(BlockDriverState *bs, uint64_t offset, mode = PREALLOC_MODE_OFF; } ret = bdrv_co_truncate(s->data_file, host_offset + cur_bytes, false, - mode, errp); + mode, 0, errp); if (ret < 0) { return ret; } @@ -XXX,XX +XXX,XX @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset, * always fulfilled, so there is no need to pass it on.) */ bdrv_co_truncate(bs->file, (last_cluster + 1) * s->cluster_size, - false, PREALLOC_MODE_OFF, &local_err); + false, PREALLOC_MODE_OFF, 0, &local_err); if (local_err) { warn_reportf_err(local_err, "Failed to truncate the tail of the image: "); @@ -XXX,XX +XXX,XX @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset, * file should be resized to the exact target size, too, * so we pass @exact here. */ - ret = bdrv_co_truncate(s->data_file, offset, exact, prealloc, errp); + ret = bdrv_co_truncate(s->data_file, offset, exact, prealloc, 0, + errp); if (ret < 0) { goto fail; } @@ -XXX,XX +XXX,XX @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset, new_file_size = allocation_start + nb_new_data_clusters * s->cluster_size; /* Image file grows, so @exact does not matter */ - ret = bdrv_co_truncate(bs->file, new_file_size, false, prealloc, errp); + ret = bdrv_co_truncate(bs->file, new_file_size, false, prealloc, 0, + errp); if (ret < 0) { error_prepend(errp, "Failed to resize underlying file: "); qcow2_free_clusters(bs, allocation_start, @@ -XXX,XX +XXX,XX @@ qcow2_co_pwritev_compressed_part(BlockDriverState *bs, if (len < 0) { return len; } - return bdrv_co_truncate(bs->file, len, false, PREALLOC_MODE_OFF, NULL); + return bdrv_co_truncate(bs->file, len, false, PREALLOC_MODE_OFF, 0, + NULL); } if (offset_into_cluster(s, offset)) { @@ -XXX,XX +XXX,XX @@ static int make_completely_empty(BlockDriverState *bs) } ret = bdrv_truncate(bs->file, (3 + l1_clusters) * s->cluster_size, false, - PREALLOC_MODE_OFF, &local_err); + PREALLOC_MODE_OFF, 0, &local_err); if (ret < 0) { error_report_err(local_err); goto fail; diff --git a/block/raw-format.c b/block/raw-format.c index XXXXXXX..XXXXXXX 100644 --- a/block/raw-format.c +++ b/block/raw-format.c @@ -XXX,XX +XXX,XX @@ static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset, s->size = offset; offset += s->offset; - return bdrv_co_truncate(bs->file, offset, exact, prealloc, errp); + return bdrv_co_truncate(bs->file, offset, exact, prealloc, 0, errp); } static void raw_eject(BlockDriverState *bs, bool eject_flag) diff --git a/block/vhdx-log.c b/block/vhdx-log.c index XXXXXXX..XXXXXXX 100644 --- a/block/vhdx-log.c +++ b/block/vhdx-log.c @@ -XXX,XX +XXX,XX @@ static int vhdx_log_flush(BlockDriverState *bs, BDRVVHDXState *s, goto exit; } ret = bdrv_truncate(bs->file, new_file_size, false, - PREALLOC_MODE_OFF, NULL); + PREALLOC_MODE_OFF, 0, NULL); if (ret < 0) { goto exit; } diff --git a/block/vhdx.c b/block/vhdx.c index XXXXXXX..XXXXXXX 100644 --- a/block/vhdx.c +++ b/block/vhdx.c @@ -XXX,XX +XXX,XX @@ static int vhdx_allocate_block(BlockDriverState *bs, BDRVVHDXState *s, } return bdrv_truncate(bs->file, *new_offset + s->block_size, false, - PREALLOC_MODE_OFF, NULL); + PREALLOC_MODE_OFF, 0, NULL); } /* diff --git a/block/vmdk.c b/block/vmdk.c index XXXXXXX..XXXXXXX 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -XXX,XX +XXX,XX @@ vmdk_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset, } length = QEMU_ALIGN_UP(length, BDRV_SECTOR_SIZE); ret = bdrv_truncate(s->extents[i].file, length, false, - PREALLOC_MODE_OFF, NULL); + PREALLOC_MODE_OFF, 0, NULL); if (ret < 0) { return ret; } diff --git a/tests/test-block-iothread.c b/tests/test-block-iothread.c index XXXXXXX..XXXXXXX 100644 --- a/tests/test-block-iothread.c +++ b/tests/test-block-iothread.c @@ -XXX,XX +XXX,XX @@ static void test_sync_op_truncate(BdrvChild *c) int ret; /* Normal success path */ - ret = bdrv_truncate(c, 65536, false, PREALLOC_MODE_OFF, NULL); + ret = bdrv_truncate(c, 65536, false, PREALLOC_MODE_OFF, 0, NULL); g_assert_cmpint(ret, ==, 0); /* Early error: Negative offset */ - ret = bdrv_truncate(c, -2, false, PREALLOC_MODE_OFF, NULL); + ret = bdrv_truncate(c, -2, false, PREALLOC_MODE_OFF, 0, NULL); g_assert_cmpint(ret, ==, -EINVAL); /* Error: Read-only image */ c->bs->read_only = true; c->bs->open_flags &= ~BDRV_O_RDWR; - ret = bdrv_truncate(c, 65536, false, PREALLOC_MODE_OFF, NULL); + ret = bdrv_truncate(c, 65536, false, PREALLOC_MODE_OFF, 0, NULL); g_assert_cmpint(ret, ==, -EACCES); c->bs->read_only = false; -- 2.25.3 Now that node level interface bdrv_truncate() supports passing request flags to the block driver, expose this on the BlockBackend level, too. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-Id: <20200424125448.63318-4-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- include/sysemu/block-backend.h | 2 +- block.c | 3 ++- block/block-backend.c | 4 ++-- block/commit.c | 4 ++-- block/crypto.c | 2 +- block/mirror.c | 2 +- block/qcow2.c | 4 ++-- block/qed.c | 2 +- block/vdi.c | 2 +- block/vhdx.c | 4 ++-- block/vmdk.c | 6 +++--- block/vpc.c | 2 +- blockdev.c | 2 +- qemu-img.c | 2 +- qemu-io-cmds.c | 2 +- 15 files changed, 22 insertions(+), 21 deletions(-) diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index XXXXXXX..XXXXXXX 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -XXX,XX +XXX,XX @@ int coroutine_fn blk_co_pwrite_zeroes(BlockBackend *blk, int64_t offset, int blk_pwrite_compressed(BlockBackend *blk, int64_t offset, const void *buf, int bytes); int blk_truncate(BlockBackend *blk, int64_t offset, bool exact, - PreallocMode prealloc, Error **errp); + PreallocMode prealloc, BdrvRequestFlags flags, Error **errp); int blk_pdiscard(BlockBackend *blk, int64_t offset, int bytes); int blk_save_vmstate(BlockBackend *blk, const uint8_t *buf, int64_t pos, int size); diff --git a/block.c b/block.c index XXXXXXX..XXXXXXX 100644 --- a/block.c +++ b/block.c @@ -XXX,XX +XXX,XX @@ static int64_t create_file_fallback_truncate(BlockBackend *blk, int64_t size; int ret; - ret = blk_truncate(blk, minimum_size, false, PREALLOC_MODE_OFF, &local_err); + ret = blk_truncate(blk, minimum_size, false, PREALLOC_MODE_OFF, 0, + &local_err); if (ret < 0 && ret != -ENOTSUP) { error_propagate(errp, local_err); return ret; diff --git a/block/block-backend.c b/block/block-backend.c index XXXXXXX..XXXXXXX 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -XXX,XX +XXX,XX @@ int blk_pwrite_compressed(BlockBackend *blk, int64_t offset, const void *buf, } int blk_truncate(BlockBackend *blk, int64_t offset, bool exact, - PreallocMode prealloc, Error **errp) + PreallocMode prealloc, BdrvRequestFlags flags, Error **errp) { if (!blk_is_available(blk)) { error_setg(errp, "No medium inserted"); return -ENOMEDIUM; } - return bdrv_truncate(blk->root, offset, exact, prealloc, 0, errp); + return bdrv_truncate(blk->root, offset, exact, prealloc, flags, errp); } int blk_save_vmstate(BlockBackend *blk, const uint8_t *buf, diff --git a/block/commit.c b/block/commit.c index XXXXXXX..XXXXXXX 100644 --- a/block/commit.c +++ b/block/commit.c @@ -XXX,XX +XXX,XX @@ static int coroutine_fn commit_run(Job *job, Error **errp) } if (base_len < len) { - ret = blk_truncate(s->base, len, false, PREALLOC_MODE_OFF, NULL); + ret = blk_truncate(s->base, len, false, PREALLOC_MODE_OFF, 0, NULL); if (ret) { goto out; } @@ -XXX,XX +XXX,XX @@ int bdrv_commit(BlockDriverState *bs) * grow the backing file image if possible. If not possible, * we must return an error */ if (length > backing_length) { - ret = blk_truncate(backing, length, false, PREALLOC_MODE_OFF, + ret = blk_truncate(backing, length, false, PREALLOC_MODE_OFF, 0, &local_err); if (ret < 0) { error_report_err(local_err); diff --git a/block/crypto.c b/block/crypto.c index XXXXXXX..XXXXXXX 100644 --- a/block/crypto.c +++ b/block/crypto.c @@ -XXX,XX +XXX,XX @@ static ssize_t block_crypto_init_func(QCryptoBlock *block, * which will be used by the crypto header */ return blk_truncate(data->blk, data->size + headerlen, false, - data->prealloc, errp); + data->prealloc, 0, errp); } diff --git a/block/mirror.c b/block/mirror.c index XXXXXXX..XXXXXXX 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -XXX,XX +XXX,XX @@ static int coroutine_fn mirror_run(Job *job, Error **errp) if (s->bdev_length > base_length) { ret = blk_truncate(s->target, s->bdev_length, false, - PREALLOC_MODE_OFF, NULL); + PREALLOC_MODE_OFF, 0, NULL); if (ret < 0) { goto immediate_exit; } diff --git a/block/qcow2.c b/block/qcow2.c index XXXXXXX..XXXXXXX 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -XXX,XX +XXX,XX @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp) /* Okay, now that we have a valid image, let's give it the right size */ ret = blk_truncate(blk, qcow2_opts->size, false, qcow2_opts->preallocation, - errp); + 0, errp); if (ret < 0) { error_prepend(errp, "Could not resize image: "); goto out; @@ -XXX,XX +XXX,XX @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts, * Amending image options should ensure that the image has * exactly the given new values, so pass exact=true here. */ - ret = blk_truncate(blk, new_size, true, PREALLOC_MODE_OFF, errp); + ret = blk_truncate(blk, new_size, true, PREALLOC_MODE_OFF, 0, errp); blk_unref(blk); if (ret < 0) { return ret; diff --git a/block/qed.c b/block/qed.c index XXXXXXX..XXXXXXX 100644 --- a/block/qed.c +++ b/block/qed.c @@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_qed_co_create(BlockdevCreateOptions *opts, * The QED format associates file length with allocation status, * so a new file (which is empty) must have a length of 0. */ - ret = blk_truncate(blk, 0, true, PREALLOC_MODE_OFF, errp); + ret = blk_truncate(blk, 0, true, PREALLOC_MODE_OFF, 0, errp); if (ret < 0) { goto out; } diff --git a/block/vdi.c b/block/vdi.c index XXXXXXX..XXXXXXX 100644 --- a/block/vdi.c +++ b/block/vdi.c @@ -XXX,XX +XXX,XX @@ static int coroutine_fn vdi_co_do_create(BlockdevCreateOptions *create_options, if (image_type == VDI_TYPE_STATIC) { ret = blk_truncate(blk, offset + blocks * block_size, false, - PREALLOC_MODE_OFF, errp); + PREALLOC_MODE_OFF, 0, errp); if (ret < 0) { error_prepend(errp, "Failed to statically allocate file"); goto exit; diff --git a/block/vhdx.c b/block/vhdx.c index XXXXXXX..XXXXXXX 100644 --- a/block/vhdx.c +++ b/block/vhdx.c @@ -XXX,XX +XXX,XX @@ static int vhdx_create_bat(BlockBackend *blk, BDRVVHDXState *s, /* All zeroes, so we can just extend the file - the end of the BAT * is the furthest thing we have written yet */ ret = blk_truncate(blk, data_file_offset, false, PREALLOC_MODE_OFF, - errp); + 0, errp); if (ret < 0) { goto exit; } } else if (type == VHDX_TYPE_FIXED) { ret = blk_truncate(blk, data_file_offset + image_size, false, - PREALLOC_MODE_OFF, errp); + PREALLOC_MODE_OFF, 0, errp); if (ret < 0) { goto exit; } diff --git a/block/vmdk.c b/block/vmdk.c index XXXXXXX..XXXXXXX 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -XXX,XX +XXX,XX @@ static int vmdk_init_extent(BlockBackend *blk, int gd_buf_size; if (flat) { - ret = blk_truncate(blk, filesize, false, PREALLOC_MODE_OFF, errp); + ret = blk_truncate(blk, filesize, false, PREALLOC_MODE_OFF, 0, errp); goto exit; } magic = cpu_to_be32(VMDK4_MAGIC); @@ -XXX,XX +XXX,XX @@ static int vmdk_init_extent(BlockBackend *blk, } ret = blk_truncate(blk, le64_to_cpu(header.grain_offset) << 9, false, - PREALLOC_MODE_OFF, errp); + PREALLOC_MODE_OFF, 0, errp); if (ret < 0) { goto exit; } @@ -XXX,XX +XXX,XX @@ static int coroutine_fn vmdk_co_do_create(int64_t size, /* bdrv_pwrite write padding zeros to align to sector, we don't need that * for description file */ if (desc_offset == 0) { - ret = blk_truncate(blk, desc_len, false, PREALLOC_MODE_OFF, errp); + ret = blk_truncate(blk, desc_len, false, PREALLOC_MODE_OFF, 0, errp); if (ret < 0) { goto exit; } diff --git a/block/vpc.c b/block/vpc.c index XXXXXXX..XXXXXXX 100644 --- a/block/vpc.c +++ b/block/vpc.c @@ -XXX,XX +XXX,XX @@ static int create_fixed_disk(BlockBackend *blk, uint8_t *buf, /* Add footer to total size */ total_size += HEADER_SIZE; - ret = blk_truncate(blk, total_size, false, PREALLOC_MODE_OFF, errp); + ret = blk_truncate(blk, total_size, false, PREALLOC_MODE_OFF, 0, errp); if (ret < 0) { return ret; } diff --git a/blockdev.c b/blockdev.c index XXXXXXX..XXXXXXX 100644 --- a/blockdev.c +++ b/blockdev.c @@ -XXX,XX +XXX,XX @@ void qmp_block_resize(bool has_device, const char *device, } bdrv_drained_begin(bs); - ret = blk_truncate(blk, size, false, PREALLOC_MODE_OFF, errp); + ret = blk_truncate(blk, size, false, PREALLOC_MODE_OFF, 0, errp); bdrv_drained_end(bs); out: diff --git a/qemu-img.c b/qemu-img.c index XXXXXXX..XXXXXXX 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -XXX,XX +XXX,XX @@ static int img_resize(int argc, char **argv) * resizing, so pass @exact=true. It is of no use to report * success when the image has not actually been resized. */ - ret = blk_truncate(blk, total_size, true, prealloc, &err); + ret = blk_truncate(blk, total_size, true, prealloc, 0, &err); if (!ret) { qprintf(quiet, "Image resized.\n"); } else { diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c index XXXXXXX..XXXXXXX 100644 --- a/qemu-io-cmds.c +++ b/qemu-io-cmds.c @@ -XXX,XX +XXX,XX @@ static int truncate_f(BlockBackend *blk, int argc, char **argv) * exact=true. It is better to err on the "emit more errors" side * than to be overly permissive. */ - ret = blk_truncate(blk, offset, true, PREALLOC_MODE_OFF, &local_err); + ret = blk_truncate(blk, offset, true, PREALLOC_MODE_OFF, 0, &local_err); if (ret < 0) { error_report_err(local_err); return ret; -- 2.25.3 If BDRV_REQ_ZERO_WRITE is set and we're extending the image, calling qcow2_cluster_zeroize() with flags=0 does the right thing: It doesn't undo any previous preallocation, but just adds the zero flag to all relevant L2 entries. If an external data file is in use, a write_zeroes request to the data file is made instead. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20200424125448.63318-5-kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block/qcow2-cluster.c | 2 +- block/qcow2.c | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index XXXXXXX..XXXXXXX 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -XXX,XX +XXX,XX @@ int qcow2_cluster_zeroize(BlockDriverState *bs, uint64_t offset, /* Caller must pass aligned values, except at image end */ assert(QEMU_IS_ALIGNED(offset, s->cluster_size)); assert(QEMU_IS_ALIGNED(end_offset, s->cluster_size) || - end_offset == bs->total_sectors << BDRV_SECTOR_BITS); + end_offset >= bs->total_sectors << BDRV_SECTOR_BITS); /* The zero flag is only supported by version 3 and newer */ if (s->qcow_version < 3) { diff --git a/block/qcow2.c b/block/qcow2.c index XXXXXXX..XXXXXXX 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -XXX,XX +XXX,XX @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options, bs->supported_zero_flags = header.version >= 3 ? BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK : 0; + bs->supported_truncate_flags = BDRV_REQ_ZERO_WRITE; /* Repair image if dirty */ if (!(flags & (BDRV_O_CHECK | BDRV_O_INACTIVE)) && !bs->read_only && @@ -XXX,XX +XXX,XX @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset, g_assert_not_reached(); } + if ((flags & BDRV_REQ_ZERO_WRITE) && offset > old_length) { + uint64_t zero_start = QEMU_ALIGN_UP(old_length, s->cluster_size); + + /* + * Use zero clusters as much as we can. qcow2_cluster_zeroize() + * requires a cluster-aligned start. The end may be unaligned if it is + * at the end of the image (which it is here). + */ + ret = qcow2_cluster_zeroize(bs, zero_start, offset - zero_start, 0); + if (ret < 0) { + error_setg_errno(errp, -ret, "Failed to zero out new clusters"); + goto fail; + } + + /* Write explicit zeros for the unaligned head */ + if (zero_start > old_length) { + uint64_t len = zero_start - old_length; + uint8_t *buf = qemu_blockalign0(bs, len); + QEMUIOVector qiov; + qemu_iovec_init_buf(&qiov, buf, len); + + qemu_co_mutex_unlock(&s->lock); + ret = qcow2_co_pwritev_part(bs, old_length, len, &qiov, 0, 0); + qemu_co_mutex_lock(&s->lock); + + qemu_vfree(buf); + if (ret < 0) { + error_setg_errno(errp, -ret, "Failed to zero out the new area"); + goto fail; + } + } + } + if (prealloc != PREALLOC_MODE_OFF) { /* Flush metadata before actually changing the image size */ ret = qcow2_write_caches(bs); -- 2.25.3 The raw format driver can simply forward the flag and let its bs->file child take care of actually providing the zeros. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200424125448.63318-6-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block/raw-format.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/block/raw-format.c b/block/raw-format.c index XXXXXXX..XXXXXXX 100644 --- a/block/raw-format.c +++ b/block/raw-format.c @@ -XXX,XX +XXX,XX @@ static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset, s->size = offset; offset += s->offset; - return bdrv_co_truncate(bs->file, offset, exact, prealloc, 0, errp); + return bdrv_co_truncate(bs->file, offset, exact, prealloc, flags, errp); } static void raw_eject(BlockDriverState *bs, bool eject_flag) @@ -XXX,XX +XXX,XX @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags, bs->supported_zero_flags = BDRV_REQ_WRITE_UNCHANGED | ((BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK) & bs->file->bs->supported_zero_flags); + bs->supported_truncate_flags = bs->file->bs->supported_truncate_flags & + BDRV_REQ_ZERO_WRITE; if (bs->probed && !bdrv_is_read_only(bs)) { bdrv_refresh_filename(bs->file->bs); -- 2.25.3 For regular files, we always get BDRV_REQ_ZERO_WRITE behaviour from the OS, so we can advertise the flag and just ignore it. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-Id: <20200424125448.63318-7-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block/file-posix.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/block/file-posix.c b/block/file-posix.c index XXXXXXX..XXXXXXX 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -XXX,XX +XXX,XX @@ static int raw_open_common(BlockDriverState *bs, QDict *options, #endif bs->supported_zero_flags = BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK; + if (S_ISREG(st.st_mode)) { + /* When extending regular files, we get zeros from the OS */ + bs->supported_truncate_flags = BDRV_REQ_ZERO_WRITE; + } ret = 0; fail: if (filename && (bdrv_flags & BDRV_O_TEMPORARY)) { -- 2.25.3 When extending the size of an image that has a backing file larger than its old size, make sure that the backing file data doesn't become visible in the guest, but the added area is properly zeroed out. Consider the following scenario where the overlay is shorter than its backing file: base.qcow2: AAAAAAAA overlay.qcow2: BBBB When resizing (extending) overlay.qcow2, the new blocks should not stay unallocated and make the additional As from base.qcow2 visible like before this patch, but zeros should be read. A similar case happens with the various variants of a commit job when an intermediate file is short (- for unallocated): base.qcow2: A-A-AAAA mid.qcow2: BB-B top.qcow2: C--C--C- After commit top.qcow2 to mid.qcow2, the following happens: mid.qcow2: CB-C00C0 (correct result) mid.qcow2: CB-C--C- (before this fix) Without the fix, blocks that previously read as zeros on top.qcow2 suddenly turn into A. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200424125448.63318-8-kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block/io.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/block/io.c b/block/io.c index XXXXXXX..XXXXXXX 100644 --- a/block/io.c +++ b/block/io.c @@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, bool exact, goto out; } + /* + * If the image has a backing file that is large enough that it would + * provide data for the new area, we cannot leave it unallocated because + * then the backing file content would become visible. Instead, zero-fill + * the new area. + * + * Note that if the image has a backing file, but was opened without the + * backing file, taking care of keeping things consistent with that backing + * file is the user's responsibility. + */ + if (new_bytes && bs->backing) { + int64_t backing_len; + + backing_len = bdrv_getlength(backing_bs(bs)); + if (backing_len < 0) { + ret = backing_len; + error_setg_errno(errp, -ret, "Could not get backing file size"); + goto out; + } + + if (backing_len > old_size) { + flags |= BDRV_REQ_ZERO_WRITE; + } + } + if (drv->bdrv_co_truncate) { if (flags & ~bs->supported_truncate_flags) { error_setg(errp, "Block driver does not support requested flags"); -- 2.25.3 We want to keep TEST_IMG for the full path of the main test image, but filter_testfiles() must be called for other test images before replacing other things like the image format because the test directory path could contain the format as a substring. Insert a filter_testfiles() call between both. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200424125448.63318-9-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- tests/qemu-iotests/iotests.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index XXXXXXX..XXXXXXX 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -XXX,XX +XXX,XX @@ def filter_img_info(output, filename): for line in output.split('\n'): if 'disk size' in line or 'actual-size' in line: continue - line = line.replace(filename, 'TEST_IMG') \ - .replace(imgfmt, 'IMGFMT') + line = line.replace(filename, 'TEST_IMG') + line = filter_testfiles(line) + line = line.replace(imgfmt, 'IMGFMT') line = re.sub('iters: [0-9]+', 'iters: XXX', line) line = re.sub('uuid: [-a-f0-9]+', 'uuid: XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX', line) line = re.sub('cid: [0-9]+', 'cid: XXXXXXXXXX', line) -- 2.25.3 Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20200424125448.63318-10-kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- tests/qemu-iotests/274 | 155 +++++++++++++++++++++ tests/qemu-iotests/274.out | 268 +++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/group | 1 + 3 files changed, 424 insertions(+) create mode 100755 tests/qemu-iotests/274 create mode 100644 tests/qemu-iotests/274.out diff --git a/tests/qemu-iotests/274 b/tests/qemu-iotests/274 new file mode 100755 index XXXXXXX..XXXXXXX --- /dev/null +++ b/tests/qemu-iotests/274 @@ -XXX,XX +XXX,XX @@ +#!/usr/bin/env python3 +# +# Copyright (C) 2019 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. +# +# Creator/Owner: Kevin Wolf <kwolf@redhat.com> +# +# Some tests for short backing files and short overlays + +import iotests + +iotests.verify_image_format(supported_fmts=['qcow2']) +iotests.verify_platform(['linux']) + +size_short = 1 * 1024 * 1024 +size_long = 2 * 1024 * 1024 +size_diff = size_long - size_short + +def create_chain() -> None: + iotests.qemu_img_log('create', '-f', iotests.imgfmt, base, + str(size_long)) + iotests.qemu_img_log('create', '-f', iotests.imgfmt, '-b', base, mid, + str(size_short)) + iotests.qemu_img_log('create', '-f', iotests.imgfmt, '-b', mid, top, + str(size_long)) + + iotests.qemu_io_log('-c', 'write -P 1 0 %d' % size_long, base) + +def create_vm() -> iotests.VM: + vm = iotests.VM() + vm.add_blockdev('file,filename=%s,node-name=base-file' % base) + vm.add_blockdev('%s,file=base-file,node-name=base' % iotests.imgfmt) + vm.add_blockdev('file,filename=%s,node-name=mid-file' % mid) + vm.add_blockdev('%s,file=mid-file,node-name=mid,backing=base' + % iotests.imgfmt) + vm.add_drive(top, 'backing=mid,node-name=top') + return vm + +with iotests.FilePath('base') as base, \ + iotests.FilePath('mid') as mid, \ + iotests.FilePath('top') as top: + + iotests.log('== Commit tests ==') + + create_chain() + + iotests.log('=== Check visible data ===') + + iotests.qemu_io_log('-c', 'read -P 1 0 %d' % size_short, top) + iotests.qemu_io_log('-c', 'read -P 0 %d %d' % (size_short, size_diff), top) + + iotests.log('=== Checking allocation status ===') + + iotests.qemu_io_log('-c', 'alloc 0 %d' % size_short, + '-c', 'alloc %d %d' % (size_short, size_diff), + base) + + iotests.qemu_io_log('-c', 'alloc 0 %d' % size_short, + '-c', 'alloc %d %d' % (size_short, size_diff), + mid) + + iotests.qemu_io_log('-c', 'alloc 0 %d' % size_short, + '-c', 'alloc %d %d' % (size_short, size_diff), + top) + + iotests.log('=== Checking map ===') + + iotests.qemu_img_log('map', '--output=json', base) + iotests.qemu_img_log('map', '--output=human', base) + iotests.qemu_img_log('map', '--output=json', mid) + iotests.qemu_img_log('map', '--output=human', mid) + iotests.qemu_img_log('map', '--output=json', top) + iotests.qemu_img_log('map', '--output=human', top) + + iotests.log('=== Testing qemu-img commit (top -> mid) ===') + + iotests.qemu_img_log('commit', top) + iotests.img_info_log(mid) + iotests.qemu_io_log('-c', 'read -P 1 0 %d' % size_short, mid) + iotests.qemu_io_log('-c', 'read -P 0 %d %d' % (size_short, size_diff), mid) + + iotests.log('=== Testing HMP commit (top -> mid) ===') + + create_chain() + with create_vm() as vm: + vm.launch() + vm.qmp_log('human-monitor-command', command_line='commit drive0') + + iotests.img_info_log(mid) + iotests.qemu_io_log('-c', 'read -P 1 0 %d' % size_short, mid) + iotests.qemu_io_log('-c', 'read -P 0 %d %d' % (size_short, size_diff), mid) + + iotests.log('=== Testing QMP active commit (top -> mid) ===') + + create_chain() + with create_vm() as vm: + vm.launch() + vm.qmp_log('block-commit', device='top', base_node='mid', + job_id='job0', auto_dismiss=False) + vm.run_job('job0', wait=5) + + iotests.img_info_log(mid) + iotests.qemu_io_log('-c', 'read -P 1 0 %d' % size_short, mid) + iotests.qemu_io_log('-c', 'read -P 0 %d %d' % (size_short, size_diff), mid) + + + iotests.log('== Resize tests ==') + + # Use different sizes for different allocation modes: + # + # We want to have at least one test where 32 bit truncation in the size of + # the overlapping area becomes visible. This is covered by the + # prealloc='off' case (1G to 6G is an overlap of 5G). + # + # However, we can only do this for modes that don't preallocate data + # because otherwise we might run out of space on the test host. + # + # We also want to test some unaligned combinations. + for (prealloc, base_size, top_size_old, top_size_new, off) in [ + ('off', '6G', '1G', '8G', '5G'), + ('metadata', '32G', '30G', '33G', '31G'), + ('falloc', '10M', '5M', '15M', '9M'), + ('full', '16M', '8M', '12M', '11M'), + ('off', '384k', '253k', '512k', '253k'), + ('off', '400k', '256k', '512k', '336k'), + ('off', '512k', '256k', '500k', '436k')]: + + iotests.log('=== preallocation=%s ===' % prealloc) + iotests.qemu_img_log('create', '-f', iotests.imgfmt, base, base_size) + iotests.qemu_img_log('create', '-f', iotests.imgfmt, '-b', base, top, + top_size_old) + iotests.qemu_io_log('-c', 'write -P 1 %s 64k' % off, base) + + # After this, top_size_old to base_size should be allocated/zeroed. + # + # In theory, leaving base_size to top_size_new unallocated would be + # correct, but in practice, if we zero out anything, we zero out + # everything up to top_size_new. + iotests.qemu_img_log('resize', '-f', iotests.imgfmt, + '--preallocation', prealloc, top, top_size_new) + iotests.qemu_io_log('-c', 'read -P 0 %s 64k' % off, top) + iotests.qemu_io_log('-c', 'map', top) + iotests.qemu_img_log('map', '--output=json', top) diff --git a/tests/qemu-iotests/274.out b/tests/qemu-iotests/274.out new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/tests/qemu-iotests/274.out @@ -XXX,XX +XXX,XX @@ +== Commit tests == +Formatting 'TEST_DIR/PID-base', fmt=qcow2 size=2097152 cluster_size=65536 lazy_refcounts=off refcount_bits=16 + +Formatting 'TEST_DIR/PID-mid', fmt=qcow2 size=1048576 backing_file=TEST_DIR/PID-base cluster_size=65536 lazy_refcounts=off refcount_bits=16 + +Formatting 'TEST_DIR/PID-top', fmt=qcow2 size=2097152 backing_file=TEST_DIR/PID-mid cluster_size=65536 lazy_refcounts=off refcount_bits=16 + +wrote 2097152/2097152 bytes at offset 0 +2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +=== Check visible data === +read 1048576/1048576 bytes at offset 0 +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +read 1048576/1048576 bytes at offset 1048576 +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +=== Checking allocation status === +1048576/1048576 bytes allocated at offset 0 bytes +1048576/1048576 bytes allocated at offset 1 MiB + +0/1048576 bytes allocated at offset 0 bytes +0/0 bytes allocated at offset 1 MiB + +0/1048576 bytes allocated at offset 0 bytes +0/1048576 bytes allocated at offset 1 MiB + +=== Checking map === +[{ "start": 0, "length": 2097152, "depth": 0, "zero": false, "data": true, "offset": 327680}] + +Offset Length Mapped to File +0 0x200000 0x50000 TEST_DIR/PID-base + +[{ "start": 0, "length": 1048576, "depth": 1, "zero": false, "data": true, "offset": 327680}] + +Offset Length Mapped to File +0 0x100000 0x50000 TEST_DIR/PID-base + +[{ "start": 0, "length": 1048576, "depth": 2, "zero": false, "data": true, "offset": 327680}, +{ "start": 1048576, "length": 1048576, "depth": 0, "zero": true, "data": false}] + +Offset Length Mapped to File +0 0x100000 0x50000 TEST_DIR/PID-base + +=== Testing qemu-img commit (top -> mid) === +Image committed. + +image: TEST_IMG +file format: IMGFMT +virtual size: 2 MiB (2097152 bytes) +cluster_size: 65536 +backing file: TEST_DIR/PID-base +Format specific information: + compat: 1.1 + lazy refcounts: false + refcount bits: 16 + corrupt: false + +read 1048576/1048576 bytes at offset 0 +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +read 1048576/1048576 bytes at offset 1048576 +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +=== Testing HMP commit (top -> mid) === +Formatting 'TEST_DIR/PID-base', fmt=qcow2 size=2097152 cluster_size=65536 lazy_refcounts=off refcount_bits=16 + +Formatting 'TEST_DIR/PID-mid', fmt=qcow2 size=1048576 backing_file=TEST_DIR/PID-base cluster_size=65536 lazy_refcounts=off refcount_bits=16 + +Formatting 'TEST_DIR/PID-top', fmt=qcow2 size=2097152 backing_file=TEST_DIR/PID-mid cluster_size=65536 lazy_refcounts=off refcount_bits=16 + +wrote 2097152/2097152 bytes at offset 0 +2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +{"execute": "human-monitor-command", "arguments": {"command-line": "commit drive0"}} +{"return": ""} +image: TEST_IMG +file format: IMGFMT +virtual size: 2 MiB (2097152 bytes) +cluster_size: 65536 +backing file: TEST_DIR/PID-base +Format specific information: + compat: 1.1 + lazy refcounts: false + refcount bits: 16 + corrupt: false + +read 1048576/1048576 bytes at offset 0 +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +read 1048576/1048576 bytes at offset 1048576 +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +=== Testing QMP active commit (top -> mid) === +Formatting 'TEST_DIR/PID-base', fmt=qcow2 size=2097152 cluster_size=65536 lazy_refcounts=off refcount_bits=16 + +Formatting 'TEST_DIR/PID-mid', fmt=qcow2 size=1048576 backing_file=TEST_DIR/PID-base cluster_size=65536 lazy_refcounts=off refcount_bits=16 + +Formatting 'TEST_DIR/PID-top', fmt=qcow2 size=2097152 backing_file=TEST_DIR/PID-mid cluster_size=65536 lazy_refcounts=off refcount_bits=16 + +wrote 2097152/2097152 bytes at offset 0 +2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +{"execute": "block-commit", "arguments": {"auto-dismiss": false, "base-node": "mid", "device": "top", "job-id": "job0"}} +{"return": {}} +{"execute": "job-complete", "arguments": {"id": "job0"}} +{"return": {}} +{"data": {"device": "job0", "len": 0, "offset": 0, "speed": 0, "type": "commit"}, "event": "BLOCK_JOB_READY", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}} +{"data": {"device": "job0", "len": 0, "offset": 0, "speed": 0, "type": "commit"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}} +{"execute": "job-dismiss", "arguments": {"id": "job0"}} +{"return": {}} +image: TEST_IMG +file format: IMGFMT +virtual size: 2 MiB (2097152 bytes) +cluster_size: 65536 +backing file: TEST_DIR/PID-base +Format specific information: + compat: 1.1 + lazy refcounts: false + refcount bits: 16 + corrupt: false + +read 1048576/1048576 bytes at offset 0 +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +read 1048576/1048576 bytes at offset 1048576 +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +== Resize tests == +=== preallocation=off === +Formatting 'TEST_DIR/PID-base', fmt=qcow2 size=6442450944 cluster_size=65536 lazy_refcounts=off refcount_bits=16 + +Formatting 'TEST_DIR/PID-top', fmt=qcow2 size=1073741824 backing_file=TEST_DIR/PID-base cluster_size=65536 lazy_refcounts=off refcount_bits=16 + +wrote 65536/65536 bytes at offset 5368709120 +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +Image resized. + +read 65536/65536 bytes at offset 5368709120 +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +1 GiB (0x40000000) bytes not allocated at offset 0 bytes (0x0) +7 GiB (0x1c0000000) bytes allocated at offset 1 GiB (0x40000000) + +[{ "start": 0, "length": 1073741824, "depth": 1, "zero": true, "data": false}, +{ "start": 1073741824, "length": 7516192768, "depth": 0, "zero": true, "data": false}] + +=== preallocation=metadata === +Formatting 'TEST_DIR/PID-base', fmt=qcow2 size=34359738368 cluster_size=65536 lazy_refcounts=off refcount_bits=16 + +Formatting 'TEST_DIR/PID-top', fmt=qcow2 size=32212254720 backing_file=TEST_DIR/PID-base cluster_size=65536 lazy_refcounts=off refcount_bits=16 + +wrote 65536/65536 bytes at offset 33285996544 +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +Image resized. + +read 65536/65536 bytes at offset 33285996544 +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +30 GiB (0x780000000) bytes not allocated at offset 0 bytes (0x0) +3 GiB (0xc0000000) bytes allocated at offset 30 GiB (0x780000000) + +[{ "start": 0, "length": 32212254720, "depth": 1, "zero": true, "data": false}, +{ "start": 32212254720, "length": 536870912, "depth": 0, "zero": true, "data": false, "offset": 327680}, +{ "start": 32749125632, "length": 536870912, "depth": 0, "zero": true, "data": false, "offset": 537264128}, +{ "start": 33285996544, "length": 536870912, "depth": 0, "zero": true, "data": false, "offset": 1074200576}, +{ "start": 33822867456, "length": 536870912, "depth": 0, "zero": true, "data": false, "offset": 1611137024}, +{ "start": 34359738368, "length": 536870912, "depth": 0, "zero": true, "data": false, "offset": 2148139008}, +{ "start": 34896609280, "length": 536870912, "depth": 0, "zero": true, "data": false, "offset": 2685075456}] + +=== preallocation=falloc === +Formatting 'TEST_DIR/PID-base', fmt=qcow2 size=10485760 cluster_size=65536 lazy_refcounts=off refcount_bits=16 + +Formatting 'TEST_DIR/PID-top', fmt=qcow2 size=5242880 backing_file=TEST_DIR/PID-base cluster_size=65536 lazy_refcounts=off refcount_bits=16 + +wrote 65536/65536 bytes at offset 9437184 +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +Image resized. + +read 65536/65536 bytes at offset 9437184 +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +5 MiB (0x500000) bytes not allocated at offset 0 bytes (0x0) +10 MiB (0xa00000) bytes allocated at offset 5 MiB (0x500000) + +[{ "start": 0, "length": 5242880, "depth": 1, "zero": true, "data": false}, +{ "start": 5242880, "length": 10485760, "depth": 0, "zero": true, "data": false, "offset": 327680}] + +=== preallocation=full === +Formatting 'TEST_DIR/PID-base', fmt=qcow2 size=16777216 cluster_size=65536 lazy_refcounts=off refcount_bits=16 + +Formatting 'TEST_DIR/PID-top', fmt=qcow2 size=8388608 backing_file=TEST_DIR/PID-base cluster_size=65536 lazy_refcounts=off refcount_bits=16 + +wrote 65536/65536 bytes at offset 11534336 +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +Image resized. + +read 65536/65536 bytes at offset 11534336 +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +8 MiB (0x800000) bytes not allocated at offset 0 bytes (0x0) +4 MiB (0x400000) bytes allocated at offset 8 MiB (0x800000) + +[{ "start": 0, "length": 8388608, "depth": 1, "zero": true, "data": false}, +{ "start": 8388608, "length": 4194304, "depth": 0, "zero": true, "data": false, "offset": 327680}] + +=== preallocation=off === +Formatting 'TEST_DIR/PID-base', fmt=qcow2 size=393216 cluster_size=65536 lazy_refcounts=off refcount_bits=16 + +Formatting 'TEST_DIR/PID-top', fmt=qcow2 size=259072 backing_file=TEST_DIR/PID-base cluster_size=65536 lazy_refcounts=off refcount_bits=16 + +wrote 65536/65536 bytes at offset 259072 +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +Image resized. + +read 65536/65536 bytes at offset 259072 +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +192 KiB (0x30000) bytes not allocated at offset 0 bytes (0x0) +320 KiB (0x50000) bytes allocated at offset 192 KiB (0x30000) + +[{ "start": 0, "length": 196608, "depth": 1, "zero": true, "data": false}, +{ "start": 196608, "length": 65536, "depth": 0, "zero": false, "data": true, "offset": 327680}, +{ "start": 262144, "length": 262144, "depth": 0, "zero": true, "data": false}] + +=== preallocation=off === +Formatting 'TEST_DIR/PID-base', fmt=qcow2 size=409600 cluster_size=65536 lazy_refcounts=off refcount_bits=16 + +Formatting 'TEST_DIR/PID-top', fmt=qcow2 size=262144 backing_file=TEST_DIR/PID-base cluster_size=65536 lazy_refcounts=off refcount_bits=16 + +wrote 65536/65536 bytes at offset 344064 +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +Image resized. + +read 65536/65536 bytes at offset 344064 +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +256 KiB (0x40000) bytes not allocated at offset 0 bytes (0x0) +256 KiB (0x40000) bytes allocated at offset 256 KiB (0x40000) + +[{ "start": 0, "length": 262144, "depth": 1, "zero": true, "data": false}, +{ "start": 262144, "length": 262144, "depth": 0, "zero": true, "data": false}] + +=== preallocation=off === +Formatting 'TEST_DIR/PID-base', fmt=qcow2 size=524288 cluster_size=65536 lazy_refcounts=off refcount_bits=16 + +Formatting 'TEST_DIR/PID-top', fmt=qcow2 size=262144 backing_file=TEST_DIR/PID-base cluster_size=65536 lazy_refcounts=off refcount_bits=16 + +wrote 65536/65536 bytes at offset 446464 +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +Image resized. + +read 65536/65536 bytes at offset 446464 +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +256 KiB (0x40000) bytes not allocated at offset 0 bytes (0x0) +244 KiB (0x3d000) bytes allocated at offset 256 KiB (0x40000) + +[{ "start": 0, "length": 262144, "depth": 1, "zero": true, "data": false}, +{ "start": 262144, "length": 249856, "depth": 0, "zero": true, "data": false}] + diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index XXXXXXX..XXXXXXX 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -XXX,XX +XXX,XX @@ 270 rw backing quick 272 rw 273 backing quick +274 rw backing 277 rw quick 279 rw backing quick 280 rw migration quick -- 2.25.3 The BDRV_REQ_ZERO_WRITE is currently implemented in a way that first the image is possibly preallocated and then the zero flag is added to all clusters. This means that a copy-on-write operation may be needed when writing to these clusters, despite having used preallocation, negating one of the major benefits of preallocation. Instead, try to forward the BDRV_REQ_ZERO_WRITE to the protocol driver, and if the protocol driver can ensure that the new area reads as zeros, we can skip setting the zero flag in the qcow2 layer. Unfortunately, the same approach doesn't work for metadata preallocation, so we'll still set the zero flag there. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-Id: <20200424142701.67053-1-kwolf@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block/qcow2.c | 22 +++++++++++++++++++--- tests/qemu-iotests/274.out | 4 ++-- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index XXXXXXX..XXXXXXX 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -XXX,XX +XXX,XX @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset, /* Allocate the data area */ new_file_size = allocation_start + nb_new_data_clusters * s->cluster_size; - /* Image file grows, so @exact does not matter */ - ret = bdrv_co_truncate(bs->file, new_file_size, false, prealloc, 0, - errp); + /* + * Image file grows, so @exact does not matter. + * + * If we need to zero out the new area, try first whether the protocol + * driver can already take care of this. + */ + if (flags & BDRV_REQ_ZERO_WRITE) { + ret = bdrv_co_truncate(bs->file, new_file_size, false, prealloc, + BDRV_REQ_ZERO_WRITE, NULL); + if (ret >= 0) { + flags &= ~BDRV_REQ_ZERO_WRITE; + } + } else { + ret = -1; + } + if (ret < 0) { + ret = bdrv_co_truncate(bs->file, new_file_size, false, prealloc, 0, + errp); + } if (ret < 0) { error_prepend(errp, "Failed to resize underlying file: "); qcow2_free_clusters(bs, allocation_start, diff --git a/tests/qemu-iotests/274.out b/tests/qemu-iotests/274.out index XXXXXXX..XXXXXXX 100644 --- a/tests/qemu-iotests/274.out +++ b/tests/qemu-iotests/274.out @@ -XXX,XX +XXX,XX @@ read 65536/65536 bytes at offset 9437184 10 MiB (0xa00000) bytes allocated at offset 5 MiB (0x500000) [{ "start": 0, "length": 5242880, "depth": 1, "zero": true, "data": false}, -{ "start": 5242880, "length": 10485760, "depth": 0, "zero": true, "data": false, "offset": 327680}] +{ "start": 5242880, "length": 10485760, "depth": 0, "zero": false, "data": true, "offset": 327680}] === preallocation=full === Formatting 'TEST_DIR/PID-base', fmt=qcow2 size=16777216 cluster_size=65536 lazy_refcounts=off refcount_bits=16 @@ -XXX,XX +XXX,XX @@ read 65536/65536 bytes at offset 11534336 4 MiB (0x400000) bytes allocated at offset 8 MiB (0x800000) [{ "start": 0, "length": 8388608, "depth": 1, "zero": true, "data": false}, -{ "start": 8388608, "length": 4194304, "depth": 0, "zero": true, "data": false, "offset": 327680}] +{ "start": 8388608, "length": 4194304, "depth": 0, "zero": false, "data": true, "offset": 327680}] === preallocation=off === Formatting 'TEST_DIR/PID-base', fmt=qcow2 size=393216 cluster_size=65536 lazy_refcounts=off refcount_bits=16 -- 2.25.3 From: Andrzej Jakowski <andrzej.jakowski@linux.intel.com> This patch introduces support for PMR that has been defined as part of NVMe 1.4 spec. User can now specify a pmrdev option that should point to HostMemoryBackend. pmrdev memory region will subsequently be exposed as PCI BAR 2 in emulated NVMe device. Guest OS can perform mmio read and writes to the PMR region that will stay persistent across system reboot. Signed-off-by: Andrzej Jakowski <andrzej.jakowski@linux.intel.com> Reviewed-by: Klaus Jensen <k.jensen@samsung.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20200330164656.9348-1-andrzej.jakowski@linux.intel.com> Reviewed-by: Keith Busch <kbusch@kernel.org> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- hw/block/nvme.h | 2 + include/block/nvme.h | 172 +++++++++++++++++++++++++++++++++++++++++ hw/block/nvme.c | 109 ++++++++++++++++++++++++++ hw/block/Makefile.objs | 2 +- hw/block/trace-events | 4 + 5 files changed, 288 insertions(+), 1 deletion(-) diff --git a/hw/block/nvme.h b/hw/block/nvme.h index XXXXXXX..XXXXXXX 100644 --- a/hw/block/nvme.h +++ b/hw/block/nvme.h @@ -XXX,XX +XXX,XX @@ typedef struct NvmeCtrl { uint64_t timestamp_set_qemu_clock_ms; /* QEMU clock time */ char *serial; + HostMemoryBackend *pmrdev; + NvmeNamespace *namespaces; NvmeSQueue **sq; NvmeCQueue **cq; diff --git a/include/block/nvme.h b/include/block/nvme.h index XXXXXXX..XXXXXXX 100644 --- a/include/block/nvme.h +++ b/include/block/nvme.h @@ -XXX,XX +XXX,XX @@ typedef struct NvmeBar { uint64_t acq; uint32_t cmbloc; uint32_t cmbsz; + uint8_t padding[3520]; /* not used by QEMU */ + uint32_t pmrcap; + uint32_t pmrctl; + uint32_t pmrsts; + uint32_t pmrebs; + uint32_t pmrswtp; + uint32_t pmrmsc; } NvmeBar; enum NvmeCapShift { @@ -XXX,XX +XXX,XX @@ enum NvmeCapShift { CAP_CSS_SHIFT = 37, CAP_MPSMIN_SHIFT = 48, CAP_MPSMAX_SHIFT = 52, + CAP_PMR_SHIFT = 56, }; enum NvmeCapMask { @@ -XXX,XX +XXX,XX @@ enum NvmeCapMask { CAP_CSS_MASK = 0xff, CAP_MPSMIN_MASK = 0xf, CAP_MPSMAX_MASK = 0xf, + CAP_PMR_MASK = 0x1, }; #define NVME_CAP_MQES(cap) (((cap) >> CAP_MQES_SHIFT) & CAP_MQES_MASK) @@ -XXX,XX +XXX,XX @@ enum NvmeCapMask { << CAP_MPSMIN_SHIFT) #define NVME_CAP_SET_MPSMAX(cap, val) (cap |= (uint64_t)(val & CAP_MPSMAX_MASK)\ << CAP_MPSMAX_SHIFT) +#define NVME_CAP_SET_PMRS(cap, val) (cap |= (uint64_t)(val & CAP_PMR_MASK)\ + << CAP_PMR_SHIFT) enum NvmeCcShift { CC_EN_SHIFT = 0, @@ -XXX,XX +XXX,XX @@ enum NvmeCmbszMask { #define NVME_CMBSZ_GETSIZE(cmbsz) \ (NVME_CMBSZ_SZ(cmbsz) * (1 << (12 + 4 * NVME_CMBSZ_SZU(cmbsz)))) +enum NvmePmrcapShift { + PMRCAP_RDS_SHIFT = 3, + PMRCAP_WDS_SHIFT = 4, + PMRCAP_BIR_SHIFT = 5, + PMRCAP_PMRTU_SHIFT = 8, + PMRCAP_PMRWBM_SHIFT = 10, + PMRCAP_PMRTO_SHIFT = 16, + PMRCAP_CMSS_SHIFT = 24, +}; + +enum NvmePmrcapMask { + PMRCAP_RDS_MASK = 0x1, + PMRCAP_WDS_MASK = 0x1, + PMRCAP_BIR_MASK = 0x7, + PMRCAP_PMRTU_MASK = 0x3, + PMRCAP_PMRWBM_MASK = 0xf, + PMRCAP_PMRTO_MASK = 0xff, + PMRCAP_CMSS_MASK = 0x1, +}; + +#define NVME_PMRCAP_RDS(pmrcap) \ + ((pmrcap >> PMRCAP_RDS_SHIFT) & PMRCAP_RDS_MASK) +#define NVME_PMRCAP_WDS(pmrcap) \ + ((pmrcap >> PMRCAP_WDS_SHIFT) & PMRCAP_WDS_MASK) +#define NVME_PMRCAP_BIR(pmrcap) \ + ((pmrcap >> PMRCAP_BIR_SHIFT) & PMRCAP_BIR_MASK) +#define NVME_PMRCAP_PMRTU(pmrcap) \ + ((pmrcap >> PMRCAP_PMRTU_SHIFT) & PMRCAP_PMRTU_MASK) +#define NVME_PMRCAP_PMRWBM(pmrcap) \ + ((pmrcap >> PMRCAP_PMRWBM_SHIFT) & PMRCAP_PMRWBM_MASK) +#define NVME_PMRCAP_PMRTO(pmrcap) \ + ((pmrcap >> PMRCAP_PMRTO_SHIFT) & PMRCAP_PMRTO_MASK) +#define NVME_PMRCAP_CMSS(pmrcap) \ + ((pmrcap >> PMRCAP_CMSS_SHIFT) & PMRCAP_CMSS_MASK) + +#define NVME_PMRCAP_SET_RDS(pmrcap, val) \ + (pmrcap |= (uint64_t)(val & PMRCAP_RDS_MASK) << PMRCAP_RDS_SHIFT) +#define NVME_PMRCAP_SET_WDS(pmrcap, val) \ + (pmrcap |= (uint64_t)(val & PMRCAP_WDS_MASK) << PMRCAP_WDS_SHIFT) +#define NVME_PMRCAP_SET_BIR(pmrcap, val) \ + (pmrcap |= (uint64_t)(val & PMRCAP_BIR_MASK) << PMRCAP_BIR_SHIFT) +#define NVME_PMRCAP_SET_PMRTU(pmrcap, val) \ + (pmrcap |= (uint64_t)(val & PMRCAP_PMRTU_MASK) << PMRCAP_PMRTU_SHIFT) +#define NVME_PMRCAP_SET_PMRWBM(pmrcap, val) \ + (pmrcap |= (uint64_t)(val & PMRCAP_PMRWBM_MASK) << PMRCAP_PMRWBM_SHIFT) +#define NVME_PMRCAP_SET_PMRTO(pmrcap, val) \ + (pmrcap |= (uint64_t)(val & PMRCAP_PMRTO_MASK) << PMRCAP_PMRTO_SHIFT) +#define NVME_PMRCAP_SET_CMSS(pmrcap, val) \ + (pmrcap |= (uint64_t)(val & PMRCAP_CMSS_MASK) << PMRCAP_CMSS_SHIFT) + +enum NvmePmrctlShift { + PMRCTL_EN_SHIFT = 0, +}; + +enum NvmePmrctlMask { + PMRCTL_EN_MASK = 0x1, +}; + +#define NVME_PMRCTL_EN(pmrctl) ((pmrctl >> PMRCTL_EN_SHIFT) & PMRCTL_EN_MASK) + +#define NVME_PMRCTL_SET_EN(pmrctl, val) \ + (pmrctl |= (uint64_t)(val & PMRCTL_EN_MASK) << PMRCTL_EN_SHIFT) + +enum NvmePmrstsShift { + PMRSTS_ERR_SHIFT = 0, + PMRSTS_NRDY_SHIFT = 8, + PMRSTS_HSTS_SHIFT = 9, + PMRSTS_CBAI_SHIFT = 12, +}; + +enum NvmePmrstsMask { + PMRSTS_ERR_MASK = 0xff, + PMRSTS_NRDY_MASK = 0x1, + PMRSTS_HSTS_MASK = 0x7, + PMRSTS_CBAI_MASK = 0x1, +}; + +#define NVME_PMRSTS_ERR(pmrsts) \ + ((pmrsts >> PMRSTS_ERR_SHIFT) & PMRSTS_ERR_MASK) +#define NVME_PMRSTS_NRDY(pmrsts) \ + ((pmrsts >> PMRSTS_NRDY_SHIFT) & PMRSTS_NRDY_MASK) +#define NVME_PMRSTS_HSTS(pmrsts) \ + ((pmrsts >> PMRSTS_HSTS_SHIFT) & PMRSTS_HSTS_MASK) +#define NVME_PMRSTS_CBAI(pmrsts) \ + ((pmrsts >> PMRSTS_CBAI_SHIFT) & PMRSTS_CBAI_MASK) + +#define NVME_PMRSTS_SET_ERR(pmrsts, val) \ + (pmrsts |= (uint64_t)(val & PMRSTS_ERR_MASK) << PMRSTS_ERR_SHIFT) +#define NVME_PMRSTS_SET_NRDY(pmrsts, val) \ + (pmrsts |= (uint64_t)(val & PMRSTS_NRDY_MASK) << PMRSTS_NRDY_SHIFT) +#define NVME_PMRSTS_SET_HSTS(pmrsts, val) \ + (pmrsts |= (uint64_t)(val & PMRSTS_HSTS_MASK) << PMRSTS_HSTS_SHIFT) +#define NVME_PMRSTS_SET_CBAI(pmrsts, val) \ + (pmrsts |= (uint64_t)(val & PMRSTS_CBAI_MASK) << PMRSTS_CBAI_SHIFT) + +enum NvmePmrebsShift { + PMREBS_PMRSZU_SHIFT = 0, + PMREBS_RBB_SHIFT = 4, + PMREBS_PMRWBZ_SHIFT = 8, +}; + +enum NvmePmrebsMask { + PMREBS_PMRSZU_MASK = 0xf, + PMREBS_RBB_MASK = 0x1, + PMREBS_PMRWBZ_MASK = 0xffffff, +}; + +#define NVME_PMREBS_PMRSZU(pmrebs) \ + ((pmrebs >> PMREBS_PMRSZU_SHIFT) & PMREBS_PMRSZU_MASK) +#define NVME_PMREBS_RBB(pmrebs) \ + ((pmrebs >> PMREBS_RBB_SHIFT) & PMREBS_RBB_MASK) +#define NVME_PMREBS_PMRWBZ(pmrebs) \ + ((pmrebs >> PMREBS_PMRWBZ_SHIFT) & PMREBS_PMRWBZ_MASK) + +#define NVME_PMREBS_SET_PMRSZU(pmrebs, val) \ + (pmrebs |= (uint64_t)(val & PMREBS_PMRSZU_MASK) << PMREBS_PMRSZU_SHIFT) +#define NVME_PMREBS_SET_RBB(pmrebs, val) \ + (pmrebs |= (uint64_t)(val & PMREBS_RBB_MASK) << PMREBS_RBB_SHIFT) +#define NVME_PMREBS_SET_PMRWBZ(pmrebs, val) \ + (pmrebs |= (uint64_t)(val & PMREBS_PMRWBZ_MASK) << PMREBS_PMRWBZ_SHIFT) + +enum NvmePmrswtpShift { + PMRSWTP_PMRSWTU_SHIFT = 0, + PMRSWTP_PMRSWTV_SHIFT = 8, +}; + +enum NvmePmrswtpMask { + PMRSWTP_PMRSWTU_MASK = 0xf, + PMRSWTP_PMRSWTV_MASK = 0xffffff, +}; + +#define NVME_PMRSWTP_PMRSWTU(pmrswtp) \ + ((pmrswtp >> PMRSWTP_PMRSWTU_SHIFT) & PMRSWTP_PMRSWTU_MASK) +#define NVME_PMRSWTP_PMRSWTV(pmrswtp) \ + ((pmrswtp >> PMRSWTP_PMRSWTV_SHIFT) & PMRSWTP_PMRSWTV_MASK) + +#define NVME_PMRSWTP_SET_PMRSWTU(pmrswtp, val) \ + (pmrswtp |= (uint64_t)(val & PMRSWTP_PMRSWTU_MASK) << PMRSWTP_PMRSWTU_SHIFT) +#define NVME_PMRSWTP_SET_PMRSWTV(pmrswtp, val) \ + (pmrswtp |= (uint64_t)(val & PMRSWTP_PMRSWTV_MASK) << PMRSWTP_PMRSWTV_SHIFT) + +enum NvmePmrmscShift { + PMRMSC_CMSE_SHIFT = 1, + PMRMSC_CBA_SHIFT = 12, +}; + +enum NvmePmrmscMask { + PMRMSC_CMSE_MASK = 0x1, + PMRMSC_CBA_MASK = 0xfffffffffffff, +}; + +#define NVME_PMRMSC_CMSE(pmrmsc) \ + ((pmrmsc >> PMRMSC_CMSE_SHIFT) & PMRMSC_CMSE_MASK) +#define NVME_PMRMSC_CBA(pmrmsc) \ + ((pmrmsc >> PMRMSC_CBA_SHIFT) & PMRMSC_CBA_MASK) + +#define NVME_PMRMSC_SET_CMSE(pmrmsc, val) \ + (pmrmsc |= (uint64_t)(val & PMRMSC_CMSE_MASK) << PMRMSC_CMSE_SHIFT) +#define NVME_PMRMSC_SET_CBA(pmrmsc, val) \ + (pmrmsc |= (uint64_t)(val & PMRMSC_CBA_MASK) << PMRMSC_CBA_SHIFT) + typedef struct NvmeCmd { uint8_t opcode; uint8_t fuse; diff --git a/hw/block/nvme.c b/hw/block/nvme.c index XXXXXXX..XXXXXXX 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -XXX,XX +XXX,XX @@ * -drive file=<file>,if=none,id=<drive_id> * -device nvme,drive=<drive_id>,serial=<serial>,id=<id[optional]>, \ * cmb_size_mb=<cmb_size_mb[optional]>, \ + * [pmrdev=<mem_backend_file_id>,] \ * num_queues=<N[optional]> * * Note cmb_size_mb denotes size of CMB in MB. CMB is assumed to be at * offset 0 in BAR2 and supports only WDS, RDS and SQS for now. + * + * cmb_size_mb= and pmrdev= options are mutually exclusive due to limitation + * in available BAR's. cmb_size_mb= will take precedence over pmrdev= when + * both provided. + * Enabling pmr emulation can be achieved by pointing to memory-backend-file. + * For example: + * -object memory-backend-file,id=<mem_id>,share=on,mem-path=<file_path>, \ + * size=<size> .... -device nvme,...,pmrdev=<mem_id> */ #include "qemu/osdep.h" @@ -XXX,XX +XXX,XX @@ #include "sysemu/sysemu.h" #include "qapi/error.h" #include "qapi/visitor.h" +#include "sysemu/hostmem.h" #include "sysemu/block-backend.h" +#include "exec/ram_addr.h" #include "qemu/log.h" #include "qemu/module.h" @@ -XXX,XX +XXX,XX @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data, NVME_GUEST_ERR(nvme_ub_mmiowr_cmbsz_readonly, "invalid write to read only CMBSZ, ignored"); return; + case 0xE00: /* PMRCAP */ + NVME_GUEST_ERR(nvme_ub_mmiowr_pmrcap_readonly, + "invalid write to PMRCAP register, ignored"); + return; + case 0xE04: /* TODO PMRCTL */ + break; + case 0xE08: /* PMRSTS */ + NVME_GUEST_ERR(nvme_ub_mmiowr_pmrsts_readonly, + "invalid write to PMRSTS register, ignored"); + return; + case 0xE0C: /* PMREBS */ + NVME_GUEST_ERR(nvme_ub_mmiowr_pmrebs_readonly, + "invalid write to PMREBS register, ignored"); + return; + case 0xE10: /* PMRSWTP */ + NVME_GUEST_ERR(nvme_ub_mmiowr_pmrswtp_readonly, + "invalid write to PMRSWTP register, ignored"); + return; + case 0xE14: /* TODO PMRMSC */ + break; default: NVME_GUEST_ERR(nvme_ub_mmiowr_invalid, "invalid MMIO write," @@ -XXX,XX +XXX,XX @@ static uint64_t nvme_mmio_read(void *opaque, hwaddr addr, unsigned size) } if (addr < sizeof(n->bar)) { + /* + * When PMRWBM bit 1 is set then read from + * from PMRSTS should ensure prior writes + * made it to persistent media + */ + if (addr == 0xE08 && + (NVME_PMRCAP_PMRWBM(n->bar.pmrcap) & 0x02)) { + qemu_ram_writeback(n->pmrdev->mr.ram_block, + 0, n->pmrdev->size); + } memcpy(&val, ptr + addr, size); } else { NVME_GUEST_ERR(nvme_ub_mmiord_invalid_ofs, @@ -XXX,XX +XXX,XX @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp) error_setg(errp, "serial property not set"); return; } + + if (!n->cmb_size_mb && n->pmrdev) { + if (host_memory_backend_is_mapped(n->pmrdev)) { + char *path = object_get_canonical_path_component(OBJECT(n->pmrdev)); + error_setg(errp, "can't use already busy memdev: %s", path); + g_free(path); + return; + } + + if (!is_power_of_2(n->pmrdev->size)) { + error_setg(errp, "pmr backend size needs to be power of 2 in size"); + return; + } + + host_memory_backend_set_mapped(n->pmrdev, true); + } + blkconf_blocksizes(&n->conf); if (!blkconf_apply_backend_options(&n->conf, blk_is_read_only(n->conf.blk), false, errp)) { @@ -XXX,XX +XXX,XX @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp) PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64 | PCI_BASE_ADDRESS_MEM_PREFETCH, &n->ctrl_mem); + } else if (n->pmrdev) { + /* Controller Capabilities register */ + NVME_CAP_SET_PMRS(n->bar.cap, 1); + + /* PMR Capabities register */ + n->bar.pmrcap = 0; + NVME_PMRCAP_SET_RDS(n->bar.pmrcap, 0); + NVME_PMRCAP_SET_WDS(n->bar.pmrcap, 0); + NVME_PMRCAP_SET_BIR(n->bar.pmrcap, 2); + NVME_PMRCAP_SET_PMRTU(n->bar.pmrcap, 0); + /* Turn on bit 1 support */ + NVME_PMRCAP_SET_PMRWBM(n->bar.pmrcap, 0x02); + NVME_PMRCAP_SET_PMRTO(n->bar.pmrcap, 0); + NVME_PMRCAP_SET_CMSS(n->bar.pmrcap, 0); + + /* PMR Control register */ + n->bar.pmrctl = 0; + NVME_PMRCTL_SET_EN(n->bar.pmrctl, 0); + + /* PMR Status register */ + n->bar.pmrsts = 0; + NVME_PMRSTS_SET_ERR(n->bar.pmrsts, 0); + NVME_PMRSTS_SET_NRDY(n->bar.pmrsts, 0); + NVME_PMRSTS_SET_HSTS(n->bar.pmrsts, 0); + NVME_PMRSTS_SET_CBAI(n->bar.pmrsts, 0); + + /* PMR Elasticity Buffer Size register */ + n->bar.pmrebs = 0; + NVME_PMREBS_SET_PMRSZU(n->bar.pmrebs, 0); + NVME_PMREBS_SET_RBB(n->bar.pmrebs, 0); + NVME_PMREBS_SET_PMRWBZ(n->bar.pmrebs, 0); + + /* PMR Sustained Write Throughput register */ + n->bar.pmrswtp = 0; + NVME_PMRSWTP_SET_PMRSWTU(n->bar.pmrswtp, 0); + NVME_PMRSWTP_SET_PMRSWTV(n->bar.pmrswtp, 0); + + /* PMR Memory Space Control register */ + n->bar.pmrmsc = 0; + NVME_PMRMSC_SET_CMSE(n->bar.pmrmsc, 0); + NVME_PMRMSC_SET_CBA(n->bar.pmrmsc, 0); + + pci_register_bar(pci_dev, NVME_PMRCAP_BIR(n->bar.pmrcap), + PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64 | + PCI_BASE_ADDRESS_MEM_PREFETCH, &n->pmrdev->mr); } for (i = 0; i < n->num_namespaces; i++) { @@ -XXX,XX +XXX,XX @@ static void nvme_exit(PCIDevice *pci_dev) if (n->cmb_size_mb) { g_free(n->cmbuf); } + + if (n->pmrdev) { + host_memory_backend_set_mapped(n->pmrdev, false); + } msix_uninit_exclusive_bar(pci_dev); } static Property nvme_props[] = { DEFINE_BLOCK_PROPERTIES(NvmeCtrl, conf), + DEFINE_PROP_LINK("pmrdev", NvmeCtrl, pmrdev, TYPE_MEMORY_BACKEND, + HostMemoryBackend *), DEFINE_PROP_STRING("serial", NvmeCtrl, serial), DEFINE_PROP_UINT32("cmb_size_mb", NvmeCtrl, cmb_size_mb, 0), DEFINE_PROP_UINT32("num_queues", NvmeCtrl, num_queues, 64), diff --git a/hw/block/Makefile.objs b/hw/block/Makefile.objs index XXXXXXX..XXXXXXX 100644 --- a/hw/block/Makefile.objs +++ b/hw/block/Makefile.objs @@ -XXX,XX +XXX,XX @@ common-obj-$(CONFIG_PFLASH_CFI02) += pflash_cfi02.o common-obj-$(CONFIG_XEN) += xen-block.o common-obj-$(CONFIG_ECC) += ecc.o common-obj-$(CONFIG_ONENAND) += onenand.o -common-obj-$(CONFIG_NVME_PCI) += nvme.o common-obj-$(CONFIG_SWIM) += swim.o common-obj-$(CONFIG_SH4) += tc58128.o obj-$(CONFIG_VIRTIO_BLK) += virtio-blk.o obj-$(CONFIG_VHOST_USER_BLK) += vhost-user-blk.o +obj-$(CONFIG_NVME_PCI) += nvme.o obj-y += dataplane/ diff --git a/hw/block/trace-events b/hw/block/trace-events index XXXXXXX..XXXXXXX 100644 --- a/hw/block/trace-events +++ b/hw/block/trace-events @@ -XXX,XX +XXX,XX @@ nvme_ub_mmiowr_ssreset_w1c_unsupported(void) "attempted to W1C CSTS.NSSRO but CA nvme_ub_mmiowr_ssreset_unsupported(void) "attempted NVM subsystem reset but CAP.NSSRS is zero (not supported)" nvme_ub_mmiowr_cmbloc_reserved(void) "invalid write to reserved CMBLOC when CMBSZ is zero, ignored" nvme_ub_mmiowr_cmbsz_readonly(void) "invalid write to read only CMBSZ, ignored" +nvme_ub_mmiowr_pmrcap_readonly(void) "invalid write to read only PMRCAP, ignored" +nvme_ub_mmiowr_pmrsts_readonly(void) "invalid write to read only PMRSTS, ignored" +nvme_ub_mmiowr_pmrebs_readonly(void) "invalid write to read only PMREBS, ignored" +nvme_ub_mmiowr_pmrswtp_readonly(void) "invalid write to read only PMRSWTP, ignored" nvme_ub_mmiowr_invalid(uint64_t offset, uint64_t data) "invalid MMIO write, offset=0x%"PRIx64", data=0x%"PRIx64"" nvme_ub_mmiord_misaligned32(uint64_t offset) "MMIO read not 32-bit aligned, offset=0x%"PRIx64"" nvme_ub_mmiord_toosmall(uint64_t offset) "MMIO read smaller than 32-bits, offset=0x%"PRIx64"" -- 2.25.3 The QMP handler qmp_object_add() and the implementation of --object in qemu-storage-daemon can share most of the code. Currently, qemu-storage-daemon calls qmp_object_add(), but this is not correct because different visitors need to be used. As a first step towards a fix, make qmp_object_add() a wrapper around a new function user_creatable_add_dict() that can get an additional parameter. The handling of "props" is only required for compatibility and not required for the qemu-storage-daemon command line, so it stays in qmp_object_add(). Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- include/qom/object_interfaces.h | 12 ++++++++++++ qom/object_interfaces.c | 27 +++++++++++++++++++++++++++ qom/qom-qmp-cmds.c | 24 +----------------------- 3 files changed, 40 insertions(+), 23 deletions(-) diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h index XXXXXXX..XXXXXXX 100644 --- a/include/qom/object_interfaces.h +++ b/include/qom/object_interfaces.h @@ -XXX,XX +XXX,XX @@ Object *user_creatable_add_type(const char *type, const char *id, const QDict *qdict, Visitor *v, Error **errp); +/** + * user_creatable_add_dict: + * @qdict: the object definition + * @errp: if an error occurs, a pointer to an area to store the error + * + * Create an instance of the user creatable object that is defined by + * @qdict. The object type is taken from the QDict key 'qom-type', its + * ID from the key 'id'. The remaining entries in @qdict are used to + * initialize the object properties. + */ +void user_creatable_add_dict(QDict *qdict, Error **errp); + /** * user_creatable_add_opts: * @opts: the object definition diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c index XXXXXXX..XXXXXXX 100644 --- a/qom/object_interfaces.c +++ b/qom/object_interfaces.c @@ -XXX,XX +XXX,XX @@ #include "qapi/qmp/qerror.h" #include "qapi/qmp/qjson.h" #include "qapi/qmp/qstring.h" +#include "qapi/qobject-input-visitor.h" #include "qom/object_interfaces.h" #include "qemu/help_option.h" #include "qemu/module.h" @@ -XXX,XX +XXX,XX @@ out: return obj; } +void user_creatable_add_dict(QDict *qdict, Error **errp) +{ + Visitor *v; + Object *obj; + g_autofree char *type = NULL; + g_autofree char *id = NULL; + + type = g_strdup(qdict_get_try_str(qdict, "qom-type")); + if (!type) { + error_setg(errp, QERR_MISSING_PARAMETER, "qom-type"); + return; + } + qdict_del(qdict, "qom-type"); + + id = g_strdup(qdict_get_try_str(qdict, "id")); + if (!id) { + error_setg(errp, QERR_MISSING_PARAMETER, "id"); + return; + } + qdict_del(qdict, "id"); + + v = qobject_input_visitor_new(QOBJECT(qdict)); + obj = user_creatable_add_type(type, id, qdict, v, errp); + visit_free(v); + object_unref(obj); +} Object *user_creatable_add_opts(QemuOpts *opts, Error **errp) { diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c index XXXXXXX..XXXXXXX 100644 --- a/qom/qom-qmp-cmds.c +++ b/qom/qom-qmp-cmds.c @@ -XXX,XX +XXX,XX @@ #include "qapi/qapi-commands-qom.h" #include "qapi/qmp/qdict.h" #include "qapi/qmp/qerror.h" -#include "qapi/qobject-input-visitor.h" #include "qemu/cutils.h" #include "qom/object_interfaces.h" #include "qom/qom-qobject.h" @@ -XXX,XX +XXX,XX @@ void qmp_object_add(QDict *qdict, QObject **ret_data, Error **errp) { QObject *props; QDict *pdict; - Visitor *v; - Object *obj; - g_autofree char *type = NULL; - g_autofree char *id = NULL; - - type = g_strdup(qdict_get_try_str(qdict, "qom-type")); - if (!type) { - error_setg(errp, QERR_MISSING_PARAMETER, "qom-type"); - return; - } - qdict_del(qdict, "qom-type"); - - id = g_strdup(qdict_get_try_str(qdict, "id")); - if (!id) { - error_setg(errp, QERR_MISSING_PARAMETER, "id"); - return; - } - qdict_del(qdict, "id"); props = qdict_get(qdict, "props"); if (props) { @@ -XXX,XX +XXX,XX @@ void qmp_object_add(QDict *qdict, QObject **ret_data, Error **errp) qobject_unref(pdict); } - v = qobject_input_visitor_new(QOBJECT(qdict)); - obj = user_creatable_add_type(type, id, qdict, v, errp); - visit_free(v); - object_unref(obj); + user_creatable_add_dict(qdict, errp); } void qmp_object_del(const char *id, Error **errp) -- 2.25.3 After processing the option string with the keyval parser, we get a QDict that contains only strings. This QDict must be fed to a keyval visitor which converts the strings into the right data types. qmp_object_add(), however, uses the normal QObject input visitor, which expects a QDict where all properties already have the QType that matches the data type required by the QOM object type. Change the --object implementation in qemu-storage-daemon so that it doesn't call qmp_object_add(), but calls user_creatable_add_dict() directly instead and pass it a new keyval boolean that decides which visitor must be used. Reported-by: Coiby Xu <coiby.xu@gmail.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- include/qom/object_interfaces.h | 6 +++++- qemu-storage-daemon.c | 4 +--- qom/object_interfaces.c | 8 ++++++-- qom/qom-qmp-cmds.c | 2 +- 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h index XXXXXXX..XXXXXXX 100644 --- a/include/qom/object_interfaces.h +++ b/include/qom/object_interfaces.h @@ -XXX,XX +XXX,XX @@ Object *user_creatable_add_type(const char *type, const char *id, /** * user_creatable_add_dict: * @qdict: the object definition + * @keyval: if true, use a keyval visitor for processing @qdict (i.e. + * assume that all @qdict values are strings); otherwise, use + * the normal QObject visitor (i.e. assume all @qdict values + * have the QType expected by the QOM object type) * @errp: if an error occurs, a pointer to an area to store the error * * Create an instance of the user creatable object that is defined by @@ -XXX,XX +XXX,XX @@ Object *user_creatable_add_type(const char *type, const char *id, * ID from the key 'id'. The remaining entries in @qdict are used to * initialize the object properties. */ -void user_creatable_add_dict(QDict *qdict, Error **errp); +void user_creatable_add_dict(QDict *qdict, bool keyval, Error **errp); /** * user_creatable_add_opts: diff --git a/qemu-storage-daemon.c b/qemu-storage-daemon.c index XXXXXXX..XXXXXXX 100644 --- a/qemu-storage-daemon.c +++ b/qemu-storage-daemon.c @@ -XXX,XX +XXX,XX @@ static void process_options(int argc, char *argv[]) QemuOpts *opts; const char *type; QDict *args; - QObject *ret_data = NULL; /* FIXME The keyval parser rejects 'help' arguments, so we must * unconditionall try QemuOpts first. */ @@ -XXX,XX +XXX,XX @@ static void process_options(int argc, char *argv[]) qemu_opts_del(opts); args = keyval_parse(optarg, "qom-type", &error_fatal); - qmp_object_add(args, &ret_data, &error_fatal); + user_creatable_add_dict(args, true, &error_fatal); qobject_unref(args); - qobject_unref(ret_data); break; } default: diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c index XXXXXXX..XXXXXXX 100644 --- a/qom/object_interfaces.c +++ b/qom/object_interfaces.c @@ -XXX,XX +XXX,XX @@ out: return obj; } -void user_creatable_add_dict(QDict *qdict, Error **errp) +void user_creatable_add_dict(QDict *qdict, bool keyval, Error **errp) { Visitor *v; Object *obj; @@ -XXX,XX +XXX,XX @@ void user_creatable_add_dict(QDict *qdict, Error **errp) } qdict_del(qdict, "id"); - v = qobject_input_visitor_new(QOBJECT(qdict)); + if (keyval) { + v = qobject_input_visitor_new_keyval(QOBJECT(qdict)); + } else { + v = qobject_input_visitor_new(QOBJECT(qdict)); + } obj = user_creatable_add_type(type, id, qdict, v, errp); visit_free(v); object_unref(obj); diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c index XXXXXXX..XXXXXXX 100644 --- a/qom/qom-qmp-cmds.c +++ b/qom/qom-qmp-cmds.c @@ -XXX,XX +XXX,XX @@ void qmp_object_add(QDict *qdict, QObject **ret_data, Error **errp) qobject_unref(pdict); } - user_creatable_add_dict(qdict, errp); + user_creatable_add_dict(qdict, false, errp); } void qmp_object_del(const char *id, Error **errp) -- 2.25.3