From b6c1bae5df8abbed73c4c0bd92e9963df8829c74 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 23 Jun 2016 14:20:24 +0200 Subject: block: Accept node-name for block-stream In order to remove the necessity to use BlockBackend names in the external API, we want to allow node-names everywhere. This converts block-stream to accept a node-name without lifting the restriction that we're operating at a root node. In case of an invalid device name, the command returns the GenericError error class now instead of DeviceNotFound, because this is what qmp_get_root_bs() returns. Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz Reviewed-by: Alberto Garcia --- include/sysemu/block-backend.h | 1 + 1 file changed, 1 insertion(+) (limited to 'include') diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index 2da4905d18..bb4fa82d1c 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -98,6 +98,7 @@ BlockDriverState *blk_bs(BlockBackend *blk); void blk_remove_bs(BlockBackend *blk); void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs); bool bdrv_has_blk(BlockDriverState *bs); +bool bdrv_is_root_node(BlockDriverState *bs); void blk_set_allow_write_beyond_eof(BlockBackend *blk, bool allow); void blk_iostatus_enable(BlockBackend *blk); -- cgit v1.2.3-55-g7522 From cd7fca952ce8456955f7f4e11df9ced14204c2f1 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 6 Jul 2016 11:22:39 +0200 Subject: nbd-server: Use a separate BlockBackend The builtin NBD server uses its own BlockBackend now instead of reusing the monitor/guest device one. This means that it has its own writethrough setting now. The builtin NBD server always uses writeback caching now regardless of whether the guest device has WCE enabled. qemu-nbd respects the cache mode given on the command line. We still need to keep a reference to the monitor BB because we put an eject notifier on it, but we don't use it for any I/O. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake Reviewed-by: Max Reitz --- block.c | 2 ++ blockdev-nbd.c | 4 ++-- include/block/nbd.h | 3 ++- nbd/server.c | 25 ++++++++++++++++++++----- qemu-nbd.c | 4 ++-- 5 files changed, 28 insertions(+), 10 deletions(-) (limited to 'include') diff --git a/block.c b/block.c index 30d64e6ca5..101f8c628f 100644 --- a/block.c +++ b/block.c @@ -25,6 +25,7 @@ #include "trace.h" #include "block/block_int.h" #include "block/blockjob.h" +#include "block/nbd.h" #include "qemu/error-report.h" #include "qemu/module.h" #include "qapi/qmp/qerror.h" @@ -2206,6 +2207,7 @@ static void bdrv_close(BlockDriverState *bs) void bdrv_close_all(void) { block_job_cancel_sync_all(); + nbd_export_close_all(); /* Drop references from requests still in flight, such as canceled block * jobs whose AIO context has not been polled yet */ diff --git a/blockdev-nbd.c b/blockdev-nbd.c index 12cae0ea72..c437d32573 100644 --- a/blockdev-nbd.c +++ b/blockdev-nbd.c @@ -176,8 +176,8 @@ void qmp_nbd_server_add(const char *device, bool has_writable, bool writable, writable = false; } - exp = nbd_export_new(blk, 0, -1, writable ? 0 : NBD_FLAG_READ_ONLY, NULL, - errp); + exp = nbd_export_new(blk_bs(blk), 0, -1, writable ? 0 : NBD_FLAG_READ_ONLY, + NULL, false, blk, errp); if (!exp) { return; } diff --git a/include/block/nbd.h b/include/block/nbd.h index 1897557a9b..80610ff31b 100644 --- a/include/block/nbd.h +++ b/include/block/nbd.h @@ -103,8 +103,9 @@ int nbd_disconnect(int fd); typedef struct NBDExport NBDExport; typedef struct NBDClient NBDClient; -NBDExport *nbd_export_new(BlockBackend *blk, off_t dev_offset, off_t size, +NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset, off_t size, uint16_t nbdflags, void (*close)(NBDExport *), + bool writethrough, BlockBackend *on_eject_blk, Error **errp); void nbd_export_close(NBDExport *exp); void nbd_export_get(NBDExport *exp); diff --git a/nbd/server.c b/nbd/server.c index 80fbb4da1d..472f584c32 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -69,6 +69,7 @@ struct NBDExport { AioContext *ctx; + BlockBackend *eject_notifier_blk; Notifier eject_notifier; }; @@ -807,11 +808,18 @@ static void nbd_eject_notifier(Notifier *n, void *data) nbd_export_close(exp); } -NBDExport *nbd_export_new(BlockBackend *blk, off_t dev_offset, off_t size, +NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset, off_t size, uint16_t nbdflags, void (*close)(NBDExport *), + bool writethrough, BlockBackend *on_eject_blk, Error **errp) { + BlockBackend *blk; NBDExport *exp = g_malloc0(sizeof(NBDExport)); + + blk = blk_new(); + blk_insert_bs(blk, bs); + blk_set_enable_write_cache(blk, !writethrough); + exp->refcount = 1; QTAILQ_INIT(&exp->clients); exp->blk = blk; @@ -827,11 +835,14 @@ NBDExport *nbd_export_new(BlockBackend *blk, off_t dev_offset, off_t size, exp->close = close; exp->ctx = blk_get_aio_context(blk); - blk_ref(blk); blk_add_aio_context_notifier(blk, blk_aio_attached, blk_aio_detach, exp); - exp->eject_notifier.notify = nbd_eject_notifier; - blk_add_remove_bs_notifier(blk, &exp->eject_notifier); + if (on_eject_blk) { + blk_ref(on_eject_blk); + exp->eject_notifier_blk = on_eject_blk; + exp->eject_notifier.notify = nbd_eject_notifier; + blk_add_remove_bs_notifier(on_eject_blk, &exp->eject_notifier); + } /* * NBD exports are used for non-shared storage migration. Make sure @@ -844,6 +855,7 @@ NBDExport *nbd_export_new(BlockBackend *blk, off_t dev_offset, off_t size, return exp; fail: + blk_unref(blk); g_free(exp); return NULL; } @@ -914,7 +926,10 @@ void nbd_export_put(NBDExport *exp) } if (exp->blk) { - notifier_remove(&exp->eject_notifier); + if (exp->eject_notifier_blk) { + notifier_remove(&exp->eject_notifier); + blk_unref(exp->eject_notifier_blk); + } blk_remove_aio_context_notifier(exp->blk, blk_aio_attached, blk_aio_detach, exp); blk_unref(exp->blk); diff --git a/qemu-nbd.c b/qemu-nbd.c index e3571c2025..99297a556f 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -910,8 +910,8 @@ int main(int argc, char **argv) } } - exp = nbd_export_new(blk, dev_offset, fd_size, nbdflags, nbd_export_closed, - &local_err); + exp = nbd_export_new(bs, dev_offset, fd_size, nbdflags, nbd_export_closed, + writethrough, NULL, &local_err); if (!exp) { error_report_err(local_err); exit(EXIT_FAILURE); -- cgit v1.2.3-55-g7522 From fe5c1355e73940dbde9b38dec6e8fab4117ec637 Mon Sep 17 00:00:00 2001 From: Pavel Butsykin Date: Fri, 22 Jul 2016 11:17:40 +0300 Subject: block: switch blk_write_compressed() to byte-based interface This is a preparatory patch, which continues the general trend of the transition to the byte-based interfaces. bdrv_check_request() and blk_check_request() are no longer used, thus we can remove them. Signed-off-by: Pavel Butsykin Reviewed-by: Stefan Hajnoczi Reviewed-by: Eric Blake Signed-off-by: Denis V. Lunev CC: Jeff Cody CC: Markus Armbruster CC: Eric Blake CC: John Snow CC: Stefan Hajnoczi CC: Kevin Wolf Signed-off-by: Kevin Wolf --- block/block-backend.c | 23 ++++------------------- block/io.c | 22 +++++++--------------- include/block/block.h | 4 ++-- include/sysemu/block-backend.h | 4 ++-- qemu-img.c | 6 ++++-- qemu-io-cmds.c | 2 +- 6 files changed, 20 insertions(+), 41 deletions(-) (limited to 'include') diff --git a/block/block-backend.c b/block/block-backend.c index dc2bc60b9e..4c704a134f 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -743,21 +743,6 @@ static int blk_check_byte_request(BlockBackend *blk, int64_t offset, return 0; } -static int blk_check_request(BlockBackend *blk, int64_t sector_num, - int nb_sectors) -{ - if (sector_num < 0 || sector_num > INT64_MAX / BDRV_SECTOR_SIZE) { - return -EIO; - } - - if (nb_sectors < 0 || nb_sectors > INT_MAX / BDRV_SECTOR_SIZE) { - return -EIO; - } - - return blk_check_byte_request(blk, sector_num * BDRV_SECTOR_SIZE, - nb_sectors * BDRV_SECTOR_SIZE); -} - int coroutine_fn blk_co_preadv(BlockBackend *blk, int64_t offset, unsigned int bytes, QEMUIOVector *qiov, BdrvRequestFlags flags) @@ -1500,15 +1485,15 @@ int coroutine_fn blk_co_pwrite_zeroes(BlockBackend *blk, int64_t offset, flags | BDRV_REQ_ZERO_WRITE); } -int blk_write_compressed(BlockBackend *blk, int64_t sector_num, - const uint8_t *buf, int nb_sectors) +int blk_pwrite_compressed(BlockBackend *blk, int64_t offset, const void *buf, + int count) { - int ret = blk_check_request(blk, sector_num, nb_sectors); + int ret = blk_check_byte_request(blk, offset, count); if (ret < 0) { return ret; } - return bdrv_write_compressed(blk_bs(blk), sector_num, buf, nb_sectors); + return bdrv_pwrite_compressed(blk_bs(blk), offset, buf, count); } int blk_truncate(BlockBackend *blk, int64_t offset) diff --git a/block/io.c b/block/io.c index 420944d80d..da874d0f19 100644 --- a/block/io.c +++ b/block/io.c @@ -540,17 +540,6 @@ static int bdrv_check_byte_request(BlockDriverState *bs, int64_t offset, return 0; } -static int bdrv_check_request(BlockDriverState *bs, int64_t sector_num, - int nb_sectors) -{ - if (nb_sectors < 0 || nb_sectors > BDRV_REQUEST_MAX_SECTORS) { - return -EIO; - } - - return bdrv_check_byte_request(bs, sector_num * BDRV_SECTOR_SIZE, - nb_sectors * BDRV_SECTOR_SIZE); -} - typedef struct RwCo { BdrvChild *child; int64_t offset; @@ -1879,8 +1868,8 @@ int bdrv_is_allocated_above(BlockDriverState *top, return 0; } -int bdrv_write_compressed(BlockDriverState *bs, int64_t sector_num, - const uint8_t *buf, int nb_sectors) +int bdrv_pwrite_compressed(BlockDriverState *bs, int64_t offset, + const void *buf, int bytes) { BlockDriver *drv = bs->drv; int ret; @@ -1891,14 +1880,17 @@ int bdrv_write_compressed(BlockDriverState *bs, int64_t sector_num, if (!drv->bdrv_write_compressed) { return -ENOTSUP; } - ret = bdrv_check_request(bs, sector_num, nb_sectors); + ret = bdrv_check_byte_request(bs, offset, bytes); if (ret < 0) { return ret; } assert(QLIST_EMPTY(&bs->dirty_bitmaps)); + assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0); + assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0); - return drv->bdrv_write_compressed(bs, sector_num, buf, nb_sectors); + return drv->bdrv_write_compressed(bs, offset >> BDRV_SECTOR_BITS, buf, + bytes >> BDRV_SECTOR_BITS); } typedef struct BdrvVmstateCo { diff --git a/include/block/block.h b/include/block/block.h index 11c162d594..b4a97f2612 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -399,8 +399,8 @@ const char *bdrv_get_node_name(const BlockDriverState *bs); const char *bdrv_get_device_name(const BlockDriverState *bs); const char *bdrv_get_device_or_node_name(const BlockDriverState *bs); int bdrv_get_flags(BlockDriverState *bs); -int bdrv_write_compressed(BlockDriverState *bs, int64_t sector_num, - const uint8_t *buf, int nb_sectors); +int bdrv_pwrite_compressed(BlockDriverState *bs, int64_t offset, + const void *buf, int bytes); int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi); ImageInfoSpecific *bdrv_get_specific_info(BlockDriverState *bs); void bdrv_round_sectors_to_clusters(BlockDriverState *bs, diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index bb4fa82d1c..4808a9621a 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -204,8 +204,8 @@ void *blk_aio_get(const AIOCBInfo *aiocb_info, BlockBackend *blk, BlockCompletionFunc *cb, void *opaque); int coroutine_fn blk_co_pwrite_zeroes(BlockBackend *blk, int64_t offset, int count, BdrvRequestFlags flags); -int blk_write_compressed(BlockBackend *blk, int64_t sector_num, - const uint8_t *buf, int nb_sectors); +int blk_pwrite_compressed(BlockBackend *blk, int64_t offset, const void *buf, + int count); int blk_truncate(BlockBackend *blk, int64_t offset); int blk_pdiscard(BlockBackend *blk, int64_t offset, int count); int blk_save_vmstate(BlockBackend *blk, const uint8_t *buf, diff --git a/qemu-img.c b/qemu-img.c index f204d04136..ef0157aa8a 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1590,7 +1590,9 @@ static int convert_write(ImgConvertState *s, int64_t sector_num, int nb_sectors, break; } - ret = blk_write_compressed(s->target, sector_num, buf, n); + ret = blk_pwrite_compressed(s->target, + sector_num << BDRV_SECTOR_BITS, + buf, n << BDRV_SECTOR_BITS); if (ret < 0) { return ret; } @@ -1727,7 +1729,7 @@ static int convert_do_copy(ImgConvertState *s) if (s->compressed) { /* signal EOF to align */ - ret = blk_write_compressed(s->target, 0, NULL, 0); + ret = blk_pwrite_compressed(s->target, 0, NULL, 0); if (ret < 0) { goto fail; } diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c index 25954f5634..3a3838a079 100644 --- a/qemu-io-cmds.c +++ b/qemu-io-cmds.c @@ -504,7 +504,7 @@ static int do_write_compressed(BlockBackend *blk, char *buf, int64_t offset, return -ERANGE; } - ret = blk_write_compressed(blk, offset >> 9, (uint8_t *)buf, count >> 9); + ret = blk_pwrite_compressed(blk, offset, buf, count); if (ret < 0) { return ret; } -- cgit v1.2.3-55-g7522 From 751e2f0698c284b4924d83ec63fa4e834bf4c80d Mon Sep 17 00:00:00 2001 From: Pavel Butsykin Date: Fri, 22 Jul 2016 11:17:41 +0300 Subject: block: Convert bdrv_pwrite_compressed() to BdrvChild Signed-off-by: Pavel Butsykin Signed-off-by: Denis V. Lunev Reviewed-by: Eric Blake CC: Jeff Cody CC: Markus Armbruster CC: Eric Blake CC: John Snow CC: Stefan Hajnoczi CC: Kevin Wolf Signed-off-by: Kevin Wolf --- block/block-backend.c | 2 +- block/io.c | 3 ++- include/block/block.h | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) (limited to 'include') diff --git a/block/block-backend.c b/block/block-backend.c index 4c704a134f..76ea45955f 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -1493,7 +1493,7 @@ int blk_pwrite_compressed(BlockBackend *blk, int64_t offset, const void *buf, return ret; } - return bdrv_pwrite_compressed(blk_bs(blk), offset, buf, count); + return bdrv_pwrite_compressed(blk->root, offset, buf, count); } int blk_truncate(BlockBackend *blk, int64_t offset) diff --git a/block/io.c b/block/io.c index da874d0f19..c528fead1b 100644 --- a/block/io.c +++ b/block/io.c @@ -1868,9 +1868,10 @@ int bdrv_is_allocated_above(BlockDriverState *top, return 0; } -int bdrv_pwrite_compressed(BlockDriverState *bs, int64_t offset, +int bdrv_pwrite_compressed(BdrvChild *child, int64_t offset, const void *buf, int bytes) { + BlockDriverState *bs = child->bs; BlockDriver *drv = bs->drv; int ret; diff --git a/include/block/block.h b/include/block/block.h index b4a97f2612..7bb5ddbf73 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -399,7 +399,7 @@ const char *bdrv_get_node_name(const BlockDriverState *bs); const char *bdrv_get_device_name(const BlockDriverState *bs); const char *bdrv_get_device_or_node_name(const BlockDriverState *bs); int bdrv_get_flags(BlockDriverState *bs); -int bdrv_pwrite_compressed(BlockDriverState *bs, int64_t offset, +int bdrv_pwrite_compressed(BdrvChild *child, int64_t offset, const void *buf, int bytes); int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi); ImageInfoSpecific *bdrv_get_specific_info(BlockDriverState *bs); -- cgit v1.2.3-55-g7522 From 29a298af9d2479cc230505b18d5a5c2da0ab578e Mon Sep 17 00:00:00 2001 From: Pavel Butsykin Date: Fri, 22 Jul 2016 11:17:42 +0300 Subject: block/io: reuse bdrv_co_pwritev() for write compressed For bdrv_pwrite_compressed() it looks like most of the code creating coroutine is duplicated in bdrv_prwv_co(). So we can just add a flag (BDRV_REQ_WRITE_COMPRESSED) and use bdrv_prwv_co() as a generic one. In the end we get coroutine oriented function for write compressed by using bdrv_co_pwritev/blk_co_pwritev with BDRV_REQ_WRITE_COMPRESSED flag. Signed-off-by: Pavel Butsykin Reviewed-by: Stefan Hajnoczi Signed-off-by: Denis V. Lunev CC: Jeff Cody CC: Markus Armbruster CC: Eric Blake CC: John Snow CC: Stefan Hajnoczi CC: Kevin Wolf Signed-off-by: Kevin Wolf --- block/io.c | 56 +++++++++++++++++++++++++++++++++-------------- include/block/block.h | 3 ++- include/block/block_int.h | 3 +++ qemu-img.c | 2 +- 4 files changed, 46 insertions(+), 18 deletions(-) (limited to 'include') diff --git a/block/io.c b/block/io.c index c528fead1b..d402076e95 100644 --- a/block/io.c +++ b/block/io.c @@ -886,6 +886,20 @@ emulate_flags: return ret; } +static int coroutine_fn +bdrv_driver_pwritev_compressed(BlockDriverState *bs, uint64_t offset, + uint64_t bytes, QEMUIOVector *qiov) +{ + BlockDriver *drv = bs->drv; + + if (!drv->bdrv_co_pwritev_compressed) { + return -ENOTSUP; + } + + assert(QLIST_EMPTY(&bs->dirty_bitmaps)); + return drv->bdrv_co_pwritev_compressed(bs, offset, bytes, qiov); +} + static int coroutine_fn bdrv_co_do_copy_on_readv(BlockDriverState *bs, int64_t offset, unsigned int bytes, QEMUIOVector *qiov) { @@ -1555,9 +1569,14 @@ int coroutine_fn bdrv_co_pwritev(BdrvChild *child, bytes = ROUND_UP(bytes, align); } - ret = bdrv_aligned_pwritev(bs, &req, offset, bytes, align, - use_local_qiov ? &local_qiov : qiov, - flags); + if (flags & BDRV_REQ_WRITE_COMPRESSED) { + ret = bdrv_driver_pwritev_compressed( + bs, offset, bytes, use_local_qiov ? &local_qiov : qiov); + } else { + ret = bdrv_aligned_pwritev(bs, &req, offset, bytes, align, + use_local_qiov ? &local_qiov : qiov, + flags); + } fail: @@ -1873,25 +1892,30 @@ int bdrv_pwrite_compressed(BdrvChild *child, int64_t offset, { BlockDriverState *bs = child->bs; BlockDriver *drv = bs->drv; - int ret; + QEMUIOVector qiov; + struct iovec iov; if (!drv) { return -ENOMEDIUM; } - if (!drv->bdrv_write_compressed) { - return -ENOTSUP; - } - ret = bdrv_check_byte_request(bs, offset, bytes); - if (ret < 0) { - return ret; + if (drv->bdrv_write_compressed) { + int ret = bdrv_check_byte_request(bs, offset, bytes); + if (ret < 0) { + return ret; + } + assert(QLIST_EMPTY(&bs->dirty_bitmaps)); + assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0); + assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0); + return drv->bdrv_write_compressed(bs, offset >> BDRV_SECTOR_BITS, buf, + bytes >> BDRV_SECTOR_BITS); } + iov = (struct iovec) { + .iov_base = (void *)buf, + .iov_len = bytes, + }; + qemu_iovec_init_external(&qiov, &iov, 1); - assert(QLIST_EMPTY(&bs->dirty_bitmaps)); - assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0); - assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0); - - return drv->bdrv_write_compressed(bs, offset >> BDRV_SECTOR_BITS, buf, - bytes >> BDRV_SECTOR_BITS); + return bdrv_prwv_co(child, offset, &qiov, true, BDRV_REQ_WRITE_COMPRESSED); } typedef struct BdrvVmstateCo { diff --git a/include/block/block.h b/include/block/block.h index 7bb5ddbf73..d8dacd2ced 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -65,9 +65,10 @@ typedef enum { BDRV_REQ_MAY_UNMAP = 0x4, BDRV_REQ_NO_SERIALISING = 0x8, BDRV_REQ_FUA = 0x10, + BDRV_REQ_WRITE_COMPRESSED = 0x20, /* Mask of valid flags */ - BDRV_REQ_MASK = 0x1f, + BDRV_REQ_MASK = 0x3f, } BdrvRequestFlags; typedef struct BlockSizes { diff --git a/include/block/block_int.h b/include/block/block_int.h index 1e939de4fe..42f8f8443d 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -207,6 +207,9 @@ struct BlockDriver { int (*bdrv_write_compressed)(BlockDriverState *bs, int64_t sector_num, const uint8_t *buf, int nb_sectors); + int coroutine_fn (*bdrv_co_pwritev_compressed)(BlockDriverState *bs, + uint64_t offset, uint64_t bytes, QEMUIOVector *qiov); + int (*bdrv_snapshot_create)(BlockDriverState *bs, QEMUSnapshotInfo *sn_info); int (*bdrv_snapshot_goto)(BlockDriverState *bs, diff --git a/qemu-img.c b/qemu-img.c index ef0157aa8a..c2ea494928 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -2034,7 +2034,7 @@ static int img_convert(int argc, char **argv) const char *preallocation = qemu_opt_get(opts, BLOCK_OPT_PREALLOC); - if (!drv->bdrv_write_compressed) { + if (!drv->bdrv_write_compressed && !drv->bdrv_co_pwritev_compressed) { error_report("Compression not supported for this file format"); ret = -1; goto out; -- cgit v1.2.3-55-g7522 From 35fadca80e6df2e7a2e57ea162db11f0219c2b2d Mon Sep 17 00:00:00 2001 From: Pavel Butsykin Date: Fri, 22 Jul 2016 11:17:48 +0300 Subject: block: remove BlockDriver.bdrv_write_compressed There are no block drivers left that implement the old .bdrv_write_compressed interface, so it can be removed. Also now we have no need to use the bdrv_pwrite_compressed function and we can remove it entirely. Signed-off-by: Pavel Butsykin Reviewed-by: Stefan Hajnoczi Signed-off-by: Denis V. Lunev CC: Jeff Cody CC: Markus Armbruster CC: Eric Blake CC: John Snow CC: Stefan Hajnoczi CC: Kevin Wolf Signed-off-by: Kevin Wolf --- block/block-backend.c | 8 ++------ block/io.c | 31 ------------------------------- include/block/block.h | 2 -- include/block/block_int.h | 3 --- qemu-img.c | 2 +- 5 files changed, 3 insertions(+), 43 deletions(-) (limited to 'include') diff --git a/block/block-backend.c b/block/block-backend.c index 76ea45955f..d1349d90e5 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -1488,12 +1488,8 @@ 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 count) { - int ret = blk_check_byte_request(blk, offset, count); - if (ret < 0) { - return ret; - } - - return bdrv_pwrite_compressed(blk->root, offset, buf, count); + return blk_prw(blk, offset, (void *) buf, count, blk_write_entry, + BDRV_REQ_WRITE_COMPRESSED); } int blk_truncate(BlockBackend *blk, int64_t offset) diff --git a/block/io.c b/block/io.c index d402076e95..0339911dbb 100644 --- a/block/io.c +++ b/block/io.c @@ -1887,37 +1887,6 @@ int bdrv_is_allocated_above(BlockDriverState *top, return 0; } -int bdrv_pwrite_compressed(BdrvChild *child, int64_t offset, - const void *buf, int bytes) -{ - BlockDriverState *bs = child->bs; - BlockDriver *drv = bs->drv; - QEMUIOVector qiov; - struct iovec iov; - - if (!drv) { - return -ENOMEDIUM; - } - if (drv->bdrv_write_compressed) { - int ret = bdrv_check_byte_request(bs, offset, bytes); - if (ret < 0) { - return ret; - } - assert(QLIST_EMPTY(&bs->dirty_bitmaps)); - assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0); - assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0); - return drv->bdrv_write_compressed(bs, offset >> BDRV_SECTOR_BITS, buf, - bytes >> BDRV_SECTOR_BITS); - } - iov = (struct iovec) { - .iov_base = (void *)buf, - .iov_len = bytes, - }; - qemu_iovec_init_external(&qiov, &iov, 1); - - return bdrv_prwv_co(child, offset, &qiov, true, BDRV_REQ_WRITE_COMPRESSED); -} - typedef struct BdrvVmstateCo { BlockDriverState *bs; QEMUIOVector *qiov; diff --git a/include/block/block.h b/include/block/block.h index d8dacd2ced..7edce5c35f 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -400,8 +400,6 @@ const char *bdrv_get_node_name(const BlockDriverState *bs); const char *bdrv_get_device_name(const BlockDriverState *bs); const char *bdrv_get_device_or_node_name(const BlockDriverState *bs); int bdrv_get_flags(BlockDriverState *bs); -int bdrv_pwrite_compressed(BdrvChild *child, int64_t offset, - const void *buf, int bytes); int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi); ImageInfoSpecific *bdrv_get_specific_info(BlockDriverState *bs); void bdrv_round_sectors_to_clusters(BlockDriverState *bs, diff --git a/include/block/block_int.h b/include/block/block_int.h index 42f8f8443d..9c61f49f94 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -204,9 +204,6 @@ struct BlockDriver { bool has_variable_length; int64_t (*bdrv_get_allocated_file_size)(BlockDriverState *bs); - int (*bdrv_write_compressed)(BlockDriverState *bs, int64_t sector_num, - const uint8_t *buf, int nb_sectors); - int coroutine_fn (*bdrv_co_pwritev_compressed)(BlockDriverState *bs, uint64_t offset, uint64_t bytes, QEMUIOVector *qiov); diff --git a/qemu-img.c b/qemu-img.c index c2ea494928..1090286a9f 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -2034,7 +2034,7 @@ static int img_convert(int argc, char **argv) const char *preallocation = qemu_opt_get(opts, BLOCK_OPT_PREALLOC); - if (!drv->bdrv_write_compressed && !drv->bdrv_co_pwritev_compressed) { + if (!drv->bdrv_co_pwritev_compressed) { error_report("Compression not supported for this file format"); ret = -1; goto out; -- cgit v1.2.3-55-g7522 From 13b9414b5798539e2dbb87a570d96184fe21edf4 Mon Sep 17 00:00:00 2001 From: Pavel Butsykin Date: Fri, 22 Jul 2016 11:17:52 +0300 Subject: drive-backup: added support for data compression The idea is simple - backup is "written-once" data. It is written block by block and it is large enough. It would be nice to save storage space and compress it. The patch adds a flag to the qmp/hmp drive-backup command which enables block compression. Compression should be implemented in the format driver to enable this feature. There are some limitations of the format driver to allow compressed writes. We can write data only once. Though for backup this is perfectly fine. These limitations are maintained by the driver and the error will be reported if we are doing something wrong. Signed-off-by: Pavel Butsykin Reviewed-by: Stefan Hajnoczi Signed-off-by: Denis V. Lunev CC: Jeff Cody CC: Markus Armbruster CC: Eric Blake CC: John Snow CC: Stefan Hajnoczi CC: Kevin Wolf Signed-off-by: Kevin Wolf --- block/backup.c | 12 +++++++++++- blockdev.c | 9 ++++++--- hmp-commands.hx | 8 +++++--- hmp.c | 3 +++ include/block/block_int.h | 1 + qapi/block-core.json | 5 ++++- qmp-commands.hx | 5 ++++- 7 files changed, 34 insertions(+), 9 deletions(-) (limited to 'include') diff --git a/block/backup.c b/block/backup.c index 2c0532314f..bb3bb9a9eb 100644 --- a/block/backup.c +++ b/block/backup.c @@ -47,6 +47,7 @@ typedef struct BackupBlockJob { uint64_t sectors_read; unsigned long *done_bitmap; int64_t cluster_size; + bool compress; NotifierWithReturn before_write; QLIST_HEAD(, CowRequest) inflight_reqs; } BackupBlockJob; @@ -154,7 +155,8 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job, bounce_qiov.size, BDRV_REQ_MAY_UNMAP); } else { ret = blk_co_pwritev(job->target, start * job->cluster_size, - bounce_qiov.size, &bounce_qiov, 0); + bounce_qiov.size, &bounce_qiov, + job->compress ? BDRV_REQ_WRITE_COMPRESSED : 0); } if (ret < 0) { trace_backup_do_cow_write_fail(job, start, ret); @@ -477,6 +479,7 @@ static void coroutine_fn backup_run(void *opaque) void backup_start(const char *job_id, BlockDriverState *bs, BlockDriverState *target, int64_t speed, MirrorSyncMode sync_mode, BdrvDirtyBitmap *sync_bitmap, + bool compress, BlockdevOnError on_source_error, BlockdevOnError on_target_error, BlockCompletionFunc *cb, void *opaque, @@ -507,6 +510,12 @@ void backup_start(const char *job_id, BlockDriverState *bs, return; } + if (compress && target->drv->bdrv_co_pwritev_compressed == NULL) { + error_setg(errp, "Compression is not supported for this drive %s", + bdrv_get_device_name(target)); + return; + } + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) { return; } @@ -555,6 +564,7 @@ void backup_start(const char *job_id, BlockDriverState *bs, job->sync_mode = sync_mode; job->sync_bitmap = sync_mode == MIRROR_SYNC_MODE_INCREMENTAL ? sync_bitmap : NULL; + job->compress = compress; /* If there is no backing file on the target, we cannot rely on COW if our * backup cluster size is smaller than the target cluster size. Even for diff --git a/blockdev.c b/blockdev.c index 5f1b01518d..f5857d0187 100644 --- a/blockdev.c +++ b/blockdev.c @@ -3132,6 +3132,9 @@ static void do_drive_backup(DriveBackup *backup, BlockJobTxn *txn, Error **errp) if (!backup->has_job_id) { backup->job_id = NULL; } + if (!backup->has_compress) { + backup->compress = false; + } bs = qmp_get_root_bs(backup->device, errp); if (!bs) { @@ -3210,8 +3213,8 @@ static void do_drive_backup(DriveBackup *backup, BlockJobTxn *txn, Error **errp) } backup_start(backup->job_id, bs, target_bs, backup->speed, backup->sync, - bmap, backup->on_source_error, backup->on_target_error, - block_job_cb, bs, txn, &local_err); + bmap, backup->compress, backup->on_source_error, + backup->on_target_error, block_job_cb, bs, txn, &local_err); bdrv_unref(target_bs); if (local_err != NULL) { error_propagate(errp, local_err); @@ -3277,7 +3280,7 @@ void do_blockdev_backup(BlockdevBackup *backup, BlockJobTxn *txn, Error **errp) } } backup_start(backup->job_id, bs, target_bs, backup->speed, backup->sync, - NULL, backup->on_source_error, backup->on_target_error, + NULL, false, backup->on_source_error, backup->on_target_error, block_job_cb, bs, txn, &local_err); if (local_err != NULL) { error_propagate(errp, local_err); diff --git a/hmp-commands.hx b/hmp-commands.hx index 848efee5d1..74f32e515c 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1182,8 +1182,8 @@ ETEXI { .name = "drive_backup", - .args_type = "reuse:-n,full:-f,device:B,target:s,format:s?", - .params = "[-n] [-f] device target [format]", + .args_type = "reuse:-n,full:-f,compress:-c,device:B,target:s,format:s?", + .params = "[-n] [-f] [-c] device target [format]", .help = "initiates a point-in-time\n\t\t\t" "copy for a device. The device's contents are\n\t\t\t" "copied to the new image file, excluding data that\n\t\t\t" @@ -1191,7 +1191,9 @@ ETEXI "The -n flag requests QEMU to reuse the image found\n\t\t\t" "in new-image-file, instead of recreating it from scratch.\n\t\t\t" "The -f flag requests QEMU to copy the whole disk,\n\t\t\t" - "so that the result does not need a backing file.\n\t\t\t", + "so that the result does not need a backing file.\n\t\t\t" + "The -c flag requests QEMU to compress backup data\n\t\t\t" + "(if the target format supports it).\n\t\t\t", .mhandler.cmd = hmp_drive_backup, }, STEXI diff --git a/hmp.c b/hmp.c index 3c06200e48..a7dfe6fa11 100644 --- a/hmp.c +++ b/hmp.c @@ -1109,6 +1109,7 @@ void hmp_drive_backup(Monitor *mon, const QDict *qdict) const char *format = qdict_get_try_str(qdict, "format"); bool reuse = qdict_get_try_bool(qdict, "reuse", false); bool full = qdict_get_try_bool(qdict, "full", false); + bool compress = qdict_get_try_bool(qdict, "compress", false); Error *err = NULL; DriveBackup backup = { .device = (char *)device, @@ -1118,6 +1119,8 @@ void hmp_drive_backup(Monitor *mon, const QDict *qdict) .sync = full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP, .has_mode = true, .mode = reuse ? NEW_IMAGE_MODE_EXISTING : NEW_IMAGE_MODE_ABSOLUTE_PATHS, + .has_compress = !!compress, + .compress = compress, }; if (!filename) { diff --git a/include/block/block_int.h b/include/block/block_int.h index 9c61f49f94..0ca6a78eb3 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -765,6 +765,7 @@ void mirror_start(const char *job_id, BlockDriverState *bs, void backup_start(const char *job_id, BlockDriverState *bs, BlockDriverState *target, int64_t speed, MirrorSyncMode sync_mode, BdrvDirtyBitmap *sync_bitmap, + bool compress, BlockdevOnError on_source_error, BlockdevOnError on_target_error, BlockCompletionFunc *cb, void *opaque, diff --git a/qapi/block-core.json b/qapi/block-core.json index 4c8cf057b5..300a68b63a 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -898,6 +898,9 @@ # Must be present if sync is "incremental", must NOT be present # otherwise. (Since 2.4) # +# @compress: #optional true to compress data, if the target format supports it. +# (default: false) (since 2.7) +# # @on-source-error: #optional the action to take on an error on the source, # default 'report'. 'stop' and 'enospc' can only be used # if the block device supports io-status (see BlockInfo). @@ -915,7 +918,7 @@ { 'struct': 'DriveBackup', 'data': { '*job-id': 'str', 'device': 'str', 'target': 'str', '*format': 'str', 'sync': 'MirrorSyncMode', '*mode': 'NewImageMode', - '*speed': 'int', '*bitmap': 'str', + '*speed': 'int', '*bitmap': 'str', '*compress': 'bool', '*on-source-error': 'BlockdevOnError', '*on-target-error': 'BlockdevOnError' } } diff --git a/qmp-commands.hx b/qmp-commands.hx index 956f1b0f47..d1593c91ff 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -1217,7 +1217,8 @@ EQMP { .name = "drive-backup", .args_type = "job-id:s?,sync:s,device:B,target:s,speed:i?,mode:s?," - "format:s?,bitmap:s?,on-source-error:s?,on-target-error:s?", + "format:s?,bitmap:s?,compress:b?," + "on-source-error:s?,on-target-error:s?", .mhandler.cmd_new = qmp_marshal_drive_backup, }, @@ -1253,6 +1254,8 @@ Arguments: - "mode": whether and how QEMU should create a new image (NewImageMode, optional, default 'absolute-paths') - "speed": the maximum speed, in bytes per second (json-int, optional) +- "compress": true to compress data, if the target format supports it. + (json-bool, optional, default false) - "on-source-error": the action to take on an error on the source, default 'report'. 'stop' and 'enospc' can only be used if the block device supports io-status. -- cgit v1.2.3-55-g7522 From 0e438cdc932a785de72166af4641aafa103a6670 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 11 Aug 2016 17:45:06 +0200 Subject: coroutine: Let CoMutex remember who holds it In cases of deadlocks, knowing who holds a given CoMutex is really helpful for debugging. Keeping the information around doesn't cost much and allows us to add another assertion to keep the code correct, so let's just add it. Signed-off-by: Kevin Wolf Reviewed-by: Paolo Bonzini Reviewed-by: Stefan Hajnoczi --- include/qemu/coroutine.h | 1 + util/qemu-coroutine-lock.c | 3 +++ 2 files changed, 4 insertions(+) (limited to 'include') diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h index ac8d4c9cc8..29a20782f0 100644 --- a/include/qemu/coroutine.h +++ b/include/qemu/coroutine.h @@ -143,6 +143,7 @@ bool qemu_co_queue_empty(CoQueue *queue); */ typedef struct CoMutex { bool locked; + Coroutine *holder; CoQueue queue; } CoMutex; diff --git a/util/qemu-coroutine-lock.c b/util/qemu-coroutine-lock.c index 22aa9abb30..f30ee8184d 100644 --- a/util/qemu-coroutine-lock.c +++ b/util/qemu-coroutine-lock.c @@ -129,6 +129,7 @@ void coroutine_fn qemu_co_mutex_lock(CoMutex *mutex) } mutex->locked = true; + mutex->holder = self; trace_qemu_co_mutex_lock_return(mutex, self); } @@ -140,9 +141,11 @@ void coroutine_fn qemu_co_mutex_unlock(CoMutex *mutex) trace_qemu_co_mutex_unlock_entry(mutex, self); assert(mutex->locked == true); + assert(mutex->holder == self); assert(qemu_in_coroutine()); mutex->locked = false; + mutex->holder = NULL; qemu_co_queue_next(&mutex->queue); trace_qemu_co_mutex_unlock_return(mutex, self); -- cgit v1.2.3-55-g7522 From 1b7f01d966f97b7820f3cdd471461cf0799a93cc Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 11 Aug 2016 17:51:59 +0200 Subject: coroutine: Assert that no locks are held on termination A coroutine that takes a lock must also release it again. If the coroutine terminates without having released all its locks, it's buggy and we'll probably run into a deadlock sooner or later. Make sure that we don't get such cases. Signed-off-by: Kevin Wolf Reviewed-by: Paolo Bonzini Reviewed-by: Stefan Hajnoczi --- include/qemu/coroutine_int.h | 1 + util/qemu-coroutine-lock.c | 11 +++++++++++ util/qemu-coroutine.c | 1 + 3 files changed, 13 insertions(+) (limited to 'include') diff --git a/include/qemu/coroutine_int.h b/include/qemu/coroutine_int.h index 581a7f5140..6df9d33352 100644 --- a/include/qemu/coroutine_int.h +++ b/include/qemu/coroutine_int.h @@ -39,6 +39,7 @@ struct Coroutine { void *entry_arg; Coroutine *caller; QSLIST_ENTRY(Coroutine) pool_next; + size_t locks_held; /* Coroutines that should be woken up when we yield or terminate */ QSIMPLEQ_HEAD(, Coroutine) co_queue_wakeup; diff --git a/util/qemu-coroutine-lock.c b/util/qemu-coroutine-lock.c index f30ee8184d..14cf9ce458 100644 --- a/util/qemu-coroutine-lock.c +++ b/util/qemu-coroutine-lock.c @@ -130,6 +130,7 @@ void coroutine_fn qemu_co_mutex_lock(CoMutex *mutex) mutex->locked = true; mutex->holder = self; + self->locks_held++; trace_qemu_co_mutex_lock_return(mutex, self); } @@ -146,6 +147,7 @@ void coroutine_fn qemu_co_mutex_unlock(CoMutex *mutex) mutex->locked = false; mutex->holder = NULL; + self->locks_held--; qemu_co_queue_next(&mutex->queue); trace_qemu_co_mutex_unlock_return(mutex, self); @@ -159,14 +161,19 @@ void qemu_co_rwlock_init(CoRwlock *lock) void qemu_co_rwlock_rdlock(CoRwlock *lock) { + Coroutine *self = qemu_coroutine_self(); + while (lock->writer) { qemu_co_queue_wait(&lock->queue); } lock->reader++; + self->locks_held++; } void qemu_co_rwlock_unlock(CoRwlock *lock) { + Coroutine *self = qemu_coroutine_self(); + assert(qemu_in_coroutine()); if (lock->writer) { lock->writer = false; @@ -179,12 +186,16 @@ void qemu_co_rwlock_unlock(CoRwlock *lock) qemu_co_queue_next(&lock->queue); } } + self->locks_held--; } void qemu_co_rwlock_wrlock(CoRwlock *lock) { + Coroutine *self = qemu_coroutine_self(); + while (lock->writer || lock->reader) { qemu_co_queue_wait(&lock->queue); } lock->writer = true; + self->locks_held++; } diff --git a/util/qemu-coroutine.c b/util/qemu-coroutine.c index 89f21a9cec..3cbf225487 100644 --- a/util/qemu-coroutine.c +++ b/util/qemu-coroutine.c @@ -122,6 +122,7 @@ void qemu_coroutine_enter(Coroutine *co) case COROUTINE_YIELD: return; case COROUTINE_TERMINATE: + assert(!co->locks_held); trace_qemu_coroutine_terminate(co); coroutine_delete(co); return; -- cgit v1.2.3-55-g7522