[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/4] block: Add bdrv_make_empty()
From: |
Eric Blake |
Subject: |
Re: [PATCH 1/4] block: Add bdrv_make_empty() |
Date: |
Tue, 28 Apr 2020 08:53:38 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 |
On 4/28/20 8:26 AM, Max Reitz wrote:
Right now, all users of bdrv_make_empty() call the BlockDriver method
directly. That is not only bad style, it is also wrong, unless the
caller has a BdrvChild with a WRITE permission.
Introduce bdrv_make_empty() that verifies that it does.
Signed-off-by: Max Reitz <address@hidden>
---
include/block/block.h | 1 +
block.c | 23 +++++++++++++++++++++++
2 files changed, 24 insertions(+)
diff --git a/include/block/block.h b/include/block/block.h
index b05995fe9c..d947fb4080 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -351,6 +351,7 @@ BlockMeasureInfo *bdrv_measure(BlockDriver *drv, QemuOpts
*opts,
void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr);
void bdrv_refresh_limits(BlockDriverState *bs, Error **errp);
int bdrv_commit(BlockDriverState *bs);
+int bdrv_make_empty(BdrvChild *c, Error **errp);
Can we please fix this to take a flags parameter? I want to make it
easier for callers to request BDRV_REQ_NO_FALLBACK for distinguishing
between callers where the image must be made empty (read as all zeroes)
regardless of time spent, vs. made empty quickly (including if it is
already all zero) but where the caller is prepared for the operation to
fail and will write zeroes itself if fast bulk zeroing was not possible.
+int bdrv_make_empty(BdrvChild *c, Error **errp)
+{
+ BlockDriver *drv = c->bs->drv;
+ int ret;
+
+ assert(c->perm & BLK_PERM_WRITE);
+
+ if (!drv->bdrv_make_empty) {
+ error_setg(errp, "%s does not support emptying nodes",
+ drv->format_name);
+ return -ENOTSUP;
+ }
And here's where we can add some automatic fallbacks, such as
recognizing if the image already reads as all zeroes. But those
optimizations can come in separate patches; for YOUR patch, just getting
the proper API in place is fine.
+
+ ret = drv->bdrv_make_empty(c->bs);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "Failed to empty %s",
+ c->bs->filename);
+ return ret;
+ }
+
+ return 0;
+}
Other than a missing flag parameter, this looks fine.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH 1/4] block: Add bdrv_make_empty(), Kevin Wolf, 2020/04/28
[PATCH 2/4] block: Use bdrv_make_empty() where possible, Max Reitz, 2020/04/28
[PATCH 3/4] block: Add blk_make_empty(), Max Reitz, 2020/04/28