From 9f4ed6fbd264a7c097590d830dcfd93996a80acb Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Mon, 26 Oct 2015 16:46:49 +0200 Subject: [PATCH 01/41] block: Don't call blk_bs() twice in bdrv_lookup_bs() Signed-off-by: Alberto Garcia Signed-off-by: Kevin Wolf --- block.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/block.c b/block.c index e9f40dc768..eb8158ac33 100644 --- a/block.c +++ b/block.c @@ -2683,12 +2683,12 @@ BlockDriverState *bdrv_lookup_bs(const char *device, blk = blk_by_name(device); if (blk) { - if (!blk_bs(blk)) { + bs = blk_bs(blk); + if (!bs) { error_setg(errp, "Device '%s' has no medium", device); - return NULL; } - return blk_bs(blk); + return bs; } } From 1c95f7e1aff8417ff6e6cc23bc2d04fbcf79d37e Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 26 Oct 2015 21:39:05 +0100 Subject: [PATCH 02/41] block: Add blk_remove_bs() This function removes the BlockDriverState associated with the given BlockBackend from that BB and sets the BDS pointer in the BB to NULL. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/block-backend.c | 12 ++++++++++++ include/sysemu/block-backend.h | 1 + 2 files changed, 13 insertions(+) diff --git a/block/block-backend.c b/block/block-backend.c index 19fdaaec1a..878c448855 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -333,6 +333,18 @@ void blk_hide_on_behalf_of_hmp_drive_del(BlockBackend *blk) } } +/* + * Disassociates the currently associated BlockDriverState from @blk. + */ +void blk_remove_bs(BlockBackend *blk) +{ + blk_update_root_state(blk); + + blk->bs->blk = NULL; + bdrv_unref(blk->bs); + blk->bs = NULL; +} + /* * Associates a new BlockDriverState with @blk. */ diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index 9306a527bb..14a6d32bb2 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -72,6 +72,7 @@ BlockBackend *blk_by_name(const char *name); BlockBackend *blk_next(BlockBackend *blk); BlockDriverState *blk_bs(BlockBackend *blk); +void blk_remove_bs(BlockBackend *blk); void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs); void blk_hide_on_behalf_of_hmp_drive_del(BlockBackend *blk); From c69a4dd89989b483b06d765b13e41594c78d32b9 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 26 Oct 2015 21:39:06 +0100 Subject: [PATCH 03/41] block: Make bdrv_states public When inserting a BDS tree into a BB, we will need to add the root BDS to this list. Since we will want to do that in the blockdev-insert-medium implementation in blockdev.c, we will need access to it there. This patch is not exactly elegant, but bdrv_states will be removed in the future anyway because we no longer need it since we have BBs. Signed-off-by: Max Reitz Reviewed-by: Alberto Garcia Signed-off-by: Kevin Wolf --- block.c | 3 +-- include/block/block_int.h | 2 ++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/block.c b/block.c index eb8158ac33..a99e6d8509 100644 --- a/block.c +++ b/block.c @@ -73,8 +73,7 @@ struct BdrvDirtyBitmap { #define NOT_DONE 0x7fffffff /* used while emulated sync operation in progress */ -static QTAILQ_HEAD(, BlockDriverState) bdrv_states = - QTAILQ_HEAD_INITIALIZER(bdrv_states); +struct BdrvStates bdrv_states = QTAILQ_HEAD_INITIALIZER(bdrv_states); static QTAILQ_HEAD(, BlockDriverState) graph_bdrv_states = QTAILQ_HEAD_INITIALIZER(graph_bdrv_states); diff --git a/include/block/block_int.h b/include/block/block_int.h index 3ceeb5a940..6a3f64da12 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -473,6 +473,8 @@ extern BlockDriver bdrv_file; extern BlockDriver bdrv_raw; extern BlockDriver bdrv_qcow2; +extern QTAILQ_HEAD(BdrvStates, BlockDriverState) bdrv_states; + /** * bdrv_setup_io_funcs: * From 38cb18f5b71428fb8a3e3759ac8fa21ac70cb1b5 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 26 Oct 2015 21:39:07 +0100 Subject: [PATCH 04/41] block: Add functions for inheriting a BBRS In order to open a BDS which inherits a BB's root state, blk_get_open_flags_from_root_state() is used to inquire the flags to be passed to bdrv_open(), and blk_apply_root_state() is used to apply the remaining state after the BDS has been opened. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/block-backend.c | 27 +++++++++++++++++++++++++++ include/sysemu/block-backend.h | 2 ++ 2 files changed, 29 insertions(+) diff --git a/block/block-backend.c b/block/block-backend.c index 878c448855..7d495395de 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -1239,6 +1239,33 @@ void blk_update_root_state(BlockBackend *blk) } } +/* + * Applies the information in the root state to the given BlockDriverState. This + * does not include the flags which have to be specified for bdrv_open(), use + * blk_get_open_flags_from_root_state() to inquire them. + */ +void blk_apply_root_state(BlockBackend *blk, BlockDriverState *bs) +{ + bs->detect_zeroes = blk->root_state.detect_zeroes; + if (blk->root_state.throttle_group) { + bdrv_io_limits_enable(bs, blk->root_state.throttle_group); + } +} + +/* + * Returns the flags to be used for bdrv_open() of a BlockDriverState which is + * supposed to inherit the root state. + */ +int blk_get_open_flags_from_root_state(BlockBackend *blk) +{ + int bs_flags; + + bs_flags = blk->root_state.read_only ? 0 : BDRV_O_RDWR; + bs_flags |= blk->root_state.open_flags & ~BDRV_O_RDWR; + + return bs_flags; +} + BlockBackendRootState *blk_get_root_state(BlockBackend *blk) { return &blk->root_state; diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index 14a6d32bb2..40e315b5bb 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -167,6 +167,8 @@ void blk_io_unplug(BlockBackend *blk); BlockAcctStats *blk_get_stats(BlockBackend *blk); BlockBackendRootState *blk_get_root_state(BlockBackend *blk); void blk_update_root_state(BlockBackend *blk); +void blk_apply_root_state(BlockBackend *blk, BlockDriverState *bs); +int blk_get_open_flags_from_root_state(BlockBackend *blk); void *blk_aio_get(const AIOCBInfo *aiocb_info, BlockBackend *blk, BlockCompletionFunc *cb, void *opaque); From 7d8a9f71b9ec9fa295265392218efaf0771d96e0 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 26 Oct 2015 21:39:08 +0100 Subject: [PATCH 05/41] blockdev: Add blockdev-open-tray Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- blockdev.c | 36 +++++++++++++++++++++++++++++++++ qapi/block-core.json | 32 +++++++++++++++++++++++++++++ qmp-commands.hx | 48 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 116 insertions(+) diff --git a/blockdev.c b/blockdev.c index 97be42f6cd..d01a99e9cf 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2088,6 +2088,42 @@ out: aio_context_release(aio_context); } +void qmp_blockdev_open_tray(const char *device, bool has_force, bool force, + Error **errp) +{ + BlockBackend *blk; + bool locked; + + if (!has_force) { + force = false; + } + + blk = blk_by_name(device); + if (!blk) { + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, + "Device '%s' not found", device); + return; + } + + if (!blk_dev_has_removable_media(blk)) { + error_setg(errp, "Device '%s' is not removable", device); + return; + } + + if (blk_dev_is_tray_open(blk)) { + return; + } + + locked = blk_dev_is_medium_locked(blk); + if (locked) { + blk_dev_eject_request(blk, force); + } + + if (!locked || force) { + blk_dev_change_media_cb(blk, false); + } +} + /* throttling disk I/O limits */ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd, int64_t bps_wr, diff --git a/qapi/block-core.json b/qapi/block-core.json index 425fdab706..b65ab81218 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -1876,6 +1876,38 @@ ## { 'command': 'blockdev-add', 'data': { 'options': 'BlockdevOptions' } } +## +# @blockdev-open-tray: +# +# Opens a block device's tray. If there is a block driver state tree inserted as +# a medium, it will become inaccessible to the guest (but it will remain +# associated to the block device, so closing the tray will make it accessible +# again). +# +# If the tray was already open before, this will be a no-op. +# +# Once the tray opens, a DEVICE_TRAY_MOVED event is emitted. There are cases in +# which no such event will be generated, these include: +# - if the guest has locked the tray, @force is false and the guest does not +# respond to the eject request +# - if the BlockBackend denoted by @device does not have a guest device attached +# to it +# - if the guest device does not have an actual tray and is empty, for instance +# for floppy disk drives +# +# @device: block device name +# +# @force: #optional if false (the default), an eject request will be sent to +# the guest if it has locked the tray (and the tray will not be opened +# immediately); if true, the tray will be opened regardless of whether +# it is locked +# +# Since: 2.5 +## +{ 'command': 'blockdev-open-tray', + 'data': { 'device': 'str', + '*force': 'bool' } } + ## # @BlockErrorAction diff --git a/qmp-commands.hx b/qmp-commands.hx index 7f85d4046c..d9ae25c498 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -3952,6 +3952,54 @@ Example (2): <- { "return": {} } +EQMP + + { + .name = "blockdev-open-tray", + .args_type = "device:s,force:b?", + .mhandler.cmd_new = qmp_marshal_blockdev_open_tray, + }, + +SQMP +blockdev-open-tray +------------------ + +Opens a block device's tray. If there is a block driver state tree inserted as a +medium, it will become inaccessible to the guest (but it will remain associated +to the block device, so closing the tray will make it accessible again). + +If the tray was already open before, this will be a no-op. + +Once the tray opens, a DEVICE_TRAY_MOVED event is emitted. There are cases in +which no such event will be generated, these include: +- if the guest has locked the tray, @force is false and the guest does not + respond to the eject request +- if the BlockBackend denoted by @device does not have a guest device attached + to it +- if the guest device does not have an actual tray and is empty, for instance + for floppy disk drives + +Arguments: + +- "device": block device name (json-string) +- "force": if false (the default), an eject request will be sent to the guest if + it has locked the tray (and the tray will not be opened immediately); + if true, the tray will be opened regardless of whether it is locked + (json-bool, optional) + +Example: + +-> { "execute": "blockdev-open-tray", + "arguments": { "device": "ide1-cd0" } } + +<- { "timestamp": { "seconds": 1418751016, + "microseconds": 716996 }, + "event": "DEVICE_TRAY_MOVED", + "data": { "device": "ide1-cd0", + "tray-open": true } } + +<- { "return": {} } + EQMP { From abaaf59d245b6984644497b6745f88912c715c39 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 26 Oct 2015 21:39:09 +0100 Subject: [PATCH 06/41] blockdev: Add blockdev-close-tray Signed-off-by: Max Reitz Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- blockdev.c | 23 +++++++++++++++++++++++ qapi/block-core.json | 16 ++++++++++++++++ qmp-commands.hx | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+) diff --git a/blockdev.c b/blockdev.c index d01a99e9cf..cb8a2f095b 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2124,6 +2124,29 @@ void qmp_blockdev_open_tray(const char *device, bool has_force, bool force, } } +void qmp_blockdev_close_tray(const char *device, Error **errp) +{ + BlockBackend *blk; + + blk = blk_by_name(device); + if (!blk) { + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, + "Device '%s' not found", device); + return; + } + + if (!blk_dev_has_removable_media(blk)) { + error_setg(errp, "Device '%s' is not removable", device); + return; + } + + if (!blk_dev_is_tray_open(blk)) { + return; + } + + blk_dev_change_media_cb(blk, true); +} + /* throttling disk I/O limits */ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd, int64_t bps_wr, diff --git a/qapi/block-core.json b/qapi/block-core.json index b65ab81218..1cb719a4e1 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -1908,6 +1908,22 @@ 'data': { 'device': 'str', '*force': 'bool' } } +## +# @blockdev-close-tray: +# +# Closes a block device's tray. If there is a block driver state tree associated +# with the block device (which is currently ejected), that tree will be loaded +# as the medium. +# +# If the tray was already closed before, this will be a no-op. +# +# @device: block device name +# +# Since: 2.5 +## +{ 'command': 'blockdev-close-tray', + 'data': { 'device': 'str' } } + ## # @BlockErrorAction diff --git a/qmp-commands.hx b/qmp-commands.hx index d9ae25c498..0e896c8e45 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -4000,6 +4000,41 @@ Example: <- { "return": {} } +EQMP + + { + .name = "blockdev-close-tray", + .args_type = "device:s", + .mhandler.cmd_new = qmp_marshal_blockdev_close_tray, + }, + +SQMP +blockdev-close-tray +------------------- + +Closes a block device's tray. If there is a block driver state tree associated +with the block device (which is currently ejected), that tree will be loaded as +the medium. + +If the tray was already closed before, this will be a no-op. + +Arguments: + +- "device": block device name (json-string) + +Example: + +-> { "execute": "blockdev-close-tray", + "arguments": { "device": "ide1-cd0" } } + +<- { "timestamp": { "seconds": 1418751345, + "microseconds": 272147 }, + "event": "DEVICE_TRAY_MOVED", + "data": { "device": "ide1-cd0", + "tray-open": false } } + +<- { "return": {} } + EQMP { From 2814f67271bce537f29c6a7832f89fd4f1cdaa1a Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 26 Oct 2015 21:39:10 +0100 Subject: [PATCH 07/41] blockdev: Add blockdev-remove-medium Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- blockdev.c | 51 ++++++++++++++++++++++++++++++++++++++++++++ qapi/block-core.json | 16 ++++++++++++++ qmp-commands.hx | 45 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 112 insertions(+) diff --git a/blockdev.c b/blockdev.c index cb8a2f095b..25635e3597 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2147,6 +2147,57 @@ void qmp_blockdev_close_tray(const char *device, Error **errp) blk_dev_change_media_cb(blk, true); } +void qmp_blockdev_remove_medium(const char *device, Error **errp) +{ + BlockBackend *blk; + BlockDriverState *bs; + AioContext *aio_context; + bool has_device; + + blk = blk_by_name(device); + if (!blk) { + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, + "Device '%s' not found", device); + return; + } + + /* For BBs without a device, we can exchange the BDS tree at will */ + has_device = blk_get_attached_dev(blk); + + if (has_device && !blk_dev_has_removable_media(blk)) { + error_setg(errp, "Device '%s' is not removable", device); + return; + } + + if (has_device && !blk_dev_is_tray_open(blk)) { + error_setg(errp, "Tray of device '%s' is not open", device); + return; + } + + bs = blk_bs(blk); + if (!bs) { + return; + } + + aio_context = bdrv_get_aio_context(bs); + aio_context_acquire(aio_context); + + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_EJECT, errp)) { + goto out; + } + + /* This follows the convention established by bdrv_make_anon() */ + if (bs->device_list.tqe_prev) { + QTAILQ_REMOVE(&bdrv_states, bs, device_list); + bs->device_list.tqe_prev = NULL; + } + + blk_remove_bs(blk); + +out: + aio_context_release(aio_context); +} + /* throttling disk I/O limits */ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd, int64_t bps_wr, diff --git a/qapi/block-core.json b/qapi/block-core.json index 1cb719a4e1..e19e82cbaf 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -1924,6 +1924,22 @@ { 'command': 'blockdev-close-tray', 'data': { 'device': 'str' } } +## +# @blockdev-remove-medium: +# +# Removes a medium (a block driver state tree) from a block device. That block +# device's tray must currently be open (unless there is no attached guest +# device). +# +# If the tray is open and there is no medium inserted, this will be a no-op. +# +# @device: block device name +# +# Since: 2.5 +## +{ 'command': 'blockdev-remove-medium', + 'data': { 'device': 'str' } } + ## # @BlockErrorAction diff --git a/qmp-commands.hx b/qmp-commands.hx index 0e896c8e45..282596265a 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -4035,6 +4035,51 @@ Example: <- { "return": {} } +EQMP + + { + .name = "blockdev-remove-medium", + .args_type = "device:s", + .mhandler.cmd_new = qmp_marshal_blockdev_remove_medium, + }, + +SQMP +blockdev-remove-medium +---------------------- + +Removes a medium (a block driver state tree) from a block device. That block +device's tray must currently be open (unless there is no attached guest device). + +If the tray is open and there is no medium inserted, this will be a no-op. + +Arguments: + +- "device": block device name (json-string) + +Example: + +-> { "execute": "blockdev-remove-medium", + "arguments": { "device": "ide1-cd0" } } + +<- { "error": { "class": "GenericError", + "desc": "Tray of device 'ide1-cd0' is not open" } } + +-> { "execute": "blockdev-open-tray", + "arguments": { "device": "ide1-cd0" } } + +<- { "timestamp": { "seconds": 1418751627, + "microseconds": 549958 }, + "event": "DEVICE_TRAY_MOVED", + "data": { "device": "ide1-cd0", + "tray-open": true } } + +<- { "return": {} } + +-> { "execute": "blockdev-remove-medium", + "arguments": { "device": "ide1-cd0" } } + +<- { "return": {} } + EQMP { From d129988289a885e57aa8790097e2d814764571bd Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 26 Oct 2015 21:39:11 +0100 Subject: [PATCH 08/41] blockdev: Add blockdev-insert-medium And a helper function for that, which directly takes a pointer to the BDS to be inserted instead of its node-name (which will be used for implementing 'change' using blockdev-insert-medium). Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- blockdev.c | 56 ++++++++++++++++++++++++++++++++++++++++++++ qapi/block-core.json | 17 ++++++++++++++ qmp-commands.hx | 37 +++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+) diff --git a/blockdev.c b/blockdev.c index 25635e3597..0f2a7e2988 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2198,6 +2198,62 @@ out: aio_context_release(aio_context); } +static void qmp_blockdev_insert_anon_medium(const char *device, + BlockDriverState *bs, Error **errp) +{ + BlockBackend *blk; + bool has_device; + + blk = blk_by_name(device); + if (!blk) { + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, + "Device '%s' not found", device); + return; + } + + /* For BBs without a device, we can exchange the BDS tree at will */ + has_device = blk_get_attached_dev(blk); + + if (has_device && !blk_dev_has_removable_media(blk)) { + error_setg(errp, "Device '%s' is not removable", device); + return; + } + + if (has_device && !blk_dev_is_tray_open(blk)) { + error_setg(errp, "Tray of device '%s' is not open", device); + return; + } + + if (blk_bs(blk)) { + error_setg(errp, "There already is a medium in device '%s'", device); + return; + } + + blk_insert_bs(blk, bs); + + QTAILQ_INSERT_TAIL(&bdrv_states, bs, device_list); +} + +void qmp_blockdev_insert_medium(const char *device, const char *node_name, + Error **errp) +{ + BlockDriverState *bs; + + bs = bdrv_find_node(node_name); + if (!bs) { + error_setg(errp, "Node '%s' not found", node_name); + return; + } + + if (bs->blk) { + error_setg(errp, "Node '%s' is already in use by '%s'", node_name, + blk_name(bs->blk)); + return; + } + + qmp_blockdev_insert_anon_medium(device, bs, errp); +} + /* throttling disk I/O limits */ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd, int64_t bps_wr, diff --git a/qapi/block-core.json b/qapi/block-core.json index e19e82cbaf..5c4fc727dc 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -1940,6 +1940,23 @@ { 'command': 'blockdev-remove-medium', 'data': { 'device': 'str' } } +## +# @blockdev-insert-medium: +# +# Inserts a medium (a block driver state tree) into a block device. That block +# device's tray must currently be open (unless there is no attached guest +# device) and there must be no medium inserted already. +# +# @device: block device name +# +# @node-name: name of a node in the block driver state graph +# +# Since: 2.5 +## +{ 'command': 'blockdev-insert-medium', + 'data': { 'device': 'str', + 'node-name': 'str'} } + ## # @BlockErrorAction diff --git a/qmp-commands.hx b/qmp-commands.hx index 282596265a..203ce6ac34 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -4080,6 +4080,43 @@ Example: <- { "return": {} } +EQMP + + { + .name = "blockdev-insert-medium", + .args_type = "device:s,node-name:s", + .mhandler.cmd_new = qmp_marshal_blockdev_insert_medium, + }, + +SQMP +blockdev-insert-medium +---------------------- + +Inserts a medium (a block driver state tree) into a block device. That block +device's tray must currently be open (unless there is no attached guest device) +and there must be no medium inserted already. + +Arguments: + +- "device": block device name (json-string) +- "node-name": root node of the BDS tree to insert into the block device + +Example: + +-> { "execute": "blockdev-add", + "arguments": { "options": { "node-name": "node0", + "driver": "raw", + "file": { "driver": "file", + "filename": "fedora.iso" } } } } + +<- { "return": {} } + +-> { "execute": "blockdev-insert-medium", + "arguments": { "device": "ide1-cd0", + "node-name": "node0" } } + +<- { "return": {} } + EQMP { From 38f54bd1ee0ed0f13b7326eb79403e320c3a28d3 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 26 Oct 2015 21:39:12 +0100 Subject: [PATCH 09/41] blockdev: Implement eject with basic operations Implement 'eject' by calling blockdev-open-tray and blockdev-remove-medium. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- blockdev.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/blockdev.c b/blockdev.c index 0f2a7e2988..1624297296 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1980,16 +1980,15 @@ out: void qmp_eject(const char *device, bool has_force, bool force, Error **errp) { - BlockBackend *blk; + Error *local_err = NULL; - blk = blk_by_name(device); - if (!blk) { - error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, - "Device '%s' not found", device); + qmp_blockdev_open_tray(device, has_force, force, &local_err); + if (local_err) { + error_propagate(errp, local_err); return; } - eject_device(blk, force, errp); + qmp_blockdev_remove_medium(device, errp); } void qmp_block_passwd(bool has_device, const char *device, From de2c6c0536c5c5ebb6e0ce7dcfd8fa9476edab52 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 26 Oct 2015 21:39:13 +0100 Subject: [PATCH 10/41] blockdev: Implement change with basic operations Implement 'change' on block devices by calling blockdev-open-tray, blockdev-remove-medium, blockdev-insert-medium (a variation of that which does not need a node-name) and blockdev-close-tray. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- blockdev.c | 178 ++++++++++++++++++++--------------------------------- 1 file changed, 68 insertions(+), 110 deletions(-) diff --git a/blockdev.c b/blockdev.c index 1624297296..53d4edfe20 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1940,44 +1940,6 @@ exit: } } - -static void eject_device(BlockBackend *blk, int force, Error **errp) -{ - BlockDriverState *bs = blk_bs(blk); - AioContext *aio_context; - - if (!bs) { - /* No medium inserted, so there is nothing to do */ - return; - } - - aio_context = bdrv_get_aio_context(bs); - aio_context_acquire(aio_context); - - if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_EJECT, errp)) { - goto out; - } - if (!blk_dev_has_removable_media(blk)) { - error_setg(errp, "Device '%s' is not removable", - bdrv_get_device_name(bs)); - goto out; - } - - if (blk_dev_is_medium_locked(blk) && !blk_dev_is_tray_open(blk)) { - blk_dev_eject_request(blk, force); - if (!force) { - error_setg(errp, "Device '%s' is locked", - bdrv_get_device_name(bs)); - goto out; - } - } - - bdrv_close(bs); - -out: - aio_context_release(aio_context); -} - void qmp_eject(const char *device, bool has_force, bool force, Error **errp) { Error *local_err = NULL; @@ -2015,78 +1977,6 @@ void qmp_block_passwd(bool has_device, const char *device, aio_context_release(aio_context); } -/* Assumes AioContext is held */ -static void qmp_bdrv_open_encrypted(BlockDriverState **pbs, - const char *filename, - int bdrv_flags, const char *format, - const char *password, Error **errp) -{ - Error *local_err = NULL; - QDict *options = NULL; - int ret; - - if (format) { - options = qdict_new(); - qdict_put(options, "driver", qstring_from_str(format)); - } - - ret = bdrv_open(pbs, filename, NULL, options, bdrv_flags, &local_err); - if (ret < 0) { - error_propagate(errp, local_err); - return; - } - - bdrv_add_key(*pbs, password, errp); -} - -void qmp_change_blockdev(const char *device, const char *filename, - const char *format, Error **errp) -{ - BlockBackend *blk; - BlockDriverState *bs; - AioContext *aio_context; - int bdrv_flags; - bool new_bs; - Error *err = NULL; - - blk = blk_by_name(device); - if (!blk) { - error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, - "Device '%s' not found", device); - return; - } - bs = blk_bs(blk); - new_bs = !bs; - - aio_context = blk_get_aio_context(blk); - aio_context_acquire(aio_context); - - eject_device(blk, 0, &err); - if (err) { - error_propagate(errp, err); - goto out; - } - - bdrv_flags = blk_is_read_only(blk) ? 0 : BDRV_O_RDWR; - bdrv_flags |= blk_get_root_state(blk)->open_flags & ~BDRV_O_RDWR; - - qmp_bdrv_open_encrypted(&bs, filename, bdrv_flags, format, NULL, &err); - if (err) { - error_propagate(errp, err); - goto out; - } - - if (new_bs) { - blk_insert_bs(blk, bs); - /* Has been sent automatically by bdrv_open() if blk_bs(blk) was not - * NULL */ - blk_dev_change_media_cb(blk, true); - } - -out: - aio_context_release(aio_context); -} - void qmp_blockdev_open_tray(const char *device, bool has_force, bool force, Error **errp) { @@ -2253,6 +2143,74 @@ void qmp_blockdev_insert_medium(const char *device, const char *node_name, qmp_blockdev_insert_anon_medium(device, bs, errp); } +void qmp_change_blockdev(const char *device, const char *filename, + const char *format, Error **errp) +{ + BlockBackend *blk; + BlockDriverState *medium_bs = NULL; + int bdrv_flags, ret; + QDict *options = NULL; + Error *err = NULL; + + blk = blk_by_name(device); + if (!blk) { + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, + "Device '%s' not found", device); + goto fail; + } + + if (blk_bs(blk)) { + blk_update_root_state(blk); + } + + bdrv_flags = blk_get_open_flags_from_root_state(blk); + + if (format) { + options = qdict_new(); + qdict_put(options, "driver", qstring_from_str(format)); + } + + assert(!medium_bs); + ret = bdrv_open(&medium_bs, filename, NULL, options, bdrv_flags, errp); + if (ret < 0) { + goto fail; + } + + blk_apply_root_state(blk, medium_bs); + + bdrv_add_key(medium_bs, NULL, &err); + if (err) { + error_propagate(errp, err); + goto fail; + } + + qmp_blockdev_open_tray(device, false, false, &err); + if (err) { + error_propagate(errp, err); + goto fail; + } + + qmp_blockdev_remove_medium(device, &err); + if (err) { + error_propagate(errp, err); + goto fail; + } + + qmp_blockdev_insert_anon_medium(device, medium_bs, &err); + if (err) { + error_propagate(errp, err); + goto fail; + } + + qmp_blockdev_close_tray(device, errp); + +fail: + /* If the medium has been inserted, the device has its own reference, so + * ours must be relinquished; and if it has not been inserted successfully, + * the reference must be relinquished anyway */ + bdrv_unref(medium_bs); +} + /* throttling disk I/O limits */ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd, int64_t bps_wr, From f1f57066573e832438cd87600310589fa9cee202 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 26 Oct 2015 21:39:14 +0100 Subject: [PATCH 11/41] block: Inquire tray state before tray-moved events blk_dev_change_media_cb() is called for all potential tray movements; however, it is possible to request closing the tray but nothing actually happening (on a floppy disk drive without a medium). Thus, the actual tray status should be inquired before sending a tray-moved event (and an event should be sent whenever the status changed). Checking @load is now superfluous; it was necessary because it was possible to change a medium without having explicitly opened the tray and closed it again (or it might have been possible, at least). This is no longer possible, though. Signed-off-by: Max Reitz Reviewed-by: Eric Blake Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- block/block-backend.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/block/block-backend.c b/block/block-backend.c index 7d495395de..1ac69824ca 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -429,18 +429,15 @@ void blk_set_dev_ops(BlockBackend *blk, const BlockDevOps *ops, void blk_dev_change_media_cb(BlockBackend *blk, bool load) { if (blk->dev_ops && blk->dev_ops->change_media_cb) { - bool tray_was_closed = !blk_dev_is_tray_open(blk); + bool tray_was_open, tray_is_open; + tray_was_open = blk_dev_is_tray_open(blk); blk->dev_ops->change_media_cb(blk->dev_opaque, load); - if (tray_was_closed) { - /* tray open */ - qapi_event_send_device_tray_moved(blk_name(blk), - true, &error_abort); - } - if (load) { - /* tray close */ - qapi_event_send_device_tray_moved(blk_name(blk), - false, &error_abort); + tray_is_open = blk_dev_is_tray_open(blk); + + if (tray_was_open != tray_is_open) { + qapi_event_send_device_tray_moved(blk_name(blk), tray_is_open, + &error_abort); } } } From 24fb4133001e1f54a526f0927837f30c1507169a Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 6 Nov 2015 16:27:06 +0100 Subject: [PATCH 12/41] qmp: Introduce blockdev-change-medium Introduce a new QMP command 'blockdev-change-medium' which is intended to replace the 'change' command for block devices. The existing function qmp_change_blockdev() is accordingly renamed to qmp_blockdev_change_medium(). Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- blockdev.c | 7 ++++--- include/sysemu/blockdev.h | 2 -- qapi-schema.json | 6 ++++-- qapi/block-core.json | 23 +++++++++++++++++++++++ qmp-commands.hx | 31 +++++++++++++++++++++++++++++++ qmp.c | 2 +- ui/cocoa.m | 10 ++++++---- 7 files changed, 69 insertions(+), 12 deletions(-) diff --git a/blockdev.c b/blockdev.c index 53d4edfe20..b3a958c4aa 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2143,8 +2143,9 @@ void qmp_blockdev_insert_medium(const char *device, const char *node_name, qmp_blockdev_insert_anon_medium(device, bs, errp); } -void qmp_change_blockdev(const char *device, const char *filename, - const char *format, Error **errp) +void qmp_blockdev_change_medium(const char *device, const char *filename, + bool has_format, const char *format, + Error **errp) { BlockBackend *blk; BlockDriverState *medium_bs = NULL; @@ -2165,7 +2166,7 @@ void qmp_change_blockdev(const char *device, const char *filename, bdrv_flags = blk_get_open_flags_from_root_state(blk); - if (format) { + if (has_format) { options = qdict_new(); qdict_put(options, "driver", qstring_from_str(format)); } diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h index a00be94895..b06a0607a9 100644 --- a/include/sysemu/blockdev.h +++ b/include/sysemu/blockdev.h @@ -63,8 +63,6 @@ DriveInfo *drive_new(QemuOpts *arg, BlockInterfaceType block_default_type); /* device-hotplug */ -void qmp_change_blockdev(const char *device, const char *filename, - const char *format, Error **errp); void hmp_commit(Monitor *mon, const QDict *qdict); void hmp_drive_del(Monitor *mon, const QDict *qdict); #endif diff --git a/qapi-schema.json b/qapi-schema.json index 8c3a42a1ac..691200e16a 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -1856,8 +1856,10 @@ # device's password. The behavior of reads and writes to the block # device between when these calls are executed is undefined. # -# Notes: It is strongly recommended that this interface is not used especially -# for changing block devices. +# Notes: This interface is deprecated, and it is strongly recommended that you +# avoid using it. For changing block devices, use +# blockdev-change-medium; for changing VNC parameters, use +# change-vnc-password. # # Since: 0.14.0 ## diff --git a/qapi/block-core.json b/qapi/block-core.json index 5c4fc727dc..e9fa6493be 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -1958,6 +1958,29 @@ 'node-name': 'str'} } +## +# @blockdev-change-medium: +# +# Changes the medium inserted into a block device by ejecting the current medium +# and loading a new image file which is inserted as the new medium (this command +# combines blockdev-open-tray, blockdev-remove-medium, blockdev-insert-medium +# and blockdev-close-tray). +# +# @device: block device name +# +# @filename: filename of the new image to be loaded +# +# @format: #optional, format to open the new image with (defaults to +# the probed format) +# +# Since: 2.5 +## +{ 'command': 'blockdev-change-medium', + 'data': { 'device': 'str', + 'filename': 'str', + '*format': 'str' } } + + ## # @BlockErrorAction # diff --git a/qmp-commands.hx b/qmp-commands.hx index 203ce6ac34..f6d9c256f2 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -4179,6 +4179,37 @@ Example: } } } ] } +EQMP + + { + .name = "blockdev-change-medium", + .args_type = "device:B,filename:F,format:s?", + .mhandler.cmd_new = qmp_marshal_blockdev_change_medium, + }, + +SQMP +blockdev-change-medium +---------------------- + +Changes the medium inserted into a block device by ejecting the current medium +and loading a new image file which is inserted as the new medium. + +Arguments: + +- "device": device name (json-string) +- "filename": filename of the new image (json-string) +- "format": format of the new image (json-string, optional) + +Examples: + +1. Change a removable medium + +-> { "execute": "blockdev-change-medium", + "arguments": { "device": "ide1-cd0", + "filename": "/srv/images/Fedora-12-x86_64-DVD.iso", + "format": "raw" } } +<- { "return": {} } + EQMP { diff --git a/qmp.c b/qmp.c index ff54e5a765..4e44f98e9d 100644 --- a/qmp.c +++ b/qmp.c @@ -414,7 +414,7 @@ void qmp_change(const char *device, const char *target, if (strcmp(device, "vnc") == 0) { qmp_change_vnc(target, has_arg, arg, errp); } else { - qmp_change_blockdev(device, target, arg, errp); + qmp_blockdev_change_medium(device, target, has_arg, arg, errp); } } diff --git a/ui/cocoa.m b/ui/cocoa.m index c0d6bb2f70..2d8e4e27c5 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -1113,10 +1113,12 @@ QemuCocoaView *cocoaView; } Error *err = NULL; - qmp_change_blockdev([drive cStringUsingEncoding: NSASCIIStringEncoding], - [file cStringUsingEncoding: NSASCIIStringEncoding], - "raw", - &err); + qmp_blockdev_change_medium([drive cStringUsingEncoding: + NSASCIIStringEncoding], + [file cStringUsingEncoding: + NSASCIIStringEncoding], + true, "raw", + &err); handleAnyDeviceErrors(err); } } From 1068674927a08cb9f535946abe2f91529b13160c Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 26 Oct 2015 21:39:16 +0100 Subject: [PATCH 13/41] hmp: Use blockdev-change-medium for change command Use separate code paths for the two overloaded functions of the 'change' HMP command, and invoke the 'blockdev-change-medium' QMP command if used on a block device (by calling qmp_blockdev_change_medium()). Signed-off-by: Max Reitz Reviewed-by: Eric Blake Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- hmp.c | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/hmp.c b/hmp.c index e1f854aefe..e5ad944811 100644 --- a/hmp.c +++ b/hmp.c @@ -1345,22 +1345,25 @@ void hmp_change(Monitor *mon, const QDict *qdict) const char *arg = qdict_get_try_str(qdict, "arg"); Error *err = NULL; - if (strcmp(device, "vnc") == 0 && - (strcmp(target, "passwd") == 0 || - strcmp(target, "password") == 0)) { - if (!arg) { - monitor_read_password(mon, hmp_change_read_arg, NULL); + if (strcmp(device, "vnc") == 0) { + if (strcmp(target, "passwd") == 0 || + strcmp(target, "password") == 0) { + if (!arg) { + monitor_read_password(mon, hmp_change_read_arg, NULL); + return; + } + } + qmp_change("vnc", target, !!arg, arg, &err); + } else { + qmp_blockdev_change_medium(device, target, !!arg, arg, &err); + if (err && + error_get_class(err) == ERROR_CLASS_DEVICE_ENCRYPTED) { + error_free(err); + monitor_read_block_device_key(mon, device, NULL, NULL); return; } } - qmp_change(device, target, !!arg, arg, &err); - if (err && - error_get_class(err) == ERROR_CLASS_DEVICE_ENCRYPTED) { - error_free(err); - monitor_read_block_device_key(mon, device, NULL, NULL); - return; - } hmp_handle_error(mon, &err); } From 39ff43d9e1f42b1d829a955e546cddab87ac0626 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 11 Nov 2015 04:49:44 +0100 Subject: [PATCH 14/41] blockdev: read-only-mode for blockdev-change-medium Add an option to qmp_blockdev_change_medium() which allows changing the read-only status of the block device whose medium is changed. Some drives do not have a inherently fixed read-only status; for instance, floppy disks can be set read-only or writable independently of the drive. Some users may find it useful to be able to therefore change the read-only status of a block device when changing the medium. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- blockdev.c | 22 ++++++++++++++++++++++ hmp.c | 2 +- qapi/block-core.json | 24 +++++++++++++++++++++++- qmp-commands.hx | 24 +++++++++++++++++++++++- qmp.c | 3 ++- ui/cocoa.m | 1 + 6 files changed, 72 insertions(+), 4 deletions(-) diff --git a/blockdev.c b/blockdev.c index b3a958c4aa..34f6e5bbb0 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2145,6 +2145,8 @@ void qmp_blockdev_insert_medium(const char *device, const char *node_name, void qmp_blockdev_change_medium(const char *device, const char *filename, bool has_format, const char *format, + bool has_read_only, + BlockdevChangeReadOnlyMode read_only, Error **errp) { BlockBackend *blk; @@ -2166,6 +2168,26 @@ void qmp_blockdev_change_medium(const char *device, const char *filename, bdrv_flags = blk_get_open_flags_from_root_state(blk); + if (!has_read_only) { + read_only = BLOCKDEV_CHANGE_READ_ONLY_MODE_RETAIN; + } + + switch (read_only) { + case BLOCKDEV_CHANGE_READ_ONLY_MODE_RETAIN: + break; + + case BLOCKDEV_CHANGE_READ_ONLY_MODE_READ_ONLY: + bdrv_flags &= ~BDRV_O_RDWR; + break; + + case BLOCKDEV_CHANGE_READ_ONLY_MODE_READ_WRITE: + bdrv_flags |= BDRV_O_RDWR; + break; + + default: + abort(); + } + if (has_format) { options = qdict_new(); qdict_put(options, "driver", qstring_from_str(format)); diff --git a/hmp.c b/hmp.c index e5ad944811..16006aa7bc 100644 --- a/hmp.c +++ b/hmp.c @@ -1355,7 +1355,7 @@ void hmp_change(Monitor *mon, const QDict *qdict) } qmp_change("vnc", target, !!arg, arg, &err); } else { - qmp_blockdev_change_medium(device, target, !!arg, arg, &err); + qmp_blockdev_change_medium(device, target, !!arg, arg, false, 0, &err); if (err && error_get_class(err) == ERROR_CLASS_DEVICE_ENCRYPTED) { error_free(err); diff --git a/qapi/block-core.json b/qapi/block-core.json index e9fa6493be..fa08ba946a 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -1958,6 +1958,24 @@ 'node-name': 'str'} } +## +# @BlockdevChangeReadOnlyMode: +# +# Specifies the new read-only mode of a block device subject to the +# @blockdev-change-medium command. +# +# @retain: Retains the current read-only mode +# +# @read-only: Makes the device read-only +# +# @read-write: Makes the device writable +# +# Since: 2.3 +## +{ 'enum': 'BlockdevChangeReadOnlyMode', + 'data': ['retain', 'read-only', 'read-write'] } + + ## # @blockdev-change-medium: # @@ -1973,12 +1991,16 @@ # @format: #optional, format to open the new image with (defaults to # the probed format) # +# @read-only-mode: #optional, change the read-only mode of the device; defaults +# to 'retain' +# # Since: 2.5 ## { 'command': 'blockdev-change-medium', 'data': { 'device': 'str', 'filename': 'str', - '*format': 'str' } } + '*format': 'str', + '*read-only-mode': 'BlockdevChangeReadOnlyMode' } } ## diff --git a/qmp-commands.hx b/qmp-commands.hx index f6d9c256f2..39d6e25121 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -4183,7 +4183,7 @@ EQMP { .name = "blockdev-change-medium", - .args_type = "device:B,filename:F,format:s?", + .args_type = "device:B,filename:F,format:s?,read-only-mode:s?", .mhandler.cmd_new = qmp_marshal_blockdev_change_medium, }, @@ -4199,6 +4199,8 @@ Arguments: - "device": device name (json-string) - "filename": filename of the new image (json-string) - "format": format of the new image (json-string, optional) +- "read-only-mode": new read-only mode (json-string, optional) + - Possible values: "retain" (default), "read-only", "read-write" Examples: @@ -4210,6 +4212,26 @@ Examples: "format": "raw" } } <- { "return": {} } +2. Load a read-only medium into a writable drive + +-> { "execute": "blockdev-change-medium", + "arguments": { "device": "isa-fd0", + "filename": "/srv/images/ro.img", + "format": "raw", + "read-only-mode": "retain" } } + +<- { "error": + { "class": "GenericError", + "desc": "Could not open '/srv/images/ro.img': Permission denied" } } + +-> { "execute": "blockdev-change-medium", + "arguments": { "device": "isa-fd0", + "filename": "/srv/images/ro.img", + "format": "raw", + "read-only-mode": "read-only" } } + +<- { "return": {} } + EQMP { diff --git a/qmp.c b/qmp.c index 4e44f98e9d..ddc63ea9f6 100644 --- a/qmp.c +++ b/qmp.c @@ -414,7 +414,8 @@ void qmp_change(const char *device, const char *target, if (strcmp(device, "vnc") == 0) { qmp_change_vnc(target, has_arg, arg, errp); } else { - qmp_blockdev_change_medium(device, target, has_arg, arg, errp); + qmp_blockdev_change_medium(device, target, has_arg, arg, false, 0, + errp); } } diff --git a/ui/cocoa.m b/ui/cocoa.m index 2d8e4e27c5..1554331554 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -1118,6 +1118,7 @@ QemuCocoaView *cocoaView; [file cStringUsingEncoding: NSASCIIStringEncoding], true, "raw", + false, 0, &err); handleAnyDeviceErrors(err); } From baead0abefa8e95b18e53281ac182c96b24ba0cb Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 26 Oct 2015 21:39:18 +0100 Subject: [PATCH 15/41] hmp: Add read-only-mode option to change command Expose the new read-only-mode option of 'blockdev-change-medium' for the 'change' HMP command. Signed-off-by: Max Reitz Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- hmp-commands.hx | 20 +++++++++++++++++--- hmp.c | 22 +++++++++++++++++++++- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/hmp-commands.hx b/hmp-commands.hx index 8939b9838a..f3de407647 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -194,8 +194,8 @@ ETEXI { .name = "change", - .args_type = "device:B,target:F,arg:s?", - .params = "device filename [format]", + .args_type = "device:B,target:F,arg:s?,read-only-mode:s?", + .params = "device filename [format [read-only-mode]]", .help = "change a removable medium, optional format", .mhandler.cmd = hmp_change, }, @@ -206,7 +206,7 @@ STEXI Change the configuration of a device. @table @option -@item change @var{diskdevice} @var{filename} [@var{format}] +@item change @var{diskdevice} @var{filename} [@var{format} [@var{read-only-mode}]] Change the medium for a removable disk device to point to @var{filename}. eg @example @@ -215,6 +215,20 @@ Change the medium for a removable disk device to point to @var{filename}. eg @var{format} is optional. +@var{read-only-mode} may be used to change the read-only status of the device. +It accepts the following values: + +@table @var +@item retain +Retains the current status; this is the default. + +@item read-only +Makes the device read-only. + +@item read-write +Makes the device writable. +@end table + @item change vnc @var{display},@var{options} Change the configuration of the VNC server. The valid syntax for @var{display} and @var{options} are described at @ref{sec_invocation}. eg diff --git a/hmp.c b/hmp.c index 16006aa7bc..d3812831ce 100644 --- a/hmp.c +++ b/hmp.c @@ -27,6 +27,7 @@ #include "qapi/opts-visitor.h" #include "qapi/qmp/qerror.h" #include "qapi/string-output-visitor.h" +#include "qapi/util.h" #include "qapi-visit.h" #include "ui/console.h" #include "block/qapi.h" @@ -1343,9 +1344,16 @@ void hmp_change(Monitor *mon, const QDict *qdict) const char *device = qdict_get_str(qdict, "device"); const char *target = qdict_get_str(qdict, "target"); const char *arg = qdict_get_try_str(qdict, "arg"); + const char *read_only = qdict_get_try_str(qdict, "read-only-mode"); + BlockdevChangeReadOnlyMode read_only_mode = 0; Error *err = NULL; if (strcmp(device, "vnc") == 0) { + if (read_only) { + monitor_printf(mon, + "Parameter 'read-only-mode' is invalid for VNC\n"); + return; + } if (strcmp(target, "passwd") == 0 || strcmp(target, "password") == 0) { if (!arg) { @@ -1355,7 +1363,19 @@ void hmp_change(Monitor *mon, const QDict *qdict) } qmp_change("vnc", target, !!arg, arg, &err); } else { - qmp_blockdev_change_medium(device, target, !!arg, arg, false, 0, &err); + if (read_only) { + read_only_mode = + qapi_enum_parse(BlockdevChangeReadOnlyMode_lookup, + read_only, BLOCKDEV_CHANGE_READ_ONLY_MODE_MAX, + BLOCKDEV_CHANGE_READ_ONLY_MODE_RETAIN, &err); + if (err) { + hmp_handle_error(mon, &err); + return; + } + } + + qmp_blockdev_change_medium(device, target, !!arg, arg, + !!read_only, read_only_mode, &err); if (err && error_get_class(err) == ERROR_CLASS_DEVICE_ENCRYPTED) { error_free(err); From adfe20303f2a897ce64b77fe579a0c7dc1e81771 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 26 Oct 2015 21:39:19 +0100 Subject: [PATCH 16/41] iotests: Add test for change-related QMP commands Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- tests/qemu-iotests/118 | 720 +++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/118.out | 5 + tests/qemu-iotests/group | 1 + 3 files changed, 726 insertions(+) create mode 100755 tests/qemu-iotests/118 create mode 100644 tests/qemu-iotests/118.out diff --git a/tests/qemu-iotests/118 b/tests/qemu-iotests/118 new file mode 100755 index 0000000000..a2bcd548c1 --- /dev/null +++ b/tests/qemu-iotests/118 @@ -0,0 +1,720 @@ +#!/usr/bin/env python +# +# Test case for the QMP 'change' command and all other associated +# commands +# +# Copyright (C) 2015 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +import os +import stat +import time +import iotests +from iotests import qemu_img + +old_img = os.path.join(iotests.test_dir, 'test0.img') +new_img = os.path.join(iotests.test_dir, 'test1.img') + +class ChangeBaseClass(iotests.QMPTestCase): + has_opened = False + has_closed = False + + def process_events(self): + for event in self.vm.get_qmp_events(wait=False): + if (event['event'] == 'DEVICE_TRAY_MOVED' and + event['data']['device'] == 'drive0'): + if event['data']['tray-open'] == False: + self.has_closed = True + else: + self.has_opened = True + + def wait_for_open(self): + timeout = time.clock() + 3 + while not self.has_opened and time.clock() < timeout: + self.process_events() + if not self.has_opened: + self.fail('Timeout while waiting for the tray to open') + + def wait_for_close(self): + timeout = time.clock() + 3 + while not self.has_closed and time.clock() < timeout: + self.process_events() + if not self.has_opened: + self.fail('Timeout while waiting for the tray to close') + +class GeneralChangeTestsBaseClass(ChangeBaseClass): + def test_change(self): + result = self.vm.qmp('change', device='drive0', target=new_img, + arg=iotests.imgfmt) + self.assert_qmp(result, 'return', {}) + + self.wait_for_open() + self.wait_for_close() + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', False) + self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img) + + def test_blockdev_change_medium(self): + result = self.vm.qmp('blockdev-change-medium', device='drive0', + filename=new_img, + format=iotests.imgfmt) + self.assert_qmp(result, 'return', {}) + + self.wait_for_open() + self.wait_for_close() + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', False) + self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img) + + def test_eject(self): + result = self.vm.qmp('eject', device='drive0', force=True) + self.assert_qmp(result, 'return', {}) + + self.wait_for_open() + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', True) + self.assert_qmp_absent(result, 'return[0]/inserted') + + def test_tray_eject_change(self): + result = self.vm.qmp('eject', device='drive0', force=True) + self.assert_qmp(result, 'return', {}) + + self.wait_for_open() + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', True) + self.assert_qmp_absent(result, 'return[0]/inserted') + + result = self.vm.qmp('blockdev-change-medium', device='drive0', + filename=new_img, + format=iotests.imgfmt) + self.assert_qmp(result, 'return', {}) + + self.wait_for_close() + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', False) + self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img) + + def test_tray_open_close(self): + result = self.vm.qmp('blockdev-open-tray', device='drive0', force=True) + self.assert_qmp(result, 'return', {}) + + self.wait_for_open() + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', True) + if self.was_empty == True: + self.assert_qmp_absent(result, 'return[0]/inserted') + else: + self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) + + result = self.vm.qmp('blockdev-close-tray', device='drive0') + self.assert_qmp(result, 'return', {}) + + if self.has_real_tray or not self.was_empty: + self.wait_for_close() + + result = self.vm.qmp('query-block') + if self.has_real_tray or not self.was_empty: + self.assert_qmp(result, 'return[0]/tray_open', False) + else: + self.assert_qmp(result, 'return[0]/tray_open', True) + if self.was_empty == True: + self.assert_qmp_absent(result, 'return[0]/inserted') + else: + self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) + + def test_tray_eject_close(self): + result = self.vm.qmp('eject', device='drive0', force=True) + self.assert_qmp(result, 'return', {}) + + self.wait_for_open() + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', True) + self.assert_qmp_absent(result, 'return[0]/inserted') + + result = self.vm.qmp('blockdev-close-tray', device='drive0') + self.assert_qmp(result, 'return', {}) + + if self.has_real_tray: + self.wait_for_close() + + result = self.vm.qmp('query-block') + if self.has_real_tray: + self.assert_qmp(result, 'return[0]/tray_open', False) + else: + self.assert_qmp(result, 'return[0]/tray_open', True) + self.assert_qmp_absent(result, 'return[0]/inserted') + + def test_tray_open_change(self): + result = self.vm.qmp('blockdev-open-tray', device='drive0', force=True) + self.assert_qmp(result, 'return', {}) + + self.wait_for_open() + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', True) + if self.was_empty == True: + self.assert_qmp_absent(result, 'return[0]/inserted') + else: + self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) + + result = self.vm.qmp('blockdev-change-medium', device='drive0', + filename=new_img, + format=iotests.imgfmt) + self.assert_qmp(result, 'return', {}) + + self.wait_for_close() + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', False) + self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img) + + def test_cycle(self): + result = self.vm.qmp('blockdev-add', + options={'node-name': 'new', + 'driver': iotests.imgfmt, + 'file': {'filename': new_img, + 'driver': 'file'}}) + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('blockdev-open-tray', device='drive0', force=True) + self.assert_qmp(result, 'return', {}) + + self.wait_for_open() + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', True) + if self.was_empty == True: + self.assert_qmp_absent(result, 'return[0]/inserted') + else: + self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) + + result = self.vm.qmp('blockdev-remove-medium', device='drive0') + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', True) + self.assert_qmp_absent(result, 'return[0]/inserted') + + result = self.vm.qmp('blockdev-insert-medium', device='drive0', + node_name='new') + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', True) + self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img) + + result = self.vm.qmp('blockdev-close-tray', device='drive0') + self.assert_qmp(result, 'return', {}) + + self.wait_for_close() + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', False) + self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img) + + def test_close_on_closed(self): + result = self.vm.qmp('blockdev-close-tray', device='drive0') + # Should be a no-op + self.assert_qmp(result, 'return', {}) + self.assertEquals(self.vm.get_qmp_events(wait=False), []) + + def test_remove_on_closed(self): + if self.has_opened: + # Empty floppy drive + return + + result = self.vm.qmp('blockdev-remove-medium', device='drive0') + self.assert_qmp(result, 'error/class', 'GenericError') + + def test_insert_on_closed(self): + if self.has_opened: + # Empty floppy drive + return + + result = self.vm.qmp('blockdev-add', + options={'node-name': 'new', + 'driver': iotests.imgfmt, + 'file': {'filename': new_img, + 'driver': 'file'}}) + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('blockdev-insert-medium', device='drive0', + node_name='new') + self.assert_qmp(result, 'error/class', 'GenericError') + +class TestInitiallyFilled(GeneralChangeTestsBaseClass): + was_empty = False + + def setUp(self, media, interface): + qemu_img('create', '-f', iotests.imgfmt, old_img, '1440k') + qemu_img('create', '-f', iotests.imgfmt, new_img, '1440k') + self.vm = iotests.VM().add_drive(old_img, 'media=%s' % media, interface) + self.vm.launch() + + def tearDown(self): + self.vm.shutdown() + os.remove(old_img) + os.remove(new_img) + + def test_insert_on_filled(self): + result = self.vm.qmp('blockdev-add', + options={'node-name': 'new', + 'driver': iotests.imgfmt, + 'file': {'filename': new_img, + 'driver': 'file'}}) + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('blockdev-open-tray', device='drive0') + self.assert_qmp(result, 'return', {}) + + self.wait_for_open() + + result = self.vm.qmp('blockdev-insert-medium', device='drive0', + node_name='new') + self.assert_qmp(result, 'error/class', 'GenericError') + +class TestInitiallyEmpty(GeneralChangeTestsBaseClass): + was_empty = True + + def setUp(self, media, interface): + qemu_img('create', '-f', iotests.imgfmt, new_img, '1440k') + self.vm = iotests.VM().add_drive(None, 'media=%s' % media, interface) + self.vm.launch() + + def tearDown(self): + self.vm.shutdown() + os.remove(new_img) + + def test_remove_on_empty(self): + result = self.vm.qmp('blockdev-open-tray', device='drive0') + self.assert_qmp(result, 'return', {}) + + self.wait_for_open() + + result = self.vm.qmp('blockdev-remove-medium', device='drive0') + # Should be a no-op + self.assert_qmp(result, 'return', {}) + +class TestCDInitiallyFilled(TestInitiallyFilled): + TestInitiallyFilled = TestInitiallyFilled + has_real_tray = True + + def setUp(self): + self.TestInitiallyFilled.setUp(self, 'cdrom', 'ide') + +class TestCDInitiallyEmpty(TestInitiallyEmpty): + TestInitiallyEmpty = TestInitiallyEmpty + has_real_tray = True + + def setUp(self): + self.TestInitiallyEmpty.setUp(self, 'cdrom', 'ide') + +class TestFloppyInitiallyFilled(TestInitiallyFilled): + TestInitiallyFilled = TestInitiallyFilled + has_real_tray = False + + def setUp(self): + self.TestInitiallyFilled.setUp(self, 'disk', 'floppy') + +class TestFloppyInitiallyEmpty(TestInitiallyEmpty): + TestInitiallyEmpty = TestInitiallyEmpty + has_real_tray = False + + def setUp(self): + self.TestInitiallyEmpty.setUp(self, 'disk', 'floppy') + # FDDs not having a real tray and there not being a medium inside the + # tray at startup means the tray will be considered open + self.has_opened = True + +class TestChangeReadOnly(ChangeBaseClass): + def setUp(self): + qemu_img('create', '-f', iotests.imgfmt, old_img, '1440k') + qemu_img('create', '-f', iotests.imgfmt, new_img, '1440k') + self.vm = iotests.VM() + + def tearDown(self): + self.vm.shutdown() + os.chmod(old_img, 0666) + os.chmod(new_img, 0666) + os.remove(old_img) + os.remove(new_img) + + def test_ro_ro_retain(self): + os.chmod(old_img, 0444) + os.chmod(new_img, 0444) + self.vm.add_drive(old_img, 'media=disk,read-only=on', 'floppy') + self.vm.launch() + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', False) + self.assert_qmp(result, 'return[0]/inserted/ro', True) + self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) + + result = self.vm.qmp('blockdev-change-medium', device='drive0', + filename=new_img, + format=iotests.imgfmt, + read_only_mode='retain') + self.assert_qmp(result, 'return', {}) + + self.wait_for_open() + self.wait_for_close() + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', False) + self.assert_qmp(result, 'return[0]/inserted/ro', True) + self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img) + + def test_ro_rw_retain(self): + os.chmod(old_img, 0444) + self.vm.add_drive(old_img, 'media=disk,read-only=on', 'floppy') + self.vm.launch() + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', False) + self.assert_qmp(result, 'return[0]/inserted/ro', True) + self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) + + result = self.vm.qmp('blockdev-change-medium', device='drive0', + filename=new_img, + format=iotests.imgfmt, + read_only_mode='retain') + self.assert_qmp(result, 'return', {}) + + self.wait_for_open() + self.wait_for_close() + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', False) + self.assert_qmp(result, 'return[0]/inserted/ro', True) + self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img) + + def test_rw_ro_retain(self): + os.chmod(new_img, 0444) + self.vm.add_drive(old_img, 'media=disk', 'floppy') + self.vm.launch() + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', False) + self.assert_qmp(result, 'return[0]/inserted/ro', False) + self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) + + result = self.vm.qmp('blockdev-change-medium', device='drive0', + filename=new_img, + format=iotests.imgfmt, + read_only_mode='retain') + self.assert_qmp(result, 'error/class', 'GenericError') + + self.assertEquals(self.vm.get_qmp_events(wait=False), []) + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', False) + self.assert_qmp(result, 'return[0]/inserted/ro', False) + self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) + + def test_ro_rw(self): + os.chmod(old_img, 0444) + self.vm.add_drive(old_img, 'media=disk,read-only=on', 'floppy') + self.vm.launch() + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', False) + self.assert_qmp(result, 'return[0]/inserted/ro', True) + self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) + + result = self.vm.qmp('blockdev-change-medium', + device='drive0', + filename=new_img, + format=iotests.imgfmt, + read_only_mode='read-write') + self.assert_qmp(result, 'return', {}) + + self.wait_for_open() + self.wait_for_close() + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', False) + self.assert_qmp(result, 'return[0]/inserted/ro', False) + self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img) + + def test_rw_ro(self): + os.chmod(new_img, 0444) + self.vm.add_drive(old_img, 'media=disk', 'floppy') + self.vm.launch() + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', False) + self.assert_qmp(result, 'return[0]/inserted/ro', False) + self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) + + result = self.vm.qmp('blockdev-change-medium', + device='drive0', + filename=new_img, + format=iotests.imgfmt, + read_only_mode='read-only') + self.assert_qmp(result, 'return', {}) + + self.wait_for_open() + self.wait_for_close() + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', False) + self.assert_qmp(result, 'return[0]/inserted/ro', True) + self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img) + + def test_make_rw_ro(self): + self.vm.add_drive(old_img, 'media=disk', 'floppy') + self.vm.launch() + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', False) + self.assert_qmp(result, 'return[0]/inserted/ro', False) + self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) + + result = self.vm.qmp('blockdev-change-medium', + device='drive0', + filename=new_img, + format=iotests.imgfmt, + read_only_mode='read-only') + self.assert_qmp(result, 'return', {}) + + self.wait_for_open() + self.wait_for_close() + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', False) + self.assert_qmp(result, 'return[0]/inserted/ro', True) + self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img) + + def test_make_ro_rw(self): + os.chmod(new_img, 0444) + self.vm.add_drive(old_img, 'media=disk', 'floppy') + self.vm.launch() + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', False) + self.assert_qmp(result, 'return[0]/inserted/ro', False) + self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) + + result = self.vm.qmp('blockdev-change-medium', + device='drive0', + filename=new_img, + format=iotests.imgfmt, + read_only_mode='read-write') + self.assert_qmp(result, 'error/class', 'GenericError') + + self.assertEquals(self.vm.get_qmp_events(wait=False), []) + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', False) + self.assert_qmp(result, 'return[0]/inserted/ro', False) + self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) + + def test_make_rw_ro_by_retain(self): + os.chmod(old_img, 0444) + self.vm.add_drive(old_img, 'media=disk,read-only=on', 'floppy') + self.vm.launch() + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', False) + self.assert_qmp(result, 'return[0]/inserted/ro', True) + self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) + + result = self.vm.qmp('blockdev-change-medium', device='drive0', + filename=new_img, + format=iotests.imgfmt, + read_only_mode='retain') + self.assert_qmp(result, 'return', {}) + + self.wait_for_open() + self.wait_for_close() + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', False) + self.assert_qmp(result, 'return[0]/inserted/ro', True) + self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img) + + def test_make_ro_rw_by_retain(self): + os.chmod(new_img, 0444) + self.vm.add_drive(old_img, 'media=disk', 'floppy') + self.vm.launch() + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', False) + self.assert_qmp(result, 'return[0]/inserted/ro', False) + self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) + + result = self.vm.qmp('blockdev-change-medium', device='drive0', + filename=new_img, + format=iotests.imgfmt, + read_only_mode='retain') + self.assert_qmp(result, 'error/class', 'GenericError') + + self.assertEquals(self.vm.get_qmp_events(wait=False), []) + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', False) + self.assert_qmp(result, 'return[0]/inserted/ro', False) + self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) + + def test_rw_ro_cycle(self): + os.chmod(new_img, 0444) + self.vm.add_drive(old_img, 'media=disk', 'floppy') + self.vm.launch() + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', False) + self.assert_qmp(result, 'return[0]/inserted/ro', False) + self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) + + result = self.vm.qmp('blockdev-add', + options={'node-name': 'new', + 'driver': iotests.imgfmt, + 'read-only': True, + 'file': {'filename': new_img, + 'driver': 'file'}}) + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('blockdev-open-tray', device='drive0', force=True) + self.assert_qmp(result, 'return', {}) + + self.wait_for_open() + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', True) + self.assert_qmp(result, 'return[0]/inserted/ro', False) + self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) + + result = self.vm.qmp('blockdev-remove-medium', device='drive0') + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', True) + self.assert_qmp_absent(result, 'return[0]/inserted') + + result = self.vm.qmp('blockdev-insert-medium', device='drive0', + node_name='new') + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', True) + self.assert_qmp(result, 'return[0]/inserted/ro', True) + self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img) + + result = self.vm.qmp('blockdev-close-tray', device='drive0') + self.assert_qmp(result, 'return', {}) + + self.wait_for_close() + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', False) + self.assert_qmp(result, 'return[0]/inserted/ro', True) + self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img) + +GeneralChangeTestsBaseClass = None +TestInitiallyFilled = None +TestInitiallyEmpty = None + + +class TestBlockJobsAfterCycle(ChangeBaseClass): + def setUp(self): + qemu_img('create', '-f', iotests.imgfmt, old_img, '1M') + + self.vm = iotests.VM() + self.vm.launch() + + result = self.vm.qmp('blockdev-add', + options={'id': 'drive0', + 'driver': 'null-co'}) + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', False) + self.assert_qmp(result, 'return[0]/inserted/image/format', 'null-co') + + # For device-less BBs, calling blockdev-open-tray or blockdev-close-tray + # is not necessary + result = self.vm.qmp('blockdev-remove-medium', device='drive0') + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('query-block') + self.assert_qmp_absent(result, 'return[0]/inserted') + + result = self.vm.qmp('blockdev-add', + options={'node-name': 'node0', + 'driver': iotests.imgfmt, + 'file': {'filename': old_img, + 'driver': 'file'}}) + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('blockdev-insert-medium', device='drive0', + node_name='node0') + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/tray_open', False) + self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) + + def tearDown(self): + self.vm.shutdown() + os.remove(old_img) + try: + os.remove(new_img) + except OSError: + pass + + def test_snapshot_and_commit(self): + # We need backing file support + if iotests.imgfmt != 'qcow2' and iotests.imgfmt != 'qed': + return + + result = self.vm.qmp('blockdev-snapshot-sync', device='drive0', + snapshot_file=new_img, + format=iotests.imgfmt) + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('query-block') + self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img) + self.assert_qmp(result, + 'return[0]/inserted/image/backing-image/filename', + old_img) + + result = self.vm.qmp('block-commit', device='drive0') + self.assert_qmp(result, 'return', {}) + + self.vm.event_wait(name='BLOCK_JOB_READY') + + result = self.vm.qmp('query-block-jobs') + self.assert_qmp(result, 'return[0]/device', 'drive0') + + result = self.vm.qmp('block-job-complete', device='drive0') + self.assert_qmp(result, 'return', {}) + + self.vm.event_wait(name='BLOCK_JOB_COMPLETED') + + +if __name__ == '__main__': + if iotests.qemu_default_machine != 'pc': + # We need floppy and IDE CD-ROM + iotests.notrun('not suitable for this machine type: %s' % + iotests.qemu_default_machine) + iotests.main() diff --git a/tests/qemu-iotests/118.out b/tests/qemu-iotests/118.out new file mode 100644 index 0000000000..6a917130b6 --- /dev/null +++ b/tests/qemu-iotests/118.out @@ -0,0 +1,5 @@ +........................................................... +---------------------------------------------------------------------- +Ran 59 tests + +OK diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index 30c784e940..a6a61aeddd 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -122,6 +122,7 @@ 114 rw auto quick 115 rw auto 116 rw auto quick +118 rw auto 119 rw auto quick 120 rw auto quick 121 rw auto From a39a24fbb05055d00026eb569ff3f7b868ca8785 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Mon, 26 Oct 2015 14:27:13 +0200 Subject: [PATCH 17/41] block: check for existing device IDs in external_snapshot_prepare() The 'snapshot-node-name' parameter of blockdev-snapshot-sync allows setting the node name of the image that is going to be created. Before creating the image, external_snapshot_prepare() checks that the name is not already being used. The check is however incomplete since it only considers existing node names, but node names must not clash with device IDs either because they share the same namespace. If the user attempts to create a snapshot using the name of an existing device for the 'snapshot-node-name' parameter the operation will eventually fail, but only after the new image has been created. This patch replaces bdrv_find_node() with bdrv_lookup_bs() to extend the check to existing device IDs, and thus detect possible name clashes before the new image is created. Signed-off-by: Alberto Garcia Reviewed-by: Max Reitz Reviewed-by: Jeff Cody Signed-off-by: Kevin Wolf --- blockdev.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/blockdev.c b/blockdev.c index 34f6e5bbb0..0c6c2a7845 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1579,8 +1579,9 @@ static void external_snapshot_prepare(BlkTransactionState *common, return; } - if (has_snapshot_node_name && bdrv_find_node(snapshot_node_name)) { - error_setg(errp, "New snapshot node name already existing"); + if (has_snapshot_node_name && + bdrv_lookup_bs(snapshot_node_name, snapshot_node_name, NULL)) { + error_setg(errp, "New snapshot node name already in use"); return; } From a911e6ae7ce47d51b519d462c1d99d53b37b0f8c Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Mon, 26 Oct 2015 14:27:14 +0200 Subject: [PATCH 18/41] block: rename BlockdevSnapshot to BlockdevSnapshotSync We will introduce the 'blockdev-snapshot' command that will require its own struct for the parameters, so we need to rename this one in order to avoid name clashes. Signed-off-by: Alberto Garcia Reviewed-by: Eric Blake Reviewed-by: Max Reitz Reviewed-by: Kevin Wolf Reviewed-by: Jeff Cody Signed-off-by: Kevin Wolf --- blockdev.c | 2 +- qapi-schema.json | 2 +- qapi/block-core.json | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/blockdev.c b/blockdev.c index 0c6c2a7845..86a29d8fa3 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1168,7 +1168,7 @@ void qmp_blockdev_snapshot_sync(bool has_device, const char *device, bool has_format, const char *format, bool has_mode, NewImageMode mode, Error **errp) { - BlockdevSnapshot snapshot = { + BlockdevSnapshotSync snapshot = { .has_device = has_device, .device = (char *) device, .has_node_name = has_node_name, diff --git a/qapi-schema.json b/qapi-schema.json index 691200e16a..04a01b1247 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -1548,7 +1548,7 @@ ## { 'union': 'TransactionAction', 'data': { - 'blockdev-snapshot-sync': 'BlockdevSnapshot', + 'blockdev-snapshot-sync': 'BlockdevSnapshotSync', 'drive-backup': 'DriveBackup', 'blockdev-backup': 'BlockdevBackup', 'abort': 'Abort', diff --git a/qapi/block-core.json b/qapi/block-core.json index fa08ba946a..f592adc8df 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -682,7 +682,7 @@ 'data': [ 'existing', 'absolute-paths' ] } ## -# @BlockdevSnapshot +# @BlockdevSnapshotSync # # Either @device or @node-name must be set but not both. # @@ -699,7 +699,7 @@ # @mode: #optional whether and how QEMU should create a new image, default is # 'absolute-paths'. ## -{ 'struct': 'BlockdevSnapshot', +{ 'struct': 'BlockdevSnapshotSync', 'data': { '*device': 'str', '*node-name': 'str', 'snapshot-file': 'str', '*snapshot-node-name': 'str', '*format': 'str', '*mode': 'NewImageMode' } } @@ -790,7 +790,7 @@ # # Generates a synchronous snapshot of a block device. # -# For the arguments, see the documentation of BlockdevSnapshot. +# For the arguments, see the documentation of BlockdevSnapshotSync. # # Returns: nothing on success # If @device is not a valid block device, DeviceNotFound @@ -798,7 +798,7 @@ # Since 0.14.0 ## { 'command': 'blockdev-snapshot-sync', - 'data': 'BlockdevSnapshot' } + 'data': 'BlockdevSnapshotSync' } ## # @change-backing-file From 3e8c2e57056614933fc5eb022e1d6d0f28071b44 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Mon, 26 Oct 2015 14:27:15 +0200 Subject: [PATCH 19/41] block: support passing 'backing': '' to 'blockdev-add' Passing an empty string allows opening an image but not its backing file. This was already described in the API documentation, only the implementation was missing. This is useful for creating snapshots using images opened with blockdev-add, since they are not supposed to have a backing image before the operation. Signed-off-by: Alberto Garcia Reviewed-by: Max Reitz Reviewed-by: Eric Blake Reviewed-by: Kevin Wolf Reviewed-by: Jeff Cody Signed-off-by: Kevin Wolf --- block.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/block.c b/block.c index a99e6d8509..34935015dd 100644 --- a/block.c +++ b/block.c @@ -1393,6 +1393,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename, BlockDriverState *bs; BlockDriver *drv = NULL; const char *drvname; + const char *backing; Error *local_err = NULL; int snapshot_flags = 0; @@ -1460,6 +1461,12 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename, assert(drvname || !(flags & BDRV_O_PROTOCOL)); + backing = qdict_get_try_str(options, "backing"); + if (backing && *backing == '\0') { + flags |= BDRV_O_NO_BACKING; + qdict_del(options, "backing"); + } + bs->open_flags = flags; bs->options = options; options = qdict_clone_shallow(options); From 43de7e2de07093e47c7f25386aff280875dc3c62 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Mon, 26 Oct 2015 14:27:16 +0200 Subject: [PATCH 20/41] block: add a 'blockdev-snapshot' QMP command One of the limitations of the 'blockdev-snapshot-sync' command is that it does not allow passing BlockdevOptions to the newly created snapshots, so they are always opened using the default values. Extending the command to allow passing options is not a practical solution because there is overlap between those options and some of the existing parameters of the command. This patch introduces a new 'blockdev-snapshot' command with a simpler interface: it just takes two references to existing block devices that will be used as the source and target for the snapshot. Since the main difference between the two commands is that one of them creates and opens the target image, while the other uses an already opened one, the bulk of the implementation is shared. Signed-off-by: Alberto Garcia Reviewed-by: Max Reitz Signed-off-by: Kevin Wolf --- blockdev.c | 167 +++++++++++++++++++++++++++---------------- qapi-schema.json | 2 + qapi/block-core.json | 28 ++++++++ qmp-commands.hx | 38 ++++++++++ 4 files changed, 173 insertions(+), 62 deletions(-) diff --git a/blockdev.c b/blockdev.c index 86a29d8fa3..7645d491cb 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1185,6 +1185,18 @@ void qmp_blockdev_snapshot_sync(bool has_device, const char *device, &snapshot, errp); } +void qmp_blockdev_snapshot(const char *node, const char *overlay, + Error **errp) +{ + BlockdevSnapshot snapshot_data = { + .node = (char *) node, + .overlay = (char *) overlay + }; + + blockdev_do_action(TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT, + &snapshot_data, errp); +} + void qmp_blockdev_snapshot_internal_sync(const char *device, const char *name, Error **errp) @@ -1530,58 +1542,48 @@ typedef struct ExternalSnapshotState { static void external_snapshot_prepare(BlkTransactionState *common, Error **errp) { - int flags, ret; - QDict *options; + int flags = 0, ret; + QDict *options = NULL; Error *local_err = NULL; - bool has_device = false; + /* Device and node name of the image to generate the snapshot from */ const char *device; - bool has_node_name = false; const char *node_name; - bool has_snapshot_node_name = false; - const char *snapshot_node_name; + /* Reference to the new image (for 'blockdev-snapshot') */ + const char *snapshot_ref; + /* File name of the new image (for 'blockdev-snapshot-sync') */ const char *new_image_file; - const char *format = "qcow2"; - enum NewImageMode mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS; ExternalSnapshotState *state = DO_UPCAST(ExternalSnapshotState, common, common); TransactionAction *action = common->action; - /* get parameters */ - g_assert(action->type == TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC); - - has_device = action->u.blockdev_snapshot_sync->has_device; - device = action->u.blockdev_snapshot_sync->device; - has_node_name = action->u.blockdev_snapshot_sync->has_node_name; - node_name = action->u.blockdev_snapshot_sync->node_name; - has_snapshot_node_name = - action->u.blockdev_snapshot_sync->has_snapshot_node_name; - snapshot_node_name = action->u.blockdev_snapshot_sync->snapshot_node_name; - - new_image_file = action->u.blockdev_snapshot_sync->snapshot_file; - if (action->u.blockdev_snapshot_sync->has_format) { - format = action->u.blockdev_snapshot_sync->format; - } - if (action->u.blockdev_snapshot_sync->has_mode) { - mode = action->u.blockdev_snapshot_sync->mode; + /* 'blockdev-snapshot' and 'blockdev-snapshot-sync' have similar + * purpose but a different set of parameters */ + switch (action->type) { + case TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT: + { + BlockdevSnapshot *s = action->u.blockdev_snapshot; + device = s->node; + node_name = s->node; + new_image_file = NULL; + snapshot_ref = s->overlay; + } + break; + case TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC: + { + BlockdevSnapshotSync *s = action->u.blockdev_snapshot_sync; + device = s->has_device ? s->device : NULL; + node_name = s->has_node_name ? s->node_name : NULL; + new_image_file = s->snapshot_file; + snapshot_ref = NULL; + } + break; + default: + g_assert_not_reached(); } /* start processing */ - state->old_bs = bdrv_lookup_bs(has_device ? device : NULL, - has_node_name ? node_name : NULL, - &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; - } - - if (has_node_name && !has_snapshot_node_name) { - error_setg(errp, "New snapshot node name missing"); - return; - } - - if (has_snapshot_node_name && - bdrv_lookup_bs(snapshot_node_name, snapshot_node_name, NULL)) { - error_setg(errp, "New snapshot node name already in use"); + state->old_bs = bdrv_lookup_bs(device, node_name, errp); + if (!state->old_bs) { return; } @@ -1612,35 +1614,70 @@ static void external_snapshot_prepare(BlkTransactionState *common, return; } - flags = state->old_bs->open_flags; + if (action->type == TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC) { + BlockdevSnapshotSync *s = action->u.blockdev_snapshot_sync; + const char *format = s->has_format ? s->format : "qcow2"; + enum NewImageMode mode; + const char *snapshot_node_name = + s->has_snapshot_node_name ? s->snapshot_node_name : NULL; - /* create new image w/backing file */ - if (mode != NEW_IMAGE_MODE_EXISTING) { - bdrv_img_create(new_image_file, format, - state->old_bs->filename, - state->old_bs->drv->format_name, - NULL, -1, flags, &local_err, false); - if (local_err) { - error_propagate(errp, local_err); + if (node_name && !snapshot_node_name) { + error_setg(errp, "New snapshot node name missing"); return; } + + if (snapshot_node_name && + bdrv_lookup_bs(snapshot_node_name, snapshot_node_name, NULL)) { + error_setg(errp, "New snapshot node name already in use"); + return; + } + + flags = state->old_bs->open_flags; + + /* create new image w/backing file */ + mode = s->has_mode ? s->mode : NEW_IMAGE_MODE_ABSOLUTE_PATHS; + if (mode != NEW_IMAGE_MODE_EXISTING) { + bdrv_img_create(new_image_file, format, + state->old_bs->filename, + state->old_bs->drv->format_name, + NULL, -1, flags, &local_err, false); + if (local_err) { + error_propagate(errp, local_err); + return; + } + } + + options = qdict_new(); + if (s->has_snapshot_node_name) { + qdict_put(options, "node-name", + qstring_from_str(snapshot_node_name)); + } + qdict_put(options, "driver", qstring_from_str(format)); + + flags |= BDRV_O_NO_BACKING; } - options = qdict_new(); - if (has_snapshot_node_name) { - qdict_put(options, "node-name", - qstring_from_str(snapshot_node_name)); - } - qdict_put(options, "driver", qstring_from_str(format)); - - /* TODO Inherit bs->options or only take explicit options with an - * extended QMP command? */ assert(state->new_bs == NULL); - ret = bdrv_open(&state->new_bs, new_image_file, NULL, options, - flags | BDRV_O_NO_BACKING, &local_err); + ret = bdrv_open(&state->new_bs, new_image_file, snapshot_ref, options, + flags, errp); /* We will manually add the backing_hd field to the bs later */ if (ret != 0) { - error_propagate(errp, local_err); + return; + } + + if (state->new_bs->blk != NULL) { + error_setg(errp, "The snapshot is already in use by %s", + blk_name(state->new_bs->blk)); + return; + } + + if (bdrv_op_is_blocked(state->new_bs, BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT, + errp)) { + return; + } + + if (state->new_bs->backing != NULL) { + error_setg(errp, "The snapshot already has a backing image"); } } @@ -1843,6 +1880,12 @@ static void abort_commit(BlkTransactionState *common) } static const BdrvActionOps actions[] = { + [TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT] = { + .instance_size = sizeof(ExternalSnapshotState), + .prepare = external_snapshot_prepare, + .commit = external_snapshot_commit, + .abort = external_snapshot_abort, + }, [TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC] = { .instance_size = sizeof(ExternalSnapshotState), .prepare = external_snapshot_prepare, diff --git a/qapi-schema.json b/qapi-schema.json index 04a01b1247..c3f95ab170 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -1545,9 +1545,11 @@ # abort since 1.6 # blockdev-snapshot-internal-sync since 1.7 # blockdev-backup since 2.3 +# blockdev-snapshot since 2.5 ## { 'union': 'TransactionAction', 'data': { + 'blockdev-snapshot': 'BlockdevSnapshot', 'blockdev-snapshot-sync': 'BlockdevSnapshotSync', 'drive-backup': 'DriveBackup', 'blockdev-backup': 'BlockdevBackup', diff --git a/qapi/block-core.json b/qapi/block-core.json index f592adc8df..083d2cd346 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -704,6 +704,21 @@ 'snapshot-file': 'str', '*snapshot-node-name': 'str', '*format': 'str', '*mode': 'NewImageMode' } } +## +# @BlockdevSnapshot +# +# @node: device or node name that will have a snapshot created. +# +# @overlay: reference to the existing block device that will become +# the overlay of @node, as part of creating the snapshot. +# It must not have a current backing file (this can be +# achieved by passing "backing": "" to blockdev-add). +# +# Since 2.5 +## +{ 'struct': 'BlockdevSnapshot', + 'data': { 'node': 'str', 'overlay': 'str' } } + ## # @DriveBackup # @@ -800,6 +815,19 @@ { 'command': 'blockdev-snapshot-sync', 'data': 'BlockdevSnapshotSync' } + +## +# @blockdev-snapshot +# +# Generates a snapshot of a block device. +# +# For the arguments, see the documentation of BlockdevSnapshot. +# +# Since 2.5 +## +{ 'command': 'blockdev-snapshot', + 'data': 'BlockdevSnapshot' } + ## # @change-backing-file # diff --git a/qmp-commands.hx b/qmp-commands.hx index 39d6e25121..5fd6f9c0e3 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -1492,6 +1492,44 @@ Example: "format": "qcow2" } } <- { "return": {} } +EQMP + + { + .name = "blockdev-snapshot", + .args_type = "node:s,overlay:s", + .mhandler.cmd_new = qmp_marshal_blockdev_snapshot, + }, + +SQMP +blockdev-snapshot +----------------- +Since 2.5 + +Create a snapshot, by installing 'node' as the backing image of +'overlay'. Additionally, if 'node' is associated with a block +device, the block device changes to using 'overlay' as its new active +image. + +Arguments: + +- "node": device that will have a snapshot created (json-string) +- "overlay": device that will have 'node' as its backing image (json-string) + +Example: + +-> { "execute": "blockdev-add", + "arguments": { "options": { "driver": "qcow2", + "node-name": "node1534", + "file": { "driver": "file", + "filename": "hd1.qcow2" }, + "backing": "" } } } + +<- { "return": {} } + +-> { "execute": "blockdev-snapshot", "arguments": { "node": "ide-hd0", + "overlay": "node1534" } } +<- { "return": {} } + EQMP { From 89e3a2d86d31212d09ca688193ccecb92c0c77b5 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Mon, 26 Oct 2015 14:27:17 +0200 Subject: [PATCH 21/41] block: add tests for the 'blockdev-snapshot' command Signed-off-by: Alberto Garcia Reviewed-by: Max Reitz Reviewed-by: Jeff Cody Signed-off-by: Kevin Wolf --- tests/qemu-iotests/085 | 102 ++++++++++++++++++++++++++++++++++--- tests/qemu-iotests/085.out | 34 ++++++++++++- 2 files changed, 128 insertions(+), 8 deletions(-) diff --git a/tests/qemu-iotests/085 b/tests/qemu-iotests/085 index 56cd6f89b7..9484117031 100755 --- a/tests/qemu-iotests/085 +++ b/tests/qemu-iotests/085 @@ -7,6 +7,7 @@ # snapshots are performed. # # Copyright (C) 2014 Red Hat, Inc. +# Copyright (C) 2015 Igalia, S.L. # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -34,17 +35,17 @@ status=1 # failure is the default! snapshot_virt0="snapshot-v0.qcow2" snapshot_virt1="snapshot-v1.qcow2" -MAX_SNAPSHOTS=10 +SNAPSHOTS=10 _cleanup() { _cleanup_qemu - for i in $(seq 1 ${MAX_SNAPSHOTS}) + for i in $(seq 1 ${SNAPSHOTS}) do rm -f "${TEST_DIR}/${i}-${snapshot_virt0}" rm -f "${TEST_DIR}/${i}-${snapshot_virt1}" done - _cleanup_test_img + rm -f "${TEST_IMG}.1" "${TEST_IMG}.2" } trap "_cleanup; exit \$status" 0 1 2 3 15 @@ -85,18 +86,50 @@ function create_group_snapshot() _send_qemu_cmd $h "${cmd}" "return" } +# ${1}: unique identifier for the snapshot filename +# ${2}: true: open backing images; false: don't open them (default) +function add_snapshot_image() +{ + if [ "${2}" = "true" ]; then + extra_params="" + else + extra_params="'backing': '', " + fi + base_image="${TEST_DIR}/$((${1}-1))-${snapshot_virt0}" + snapshot_file="${TEST_DIR}/${1}-${snapshot_virt0}" + _make_test_img -b "${base_image}" "$size" + mv "${TEST_IMG}" "${snapshot_file}" + cmd="{ 'execute': 'blockdev-add', 'arguments': + { 'options': + { 'driver': 'qcow2', 'node-name': 'snap_"${1}"', "${extra_params}" + 'file': + { 'driver': 'file', 'filename': '"${snapshot_file}"' } } } }" + _send_qemu_cmd $h "${cmd}" "return" +} + +# ${1}: unique identifier for the snapshot filename +# ${2}: expected response, defaults to 'return' +function blockdev_snapshot() +{ + cmd="{ 'execute': 'blockdev-snapshot', + 'arguments': { 'node': 'virtio0', + 'overlay':'snap_"${1}"' } }" + _send_qemu_cmd $h "${cmd}" "${2:-return}" +} + size=128M _make_test_img $size -mv "${TEST_IMG}" "${TEST_IMG}.orig" +mv "${TEST_IMG}" "${TEST_IMG}.1" _make_test_img $size +mv "${TEST_IMG}" "${TEST_IMG}.2" echo echo === Running QEMU === echo qemu_comm_method="qmp" -_launch_qemu -drive file="${TEST_IMG}.orig",if=virtio -drive file="${TEST_IMG}",if=virtio +_launch_qemu -drive file="${TEST_IMG}.1",if=virtio -drive file="${TEST_IMG}.2",if=virtio h=$QEMU_HANDLE echo @@ -105,6 +138,8 @@ echo _send_qemu_cmd $h "{ 'execute': 'qmp_capabilities' }" "return" +# Tests for the blockdev-snapshot-sync command + echo echo === Create a single snapshot on virtio0 === echo @@ -132,11 +167,66 @@ echo echo === Create several transactional group snapshots === echo -for i in $(seq 2 ${MAX_SNAPSHOTS}) +for i in $(seq 2 ${SNAPSHOTS}) do create_group_snapshot ${i} done +# Tests for the blockdev-snapshot command + +echo +echo === Create a couple of snapshots using blockdev-snapshot === +echo + +SNAPSHOTS=$((${SNAPSHOTS}+1)) +add_snapshot_image ${SNAPSHOTS} +blockdev_snapshot ${SNAPSHOTS} + +SNAPSHOTS=$((${SNAPSHOTS}+1)) +add_snapshot_image ${SNAPSHOTS} +blockdev_snapshot ${SNAPSHOTS} + +echo +echo === Invalid command - snapshot node used as active layer === +echo + +blockdev_snapshot ${SNAPSHOTS} error + +_send_qemu_cmd $h "{ 'execute': 'blockdev-snapshot', + 'arguments': { 'node':'virtio0', + 'overlay':'virtio0' } + }" "error" + +_send_qemu_cmd $h "{ 'execute': 'blockdev-snapshot', + 'arguments': { 'node':'virtio0', + 'overlay':'virtio1' } + }" "error" + +echo +echo === Invalid command - snapshot node used as backing hd === +echo + +blockdev_snapshot $((${SNAPSHOTS}-1)) error + +echo +echo === Invalid command - snapshot node has a backing image === +echo + +SNAPSHOTS=$((${SNAPSHOTS}+1)) +add_snapshot_image ${SNAPSHOTS} true +blockdev_snapshot ${SNAPSHOTS} error + +echo +echo === Invalid command - The node does not exist === +echo + +blockdev_snapshot $((${SNAPSHOTS}+1)) error + +_send_qemu_cmd $h "{ 'execute': 'blockdev-snapshot', + 'arguments': { 'node':'nodevice', + 'overlay':'snap_"${SNAPSHOTS}"' } + }" "error" + # success, all done echo "*** done" rm -f $seq.full diff --git a/tests/qemu-iotests/085.out b/tests/qemu-iotests/085.out index a6cf19e57a..52292eac4e 100644 --- a/tests/qemu-iotests/085.out +++ b/tests/qemu-iotests/085.out @@ -11,7 +11,7 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 === Create a single snapshot on virtio0 === -Formatting 'TEST_DIR/1-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file=TEST_DIR/t.qcow2.orig backing_fmt=qcow2 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16 +Formatting 'TEST_DIR/1-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file=TEST_DIR/t.qcow2.1 backing_fmt=qcow2 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16 {"return": {}} === Invalid command - missing device and nodename === @@ -26,7 +26,7 @@ Formatting 'TEST_DIR/1-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file === Create several transactional group snapshots === Formatting 'TEST_DIR/2-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file=TEST_DIR/1-snapshot-v0.qcow2 backing_fmt=qcow2 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16 -Formatting 'TEST_DIR/2-snapshot-v1.qcow2', fmt=qcow2 size=134217728 backing_file=TEST_DIR/t.qcow2 backing_fmt=qcow2 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16 +Formatting 'TEST_DIR/2-snapshot-v1.qcow2', fmt=qcow2 size=134217728 backing_file=TEST_DIR/t.qcow2.2 backing_fmt=qcow2 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16 {"return": {}} Formatting 'TEST_DIR/3-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file=TEST_DIR/2-snapshot-v0.qcow2 backing_fmt=qcow2 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16 Formatting 'TEST_DIR/3-snapshot-v1.qcow2', fmt=qcow2 size=134217728 backing_file=TEST_DIR/2-snapshot-v1.qcow2 backing_fmt=qcow2 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16 @@ -52,4 +52,34 @@ Formatting 'TEST_DIR/9-snapshot-v1.qcow2', fmt=qcow2 size=134217728 backing_file Formatting 'TEST_DIR/10-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file=TEST_DIR/9-snapshot-v0.qcow2 backing_fmt=qcow2 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16 Formatting 'TEST_DIR/10-snapshot-v1.qcow2', fmt=qcow2 size=134217728 backing_file=TEST_DIR/9-snapshot-v1.qcow2 backing_fmt=qcow2 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16 {"return": {}} + +=== Create a couple of snapshots using blockdev-snapshot === + +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/10-snapshot-v0.IMGFMT +{"return": {}} +{"return": {}} +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/11-snapshot-v0.IMGFMT +{"return": {}} +{"return": {}} + +=== Invalid command - snapshot node used as active layer === + +{"error": {"class": "GenericError", "desc": "The snapshot is already in use by virtio0"}} +{"error": {"class": "GenericError", "desc": "The snapshot is already in use by virtio0"}} +{"error": {"class": "GenericError", "desc": "The snapshot is already in use by virtio1"}} + +=== Invalid command - snapshot node used as backing hd === + +{"error": {"class": "GenericError", "desc": "Node 'snap_11' is busy: node is used as backing hd of 'virtio0'"}} + +=== Invalid command - snapshot node has a backing image === + +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/12-snapshot-v0.IMGFMT +{"return": {}} +{"error": {"class": "GenericError", "desc": "The snapshot already has a backing image"}} + +=== Invalid command - The node does not exist === + +{"error": {"class": "GenericError", "desc": "Cannot find device=snap_14 nor node_name=snap_14"}} +{"error": {"class": "GenericError", "desc": "Cannot find device=nodevice nor node_name=nodevice"}} *** done From 3db2bd5508c86a1605258bc77c9672d93b5c350e Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Wed, 28 Oct 2015 15:43:49 +0200 Subject: [PATCH 22/41] commit: reopen overlay_bs before base 'block-commit' needs write access to two different nodes of the chain: - 'base', because that's where the data is written to. - the overlay of 'top', because it needs to update the backing file string to point to 'base' after the operation. Both images have to be opened in read-write mode, and commit_start() takes care of reopening them if necessary. With the current implementation, however, when overlay_bs is reopened in read-write mode it has the side effect of making 'base' read-only again, eventually making 'block-commit' fail. This needs to be fixed in bdrv_reopen(), but until we get to that it can be worked around simply by swapping the order of base and overlay_bs in the reopen queue. In order to reproduce this bug, overlay_bs needs to be initially in read-only mode. That is: the 'top' parameter of 'block-commit' cannot be the active layer nor its immediate backing chain. Cc: qemu-stable@nongnu.org Signed-off-by: Alberto Garcia Reviewed-by: Max Reitz Signed-off-by: Kevin Wolf --- block/commit.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/block/commit.c b/block/commit.c index fdebe87c16..a5d02aa560 100644 --- a/block/commit.c +++ b/block/commit.c @@ -236,14 +236,14 @@ void commit_start(BlockDriverState *bs, BlockDriverState *base, orig_overlay_flags = bdrv_get_flags(overlay_bs); /* convert base & overlay_bs to r/w, if necessary */ - if (!(orig_base_flags & BDRV_O_RDWR)) { - reopen_queue = bdrv_reopen_queue(reopen_queue, base, NULL, - orig_base_flags | BDRV_O_RDWR); - } if (!(orig_overlay_flags & BDRV_O_RDWR)) { reopen_queue = bdrv_reopen_queue(reopen_queue, overlay_bs, NULL, orig_overlay_flags | BDRV_O_RDWR); } + if (!(orig_base_flags & BDRV_O_RDWR)) { + reopen_queue = bdrv_reopen_queue(reopen_queue, base, NULL, + orig_base_flags | BDRV_O_RDWR); + } if (reopen_queue) { bdrv_reopen_multiple(reopen_queue, &local_err); if (local_err != NULL) { From bcdce5a73cb28f32b3ca3a51e8fa89879685e015 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Wed, 28 Oct 2015 15:43:50 +0200 Subject: [PATCH 23/41] qemu-iotests: Test the reopening of overlay_bs in 'block-commit' The 'block-commit' command needs the overlay image of 'top' to be opened in read-write mode in order to update the backing file string. If 'top' is not the active layer or its backing file then its overlay needs to be reopened during the block job. This is a test case for that scenario. Signed-off-by: Alberto Garcia Reviewed-by: Max Reitz Signed-off-by: Kevin Wolf --- tests/qemu-iotests/040 | 30 ++++++++++++++++++++++++++++++ tests/qemu-iotests/040.out | 4 ++-- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040 index ea2f98e51d..5bdaf3d48d 100755 --- a/tests/qemu-iotests/040 +++ b/tests/qemu-iotests/040 @@ -41,6 +41,7 @@ class ImageCommitTestCase(iotests.QMPTestCase): while not completed: for event in self.vm.get_qmp_events(wait=True): if event['event'] == 'BLOCK_JOB_COMPLETED': + self.assert_qmp_absent(event, 'data/error') self.assert_qmp(event, 'data/type', 'commit') self.assert_qmp(event, 'data/device', 'drive0') self.assert_qmp(event, 'data/offset', event['data']['len']) @@ -251,5 +252,34 @@ class TestSetSpeed(ImageCommitTestCase): class TestActiveZeroLengthImage(TestSingleDrive): image_len = 0 +class TestReopenOverlay(ImageCommitTestCase): + image_len = 1024 * 1024 + img0 = os.path.join(iotests.test_dir, '0.img') + img1 = os.path.join(iotests.test_dir, '1.img') + img2 = os.path.join(iotests.test_dir, '2.img') + img3 = os.path.join(iotests.test_dir, '3.img') + + def setUp(self): + iotests.create_image(self.img0, self.image_len) + qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % self.img0, self.img1) + qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % self.img1, self.img2) + qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % self.img2, self.img3) + qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0xab 0 128K', self.img1) + self.vm = iotests.VM().add_drive(self.img3) + self.vm.launch() + + def tearDown(self): + self.vm.shutdown() + os.remove(self.img0) + os.remove(self.img1) + os.remove(self.img2) + os.remove(self.img3) + + # This tests what happens when the overlay image of the 'top' node + # needs to be reopened in read-write mode in order to update the + # backing image string. + def test_reopen_overlay(self): + self.run_commit_test(self.img1, self.img0) + if __name__ == '__main__': iotests.main(supported_fmts=['qcow2', 'qed']) diff --git a/tests/qemu-iotests/040.out b/tests/qemu-iotests/040.out index 42314e9c00..4fd1c2dcd2 100644 --- a/tests/qemu-iotests/040.out +++ b/tests/qemu-iotests/040.out @@ -1,5 +1,5 @@ -........................ +......................... ---------------------------------------------------------------------- -Ran 24 tests +Ran 25 tests OK From 95334230637cef9fbd199bb79a56271ec73d4732 Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 2 Nov 2015 18:32:06 -0500 Subject: [PATCH 24/41] qcow2: avoid misaligned 64bit bswap If we create a buffer directly on the stack by using 12 bytes, there's no guarantee the 64bit value we want to swap will be aligned, which could cause errors with undefined behavior. Spotted with clang -fsanitize=undefined and observed in iotests 15, 26, 44, 115 and 121. Signed-off-by: John Snow Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- block/qcow2-refcount.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index 4b81c8db61..6e0e5bd9ae 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -560,13 +560,16 @@ static int alloc_refcount_block(BlockDriverState *bs, } /* Hook up the new refcount table in the qcow2 header */ - uint8_t data[12]; - cpu_to_be64w((uint64_t*)data, table_offset); - cpu_to_be32w((uint32_t*)(data + 8), table_clusters); + struct QEMU_PACKED { + uint64_t d64; + uint32_t d32; + } data; + cpu_to_be64w(&data.d64, table_offset); + cpu_to_be32w(&data.d32, table_clusters); BLKDBG_EVENT(bs->file, BLKDBG_REFBLOCK_ALLOC_SWITCH_TABLE); ret = bdrv_pwrite_sync(bs->file->bs, offsetof(QCowHeader, refcount_table_offset), - data, sizeof(data)); + &data, sizeof(data)); if (ret < 0) { goto fail_table; } From 62547b8a1c04daf30f50e3db362ade53e22bf222 Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 2 Nov 2015 18:28:20 -0500 Subject: [PATCH 25/41] qemu-img: add check for zero-length job len The mirror job doesn't update its total length until it has already started running, so we should translate a zero-length job-len as meaning 0%. Otherwise, we may get divide-by-zero faults. Signed-off-by: John Snow Reviewed-by: Jeff Cody Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- qemu-img.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/qemu-img.c b/qemu-img.c index 3025776e14..9831db75ef 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -656,7 +656,8 @@ static void run_block_job(BlockJob *job, Error **errp) do { aio_poll(aio_context, true); - qemu_progress_print((float)job->offset / job->len * 100.f, 0); + qemu_progress_print(job->len ? + ((float)job->offset / job->len * 100.f) : 0.0f, 0); } while (!job->ready); block_job_complete_sync(job, errp); From 5ac724184c286b367525035eabf4b8bb4a386c54 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Wed, 4 Nov 2015 15:15:35 +0200 Subject: [PATCH 26/41] throttle: Check for pending requests in throttle_group_unregister_bs() throttle_group_unregister_bs() removes a BlockDriverState from its throttling group and destroys the timers. This means that there must be no pending throttled requests at that point (because it would be impossible to complete them), so the caller has to drain them first. At the moment throttle_group_unregister_bs() is only called from bdrv_io_limits_disable(), which already takes care of draining the requests, so there's nothing to worry about, but this patch makes this invariant explicit in the documentation and adds the relevant assertions. Signed-off-by: Alberto Garcia Signed-off-by: Kevin Wolf --- block/throttle-groups.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/block/throttle-groups.c b/block/throttle-groups.c index 3419af7d96..13b5baa5d7 100644 --- a/block/throttle-groups.c +++ b/block/throttle-groups.c @@ -437,6 +437,9 @@ void throttle_group_register_bs(BlockDriverState *bs, const char *groupname) * list, destroying the timers and setting the throttle_state pointer * to NULL. * + * The BlockDriverState must not have pending throttled requests, so + * the caller has to drain them first. + * * The group will be destroyed if it's empty after this operation. * * @bs: the BlockDriverState to remove @@ -446,6 +449,10 @@ void throttle_group_unregister_bs(BlockDriverState *bs) ThrottleGroup *tg = container_of(bs->throttle_state, ThrottleGroup, ts); int i; + assert(bs->pending_reqs[0] == 0 && bs->pending_reqs[1] == 0); + assert(qemu_co_queue_empty(&bs->throttled_reqs[0])); + assert(qemu_co_queue_empty(&bs->throttled_reqs[1])); + qemu_mutex_lock(&tg->lock); for (i = 0; i < 2; i++) { if (tg->tokens[i] == bs) { From a0d64a61db602696f4f1895a890c65eda5b3b618 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Wed, 4 Nov 2015 15:15:36 +0200 Subject: [PATCH 27/41] throttle: Use bs->throttle_state instead of bs->io_limits_enabled There are two ways to check for I/O limits in a BlockDriverState: - bs->throttle_state: if this pointer is not NULL, it means that this BDS is member of a throttling group, its ThrottleTimers structure has been initialized and its I/O limits are ready to be applied. - bs->io_limits_enabled: if true it means that the throttle_state pointer is valid _and_ the limits are currently enabled. The latter is used in several places to check whether a BDS has I/O limits configured, but what it really checks is whether requests are being throttled or not. For example, io_limits_enabled can be temporarily set to false in cases like bdrv_read_unthrottled() without otherwise touching the throtting configuration of that BDS. This patch replaces bs->io_limits_enabled with bs->throttle_state in all cases where what we really want to check is the existence of I/O limits, not whether they are currently enabled or not. Signed-off-by: Alberto Garcia Signed-off-by: Kevin Wolf --- block.c | 6 +++--- block/qapi.c | 2 +- blockdev.c | 4 ++-- include/block/block_int.h | 5 ++++- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/block.c b/block.c index 34935015dd..cffac75016 100644 --- a/block.c +++ b/block.c @@ -1907,7 +1907,7 @@ void bdrv_close(BlockDriverState *bs) } /* Disable I/O limits and drain all pending throttled requests */ - if (bs->io_limits_enabled) { + if (bs->throttle_state) { bdrv_io_limits_disable(bs); } @@ -3712,7 +3712,7 @@ void bdrv_detach_aio_context(BlockDriverState *bs) baf->detach_aio_context(baf->opaque); } - if (bs->io_limits_enabled) { + if (bs->throttle_state) { throttle_timers_detach_aio_context(&bs->throttle_timers); } if (bs->drv->bdrv_detach_aio_context) { @@ -3748,7 +3748,7 @@ void bdrv_attach_aio_context(BlockDriverState *bs, if (bs->drv->bdrv_attach_aio_context) { bs->drv->bdrv_attach_aio_context(bs, new_context); } - if (bs->io_limits_enabled) { + if (bs->throttle_state) { throttle_timers_attach_aio_context(&bs->throttle_timers, new_context); } diff --git a/block/qapi.c b/block/qapi.c index ec0f5139e2..89d4274177 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -64,7 +64,7 @@ BlockDeviceInfo *bdrv_block_device_info(BlockDriverState *bs, Error **errp) info->backing_file_depth = bdrv_get_backing_file_depth(bs); info->detect_zeroes = bs->detect_zeroes; - if (bs->io_limits_enabled) { + if (bs->throttle_state) { ThrottleConfig cfg; throttle_group_get_config(bs, &cfg); diff --git a/blockdev.c b/blockdev.c index 7645d491cb..3598b01419 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2361,14 +2361,14 @@ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd, if (throttle_enabled(&cfg)) { /* Enable I/O limits if they're not enabled yet, otherwise * just update the throttling group. */ - if (!bs->io_limits_enabled) { + if (!bs->throttle_state) { bdrv_io_limits_enable(bs, has_group ? group : device); } else if (has_group) { bdrv_io_limits_update_group(bs, group); } /* Set the new throttling configuration */ bdrv_set_io_limits(bs, &cfg); - } else if (bs->io_limits_enabled) { + } else if (bs->throttle_state) { /* If all throttling settings are set to 0, disable I/O limits */ bdrv_io_limits_disable(bs); } diff --git a/include/block/block_int.h b/include/block/block_int.h index 6a3f64da12..603145a21d 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -390,7 +390,10 @@ struct BlockDriverState { /* number of in-flight serialising requests */ unsigned int serialising_in_flight; - /* I/O throttling */ + /* I/O throttling. + * throttle_state tells us if this BDS has I/O limits configured. + * io_limits_enabled tells us if they are currently being + * enforced, but it can be temporarily set to false */ CoQueue throttled_reqs[2]; bool io_limits_enabled; /* The following fields are protected by the ThrottleGroup lock. From 08b24cfe3765f4b739700778814048e7d9a045fe Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Tue, 3 Nov 2015 12:32:35 +0200 Subject: [PATCH 28/41] block: Disallow snapshots if the overlay doesn't support backing files This addresses scenarios like this one: { 'execute': 'blockdev-add', 'arguments': { 'options': { 'driver': 'qcow2', 'node-name': 'new0', 'file': { 'driver': 'file', 'filename': 'new.qcow2', 'node-name': 'file0' } } } } { 'execute': 'blockdev-snapshot', 'arguments': { 'node': 'virtio0', 'overlay': 'file0' } } Signed-off-by: Alberto Garcia Reviewed-by: Eric Blake Reviewed-by: Max Reitz Signed-off-by: Kevin Wolf --- blockdev.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/blockdev.c b/blockdev.c index 3598b01419..3197791c7b 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1678,6 +1678,11 @@ static void external_snapshot_prepare(BlkTransactionState *common, if (state->new_bs->backing != NULL) { error_setg(errp, "The snapshot already has a backing image"); + return; + } + + if (!state->new_bs->drv->supports_backing) { + error_setg(errp, "The snapshot does not support backing images"); } } From f2d7f16f948b2ecfbb039c6bd62d6c7e2d61fac4 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Tue, 3 Nov 2015 12:32:36 +0200 Subject: [PATCH 29/41] block: Remove inner quotation marks in iotest 085 This patch removes the inner quotation marks in all cases like this: cmd=" ... "${variable}" ... " Signed-off-by: Alberto Garcia Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- tests/qemu-iotests/085 | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/qemu-iotests/085 b/tests/qemu-iotests/085 index 9484117031..80e547d81b 100755 --- a/tests/qemu-iotests/085 +++ b/tests/qemu-iotests/085 @@ -65,7 +65,7 @@ function create_single_snapshot() { cmd="{ 'execute': 'blockdev-snapshot-sync', 'arguments': { 'device': 'virtio0', - 'snapshot-file':'"${TEST_DIR}/${1}-${snapshot_virt0}"', + 'snapshot-file':'${TEST_DIR}/${1}-${snapshot_virt0}', 'format': 'qcow2' } }" _send_qemu_cmd $h "${cmd}" "return" } @@ -77,10 +77,10 @@ function create_group_snapshot() {'actions': [ { 'type': 'blockdev-snapshot-sync', 'data' : { 'device': 'virtio0', - 'snapshot-file': '"${TEST_DIR}/${1}-${snapshot_virt0}"' } }, + 'snapshot-file': '${TEST_DIR}/${1}-${snapshot_virt0}' } }, { 'type': 'blockdev-snapshot-sync', 'data' : { 'device': 'virtio1', - 'snapshot-file': '"${TEST_DIR}/${1}-${snapshot_virt1}"' } } ] + 'snapshot-file': '${TEST_DIR}/${1}-${snapshot_virt1}' } } ] } }" _send_qemu_cmd $h "${cmd}" "return" @@ -101,9 +101,9 @@ function add_snapshot_image() mv "${TEST_IMG}" "${snapshot_file}" cmd="{ 'execute': 'blockdev-add', 'arguments': { 'options': - { 'driver': 'qcow2', 'node-name': 'snap_"${1}"', "${extra_params}" + { 'driver': 'qcow2', 'node-name': 'snap_${1}', ${extra_params} 'file': - { 'driver': 'file', 'filename': '"${snapshot_file}"' } } } }" + { 'driver': 'file', 'filename': '${snapshot_file}' } } } }" _send_qemu_cmd $h "${cmd}" "return" } @@ -113,7 +113,7 @@ function blockdev_snapshot() { cmd="{ 'execute': 'blockdev-snapshot', 'arguments': { 'node': 'virtio0', - 'overlay':'snap_"${1}"' } }" + 'overlay':'snap_${1}' } }" _send_qemu_cmd $h "${cmd}" "${2:-return}" } @@ -152,7 +152,7 @@ echo === Invalid command - missing device and nodename === echo _send_qemu_cmd $h "{ 'execute': 'blockdev-snapshot-sync', - 'arguments': { 'snapshot-file':'"${TEST_DIR}/1-${snapshot_virt0}"', + 'arguments': { 'snapshot-file':'${TEST_DIR}/1-${snapshot_virt0}', 'format': 'qcow2' } }" "error" echo @@ -224,7 +224,7 @@ blockdev_snapshot $((${SNAPSHOTS}+1)) error _send_qemu_cmd $h "{ 'execute': 'blockdev-snapshot', 'arguments': { 'node':'nodevice', - 'overlay':'snap_"${SNAPSHOTS}"' } + 'overlay':'snap_${SNAPSHOTS}' } }" "error" # success, all done From 3fa123d05964b07f9d4d972f131cce847091926d Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Tue, 3 Nov 2015 12:32:37 +0200 Subject: [PATCH 30/41] block: test 'blockdev-snapshot' using a file BDS as the overlay This test checks that it is not possible to create a snapshot if the requested overlay node is a BDS which does not support backing images. Signed-off-by: Alberto Garcia Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- tests/qemu-iotests/085 | 12 +++++++++++- tests/qemu-iotests/085.out | 4 ++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/tests/qemu-iotests/085 b/tests/qemu-iotests/085 index 80e547d81b..aa77eca77d 100755 --- a/tests/qemu-iotests/085 +++ b/tests/qemu-iotests/085 @@ -103,7 +103,8 @@ function add_snapshot_image() { 'options': { 'driver': 'qcow2', 'node-name': 'snap_${1}', ${extra_params} 'file': - { 'driver': 'file', 'filename': '${snapshot_file}' } } } }" + { 'driver': 'file', 'filename': '${snapshot_file}', + 'node-name': 'file_${1}' } } } }" _send_qemu_cmd $h "${cmd}" "return" } @@ -186,6 +187,15 @@ SNAPSHOTS=$((${SNAPSHOTS}+1)) add_snapshot_image ${SNAPSHOTS} blockdev_snapshot ${SNAPSHOTS} +echo +echo === Invalid command - cannot create a snapshot using a file BDS === +echo + +_send_qemu_cmd $h "{ 'execute': 'blockdev-snapshot', + 'arguments': { 'node':'virtio0', + 'overlay':'file_${SNAPSHOTS}' } + }" "error" + echo echo === Invalid command - snapshot node used as active layer === echo diff --git a/tests/qemu-iotests/085.out b/tests/qemu-iotests/085.out index 52292eac4e..01c78d6894 100644 --- a/tests/qemu-iotests/085.out +++ b/tests/qemu-iotests/085.out @@ -62,6 +62,10 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/ {"return": {}} {"return": {}} +=== Invalid command - cannot create a snapshot using a file BDS === + +{"error": {"class": "GenericError", "desc": "The snapshot does not support backing images"}} + === Invalid command - snapshot node used as active layer === {"error": {"class": "GenericError", "desc": "The snapshot is already in use by virtio0"}} From 9b0beaf3de1396a23d5c287283e6f36c4b5d4385 Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 5 Nov 2015 18:53:02 -0500 Subject: [PATCH 31/41] qemu-io: fix cvtnum lval types cvtnum() returns int64_t: we should not be storing this result inside of an int. In a few cases, we need an extra sprinkling of error handling where we expect to pass this number on towards a function that expects something smaller than int64_t. Reported-by: Max Reitz Signed-off-by: John Snow Signed-off-by: Kevin Wolf --- qemu-io-cmds.c | 125 +++++++++++++++++++++++++++++++++++-------------- 1 file changed, 89 insertions(+), 36 deletions(-) diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c index 6e5d1e4d38..1e97f69f56 100644 --- a/qemu-io-cmds.c +++ b/qemu-io-cmds.c @@ -294,9 +294,10 @@ static void qemu_io_free(void *p) qemu_vfree(p); } -static void dump_buffer(const void *buffer, int64_t offset, int len) +static void dump_buffer(const void *buffer, int64_t offset, int64_t len) { - int i, j; + uint64_t i; + int j; const uint8_t *p; for (i = 0, p = buffer; i < len; i += 16) { @@ -319,7 +320,7 @@ static void dump_buffer(const void *buffer, int64_t offset, int len) } static void print_report(const char *op, struct timeval *t, int64_t offset, - int count, int total, int cnt, int Cflag) + int64_t count, int64_t total, int cnt, int Cflag) { char s1[64], s2[64], ts[64]; @@ -327,12 +328,12 @@ static void print_report(const char *op, struct timeval *t, int64_t offset, if (!Cflag) { cvtstr((double)total, s1, sizeof(s1)); cvtstr(tdiv((double)total, *t), s2, sizeof(s2)); - printf("%s %d/%d bytes at offset %" PRId64 "\n", + printf("%s %"PRId64"/%"PRId64" bytes at offset %" PRId64 "\n", op, total, count, offset); printf("%s, %d ops; %s (%s/sec and %.4f ops/sec)\n", s1, cnt, ts, s2, tdiv((double)cnt, *t)); } else {/* bytes,ops,time,bytes/sec,ops/sec */ - printf("%d,%d,%s,%.3f,%.3f\n", + printf("%"PRId64",%d,%s,%.3f,%.3f\n", total, cnt, ts, tdiv((double)total, *t), tdiv((double)cnt, *t)); @@ -393,11 +394,15 @@ fail: return buf; } -static int do_read(BlockBackend *blk, char *buf, int64_t offset, int count, - int *total) +static int do_read(BlockBackend *blk, char *buf, int64_t offset, int64_t count, + int64_t *total) { int ret; + if (count >> 9 > INT_MAX) { + return -ERANGE; + } + ret = blk_read(blk, offset >> 9, (uint8_t *)buf, count >> 9); if (ret < 0) { return ret; @@ -406,11 +411,15 @@ static int do_read(BlockBackend *blk, char *buf, int64_t offset, int count, return 1; } -static int do_write(BlockBackend *blk, char *buf, int64_t offset, int count, - int *total) +static int do_write(BlockBackend *blk, char *buf, int64_t offset, int64_t count, + int64_t *total) { int ret; + if (count >> 9 > INT_MAX) { + return -ERANGE; + } + ret = blk_write(blk, offset >> 9, (uint8_t *)buf, count >> 9); if (ret < 0) { return ret; @@ -419,9 +428,13 @@ static int do_write(BlockBackend *blk, char *buf, int64_t offset, int count, return 1; } -static int do_pread(BlockBackend *blk, char *buf, int64_t offset, int count, - int *total) +static int do_pread(BlockBackend *blk, char *buf, int64_t offset, + int64_t count, int64_t *total) { + if (count > INT_MAX) { + return -ERANGE; + } + *total = blk_pread(blk, offset, (uint8_t *)buf, count); if (*total < 0) { return *total; @@ -429,9 +442,13 @@ static int do_pread(BlockBackend *blk, char *buf, int64_t offset, int count, return 1; } -static int do_pwrite(BlockBackend *blk, char *buf, int64_t offset, int count, - int *total) +static int do_pwrite(BlockBackend *blk, char *buf, int64_t offset, + int64_t count, int64_t *total) { + if (count > INT_MAX) { + return -ERANGE; + } + *total = blk_pwrite(blk, offset, (uint8_t *)buf, count); if (*total < 0) { return *total; @@ -442,8 +459,8 @@ static int do_pwrite(BlockBackend *blk, char *buf, int64_t offset, int count, typedef struct { BlockBackend *blk; int64_t offset; - int count; - int *total; + int64_t count; + int64_t *total; int ret; bool done; } CoWriteZeroes; @@ -463,8 +480,8 @@ static void coroutine_fn co_write_zeroes_entry(void *opaque) *data->total = data->count; } -static int do_co_write_zeroes(BlockBackend *blk, int64_t offset, int count, - int *total) +static int do_co_write_zeroes(BlockBackend *blk, int64_t offset, int64_t count, + int64_t *total) { Coroutine *co; CoWriteZeroes data = { @@ -475,6 +492,10 @@ static int do_co_write_zeroes(BlockBackend *blk, int64_t offset, int count, .done = false, }; + if (count >> BDRV_SECTOR_BITS > INT_MAX) { + return -ERANGE; + } + co = qemu_coroutine_create(co_write_zeroes_entry); qemu_coroutine_enter(co, &data); while (!data.done) { @@ -488,10 +509,14 @@ static int do_co_write_zeroes(BlockBackend *blk, int64_t offset, int count, } static int do_write_compressed(BlockBackend *blk, char *buf, int64_t offset, - int count, int *total) + int64_t count, int64_t *total) { int ret; + if (count >> 9 > INT_MAX) { + return -ERANGE; + } + ret = blk_write_compressed(blk, offset >> 9, (uint8_t *)buf, count >> 9); if (ret < 0) { return ret; @@ -501,8 +526,12 @@ static int do_write_compressed(BlockBackend *blk, char *buf, int64_t offset, } static int do_load_vmstate(BlockBackend *blk, char *buf, int64_t offset, - int count, int *total) + int64_t count, int64_t *total) { + if (count > INT_MAX) { + return -ERANGE; + } + *total = blk_load_vmstate(blk, (uint8_t *)buf, offset, count); if (*total < 0) { return *total; @@ -511,8 +540,12 @@ static int do_load_vmstate(BlockBackend *blk, char *buf, int64_t offset, } static int do_save_vmstate(BlockBackend *blk, char *buf, int64_t offset, - int count, int *total) + int64_t count, int64_t *total) { + if (count > INT_MAX) { + return -ERANGE; + } + *total = blk_save_vmstate(blk, (uint8_t *)buf, offset, count); if (*total < 0) { return *total; @@ -642,10 +675,11 @@ static int read_f(BlockBackend *blk, int argc, char **argv) int c, cnt; char *buf; int64_t offset; - int count; + int64_t count; /* Some compilers get confused and warn if this is not initialized. */ - int total = 0; - int pattern = 0, pattern_offset = 0, pattern_count = 0; + int64_t total = 0; + int pattern = 0; + int64_t pattern_offset = 0, pattern_count = 0; while ((c = getopt(argc, argv, "bCl:pP:qs:v")) != -1) { switch (c) { @@ -712,6 +746,10 @@ static int read_f(BlockBackend *blk, int argc, char **argv) if (count < 0) { printf("non-numeric length argument -- %s\n", argv[optind]); return 0; + } else if (count > SIZE_MAX) { + printf("length cannot exceed %" PRIu64 ", given %s\n", + (uint64_t) SIZE_MAX, argv[optind]); + return 0; } if (!Pflag && (lflag || sflag)) { @@ -734,7 +772,7 @@ static int read_f(BlockBackend *blk, int argc, char **argv) return 0; } if (count & 0x1ff) { - printf("count %d is not sector aligned\n", + printf("count %"PRId64" is not sector aligned\n", count); return 0; } @@ -762,7 +800,7 @@ static int read_f(BlockBackend *blk, int argc, char **argv) memset(cmp_buf, pattern, pattern_count); if (memcmp(buf + pattern_offset, cmp_buf, pattern_count)) { printf("Pattern verification failed at offset %" - PRId64 ", %d bytes\n", + PRId64 ", %"PRId64" bytes\n", offset + pattern_offset, pattern_count); } g_free(cmp_buf); @@ -957,9 +995,9 @@ static int write_f(BlockBackend *blk, int argc, char **argv) int c, cnt; char *buf = NULL; int64_t offset; - int count; + int64_t count; /* Some compilers get confused and warn if this is not initialized. */ - int total = 0; + int64_t total = 0; int pattern = 0xcd; while ((c = getopt(argc, argv, "bcCpP:qz")) != -1) { @@ -1019,6 +1057,10 @@ static int write_f(BlockBackend *blk, int argc, char **argv) if (count < 0) { printf("non-numeric length argument -- %s\n", argv[optind]); return 0; + } else if (count > SIZE_MAX) { + printf("length cannot exceed %" PRIu64 ", given %s\n", + (uint64_t) SIZE_MAX, argv[optind]); + return 0; } if (!pflag) { @@ -1029,7 +1071,7 @@ static int write_f(BlockBackend *blk, int argc, char **argv) } if (count & 0x1ff) { - printf("count %d is not sector aligned\n", + printf("count %"PRId64" is not sector aligned\n", count); return 0; } @@ -1777,8 +1819,7 @@ static int discard_f(BlockBackend *blk, int argc, char **argv) struct timeval t1, t2; int Cflag = 0, qflag = 0; int c, ret; - int64_t offset; - int count; + int64_t offset, count; while ((c = getopt(argc, argv, "Cq")) != -1) { switch (c) { @@ -1808,6 +1849,11 @@ static int discard_f(BlockBackend *blk, int argc, char **argv) if (count < 0) { printf("non-numeric length argument -- %s\n", argv[optind]); return 0; + } else if (count >> BDRV_SECTOR_BITS > INT_MAX) { + printf("length cannot exceed %"PRIu64", given %s\n", + (uint64_t)INT_MAX << BDRV_SECTOR_BITS, + argv[optind]); + return 0; } gettimeofday(&t1, NULL); @@ -1833,11 +1879,10 @@ out: static int alloc_f(BlockBackend *blk, int argc, char **argv) { BlockDriverState *bs = blk_bs(blk); - int64_t offset, sector_num; - int nb_sectors, remaining; + int64_t offset, sector_num, nb_sectors, remaining; char s1[64]; - int num, sum_alloc; - int ret; + int num, ret; + int64_t sum_alloc; offset = cvtnum(argv[1]); if (offset < 0) { @@ -1854,6 +1899,10 @@ static int alloc_f(BlockBackend *blk, int argc, char **argv) if (nb_sectors < 0) { printf("non-numeric length argument -- %s\n", argv[2]); return 0; + } else if (nb_sectors > INT_MAX) { + printf("length argument cannot exceed %d, given %s\n", + INT_MAX, argv[2]); + return 0; } } else { nb_sectors = 1; @@ -1881,7 +1930,7 @@ static int alloc_f(BlockBackend *blk, int argc, char **argv) cvtstr(offset, s1, sizeof(s1)); - printf("%d/%d sectors allocated at offset %s\n", + printf("%"PRId64"/%"PRId64" sectors allocated at offset %s\n", sum_alloc, nb_sectors, s1); return 0; } @@ -2191,10 +2240,14 @@ static const cmdinfo_t sigraise_cmd = { static int sigraise_f(BlockBackend *blk, int argc, char **argv) { - int sig = cvtnum(argv[1]); + int64_t sig = cvtnum(argv[1]); if (sig < 0) { printf("non-numeric signal number argument -- %s\n", argv[1]); return 0; + } else if (sig > NSIG) { + printf("signal argument '%s' is too large to be a valid signal\n", + argv[1]); + return 0; } /* Using raise() to kill this process does not necessarily flush all open From ef5a788527b2038d742b057a415ab4d0e735e98f Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 5 Nov 2015 18:53:03 -0500 Subject: [PATCH 32/41] qemu-io: Check for trailing chars Make sure there's not trailing garbage, e.g. "64k-whatever-i-want-here" Reported-by: Max Reitz Signed-off-by: John Snow Reviewed-by: Eric Blake Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- qemu-io-cmds.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c index 1e97f69f56..7ebe89a1d8 100644 --- a/qemu-io-cmds.c +++ b/qemu-io-cmds.c @@ -136,7 +136,14 @@ static char **breakline(char *input, int *count) static int64_t cvtnum(const char *s) { char *end; - return qemu_strtosz_suffix(s, &end, QEMU_STRTOSZ_DEFSUFFIX_B); + int64_t ret; + + ret = qemu_strtosz_suffix(s, &end, QEMU_STRTOSZ_DEFSUFFIX_B); + if (*end != '\0') { + /* Detritus at the end of the string */ + return -EINVAL; + } + return ret; } #define EXABYTES(x) ((long long)(x) << 60) From a9ecfa004f2dd83df612daac4a87dfc3a0feba28 Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 5 Nov 2015 18:53:04 -0500 Subject: [PATCH 33/41] qemu-io: Correct error messages Reported-by: Max Reitz Signed-off-by: John Snow Reviewed-by: Eric Blake Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- qemu-io-cmds.c | 53 ++++++++++++++++++++++++++++++++------------------ 1 file changed, 34 insertions(+), 19 deletions(-) diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c index 7ebe89a1d8..9c77aafb99 100644 --- a/qemu-io-cmds.c +++ b/qemu-io-cmds.c @@ -146,6 +146,21 @@ static int64_t cvtnum(const char *s) return ret; } +static void print_cvtnum_err(int64_t rc, const char *arg) +{ + switch (rc) { + case -EINVAL: + printf("Parsing error: non-numeric argument," + " or extraneous/unrecognized suffix -- %s\n", arg); + break; + case -ERANGE: + printf("Parsing error: argument too large -- %s\n", arg); + break; + default: + printf("Parsing error: %s\n", arg); + } +} + #define EXABYTES(x) ((long long)(x) << 60) #define PETABYTES(x) ((long long)(x) << 50) #define TERABYTES(x) ((long long)(x) << 40) @@ -367,13 +382,13 @@ create_iovec(BlockBackend *blk, QEMUIOVector *qiov, char **argv, int nr_iov, len = cvtnum(arg); if (len < 0) { - printf("non-numeric length argument -- %s\n", arg); + print_cvtnum_err(len, arg); goto fail; } /* should be SIZE_T_MAX, but that doesn't exist */ if (len > INT_MAX) { - printf("too large length argument -- %s\n", arg); + printf("Argument '%s' exceeds maximum size %d\n", arg, INT_MAX); goto fail; } @@ -700,7 +715,7 @@ static int read_f(BlockBackend *blk, int argc, char **argv) lflag = 1; pattern_count = cvtnum(optarg); if (pattern_count < 0) { - printf("non-numeric length argument -- %s\n", optarg); + print_cvtnum_err(pattern_count, optarg); return 0; } break; @@ -721,7 +736,7 @@ static int read_f(BlockBackend *blk, int argc, char **argv) sflag = 1; pattern_offset = cvtnum(optarg); if (pattern_offset < 0) { - printf("non-numeric length argument -- %s\n", optarg); + print_cvtnum_err(pattern_offset, optarg); return 0; } break; @@ -744,14 +759,14 @@ static int read_f(BlockBackend *blk, int argc, char **argv) offset = cvtnum(argv[optind]); if (offset < 0) { - printf("non-numeric length argument -- %s\n", argv[optind]); + print_cvtnum_err(offset, argv[optind]); return 0; } optind++; count = cvtnum(argv[optind]); if (count < 0) { - printf("non-numeric length argument -- %s\n", argv[optind]); + print_cvtnum_err(count, argv[optind]); return 0; } else if (count > SIZE_MAX) { printf("length cannot exceed %" PRIu64 ", given %s\n", @@ -906,7 +921,7 @@ static int readv_f(BlockBackend *blk, int argc, char **argv) offset = cvtnum(argv[optind]); if (offset < 0) { - printf("non-numeric length argument -- %s\n", argv[optind]); + print_cvtnum_err(offset, argv[optind]); return 0; } optind++; @@ -1055,14 +1070,14 @@ static int write_f(BlockBackend *blk, int argc, char **argv) offset = cvtnum(argv[optind]); if (offset < 0) { - printf("non-numeric length argument -- %s\n", argv[optind]); + print_cvtnum_err(offset, argv[optind]); return 0; } optind++; count = cvtnum(argv[optind]); if (count < 0) { - printf("non-numeric length argument -- %s\n", argv[optind]); + print_cvtnum_err(count, argv[optind]); return 0; } else if (count > SIZE_MAX) { printf("length cannot exceed %" PRIu64 ", given %s\n", @@ -1191,7 +1206,7 @@ static int writev_f(BlockBackend *blk, int argc, char **argv) offset = cvtnum(argv[optind]); if (offset < 0) { - printf("non-numeric length argument -- %s\n", argv[optind]); + print_cvtnum_err(offset, argv[optind]); return 0; } optind++; @@ -1318,7 +1333,7 @@ static int multiwrite_f(BlockBackend *blk, int argc, char **argv) /* Read the offset of the request */ offset = cvtnum(argv[optind]); if (offset < 0) { - printf("non-numeric offset argument -- %s\n", argv[optind]); + print_cvtnum_err(offset, argv[optind]); goto out; } optind++; @@ -1545,7 +1560,7 @@ static int aio_read_f(BlockBackend *blk, int argc, char **argv) ctx->offset = cvtnum(argv[optind]); if (ctx->offset < 0) { - printf("non-numeric length argument -- %s\n", argv[optind]); + print_cvtnum_err(ctx->offset, argv[optind]); g_free(ctx); return 0; } @@ -1640,7 +1655,7 @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv) ctx->offset = cvtnum(argv[optind]); if (ctx->offset < 0) { - printf("non-numeric length argument -- %s\n", argv[optind]); + print_cvtnum_err(ctx->offset, argv[optind]); g_free(ctx); return 0; } @@ -1700,7 +1715,7 @@ static int truncate_f(BlockBackend *blk, int argc, char **argv) offset = cvtnum(argv[1]); if (offset < 0) { - printf("non-numeric truncate argument -- %s\n", argv[1]); + print_cvtnum_err(offset, argv[1]); return 0; } @@ -1847,14 +1862,14 @@ static int discard_f(BlockBackend *blk, int argc, char **argv) offset = cvtnum(argv[optind]); if (offset < 0) { - printf("non-numeric length argument -- %s\n", argv[optind]); + print_cvtnum_err(offset, argv[optind]); return 0; } optind++; count = cvtnum(argv[optind]); if (count < 0) { - printf("non-numeric length argument -- %s\n", argv[optind]); + print_cvtnum_err(count, argv[optind]); return 0; } else if (count >> BDRV_SECTOR_BITS > INT_MAX) { printf("length cannot exceed %"PRIu64", given %s\n", @@ -1893,7 +1908,7 @@ static int alloc_f(BlockBackend *blk, int argc, char **argv) offset = cvtnum(argv[1]); if (offset < 0) { - printf("non-numeric offset argument -- %s\n", argv[1]); + print_cvtnum_err(offset, argv[1]); return 0; } else if (offset & 0x1ff) { printf("offset %" PRId64 " is not sector aligned\n", @@ -1904,7 +1919,7 @@ static int alloc_f(BlockBackend *blk, int argc, char **argv) if (argc == 3) { nb_sectors = cvtnum(argv[2]); if (nb_sectors < 0) { - printf("non-numeric length argument -- %s\n", argv[2]); + print_cvtnum_err(nb_sectors, argv[2]); return 0; } else if (nb_sectors > INT_MAX) { printf("length argument cannot exceed %d, given %s\n", @@ -2249,7 +2264,7 @@ static int sigraise_f(BlockBackend *blk, int argc, char **argv) { int64_t sig = cvtnum(argv[1]); if (sig < 0) { - printf("non-numeric signal number argument -- %s\n", argv[1]); + print_cvtnum_err(sig, argv[1]); return 0; } else if (sig > NSIG) { printf("signal argument '%s' is too large to be a valid signal\n", From f6c8c2e055f88e8ed74ac0c185fc9fbca1e1b775 Mon Sep 17 00:00:00 2001 From: Jeff Cody Date: Fri, 30 Oct 2015 15:25:17 -0400 Subject: [PATCH 34/41] qemu-iotests: fix cleanup of background processes Commit 934659c switched the iotests to run qemu and qemu-nbd from a bash subshell, in order to catch segfaults. Unfortunately, this means the process PID cannot be captured via '$!'. We stopped killing qemu and qemu-nbd processes, leaving a lot of orphaned, running qemu processes after executing iotests. Since the process is using exec in the subshell, the PID is the same as the subshell PID. Track these PIDs for cleanup using pidfiles in the $TEST_DIR. Only track the qemu PID, however, if requested - not all usage requires killing the process. Reported-by: John Snow Signed-off-by: Jeff Cody Message-id: 9e4f958b3895b7259b98d845bb46f000ba362869.1446232490.git.jcody@redhat.com [mreitz@redhat.com: Replaced '! -z "..."' by '-n "..."'] Signed-off-by: Max Reitz --- tests/qemu-iotests/058 | 12 ++++++++---- tests/qemu-iotests/common.config | 14 ++++++++++++-- tests/qemu-iotests/common.qemu | 18 ++++++++++++------ tests/qemu-iotests/common.rc | 8 +++++--- 4 files changed, 37 insertions(+), 15 deletions(-) diff --git a/tests/qemu-iotests/058 b/tests/qemu-iotests/058 index f2bdd0bffc..63a6598784 100755 --- a/tests/qemu-iotests/058 +++ b/tests/qemu-iotests/058 @@ -32,11 +32,17 @@ status=1 # failure is the default! nbd_unix_socket=$TEST_DIR/test_qemu_nbd_socket nbd_snapshot_img="nbd:unix:$nbd_unix_socket" +rm -f "${TEST_DIR}/qemu-nbd.pid" _cleanup_nbd() { - if [ -n "$NBD_SNAPSHOT_PID" ]; then - kill "$NBD_SNAPSHOT_PID" + local NBD_SNAPSHOT_PID + if [ -f "${TEST_DIR}/qemu-nbd.pid" ]; then + read NBD_SNAPSHOT_PID < "${TEST_DIR}/qemu-nbd.pid" + rm -f "${TEST_DIR}/qemu-nbd.pid" + if [ -n "$NBD_SNAPSHOT_PID" ]; then + kill "$NBD_SNAPSHOT_PID" + fi fi rm -f "$nbd_unix_socket" } @@ -60,7 +66,6 @@ _export_nbd_snapshot() { _cleanup_nbd $QEMU_NBD -v -t -k "$nbd_unix_socket" "$TEST_IMG" -l $1 & - NBD_SNAPSHOT_PID=$! _wait_for_nbd } @@ -68,7 +73,6 @@ _export_nbd_snapshot1() { _cleanup_nbd $QEMU_NBD -v -t -k "$nbd_unix_socket" "$TEST_IMG" -l snapshot.name=$1 & - NBD_SNAPSHOT_PID=$! _wait_for_nbd } diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config index 596bb2b1e5..be99730bb4 100644 --- a/tests/qemu-iotests/common.config +++ b/tests/qemu-iotests/common.config @@ -44,6 +44,8 @@ export HOST_OPTIONS=${HOST_OPTIONS:=local.config} export CHECK_OPTIONS=${CHECK_OPTIONS:="-g auto"} export PWD=`pwd` +export _QEMU_HANDLE=0 + # $1 = prog to look for, $2* = default pathnames if not found in $PATH set_prog_path() { @@ -105,7 +107,12 @@ fi _qemu_wrapper() { - (exec "$QEMU_PROG" $QEMU_OPTIONS "$@") + ( + if [ -n "${QEMU_NEED_PID}" ]; then + echo $BASHPID > "${TEST_DIR}/qemu-${_QEMU_HANDLE}.pid" + fi + exec "$QEMU_PROG" $QEMU_OPTIONS "$@" + ) } _qemu_img_wrapper() @@ -120,7 +127,10 @@ _qemu_io_wrapper() _qemu_nbd_wrapper() { - (exec "$QEMU_NBD_PROG" $QEMU_NBD_OPTIONS "$@") + ( + echo $BASHPID > "${TEST_DIR}/qemu-nbd.pid" + exec "$QEMU_NBD_PROG" $QEMU_NBD_OPTIONS "$@" + ) } export QEMU=_qemu_wrapper diff --git a/tests/qemu-iotests/common.qemu b/tests/qemu-iotests/common.qemu index e3faa53d22..8bf3969418 100644 --- a/tests/qemu-iotests/common.qemu +++ b/tests/qemu-iotests/common.qemu @@ -30,8 +30,6 @@ QEMU_COMM_TIMEOUT=10 QEMU_FIFO_IN="${TEST_DIR}/qmp-in-$$" QEMU_FIFO_OUT="${TEST_DIR}/qmp-out-$$" -QEMU_PID= -_QEMU_HANDLE=0 QEMU_HANDLE=0 # If bash version is >= 4.1, these will be overwritten and dynamic @@ -153,11 +151,11 @@ function _launch_qemu() mkfifo "${fifo_out}" mkfifo "${fifo_in}" + QEMU_NEED_PID='y'\ ${QEMU} -nographic -serial none ${comm} -machine accel=qtest "${@}" \ >"${fifo_out}" \ 2>&1 \ <"${fifo_in}" & - QEMU_PID[${_QEMU_HANDLE}]=$! if [[ "${BASH_VERSINFO[0]}" -ge "5" || ("${BASH_VERSINFO[0]}" -ge "4" && "${BASH_VERSINFO[1]}" -ge "1") ]] @@ -196,10 +194,18 @@ function _cleanup_qemu() # QEMU_PID[], QEMU_IN[], QEMU_OUT[] all use same indices for i in "${!QEMU_OUT[@]}" do - if [ -z "${wait}" ]; then - kill -KILL ${QEMU_PID[$i]} 2>/dev/null + local QEMU_PID + if [ -f "${TEST_DIR}/qemu-${i}.pid" ]; then + read QEMU_PID < "${TEST_DIR}/qemu-${i}.pid" + rm -f "${TEST_DIR}/qemu-${i}.pid" + if [ -z "${wait}" ] && [ -n "${QEMU_PID}" ]; then + kill -KILL ${QEMU_PID} 2>/dev/null + fi + if [ -n "${QEMU_PID}" ]; then + wait ${QEMU_PID} 2>/dev/null # silent kill + fi fi - wait ${QEMU_PID[$i]} 2>/dev/null # silent kill + if [ -n "${wait}" ]; then cat <&${QEMU_OUT[$i]} | _filter_testdir | _filter_qemu \ | _filter_qemu_io | _filter_qmp diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc index 28e4beac15..4878e99650 100644 --- a/tests/qemu-iotests/common.rc +++ b/tests/qemu-iotests/common.rc @@ -154,7 +154,6 @@ _make_test_img() # Start an NBD server on the image file, which is what we'll be talking to if [ $IMGPROTO = "nbd" ]; then eval "$QEMU_NBD -v -t -b 127.0.0.1 -p 10810 -f $IMGFMT $TEST_IMG_FILE &" - QEMU_NBD_PID=$! sleep 1 # FIXME: qemu-nbd needs to be listening before we continue fi } @@ -175,8 +174,11 @@ _cleanup_test_img() case "$IMGPROTO" in nbd) - if [ -n "$QEMU_NBD_PID" ]; then - kill $QEMU_NBD_PID + if [ -f "${TEST_DIR}/qemu-nbd.pid" ]; then + local QEMU_NBD_PID + read QEMU_NBD_PID < "${TEST_DIR}/qemu-nbd.pid" + kill ${QEMU_NBD_PID} + rm -f "${TEST_DIR}/qemu-nbd.pid" fi rm -f "$TEST_IMG_FILE" ;; From e6c17669eb0868013d9a17050d8eb2d598f06067 Mon Sep 17 00:00:00 2001 From: Jeff Cody Date: Fri, 30 Oct 2015 15:25:18 -0400 Subject: [PATCH 35/41] qemu-iotests: fix -valgrind option for check Commit 934659c switched the iotests to run qemu-io from a bash subshell, in order to catch segfaults. This method is incompatible with the current valgrind_qemu_io() bash function. Move the valgrind usage into the exec subshell in _qemu_io_wrapper(), while making sure the original return value is passed back to the caller. Update test output for tests 039, 061, and 137 as it looks for the specific subshell command when the process is terminated. Reported-by: Kevin Wolf Signed-off-by: Jeff Cody Message-id: 0066fd85d26ca641a1c25135ff2479b7985701cf.1446232490.git.jcody@redhat.com Reviewed-by: Max Reitz Signed-off-by: Max Reitz --- tests/qemu-iotests/039.out | 30 +++++++++++++++++++++++++----- tests/qemu-iotests/061.out | 12 ++++++++++-- tests/qemu-iotests/137.out | 6 +++++- tests/qemu-iotests/common | 9 ++------- tests/qemu-iotests/common.config | 18 +++++++++++++++++- tests/qemu-iotests/common.rc | 10 ---------- 6 files changed, 59 insertions(+), 26 deletions(-) diff --git a/tests/qemu-iotests/039.out b/tests/qemu-iotests/039.out index 03a31c5943..32c884694c 100644 --- a/tests/qemu-iotests/039.out +++ b/tests/qemu-iotests/039.out @@ -11,7 +11,11 @@ No errors were found on the image. Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 wrote 512/512 bytes at offset 0 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -./common.config: Killed ( exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" ) +./common.config: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then + exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"; +else + exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"; +fi ) incompatible_features 0x1 ERROR cluster 5 refcount=0 reference=1 ERROR OFLAG_COPIED data cluster: l2_entry=8000000000050000 refcount=0 @@ -46,7 +50,11 @@ read 512/512 bytes at offset 0 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 wrote 512/512 bytes at offset 0 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -./common.config: Killed ( exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" ) +./common.config: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then + exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"; +else + exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"; +fi ) incompatible_features 0x1 ERROR cluster 5 refcount=0 reference=1 Rebuilding refcount structure @@ -60,7 +68,11 @@ incompatible_features 0x0 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 wrote 512/512 bytes at offset 0 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -./common.config: Killed ( exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" ) +./common.config: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then + exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"; +else + exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"; +fi ) incompatible_features 0x0 No errors were found on the image. @@ -79,7 +91,11 @@ No errors were found on the image. Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 wrote 512/512 bytes at offset 0 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -./common.config: Killed ( exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" ) +./common.config: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then + exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"; +else + exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"; +fi ) incompatible_features 0x1 ERROR cluster 5 refcount=0 reference=1 ERROR OFLAG_COPIED data cluster: l2_entry=8000000000050000 refcount=0 @@ -89,7 +105,11 @@ Data may be corrupted, or further writes to the image may corrupt it. Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 wrote 512/512 bytes at offset 0 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -./common.config: Killed ( exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" ) +./common.config: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then + exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"; +else + exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"; +fi ) incompatible_features 0x0 No errors were found on the image. *** done diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out index b16bea95d2..f2598a8f9d 100644 --- a/tests/qemu-iotests/061.out +++ b/tests/qemu-iotests/061.out @@ -57,7 +57,11 @@ No errors were found on the image. Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 wrote 131072/131072 bytes at offset 0 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -./common.config: Killed ( exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" ) +./common.config: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then + exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"; +else + exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"; +fi ) magic 0x514649fb version 3 backing_file_offset 0x0 @@ -215,7 +219,11 @@ No errors were found on the image. Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 wrote 131072/131072 bytes at offset 0 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -./common.config: Killed ( exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" ) +./common.config: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then + exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"; +else + exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"; +fi ) magic 0x514649fb version 3 backing_file_offset 0x0 diff --git a/tests/qemu-iotests/137.out b/tests/qemu-iotests/137.out index cf55a41d8a..88c702cf77 100644 --- a/tests/qemu-iotests/137.out +++ b/tests/qemu-iotests/137.out @@ -31,7 +31,11 @@ Cache clean interval too big Unsupported value 'blubb' for qcow2 option 'overlap-check'. Allowed are any of the following: none, constant, cached, all wrote 512/512 bytes at offset 0 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -./common.config: Killed ( exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" ) +./common.config: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then + exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"; +else + exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"; +fi ) incompatible_features 0x0 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 wrote 65536/65536 bytes at offset 0 diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common index 25c351bbd1..ff84f4b0d6 100644 --- a/tests/qemu-iotests/common +++ b/tests/qemu-iotests/common @@ -41,7 +41,6 @@ sortme=false expunge=true have_test_arg=false randomize=false -valgrind=false cachemode=false rm -f $tmp.list $tmp.tmp $tmp.sed @@ -53,6 +52,7 @@ export CACHEMODE="writeback" export QEMU_IO_OPTIONS="" export CACHEMODE_IS_DEFAULT=true export QEMU_OPTIONS="-nodefaults" +export VALGRIND_QEMU= for r do @@ -278,7 +278,7 @@ testlist options ;; -valgrind) - valgrind=true + VALGRIND_QEMU='y' xpand=false ;; @@ -436,8 +436,3 @@ fi if [ "$IMGPROTO" = "nbd" ] ; then [ "$QEMU_NBD" = "" ] && _fatal "qemu-nbd not found" fi - -if $valgrind; then - export REAL_QEMU_IO="$QEMU_IO_PROG" - export QEMU_IO_PROG=valgrind_qemu_io -fi diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config index be99730bb4..3ed51b8baa 100644 --- a/tests/qemu-iotests/common.config +++ b/tests/qemu-iotests/common.config @@ -122,7 +122,23 @@ _qemu_img_wrapper() _qemu_io_wrapper() { - (exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@") + local VALGRIND_LOGFILE=/tmp/$$.valgrind + local RETVAL + ( + if [ "${VALGRIND_QEMU}" == "y" ]; then + exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" + else + exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" + fi + ) + RETVAL=$? + if [ "${VALGRIND_QEMU}" == "y" ]; then + if [ $RETVAL == 99 ]; then + cat "${VALGRIND_LOGFILE}" + fi + rm -f "${VALGRIND_LOGFILE}" + fi + (exit $RETVAL) } _qemu_nbd_wrapper() diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc index 4878e99650..d9913f8496 100644 --- a/tests/qemu-iotests/common.rc +++ b/tests/qemu-iotests/common.rc @@ -70,16 +70,6 @@ else TEST_IMG=$IMGPROTO:$TEST_DIR/t.$IMGFMT fi -function valgrind_qemu_io() -{ - valgrind --log-file=/tmp/$$.valgrind --error-exitcode=99 $REAL_QEMU_IO "$@" - if [ $? != 0 ]; then - cat /tmp/$$.valgrind - fi - rm -f /tmp/$$.valgrind -} - - _optstr_add() { if [ -n "$1" ]; then From 10f3cd15dd9913f8d959fbd061e6e00c45432093 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Mon, 2 Nov 2015 16:51:53 +0200 Subject: [PATCH 36/41] mirror: block all operations on the target image during the job There's nothing preventing the target image from being used by other operations during the 'drive-mirror' job, so we should block them all until the job is done. Signed-off-by: Alberto Garcia Reviewed-by: Max Reitz Message-id: 82b88fd04cde918a08a6f9a4ab85626d7fd7b502.1446475331.git.berto@igalia.com Signed-off-by: Max Reitz --- block/mirror.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/block/mirror.c b/block/mirror.c index b1252a1b29..60f1cb589d 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -384,6 +384,7 @@ static void mirror_exit(BlockJob *job, void *opaque) aio_context_release(replace_aio_context); } g_free(s->replaces); + bdrv_op_unblock_all(s->target, s->common.blocker); bdrv_unref(s->target); block_job_completed(&s->common, data->ret); g_free(data); @@ -744,6 +745,9 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target, block_job_release(bs); return; } + + bdrv_op_block_all(s->target, s->common.blocker); + bdrv_set_enable_write_cache(s->target, true); if (s->target->blk) { blk_set_on_error(s->target->blk, on_target_error, on_target_error); From f636ae85f3db8ffb987c79715869dba1b8217e8a Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Mon, 2 Nov 2015 16:51:54 +0200 Subject: [PATCH 37/41] block: Add blk_get_refcnt() This function returns the reference count of a given BlockBackend. For convenience, it returns 0 if the BlockBackend pointer is NULL. Signed-off-by: Alberto Garcia Reviewed-by: Max Reitz Message-id: dfdd8a17dbe3288842840636d2cfe5bb895abcb0.1446475331.git.berto@igalia.com Signed-off-by: Max Reitz --- block/block-backend.c | 5 +++++ include/sysemu/block-backend.h | 1 + 2 files changed, 6 insertions(+) diff --git a/block/block-backend.c b/block/block-backend.c index 1ac69824ca..6f9309fef4 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -189,6 +189,11 @@ static void drive_info_del(DriveInfo *dinfo) g_free(dinfo); } +int blk_get_refcnt(BlockBackend *blk) +{ + return blk ? blk->refcnt : 0; +} + /* * Increment @blk's reference count. * @blk must not be null. diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index 40e315b5bb..f4a68e291b 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -65,6 +65,7 @@ BlockBackend *blk_new_with_bs(const char *name, Error **errp); BlockBackend *blk_new_open(const char *name, const char *filename, const char *reference, QDict *options, int flags, Error **errp); +int blk_get_refcnt(BlockBackend *blk); void blk_ref(BlockBackend *blk); void blk_unref(BlockBackend *blk); const char *blk_name(BlockBackend *blk); From 81b936ae70e635557af9ca80922ee69146cb5f4c Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Mon, 2 Nov 2015 16:51:55 +0200 Subject: [PATCH 38/41] block: Add 'x-blockdev-del' QMP command This command is still experimental, hence the name. This is the companion to 'blockdev-add'. It allows deleting a BlockBackend with its associated BlockDriverState tree, or a BlockDriverState that is not attached to any backend. In either case, the command fails if the reference count is greater than 1 or the BlockDriverState has any parents. Signed-off-by: Alberto Garcia Reviewed-by: Max Reitz Message-id: 6cfc148c77aca1da942b094d811bfa3fcf7ac7bb.1446475331.git.berto@igalia.com Signed-off-by: Max Reitz --- blockdev.c | 66 ++++++++++++++++++++++++++++++++++++++++++++ qapi/block-core.json | 32 +++++++++++++++++++-- qmp-commands.hx | 61 ++++++++++++++++++++++++++++++++++++++-- 3 files changed, 155 insertions(+), 4 deletions(-) diff --git a/blockdev.c b/blockdev.c index 3197791c7b..8607df90a9 100644 --- a/blockdev.c +++ b/blockdev.c @@ -3479,6 +3479,72 @@ fail: qmp_output_visitor_cleanup(ov); } +void qmp_x_blockdev_del(bool has_id, const char *id, + bool has_node_name, const char *node_name, Error **errp) +{ + AioContext *aio_context; + BlockBackend *blk; + BlockDriverState *bs; + + if (has_id && has_node_name) { + error_setg(errp, "Only one of id and node-name must be specified"); + return; + } else if (!has_id && !has_node_name) { + error_setg(errp, "No block device specified"); + return; + } + + if (has_id) { + blk = blk_by_name(id); + if (!blk) { + error_setg(errp, "Cannot find block backend %s", id); + return; + } + if (blk_get_refcnt(blk) > 1) { + error_setg(errp, "Block backend %s is in use", id); + return; + } + bs = blk_bs(blk); + aio_context = blk_get_aio_context(blk); + } else { + bs = bdrv_find_node(node_name); + if (!bs) { + error_setg(errp, "Cannot find node %s", node_name); + return; + } + blk = bs->blk; + if (blk) { + error_setg(errp, "Node %s is in use by %s", + node_name, blk_name(blk)); + return; + } + aio_context = bdrv_get_aio_context(bs); + } + + aio_context_acquire(aio_context); + + if (bs) { + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_DRIVE_DEL, errp)) { + goto out; + } + + if (bs->refcnt > 1 || !QLIST_EMPTY(&bs->parents)) { + error_setg(errp, "Block device %s is in use", + bdrv_get_device_or_node_name(bs)); + goto out; + } + } + + if (blk) { + blk_unref(blk); + } else { + bdrv_unref(bs); + } + +out: + aio_context_release(aio_context); +} + BlockJobInfoList *qmp_query_block_jobs(Error **errp) { BlockJobInfoList *head = NULL, **p_next = &head; diff --git a/qapi/block-core.json b/qapi/block-core.json index 083d2cd346..470e86c6df 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -1895,8 +1895,8 @@ # level and no BlockBackend will be created. # # This command is still a work in progress. It doesn't support all -# block drivers, it lacks a matching blockdev-del, and more. Stay -# away from it unless you want to help with its development. +# block drivers among other things. Stay away from it unless you want +# to help with its development. # # @options: block device options for the new device # @@ -1904,6 +1904,34 @@ ## { 'command': 'blockdev-add', 'data': { 'options': 'BlockdevOptions' } } +## +# @x-blockdev-del: +# +# Deletes a block device that has been added using blockdev-add. +# The selected device can be either a block backend or a graph node. +# +# In the former case the backend will be destroyed, along with its +# inserted medium if there's any. The command will fail if the backend +# or its medium are in use. +# +# In the latter case the node will be destroyed. The command will fail +# if the node is attached to a block backend or is otherwise being +# used. +# +# One of @id or @node-name must be specified, but not both. +# +# This command is still a work in progress and is considered +# experimental. Stay away from it unless you want to help with its +# development. +# +# @id: #optional Name of the block backend device to delete. +# +# @node-name: #optional Name of the graph node to delete. +# +# Since: 2.5 +## +{ 'command': 'x-blockdev-del', 'data': { '*id': 'str', '*node-name': 'str' } } + ## # @blockdev-open-tray: # diff --git a/qmp-commands.hx b/qmp-commands.hx index 5fd6f9c0e3..02c0c5bc42 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -3946,8 +3946,8 @@ blockdev-add Add a block device. This command is still a work in progress. It doesn't support all -block drivers, it lacks a matching blockdev-del, and more. Stay away -from it unless you want to help with its development. +block drivers among other things. Stay away from it unless you want +to help with its development. Arguments: @@ -3990,6 +3990,63 @@ Example (2): <- { "return": {} } +EQMP + + { + .name = "x-blockdev-del", + .args_type = "id:s?,node-name:s?", + .mhandler.cmd_new = qmp_marshal_x_blockdev_del, + }, + +SQMP +x-blockdev-del +------------ +Since 2.5 + +Deletes a block device thas has been added using blockdev-add. +The selected device can be either a block backend or a graph node. + +In the former case the backend will be destroyed, along with its +inserted medium if there's any. The command will fail if the backend +or its medium are in use. + +In the latter case the node will be destroyed. The command will fail +if the node is attached to a block backend or is otherwise being +used. + +One of "id" or "node-name" must be specified, but not both. + +This command is still a work in progress and is considered +experimental. Stay away from it unless you want to help with its +development. + +Arguments: + +- "id": Name of the block backend device to delete (json-string, optional) +- "node-name": Name of the graph node to delete (json-string, optional) + +Example: + +-> { "execute": "blockdev-add", + "arguments": { + "options": { + "driver": "qcow2", + "id": "drive0", + "file": { + "driver": "file", + "filename": "test.qcow2" + } + } + } + } + +<- { "return": {} } + +-> { "execute": "x-blockdev-del", + "arguments": { "id": "drive0" } + } +<- { "return": {} } + EQMP { From 342075fd0ec9dce87139d71a81e1dbbe5ab5d021 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Mon, 2 Nov 2015 16:51:56 +0200 Subject: [PATCH 39/41] iotests: Add tests for the x-blockdev-del command Signed-off-by: Alberto Garcia Message-id: 57c3b0d4d0c73ddadd19e5bded9492c359cc4568.1446475331.git.berto@igalia.com Reviewed-by: Max Reitz Signed-off-by: Max Reitz --- tests/qemu-iotests/139 | 414 +++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/139.out | 5 + tests/qemu-iotests/group | 1 + 3 files changed, 420 insertions(+) create mode 100644 tests/qemu-iotests/139 create mode 100644 tests/qemu-iotests/139.out diff --git a/tests/qemu-iotests/139 b/tests/qemu-iotests/139 new file mode 100644 index 0000000000..b5470f709c --- /dev/null +++ b/tests/qemu-iotests/139 @@ -0,0 +1,414 @@ +#!/usr/bin/env python +# +# Test cases for the QMP 'x-blockdev-del' command +# +# Copyright (C) 2015 Igalia, S.L. +# Author: Alberto Garcia +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +import os +import iotests +import time + +base_img = os.path.join(iotests.test_dir, 'base.img') +new_img = os.path.join(iotests.test_dir, 'new.img') + +class TestBlockdevDel(iotests.QMPTestCase): + + def setUp(self): + iotests.qemu_img('create', '-f', iotests.imgfmt, base_img, '1M') + self.vm = iotests.VM() + self.vm.launch() + + def tearDown(self): + self.vm.shutdown() + os.remove(base_img) + if os.path.isfile(new_img): + os.remove(new_img) + + # Check whether a BlockBackend exists + def checkBlockBackend(self, backend, node, must_exist = True): + result = self.vm.qmp('query-block') + backends = filter(lambda x: x['device'] == backend, result['return']) + self.assertLessEqual(len(backends), 1) + self.assertEqual(must_exist, len(backends) == 1) + if must_exist: + if node: + self.assertEqual(backends[0]['inserted']['node-name'], node) + else: + self.assertFalse(backends[0].has_key('inserted')) + + # Check whether a BlockDriverState exists + def checkBlockDriverState(self, node, must_exist = True): + result = self.vm.qmp('query-named-block-nodes') + nodes = filter(lambda x: x['node-name'] == node, result['return']) + self.assertLessEqual(len(nodes), 1) + self.assertEqual(must_exist, len(nodes) == 1) + + # Add a new BlockBackend (with its attached BlockDriverState) + def addBlockBackend(self, backend, node): + file_node = '%s_file' % node + self.checkBlockBackend(backend, node, False) + self.checkBlockDriverState(node, False) + self.checkBlockDriverState(file_node, False) + opts = {'driver': iotests.imgfmt, + 'id': backend, + 'node-name': node, + 'file': {'driver': 'file', + 'node-name': file_node, + 'filename': base_img}} + result = self.vm.qmp('blockdev-add', conv_keys = False, options = opts) + self.assert_qmp(result, 'return', {}) + self.checkBlockBackend(backend, node) + self.checkBlockDriverState(node) + self.checkBlockDriverState(file_node) + + # Add a BlockDriverState without a BlockBackend + def addBlockDriverState(self, node): + file_node = '%s_file' % node + self.checkBlockDriverState(node, False) + self.checkBlockDriverState(file_node, False) + opts = {'driver': iotests.imgfmt, + 'node-name': node, + 'file': {'driver': 'file', + 'node-name': file_node, + 'filename': base_img}} + result = self.vm.qmp('blockdev-add', conv_keys = False, options = opts) + self.assert_qmp(result, 'return', {}) + self.checkBlockDriverState(node) + self.checkBlockDriverState(file_node) + + # Add a BlockDriverState that will be used as overlay for the base_img BDS + def addBlockDriverStateOverlay(self, node): + self.checkBlockDriverState(node, False) + iotests.qemu_img('create', '-f', iotests.imgfmt, + '-b', base_img, new_img, '1M') + opts = {'driver': iotests.imgfmt, + 'node-name': node, + 'backing': '', + 'file': {'driver': 'file', + 'filename': new_img}} + result = self.vm.qmp('blockdev-add', conv_keys = False, options = opts) + self.assert_qmp(result, 'return', {}) + self.checkBlockDriverState(node) + + # Delete a BlockBackend + def delBlockBackend(self, backend, node, expect_error = False, + destroys_media = True): + self.checkBlockBackend(backend, node) + if node: + self.checkBlockDriverState(node) + result = self.vm.qmp('x-blockdev-del', id = backend) + if expect_error: + self.assert_qmp(result, 'error/class', 'GenericError') + if node: + self.checkBlockDriverState(node) + else: + self.assert_qmp(result, 'return', {}) + if node: + self.checkBlockDriverState(node, not destroys_media) + self.checkBlockBackend(backend, node, must_exist = expect_error) + + # Delete a BlockDriverState + def delBlockDriverState(self, node, expect_error = False): + self.checkBlockDriverState(node) + result = self.vm.qmp('x-blockdev-del', node_name = node) + if expect_error: + self.assert_qmp(result, 'error/class', 'GenericError') + else: + self.assert_qmp(result, 'return', {}) + self.checkBlockDriverState(node, expect_error) + + # Add a device model + def addDeviceModel(self, device, backend): + result = self.vm.qmp('device_add', id = device, + driver = 'virtio-blk-pci', drive = backend) + self.assert_qmp(result, 'return', {}) + + # Delete a device model + def delDeviceModel(self, device): + result = self.vm.qmp('device_del', id = device) + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('system_reset') + self.assert_qmp(result, 'return', {}) + + device_path = '/machine/peripheral/%s/virtio-backend' % device + event = self.vm.event_wait(name="DEVICE_DELETED", + match={'data': {'path': device_path}}) + self.assertNotEqual(event, None) + + event = self.vm.event_wait(name="DEVICE_DELETED", + match={'data': {'device': device}}) + self.assertNotEqual(event, None) + + # Remove a BlockDriverState + def ejectDrive(self, backend, node, expect_error = False, + destroys_media = True): + self.checkBlockBackend(backend, node) + self.checkBlockDriverState(node) + result = self.vm.qmp('eject', device = backend) + if expect_error: + self.assert_qmp(result, 'error/class', 'GenericError') + self.checkBlockDriverState(node) + self.checkBlockBackend(backend, node) + else: + self.assert_qmp(result, 'return', {}) + self.checkBlockDriverState(node, not destroys_media) + self.checkBlockBackend(backend, None) + + # Insert a BlockDriverState + def insertDrive(self, backend, node): + self.checkBlockBackend(backend, None) + self.checkBlockDriverState(node) + result = self.vm.qmp('blockdev-insert-medium', + device = backend, node_name = node) + self.assert_qmp(result, 'return', {}) + self.checkBlockBackend(backend, node) + self.checkBlockDriverState(node) + + # Create a snapshot using 'blockdev-snapshot-sync' + def createSnapshotSync(self, node, overlay): + self.checkBlockDriverState(node) + self.checkBlockDriverState(overlay, False) + opts = {'node-name': node, + 'snapshot-file': new_img, + 'snapshot-node-name': overlay, + 'format': iotests.imgfmt} + result = self.vm.qmp('blockdev-snapshot-sync', conv_keys=False, **opts) + self.assert_qmp(result, 'return', {}) + self.checkBlockDriverState(node) + self.checkBlockDriverState(overlay) + + # Create a snapshot using 'blockdev-snapshot' + def createSnapshot(self, node, overlay): + self.checkBlockDriverState(node) + self.checkBlockDriverState(overlay) + result = self.vm.qmp('blockdev-snapshot', + node = node, overlay = overlay) + self.assert_qmp(result, 'return', {}) + self.checkBlockDriverState(node) + self.checkBlockDriverState(overlay) + + # Create a mirror + def createMirror(self, backend, node, new_node): + self.checkBlockBackend(backend, node) + self.checkBlockDriverState(new_node, False) + opts = {'device': backend, + 'target': new_img, + 'node-name': new_node, + 'sync': 'top', + 'format': iotests.imgfmt} + result = self.vm.qmp('drive-mirror', conv_keys=False, **opts) + self.assert_qmp(result, 'return', {}) + self.checkBlockBackend(backend, node) + self.checkBlockDriverState(new_node) + + # Complete an existing block job + def completeBlockJob(self, backend, node_before, node_after): + self.checkBlockBackend(backend, node_before) + result = self.vm.qmp('block-job-complete', device=backend) + self.assert_qmp(result, 'return', {}) + self.wait_until_completed(backend) + self.checkBlockBackend(backend, node_after) + + # Add a BlkDebug node + # Note that the purpose of this is to test the x-blockdev-del + # sanity checks, not to create a usable blkdebug drive + def addBlkDebug(self, debug, node): + self.checkBlockDriverState(node, False) + self.checkBlockDriverState(debug, False) + image = {'driver': iotests.imgfmt, + 'node-name': node, + 'file': {'driver': 'file', + 'filename': base_img}} + opts = {'driver': 'blkdebug', + 'node-name': debug, + 'image': image} + result = self.vm.qmp('blockdev-add', conv_keys = False, options = opts) + self.assert_qmp(result, 'return', {}) + self.checkBlockDriverState(node) + self.checkBlockDriverState(debug) + + # Add a BlkVerify node + # Note that the purpose of this is to test the x-blockdev-del + # sanity checks, not to create a usable blkverify drive + def addBlkVerify(self, blkverify, test, raw): + self.checkBlockDriverState(test, False) + self.checkBlockDriverState(raw, False) + self.checkBlockDriverState(blkverify, False) + iotests.qemu_img('create', '-f', iotests.imgfmt, new_img, '1M') + node_0 = {'driver': iotests.imgfmt, + 'node-name': test, + 'file': {'driver': 'file', + 'filename': base_img}} + node_1 = {'driver': iotests.imgfmt, + 'node-name': raw, + 'file': {'driver': 'file', + 'filename': new_img}} + opts = {'driver': 'blkverify', + 'node-name': blkverify, + 'test': node_0, + 'raw': node_1} + result = self.vm.qmp('blockdev-add', conv_keys = False, options = opts) + self.assert_qmp(result, 'return', {}) + self.checkBlockDriverState(test) + self.checkBlockDriverState(raw) + self.checkBlockDriverState(blkverify) + + # Add a Quorum node + def addQuorum(self, quorum, child0, child1): + self.checkBlockDriverState(child0, False) + self.checkBlockDriverState(child1, False) + self.checkBlockDriverState(quorum, False) + iotests.qemu_img('create', '-f', iotests.imgfmt, new_img, '1M') + child_0 = {'driver': iotests.imgfmt, + 'node-name': child0, + 'file': {'driver': 'file', + 'filename': base_img}} + child_1 = {'driver': iotests.imgfmt, + 'node-name': child1, + 'file': {'driver': 'file', + 'filename': new_img}} + opts = {'driver': 'quorum', + 'node-name': quorum, + 'vote-threshold': 1, + 'children': [ child_0, child_1 ]} + result = self.vm.qmp('blockdev-add', conv_keys = False, options = opts) + self.assert_qmp(result, 'return', {}) + self.checkBlockDriverState(child0) + self.checkBlockDriverState(child1) + self.checkBlockDriverState(quorum) + + ######################## + # The tests start here # + ######################## + + def testWrongParameters(self): + self.addBlockBackend('drive0', 'node0') + result = self.vm.qmp('x-blockdev-del') + self.assert_qmp(result, 'error/class', 'GenericError') + result = self.vm.qmp('x-blockdev-del', id='drive0', node_name='node0') + self.assert_qmp(result, 'error/class', 'GenericError') + self.delBlockBackend('drive0', 'node0') + + def testBlockBackend(self): + self.addBlockBackend('drive0', 'node0') + # You cannot delete a BDS that is attached to a backend + self.delBlockDriverState('node0', expect_error = True) + self.delBlockBackend('drive0', 'node0') + + def testBlockDriverState(self): + self.addBlockDriverState('node0') + # You cannot delete a file BDS directly + self.delBlockDriverState('node0_file', expect_error = True) + self.delBlockDriverState('node0') + + def testEject(self): + self.addBlockBackend('drive0', 'node0') + self.ejectDrive('drive0', 'node0') + self.delBlockBackend('drive0', None) + + def testDeviceModel(self): + self.addBlockBackend('drive0', 'node0') + self.addDeviceModel('device0', 'drive0') + self.ejectDrive('drive0', 'node0', expect_error = True) + self.delBlockBackend('drive0', 'node0', expect_error = True) + self.delDeviceModel('device0') + self.delBlockBackend('drive0', 'node0') + + def testAttachMedia(self): + # This creates a BlockBackend and removes its media + self.addBlockBackend('drive0', 'node0') + self.ejectDrive('drive0', 'node0') + # This creates a new BlockDriverState and inserts it into the backend + self.addBlockDriverState('node1') + self.insertDrive('drive0', 'node1') + # The backend can't be removed: the new BDS has an extra reference + self.delBlockBackend('drive0', 'node1', expect_error = True) + self.delBlockDriverState('node1', expect_error = True) + # The BDS still exists after being ejected, but now it can be removed + self.ejectDrive('drive0', 'node1', destroys_media = False) + self.delBlockDriverState('node1') + self.delBlockBackend('drive0', None) + + def testSnapshotSync(self): + self.addBlockBackend('drive0', 'node0') + self.createSnapshotSync('node0', 'overlay0') + # This fails because node0 is now being used as a backing image + self.delBlockDriverState('node0', expect_error = True) + # This succeeds because overlay0 only has the backend reference + self.delBlockBackend('drive0', 'overlay0') + self.checkBlockDriverState('node0', False) + + def testSnapshot(self): + self.addBlockBackend('drive0', 'node0') + self.addBlockDriverStateOverlay('overlay0') + self.createSnapshot('node0', 'overlay0') + self.delBlockBackend('drive0', 'overlay0', expect_error = True) + self.delBlockDriverState('node0', expect_error = True) + self.delBlockDriverState('overlay0', expect_error = True) + self.ejectDrive('drive0', 'overlay0', destroys_media = False) + self.delBlockBackend('drive0', None) + self.delBlockDriverState('node0', expect_error = True) + self.delBlockDriverState('overlay0') + self.checkBlockDriverState('node0', False) + + def testMirror(self): + self.addBlockBackend('drive0', 'node0') + self.createMirror('drive0', 'node0', 'mirror0') + # The block job prevents removing the device + self.delBlockBackend('drive0', 'node0', expect_error = True) + self.delBlockDriverState('node0', expect_error = True) + self.delBlockDriverState('mirror0', expect_error = True) + self.wait_ready('drive0') + self.completeBlockJob('drive0', 'node0', 'mirror0') + self.assert_no_active_block_jobs() + self.checkBlockDriverState('node0', False) + # This succeeds because the backend now points to mirror0 + self.delBlockBackend('drive0', 'mirror0') + + def testBlkDebug(self): + self.addBlkDebug('debug0', 'node0') + # 'node0' is used by the blkdebug node + self.delBlockDriverState('node0', expect_error = True) + # But we can remove the blkdebug node directly + self.delBlockDriverState('debug0') + self.checkBlockDriverState('node0', False) + + def testBlkVerify(self): + self.addBlkVerify('verify0', 'node0', 'node1') + # We cannot remove the children of a blkverify device + self.delBlockDriverState('node0', expect_error = True) + self.delBlockDriverState('node1', expect_error = True) + # But we can remove the blkverify node directly + self.delBlockDriverState('verify0') + self.checkBlockDriverState('node0', False) + self.checkBlockDriverState('node1', False) + + def testQuorum(self): + self.addQuorum('quorum0', 'node0', 'node1') + # We cannot remove the children of a Quorum device + self.delBlockDriverState('node0', expect_error = True) + self.delBlockDriverState('node1', expect_error = True) + # But we can remove the Quorum node directly + self.delBlockDriverState('quorum0') + self.checkBlockDriverState('node0', False) + self.checkBlockDriverState('node1', False) + + +if __name__ == '__main__': + iotests.main(supported_fmts=["qcow2"]) diff --git a/tests/qemu-iotests/139.out b/tests/qemu-iotests/139.out new file mode 100644 index 0000000000..281b69efea --- /dev/null +++ b/tests/qemu-iotests/139.out @@ -0,0 +1,5 @@ +............ +---------------------------------------------------------------------- +Ran 12 tests + +OK diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index a6a61aeddd..c69265decd 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -138,3 +138,4 @@ 135 rw auto 137 rw auto 138 rw auto quick +139 rw auto quick From a99dfb45f26bface6830ee5465e57bcdbc53c6c8 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 4 Nov 2015 18:16:24 +0100 Subject: [PATCH 40/41] qcow2: Fix qcow2_get_cluster_offset() for zero clusters When searching for contiguous zero clusters, we only need to check the cluster type. Before this patch, an increasing offset (L2E_OFFSET_MASK) was expected, so that the function never returned more than a single zero cluster in practice. This patch fixes it to actually return as many contiguous zero clusters as it can. Cc: qemu-stable@nongnu.org Signed-off-by: Kevin Wolf Message-id: 1446657384-5907-1-git-send-email-kwolf@redhat.com Signed-off-by: Max Reitz --- block/qcow2-cluster.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 67be0ce2c9..24a60e2236 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -312,7 +312,7 @@ static int count_contiguous_clusters(int nb_clusters, int cluster_size, if (!offset) return 0; - assert(qcow2_get_cluster_type(first_entry) != QCOW2_CLUSTER_COMPRESSED); + assert(qcow2_get_cluster_type(first_entry) == QCOW2_CLUSTER_NORMAL); for (i = 0; i < nb_clusters; i++) { uint64_t l2_entry = be64_to_cpu(l2_table[i]) & mask; @@ -324,14 +324,16 @@ static int count_contiguous_clusters(int nb_clusters, int cluster_size, return i; } -static int count_contiguous_free_clusters(int nb_clusters, uint64_t *l2_table) +static int count_contiguous_clusters_by_type(int nb_clusters, + uint64_t *l2_table, + int wanted_type) { int i; for (i = 0; i < nb_clusters; i++) { int type = qcow2_get_cluster_type(be64_to_cpu(l2_table[i])); - if (type != QCOW2_CLUSTER_UNALLOCATED) { + if (type != wanted_type) { break; } } @@ -554,13 +556,14 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t offset, ret = -EIO; goto fail; } - c = count_contiguous_clusters(nb_clusters, s->cluster_size, - &l2_table[l2_index], QCOW_OFLAG_ZERO); + c = count_contiguous_clusters_by_type(nb_clusters, &l2_table[l2_index], + QCOW2_CLUSTER_ZERO); *cluster_offset = 0; break; case QCOW2_CLUSTER_UNALLOCATED: /* how many empty clusters ? */ - c = count_contiguous_free_clusters(nb_clusters, &l2_table[l2_index]); + c = count_contiguous_clusters_by_type(nb_clusters, &l2_table[l2_index], + QCOW2_CLUSTER_UNALLOCATED); *cluster_offset = 0; break; case QCOW2_CLUSTER_NORMAL: From 92e68987745b0f89f04d29fe1d0c821010d58ea6 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Tue, 10 Nov 2015 18:28:11 +0200 Subject: [PATCH 41/41] iotests: Check for quorum support in test 139 The quorum driver is always built in, but it is disabled during run-time if there's no SHA256 support available (see commit e94867e). This patch skips the quorum test in iotest 139 in that case. Signed-off-by: Alberto Garcia Message-id: 1447172891-20410-1-git-send-email-berto@igalia.com Signed-off-by: Max Reitz --- tests/qemu-iotests/139 | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/qemu-iotests/139 b/tests/qemu-iotests/139 index b5470f709c..42f78c7baa 100644 --- a/tests/qemu-iotests/139 +++ b/tests/qemu-iotests/139 @@ -400,6 +400,8 @@ class TestBlockdevDel(iotests.QMPTestCase): self.checkBlockDriverState('node1', False) def testQuorum(self): + if not 'quorum' in iotests.qemu_img_pipe('--help'): + return self.addQuorum('quorum0', 'node0', 'node1') # We cannot remove the children of a Quorum device self.delBlockDriverState('node0', expect_error = True)