| Message ID | 20200429141126.85159-5-mreitz@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block: Do not call BlockDriver.bdrv_make_empty() directly | expand |
On 29.04.20 16:11, Max Reitz wrote: > bdrv_commit() already has a BlockBackend pointing to the BDS that we > want to empty, it just has the wrong permissions. > > qemu-img commit has no BlockBackend pointing to the old backing file > yet, but introducing one is simple. > > After this commit, bdrv_make_empty() is the only remaining caller of > BlockDriver.bdrv_make_empty(). > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > block/commit.c | 16 +++++++++------- > qemu-img.c | 19 ++++++++++++++----- > 2 files changed, 23 insertions(+), 12 deletions(-) [...] > diff --git a/qemu-img.c b/qemu-img.c > index 7f52742ef2..77f3575538 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -1065,11 +1065,20 @@ static int img_commit(int argc, char **argv) > goto unref_backing; > } > > - if (!drop && bs->drv->bdrv_make_empty) { > - ret = bs->drv->bdrv_make_empty(bs); > - if (ret) { > - error_setg_errno(&local_err, -ret, "Could not empty %s", > - filename); > + if (!drop) { > + BlockBackend *old_backing_blk; > + > + old_backing_blk = blk_new_with_bs(bs, BLK_PERM_WRITE, BLK_PERM_ALL, > + &local_err); > + if (!old_backing_blk) { > + goto unref_backing; > + } > + ret = blk_make_empty(old_backing_blk, &local_err); > + blk_unref(old_backing_blk); > + if (ret == -ENOTSUP) { > + error_free(local_err); > + local_err = NULL; > + } else if (ret < 0) { > goto unref_backing; > } > } This patch should also include a %s/Could not empty/Failed to empty/ on tests/qemu-iotests/098.out... Max
On 4/29/20 9:11 AM, Max Reitz wrote: > bdrv_commit() already has a BlockBackend pointing to the BDS that we > want to empty, it just has the wrong permissions. > > qemu-img commit has no BlockBackend pointing to the old backing file > yet, but introducing one is simple. > > After this commit, bdrv_make_empty() is the only remaining caller of > BlockDriver.bdrv_make_empty(). > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > block/commit.c | 16 +++++++++------- > qemu-img.c | 19 ++++++++++++++----- > 2 files changed, 23 insertions(+), 12 deletions(-) > Reviewed-by: Eric Blake <eblake@redhat.com>
diff --git a/block/commit.c b/block/commit.c index 87f6096d90..ba60fb7955 100644 --- a/block/commit.c +++ b/block/commit.c @@ -414,7 +414,9 @@ int bdrv_commit(BlockDriverState *bs) } ctx = bdrv_get_aio_context(bs); - src = blk_new(ctx, BLK_PERM_CONSISTENT_READ, BLK_PERM_ALL); + /* WRITE_UNCHANGED is required for bdrv_make_empty() */ + src = blk_new(ctx, BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED, + BLK_PERM_ALL); backing = blk_new(ctx, BLK_PERM_WRITE | BLK_PERM_RESIZE, BLK_PERM_ALL); ret = blk_insert_bs(src, bs, &local_err); @@ -492,14 +494,14 @@ int bdrv_commit(BlockDriverState *bs) } } - if (drv->bdrv_make_empty) { - ret = drv->bdrv_make_empty(bs); - if (ret < 0) { - goto ro_cleanup; - } - blk_flush(src); + ret = blk_make_empty(src, NULL); + /* Ignore -ENOTSUP */ + if (ret < 0 && ret != -ENOTSUP) { + goto ro_cleanup; } + blk_flush(src); + /* * Make sure all data we wrote to the backing device is actually * stable on disk. diff --git a/qemu-img.c b/qemu-img.c index 7f52742ef2..77f3575538 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1065,11 +1065,20 @@ static int img_commit(int argc, char **argv) goto unref_backing; } - if (!drop && bs->drv->bdrv_make_empty) { - ret = bs->drv->bdrv_make_empty(bs); - if (ret) { - error_setg_errno(&local_err, -ret, "Could not empty %s", - filename); + if (!drop) { + BlockBackend *old_backing_blk; + + old_backing_blk = blk_new_with_bs(bs, BLK_PERM_WRITE, BLK_PERM_ALL, + &local_err); + if (!old_backing_blk) { + goto unref_backing; + } + ret = blk_make_empty(old_backing_blk, &local_err); + blk_unref(old_backing_blk); + if (ret == -ENOTSUP) { + error_free(local_err); + local_err = NULL; + } else if (ret < 0) { goto unref_backing; } }
bdrv_commit() already has a BlockBackend pointing to the BDS that we want to empty, it just has the wrong permissions. qemu-img commit has no BlockBackend pointing to the old backing file yet, but introducing one is simple. After this commit, bdrv_make_empty() is the only remaining caller of BlockDriver.bdrv_make_empty(). Signed-off-by: Max Reitz <mreitz@redhat.com> --- block/commit.c | 16 +++++++++------- qemu-img.c | 19 ++++++++++++++----- 2 files changed, 23 insertions(+), 12 deletions(-)