From 5cf87fd68e1ea06013594682cec6feb5488fb773 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 31 Mar 2016 15:07:43 +0200 Subject: [PATCH 1/4] block: forbid x-blockdev-del from acting on DriveInfo Failing on -drive/drive_add created BlockBackends was a requirement for x-blockdev-del, but it sneaked through the patch review. Let's fix it now. Example: $ x86_64-softmmu/qemu-system-x86_64 -drive if=none,file=null-co://,id=null -qmp stdio >> {'execute':'qmp_capabilities'} << {"return": {}} >> {'execute':'x-blockdev-del','arguments':{'id':'null'}} << {"error": {"class": "GenericError", "desc": "Deleting block backend added with drive-add is not supported"}} And without a DriveInfo: >> { "execute": "blockdev-add", "arguments": { "options": { "driver":"null-co", "id":"null2"}}} << {"return": {}} >> {'execute':'x-blockdev-del','arguments':{'id':'null2'}} << {"return": {}} Suggested-by: Kevin Wolf Signed-off-by: Paolo Bonzini Signed-off-by: Kevin Wolf --- blockdev.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/blockdev.c b/blockdev.c index e50e8eabd094..332068a8375f 100644 --- a/blockdev.c +++ b/blockdev.c @@ -4027,6 +4027,11 @@ void qmp_x_blockdev_del(bool has_id, const char *id, error_setg(errp, "Cannot find block backend %s", id); return; } + if (blk_legacy_dinfo(blk)) { + error_setg(errp, "Deleting block backend added with drive-add" + " is not supported"); + return; + } if (blk_get_refcnt(blk) > 1) { error_setg(errp, "Block backend %s is in use", id); return; From 76b223200ef4fb09dd87f0e213159795eb68e7a5 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 4 Apr 2016 17:11:13 +0200 Subject: [PATCH 2/4] block: Forbid I/O throttling on nodes with multiple parents for 2.6 As the patches to move I/O throttling to BlockBackend didn't make it in time for the 2.6 release, but the release adds new ways of configuring VMs whose behaviour would change once the move is done, we need to outlaw such configurations temporarily. The problem exists whenever a BDS has more users than just its BB, for example it is used as a backing file for another node. (This wasn't possible in 2.5 yet as we introduced node references to specify a backing file only recently.) In these cases, the throttling would apply to these other users now, but after moving throttling to the BlockBackend the other users wouldn't be throttled any more. This patch prevents making new references to a throttled node as well as using monitor commands to throttle a node with multiple parents. Compared to 2.5 this changes behaviour in some corner cases where references were allowed before, like bs->file or Quorum children. It seems reasonable to assume that users didn't use I/O throttling on such low level nodes. With the upcoming move of throttling into BlockBackend, such configurations won't be possible anyway. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block.c | 7 +++++++ blockdev.c | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/block.c b/block.c index d36eb75be96a..d4939b49bf05 100644 --- a/block.c +++ b/block.c @@ -1526,6 +1526,13 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename, if (!bs) { return -ENODEV; } + + if (bs->throttle_state) { + error_setg(errp, "Cannot reference an existing block device for " + "which I/O throttling is enabled"); + return -EINVAL; + } + bdrv_ref(bs); *pbs = bs; return 0; diff --git a/blockdev.c b/blockdev.c index 332068a8375f..f1f520a2653b 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2661,6 +2661,13 @@ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd, goto out; } + /* The BlockBackend must be the only parent */ + assert(QLIST_FIRST(&bs->parents)); + if (QLIST_NEXT(QLIST_FIRST(&bs->parents), next_parent)) { + error_setg(errp, "Cannot throttle device with multiple parents"); + goto out; + } + throttle_config_init(&cfg); cfg.buckets[THROTTLE_BPS_TOTAL].avg = bps; cfg.buckets[THROTTLE_BPS_READ].avg = bps_rd; From 08db36f6ec29aff59c11c0f7524c2481d8a27636 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Fri, 1 Apr 2016 17:56:33 +0800 Subject: [PATCH 3/4] qemu-iotests: 149: Use "/usr/bin/env python" Do the same as other scripts, to pick the correct interpreter between python2 and python3 from the environment. Signed-off-by: Fam Zheng Message-id: 1459504593-2692-1-git-send-email-famz@redhat.com Signed-off-by: Max Reitz --- tests/qemu-iotests/149 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemu-iotests/149 b/tests/qemu-iotests/149 index bb5811d93b44..52e23d2946e2 100755 --- a/tests/qemu-iotests/149 +++ b/tests/qemu-iotests/149 @@ -1,4 +1,4 @@ -#!/usr/bin/python +#!/usr/bin/env python # # Copyright (C) 2016 Red Hat, Inc. # From 95c3df5a24e2f18129b58691c2ebaf0d86808525 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Fri, 1 Apr 2016 09:57:02 -0600 Subject: [PATCH 4/4] crypto: Avoid memory leak on failure Commit 7836857 introduced a memory leak due to invalid use of Error vs. visit_type_end(). If visiting the intermediate members fails, we clear the error and unconditionally use visit_end_struct() on the same error object; but if that cleanup succeeds, we then skip the qapi_free call. Until a later patch adds visit_check_struct(), the only safe approach is to use two separate error objects. Signed-off-by: Eric Blake Message-id: 1459526222-30052-1-git-send-email-eblake@redhat.com Signed-off-by: Max Reitz --- block/crypto.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/block/crypto.c b/block/crypto.c index be3498581c55..1903e84fbd85 100644 --- a/block/crypto.c +++ b/block/crypto.c @@ -196,6 +196,7 @@ block_crypto_open_opts_init(QCryptoBlockFormat format, OptsVisitor *ov; QCryptoBlockOpenOptions *ret = NULL; Error *local_err = NULL; + Error *end_err = NULL; ret = g_new0(QCryptoBlockOpenOptions, 1); ret->format = format; @@ -218,10 +219,9 @@ block_crypto_open_opts_init(QCryptoBlockFormat format, error_setg(&local_err, "Unsupported block format %d", format); break; } - error_propagate(errp, local_err); - local_err = NULL; - visit_end_struct(opts_get_visitor(ov), &local_err); + visit_end_struct(opts_get_visitor(ov), &end_err); + error_propagate(&local_err, end_err); out: if (local_err) { @@ -242,6 +242,7 @@ block_crypto_create_opts_init(QCryptoBlockFormat format, OptsVisitor *ov; QCryptoBlockCreateOptions *ret = NULL; Error *local_err = NULL; + Error *end_err = NULL; ret = g_new0(QCryptoBlockCreateOptions, 1); ret->format = format; @@ -264,10 +265,9 @@ block_crypto_create_opts_init(QCryptoBlockFormat format, error_setg(&local_err, "Unsupported block format %d", format); break; } - error_propagate(errp, local_err); - local_err = NULL; - visit_end_struct(opts_get_visitor(ov), &local_err); + visit_end_struct(opts_get_visitor(ov), &end_err); + error_propagate(&local_err, end_err); out: if (local_err) {