* [Qemu-devel] [PATCH 1/2] block: pass bdrv_* methods to bs->file by default
2017-06-27 22:46 [Qemu-devel] [PATCH 0/2] block: block driver callbacks fixes Manos Pitsidianakis
@ 2017-06-27 22:46 ` Manos Pitsidianakis
2017-06-29 14:00 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-06-29 14:05 ` Stefan Hajnoczi
2017-06-27 22:46 ` [Qemu-devel] [PATCH 2/2] block: add default implementations for bdrv_co_get_block_status() Manos Pitsidianakis
1 sibling, 2 replies; 7+ messages in thread
From: Manos Pitsidianakis @ 2017-06-27 22:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, Kevin Wolf, Stefan Hajnoczi, Max Reitz
The following functions fail if bs->drv does not implement them:
bdrv_probe_blocksizes
bdrv_probe_geometry
bdrv_truncate
bdrv_has_zero_init
bdrv_get_info
bdrv_media_changed
bdrv_eject
bdrv_lock_medium
bdrv_co_ioctl
Instead, the call should be passed to bs->file if it exists, to allow
filter drivers to support those methods without implementing them.
Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr>
---
block.c | 45 ++++++++++++++++++++++++++++++++++-----------
block/io.c | 4 ++++
2 files changed, 38 insertions(+), 11 deletions(-)
diff --git a/block.c b/block.c
index 69439628..ad11230a 100644
--- a/block.c
+++ b/block.c
@@ -494,6 +494,8 @@ int bdrv_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
if (drv && drv->bdrv_probe_blocksizes) {
return drv->bdrv_probe_blocksizes(bs, bsz);
+ } else if (bs->file && bs->file->bs) {
+ return bdrv_probe_blocksizes(bs->file->bs, bsz);
}
return -ENOTSUP;
@@ -511,6 +513,8 @@ int bdrv_probe_geometry(BlockDriverState *bs, HDGeometry *geo)
if (drv && drv->bdrv_probe_geometry) {
return drv->bdrv_probe_geometry(bs, geo);
+ } else if (bs->file && bs->file->bs) {
+ return bdrv_probe_geometry(bs->file->bs, geo);
}
return -ENOTSUP;
@@ -3406,13 +3410,18 @@ int bdrv_truncate(BdrvChild *child, int64_t offset, Error **errp)
assert(child->perm & BLK_PERM_RESIZE);
- if (!drv) {
- error_setg(errp, "No medium inserted");
- return -ENOMEDIUM;
- }
- if (!drv->bdrv_truncate) {
- error_setg(errp, "Image format driver does not support resize");
- return -ENOTSUP;
+ if (!drv || !drv->bdrv_truncate) {
+ if (bs->file && bs->file->bs) {
+ return bdrv_truncate(bs->file, offset, errp);
+ }
+ if (!drv) {
+ error_setg(errp, "No medium inserted");
+ return -ENOMEDIUM;
+ }
+ if (!drv->bdrv_truncate) {
+ error_setg(errp, "Image format driver does not support resize");
+ return -ENOTSUP;
+ }
}
if (bs->read_only) {
error_setg(errp, "Image is read-only");
@@ -3778,6 +3787,9 @@ int bdrv_has_zero_init(BlockDriverState *bs)
if (bs->drv->bdrv_has_zero_init) {
return bs->drv->bdrv_has_zero_init(bs);
}
+ if (bs->file && bs->file->bs) {
+ return bdrv_has_zero_init(bs->file->bs);
+ }
/* safe default */
return 0;
@@ -3832,10 +3844,15 @@ void bdrv_get_backing_filename(BlockDriverState *bs,
int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
{
BlockDriver *drv = bs->drv;
- if (!drv)
- return -ENOMEDIUM;
- if (!drv->bdrv_get_info)
- return -ENOTSUP;
+ if (!drv || !drv->bdrv_get_info) {
+ if (bs->file && bs->file->bs) {
+ return bdrv_get_info(bs->file->bs, bdi);
+ }
+ if (!drv)
+ return -ENOMEDIUM;
+ if (!drv->bdrv_get_info)
+ return -ENOTSUP;
+ }
memset(bdi, 0, sizeof(*bdi));
return drv->bdrv_get_info(bs, bdi);
}
@@ -4205,6 +4222,8 @@ int bdrv_media_changed(BlockDriverState *bs)
if (drv && drv->bdrv_media_changed) {
return drv->bdrv_media_changed(bs);
+ } else if (bs->file && bs->file->bs) {
+ bdrv_media_changed(bs->file->bs);
}
return -ENOTSUP;
}
@@ -4218,6 +4237,8 @@ void bdrv_eject(BlockDriverState *bs, bool eject_flag)
if (drv && drv->bdrv_eject) {
drv->bdrv_eject(bs, eject_flag);
+ } else if (bs->file && bs->file->bs) {
+ bdrv_eject(bs->file->bs, eject_flag);
}
}
@@ -4233,6 +4254,8 @@ void bdrv_lock_medium(BlockDriverState *bs, bool locked)
if (drv && drv->bdrv_lock_medium) {
drv->bdrv_lock_medium(bs, locked);
+ } else if (bs->file && bs->file->bs) {
+ bdrv_lock_medium(bs->file->bs, locked);
}
}
diff --git a/block/io.c b/block/io.c
index c72d7015..c1b73226 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2403,6 +2403,10 @@ int bdrv_co_ioctl(BlockDriverState *bs, int req, void *buf)
bdrv_inc_in_flight(bs);
if (!drv || (!drv->bdrv_aio_ioctl && !drv->bdrv_co_ioctl)) {
+ if (bs->file && bs->file->bs) {
+ bdrv_dec_in_flight(bs);
+ return bdrv_co_ioctl(bs->file->bs, req, buf);
+ }
co.ret = -ENOTSUP;
goto out;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH 2/2] block: add default implementations for bdrv_co_get_block_status()
2017-06-27 22:46 [Qemu-devel] [PATCH 0/2] block: block driver callbacks fixes Manos Pitsidianakis
2017-06-27 22:46 ` [Qemu-devel] [PATCH 1/2] block: pass bdrv_* methods to bs->file by default Manos Pitsidianakis
@ 2017-06-27 22:46 ` Manos Pitsidianakis
2017-06-29 14:04 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
1 sibling, 1 reply; 7+ messages in thread
From: Manos Pitsidianakis @ 2017-06-27 22:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, Kevin Wolf, Stefan Hajnoczi, Max Reitz
bdrv_co_get_block_status_from_file() and
bdrv_co_get_block_status_from_backing() set *file to bs->file and
bs->backing respectively, so that bdrv_co_get_block_status() can recurse
to them. Future block drivers won't have to duplicate code to implement
this.
Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr>
---
block/blkdebug.c | 12 +-----------
block/commit.c | 12 +-----------
block/io.c | 24 ++++++++++++++++++++++++
block/mirror.c | 12 +-----------
include/block/block_int.h | 16 ++++++++++++++++
5 files changed, 43 insertions(+), 33 deletions(-)
diff --git a/block/blkdebug.c b/block/blkdebug.c
index b25856c4..f1539db6 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -641,16 +641,6 @@ static int coroutine_fn blkdebug_co_pdiscard(BlockDriverState *bs,
return bdrv_co_pdiscard(bs->file->bs, offset, bytes);
}
-static int64_t coroutine_fn blkdebug_co_get_block_status(
- BlockDriverState *bs, int64_t sector_num, int nb_sectors, int *pnum,
- BlockDriverState **file)
-{
- *pnum = nb_sectors;
- *file = bs->file->bs;
- return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID |
- (sector_num << BDRV_SECTOR_BITS);
-}
-
static void blkdebug_close(BlockDriverState *bs)
{
BDRVBlkdebugState *s = bs->opaque;
@@ -925,7 +915,7 @@ static BlockDriver bdrv_blkdebug = {
.bdrv_co_flush_to_disk = blkdebug_co_flush,
.bdrv_co_pwrite_zeroes = blkdebug_co_pwrite_zeroes,
.bdrv_co_pdiscard = blkdebug_co_pdiscard,
- .bdrv_co_get_block_status = blkdebug_co_get_block_status,
+ .bdrv_co_get_block_status = bdrv_co_get_block_status_from_file,
.bdrv_debug_event = blkdebug_debug_event,
.bdrv_debug_breakpoint = blkdebug_debug_breakpoint,
diff --git a/block/commit.c b/block/commit.c
index 524bd549..5b04f832 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -247,16 +247,6 @@ static int coroutine_fn bdrv_commit_top_preadv(BlockDriverState *bs,
return bdrv_co_preadv(bs->backing, offset, bytes, qiov, flags);
}
-static int64_t coroutine_fn bdrv_commit_top_get_block_status(
- BlockDriverState *bs, int64_t sector_num, int nb_sectors, int *pnum,
- BlockDriverState **file)
-{
- *pnum = nb_sectors;
- *file = bs->backing->bs;
- return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID |
- (sector_num << BDRV_SECTOR_BITS);
-}
-
static void bdrv_commit_top_refresh_filename(BlockDriverState *bs, QDict *opts)
{
bdrv_refresh_filename(bs->backing->bs);
@@ -282,7 +272,7 @@ static void bdrv_commit_top_child_perm(BlockDriverState *bs, BdrvChild *c,
static BlockDriver bdrv_commit_top = {
.format_name = "commit_top",
.bdrv_co_preadv = bdrv_commit_top_preadv,
- .bdrv_co_get_block_status = bdrv_commit_top_get_block_status,
+ .bdrv_co_get_block_status = bdrv_co_get_block_status_from_backing,
.bdrv_refresh_filename = bdrv_commit_top_refresh_filename,
.bdrv_close = bdrv_commit_top_close,
.bdrv_child_perm = bdrv_commit_top_child_perm,
diff --git a/block/io.c b/block/io.c
index c1b73226..aec520b4 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1706,6 +1706,30 @@ typedef struct BdrvCoGetBlockStatusData {
bool done;
} BdrvCoGetBlockStatusData;
+int64_t coroutine_fn bdrv_co_get_block_status_from_file(BlockDriverState *bs,
+ int64_t sector_num,
+ int nb_sectors, int *pnum,
+ BlockDriverState **file)
+{
+ assert(bs->file && bs->file->bs);
+ *pnum = nb_sectors;
+ *file = bs->file->bs;
+ return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID |
+ (sector_num << BDRV_SECTOR_BITS);
+}
+
+int64_t coroutine_fn bdrv_co_get_block_status_from_backing(BlockDriverState *bs,
+ int64_t sector_num,
+ int nb_sectors, int *pnum,
+ BlockDriverState **file)
+{
+ assert(bs->backing && bs->backing->bs);
+ *pnum = nb_sectors;
+ *file = bs->backing->bs;
+ return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID |
+ (sector_num << BDRV_SECTOR_BITS);
+}
+
/*
* Returns the allocation status of the specified sectors.
* Drivers not implementing the functionality are assumed to not support
diff --git a/block/mirror.c b/block/mirror.c
index 61a862dc..e8bf5f40 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1052,16 +1052,6 @@ static int coroutine_fn bdrv_mirror_top_flush(BlockDriverState *bs)
return bdrv_co_flush(bs->backing->bs);
}
-static int64_t coroutine_fn bdrv_mirror_top_get_block_status(
- BlockDriverState *bs, int64_t sector_num, int nb_sectors, int *pnum,
- BlockDriverState **file)
-{
- *pnum = nb_sectors;
- *file = bs->backing->bs;
- return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID |
- (sector_num << BDRV_SECTOR_BITS);
-}
-
static int coroutine_fn bdrv_mirror_top_pwrite_zeroes(BlockDriverState *bs,
int64_t offset, int bytes, BdrvRequestFlags flags)
{
@@ -1108,7 +1098,7 @@ static BlockDriver bdrv_mirror_top = {
.bdrv_co_pwrite_zeroes = bdrv_mirror_top_pwrite_zeroes,
.bdrv_co_pdiscard = bdrv_mirror_top_pdiscard,
.bdrv_co_flush = bdrv_mirror_top_flush,
- .bdrv_co_get_block_status = bdrv_mirror_top_get_block_status,
+ .bdrv_co_get_block_status = bdrv_co_get_block_status_from_backing,
.bdrv_refresh_filename = bdrv_mirror_top_refresh_filename,
.bdrv_close = bdrv_mirror_top_close,
.bdrv_child_perm = bdrv_mirror_top_child_perm,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 15fa6021..a42c2a6f 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -945,6 +945,22 @@ void bdrv_format_default_perms(BlockDriverState *bs, BdrvChild *c,
uint64_t perm, uint64_t shared,
uint64_t *nperm, uint64_t *nshared);
+/*
+ * Default implementation for drivers to pass bdrv_co_get_block_status() to
+ * their file
+ * */
+int64_t coroutine_fn bdrv_co_get_block_status_from_file(BlockDriverState *bs,
+ int64_t sector_num,
+ int nb_sectors, int *pnum,
+ BlockDriverState **file);
+/*
+ * Default implementation for drivers to pass bdrv_co_get_block_status() to
+ * their backing file
+ * */
+int64_t coroutine_fn bdrv_co_get_block_status_from_backing(BlockDriverState *bs,
+ int64_t sector_num,
+ int nb_sectors, int *pnum,
+ BlockDriverState **file);
const char *bdrv_get_parent_name(const BlockDriverState *bs);
void blk_dev_change_media_cb(BlockBackend *blk, bool load, Error **errp);
bool blk_dev_has_removable_media(BlockBackend *blk);
--
2.11.0
^ permalink raw reply related [flat|nested] 7+ messages in thread