Message ID | 20210715124853.13335-1-vsementsov@virtuozzo.com |
---|---|
State | New |
Headers | show |
Series | block/vvfat: fix: drop backing | expand |
> On Jul 15, 2021, at 8:48 AM, Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote: > > Most probably this fake backing child doesn't work anyway (see notes > about it in a8a4d15c1c34d). > > Still, since 25f78d9e2de528473d52 drivers are required to set > .supports_backing if they want to call bdrv_set_backing_hd, so now > vvfat just doesn't work because of this check. > > Let's finally drop this fake backing file. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > > Honestly, I don't know, which scenarios may break after this patch. > So, that's just my idea that it's more correct to drop this thing than > set .supports_backing in vvfat driver. > > block/vvfat.c | 43 ++++--------------------------------------- > 1 file changed, 4 insertions(+), 39 deletions(-) > > diff --git a/block/vvfat.c b/block/vvfat.c > index ae9d387da7..34bf1e3a86 100644 > --- a/block/vvfat.c > +++ b/block/vvfat.c > @@ -3098,26 +3098,6 @@ static int coroutine_fn vvfat_co_block_status(BlockDriverState *bs, > return BDRV_BLOCK_DATA; > } > > -static int coroutine_fn > -write_target_commit(BlockDriverState *bs, uint64_t offset, uint64_t bytes, > - QEMUIOVector *qiov, int flags) > -{ > - int ret; > - > - BDRVVVFATState* s = *((BDRVVVFATState**) bs->opaque); > - qemu_co_mutex_lock(&s->lock); > - ret = try_commit(s); > - qemu_co_mutex_unlock(&s->lock); > - > - return ret; > -} > - > -static BlockDriver vvfat_write_target = { > - .format_name = "vvfat_write_target", > - .instance_size = sizeof(void*), > - .bdrv_co_pwritev = write_target_commit, > -}; > - > static void vvfat_qcow_options(BdrvChildRole role, bool parent_is_format, > int *child_flags, QDict *child_options, > int parent_flags, QDict *parent_options) > @@ -3133,7 +3113,6 @@ static int enable_write_target(BlockDriverState *bs, Error **errp) > { > BDRVVVFATState *s = bs->opaque; > BlockDriver *bdrv_qcow = NULL; > - BlockDriverState *backing; > QemuOpts *opts = NULL; > int ret; > int size = sector2cluster(s, s->sector_count); > @@ -3184,13 +3163,6 @@ static int enable_write_target(BlockDriverState *bs, Error **errp) > unlink(s->qcow_filename); > #endif > > - backing = bdrv_new_open_driver(&vvfat_write_target, NULL, BDRV_O_ALLOW_RDWR, > - &error_abort); > - *(void**) backing->opaque = s; > - > - bdrv_set_backing_hd(s->bs, backing, &error_abort); > - bdrv_unref(backing); > - > return 0; > > err: > @@ -3205,17 +3177,10 @@ static void vvfat_child_perm(BlockDriverState *bs, BdrvChild *c, > uint64_t perm, uint64_t shared, > uint64_t *nperm, uint64_t *nshared) > { > - if (role & BDRV_CHILD_DATA) { > - /* This is a private node, nobody should try to attach to it */ > - *nperm = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE; > - *nshared = BLK_PERM_WRITE_UNCHANGED; > - } else { > - assert(role & BDRV_CHILD_COW); > - /* The backing file is there so 'commit' can use it. vvfat doesn't > - * access it in any way. */ > - *nperm = 0; > - *nshared = BLK_PERM_ALL; > - } > + assert(role & BDRV_CHILD_DATA); > + /* This is a private node, nobody should try to attach to it */ > + *nperm = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE; > + *nshared = BLK_PERM_WRITE_UNCHANGED; > } > > static void vvfat_close(BlockDriverState *bs) > -- > 2.29.2 > Tested-by: John Arbuckle <programmingkidx@gmail.com>
Am 15.07.2021 um 14:48 hat Vladimir Sementsov-Ogievskiy geschrieben: > Most probably this fake backing child doesn't work anyway (see notes > about it in a8a4d15c1c34d). > > Still, since 25f78d9e2de528473d52 drivers are required to set > .supports_backing if they want to call bdrv_set_backing_hd, so now > vvfat just doesn't work because of this check. > > Let's finally drop this fake backing file. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Thanks, applied to the block branch. Kevin
diff --git a/block/vvfat.c b/block/vvfat.c index ae9d387da7..34bf1e3a86 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -3098,26 +3098,6 @@ static int coroutine_fn vvfat_co_block_status(BlockDriverState *bs, return BDRV_BLOCK_DATA; } -static int coroutine_fn -write_target_commit(BlockDriverState *bs, uint64_t offset, uint64_t bytes, - QEMUIOVector *qiov, int flags) -{ - int ret; - - BDRVVVFATState* s = *((BDRVVVFATState**) bs->opaque); - qemu_co_mutex_lock(&s->lock); - ret = try_commit(s); - qemu_co_mutex_unlock(&s->lock); - - return ret; -} - -static BlockDriver vvfat_write_target = { - .format_name = "vvfat_write_target", - .instance_size = sizeof(void*), - .bdrv_co_pwritev = write_target_commit, -}; - static void vvfat_qcow_options(BdrvChildRole role, bool parent_is_format, int *child_flags, QDict *child_options, int parent_flags, QDict *parent_options) @@ -3133,7 +3113,6 @@ static int enable_write_target(BlockDriverState *bs, Error **errp) { BDRVVVFATState *s = bs->opaque; BlockDriver *bdrv_qcow = NULL; - BlockDriverState *backing; QemuOpts *opts = NULL; int ret; int size = sector2cluster(s, s->sector_count); @@ -3184,13 +3163,6 @@ static int enable_write_target(BlockDriverState *bs, Error **errp) unlink(s->qcow_filename); #endif - backing = bdrv_new_open_driver(&vvfat_write_target, NULL, BDRV_O_ALLOW_RDWR, - &error_abort); - *(void**) backing->opaque = s; - - bdrv_set_backing_hd(s->bs, backing, &error_abort); - bdrv_unref(backing); - return 0; err: @@ -3205,17 +3177,10 @@ static void vvfat_child_perm(BlockDriverState *bs, BdrvChild *c, uint64_t perm, uint64_t shared, uint64_t *nperm, uint64_t *nshared) { - if (role & BDRV_CHILD_DATA) { - /* This is a private node, nobody should try to attach to it */ - *nperm = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE; - *nshared = BLK_PERM_WRITE_UNCHANGED; - } else { - assert(role & BDRV_CHILD_COW); - /* The backing file is there so 'commit' can use it. vvfat doesn't - * access it in any way. */ - *nperm = 0; - *nshared = BLK_PERM_ALL; - } + assert(role & BDRV_CHILD_DATA); + /* This is a private node, nobody should try to attach to it */ + *nperm = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE; + *nshared = BLK_PERM_WRITE_UNCHANGED; } static void vvfat_close(BlockDriverState *bs)
Most probably this fake backing child doesn't work anyway (see notes about it in a8a4d15c1c34d). Still, since 25f78d9e2de528473d52 drivers are required to set .supports_backing if they want to call bdrv_set_backing_hd, so now vvfat just doesn't work because of this check. Let's finally drop this fake backing file. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- Honestly, I don't know, which scenarios may break after this patch. So, that's just my idea that it's more correct to drop this thing than set .supports_backing in vvfat driver. block/vvfat.c | 43 ++++--------------------------------------- 1 file changed, 4 insertions(+), 39 deletions(-)