aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPeter Maydell <peter.maydell@linaro.org>2017-08-15 18:17:02 +0100
committerPeter Maydell <peter.maydell@linaro.org>2017-08-15 18:17:02 +0100
commit09920c53549f6097a007bc6081b6f03b9dc13d9c (patch)
treef89faaaa418a7922c811c01ab29e4d83eef73ede
parent72b384f4a7d1d120a2591fc2019cec6ae50d7998 (diff)
parent72b6ffc76653214b69a94a7b1643ff80df134486 (diff)
Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2017-08-15' into staging
nbd patches for 2017-08-15 - Eric Blake: nbd: Fix trace message for disconnect - Stefan Hajnoczi: qemu-iotests: step clock after each test iteration - Fam Zheng: 0/4 block: Fix non-shared storage migration - Eric Blake: nbd-client: Fix regression when server sends garbage # gpg: Signature made Tue 15 Aug 2017 16:06:02 BST # gpg: using RSA key 0xA7A16B4A2527436A # gpg: Good signature from "Eric Blake <eblake@redhat.com>" # gpg: aka "Eric Blake (Free Software Programmer) <ebb9@byu.net>" # gpg: aka "[jpeg image of size 6874]" # Primary key fingerprint: 71C2 CC22 B1C4 6029 27D2 F3AA A7A1 6B4A 2527 436A * remotes/ericb/tags/pull-nbd-2017-08-15: nbd-client: Fix regression when server sends garbage iotests: Add non-shared storage migration case 192 block-backend: Defer shared_perm tightening migration completion nbd: Fix order of bdrv_set_perm and bdrv_invalidate_cache stubs: Add vm state change handler stubs qemu-iotests: step clock after each test iteration nbd: Fix trace message for disconnect Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
-rw-r--r--block/block-backend.c41
-rw-r--r--block/nbd-client.c17
-rw-r--r--block/nbd-client.h1
-rw-r--r--nbd/common.c2
-rw-r--r--nbd/server.c20
-rw-r--r--stubs/Makefile.objs1
-rw-r--r--stubs/change-state-handler.c14
-rwxr-xr-xtests/qemu-iotests/0934
-rwxr-xr-xtests/qemu-iotests/19263
-rw-r--r--tests/qemu-iotests/192.out7
-rw-r--r--tests/qemu-iotests/group1
11 files changed, 157 insertions, 14 deletions
diff --git a/block/block-backend.c b/block/block-backend.c
index 968438c149..e9798e897d 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -20,6 +20,7 @@
#include "qapi-event.h"
#include "qemu/id.h"
#include "trace.h"
+#include "migration/misc.h"
/* Number of coroutines to reserve per attached device model */
#define COROUTINE_POOL_RESERVATION 64
@@ -68,6 +69,7 @@ struct BlockBackend {
NotifierList remove_bs_notifiers, insert_bs_notifiers;
int quiesce_counter;
+ VMChangeStateEntry *vmsh;
};
typedef struct BlockBackendAIOCB {
@@ -129,6 +131,23 @@ static const char *blk_root_get_name(BdrvChild *child)
return blk_name(child->opaque);
}
+static void blk_vm_state_changed(void *opaque, int running, RunState state)
+{
+ Error *local_err = NULL;
+ BlockBackend *blk = opaque;
+
+ if (state == RUN_STATE_INMIGRATE) {
+ return;
+ }
+
+ qemu_del_vm_change_state_handler(blk->vmsh);
+ blk->vmsh = NULL;
+ blk_set_perm(blk, blk->perm, blk->shared_perm, &local_err);
+ if (local_err) {
+ error_report_err(local_err);
+ }
+}
+
/*
* Notifies the user of the BlockBackend that migration has completed. qdev
* devices can tighten their permissions in response (specifically revoke
@@ -147,6 +166,24 @@ static void blk_root_activate(BdrvChild *child, Error **errp)
blk->disable_perm = false;
+ blk_set_perm(blk, blk->perm, BLK_PERM_ALL, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ blk->disable_perm = true;
+ return;
+ }
+
+ if (runstate_check(RUN_STATE_INMIGRATE)) {
+ /* Activation can happen when migration process is still active, for
+ * example when nbd_server_add is called during non-shared storage
+ * migration. Defer the shared_perm update to migration completion. */
+ if (!blk->vmsh) {
+ blk->vmsh = qemu_add_vm_change_state_handler(blk_vm_state_changed,
+ blk);
+ }
+ return;
+ }
+
blk_set_perm(blk, blk->perm, blk->shared_perm, &local_err);
if (local_err) {
error_propagate(errp, local_err);
@@ -291,6 +328,10 @@ static void blk_delete(BlockBackend *blk)
if (blk->root) {
blk_remove_bs(blk);
}
+ if (blk->vmsh) {
+ qemu_del_vm_change_state_handler(blk->vmsh);
+ blk->vmsh = NULL;
+ }
assert(QLIST_EMPTY(&blk->remove_bs_notifiers.notifiers));
assert(QLIST_EMPTY(&blk->insert_bs_notifiers.notifiers));
QTAILQ_REMOVE(&block_backends, blk, link);
diff --git a/block/nbd-client.c b/block/nbd-client.c
index 25dd28406b..422ecb4307 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -73,7 +73,7 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
int ret;
Error *local_err = NULL;
- for (;;) {
+ while (!s->quit) {
assert(s->reply.handle == 0);
ret = nbd_receive_reply(s->ioc, &s->reply, &local_err);
if (ret < 0) {
@@ -107,6 +107,9 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
qemu_coroutine_yield();
}
+ if (ret < 0) {
+ s->quit = true;
+ }
nbd_recv_coroutines_enter_all(s);
s->read_reply_co = NULL;
}
@@ -135,6 +138,10 @@ static int nbd_co_send_request(BlockDriverState *bs,
assert(i < MAX_NBD_REQUESTS);
request->handle = INDEX_TO_HANDLE(s, i);
+ if (s->quit) {
+ qemu_co_mutex_unlock(&s->send_mutex);
+ return -EIO;
+ }
if (!s->ioc) {
qemu_co_mutex_unlock(&s->send_mutex);
return -EPIPE;
@@ -143,7 +150,7 @@ static int nbd_co_send_request(BlockDriverState *bs,
if (qiov) {
qio_channel_set_cork(s->ioc, true);
rc = nbd_send_request(s->ioc, request);
- if (rc >= 0) {
+ if (rc >= 0 && !s->quit) {
ret = nbd_rwv(s->ioc, qiov->iov, qiov->niov, request->len, false,
NULL);
if (ret != request->len) {
@@ -154,6 +161,9 @@ static int nbd_co_send_request(BlockDriverState *bs,
} else {
rc = nbd_send_request(s->ioc, request);
}
+ if (rc < 0) {
+ s->quit = true;
+ }
qemu_co_mutex_unlock(&s->send_mutex);
return rc;
}
@@ -168,8 +178,7 @@ static void nbd_co_receive_reply(NBDClientSession *s,
/* Wait until we're woken up by nbd_read_reply_entry. */
qemu_coroutine_yield();
*reply = s->reply;
- if (reply->handle != request->handle ||
- !s->ioc) {
+ if (reply->handle != request->handle || !s->ioc || s->quit) {
reply->error = EIO;
} else {
if (qiov && reply->error == 0) {
diff --git a/block/nbd-client.h b/block/nbd-client.h
index df80771357..1935ffbcaa 100644
--- a/block/nbd-client.h
+++ b/block/nbd-client.h
@@ -29,6 +29,7 @@ typedef struct NBDClientSession {
Coroutine *recv_coroutine[MAX_NBD_REQUESTS];
NBDReply reply;
+ bool quit;
} NBDClientSession;
NBDClientSession *nbd_get_client_session(BlockDriverState *bs);
diff --git a/nbd/common.c b/nbd/common.c
index a2f28f2eec..e288d1b972 100644
--- a/nbd/common.c
+++ b/nbd/common.c
@@ -182,7 +182,7 @@ const char *nbd_cmd_lookup(uint16_t cmd)
case NBD_CMD_WRITE:
return "write";
case NBD_CMD_DISC:
- return "discard";
+ return "disconnect";
case NBD_CMD_FLUSH:
return "flush";
case NBD_CMD_TRIM:
diff --git a/nbd/server.c b/nbd/server.c
index 82a78bf439..993ade30bb 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1045,11 +1045,22 @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset, off_t size,
bool writethrough, BlockBackend *on_eject_blk,
Error **errp)
{
+ AioContext *ctx;
BlockBackend *blk;
NBDExport *exp = g_malloc0(sizeof(NBDExport));
uint64_t perm;
int ret;
+ /*
+ * NBD exports are used for non-shared storage migration. Make sure
+ * that BDRV_O_INACTIVE is cleared and the image is ready for write
+ * access since the export could be available before migration handover.
+ */
+ ctx = bdrv_get_aio_context(bs);
+ aio_context_acquire(ctx);
+ bdrv_invalidate_cache(bs, NULL);
+ aio_context_release(ctx);
+
/* Don't allow resize while the NBD server is running, otherwise we don't
* care what happens with the node. */
perm = BLK_PERM_CONSISTENT_READ;
@@ -1087,15 +1098,6 @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset, off_t size,
exp->eject_notifier.notify = nbd_eject_notifier;
blk_add_remove_bs_notifier(on_eject_blk, &exp->eject_notifier);
}
-
- /*
- * NBD exports are used for non-shared storage migration. Make sure
- * that BDRV_O_INACTIVE is cleared and the image is ready for write
- * access since the export could be available before migration handover.
- */
- aio_context_acquire(exp->ctx);
- blk_invalidate_cache(blk, NULL);
- aio_context_release(exp->ctx);
return exp;
fail:
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index f5b47bfd74..e69c217aff 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -19,6 +19,7 @@ stub-obj-y += is-daemonized.o
stub-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
stub-obj-y += machine-init-done.o
stub-obj-y += migr-blocker.o
+stub-obj-y += change-state-handler.o
stub-obj-y += monitor.o
stub-obj-y += notify-event.o
stub-obj-y += qtest.o
diff --git a/stubs/change-state-handler.c b/stubs/change-state-handler.c
new file mode 100644
index 0000000000..01b1c6986d
--- /dev/null
+++ b/stubs/change-state-handler.c
@@ -0,0 +1,14 @@
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "sysemu/sysemu.h"
+
+VMChangeStateEntry *qemu_add_vm_change_state_handler(VMChangeStateHandler *cb,
+ void *opaque)
+{
+ return NULL;
+}
+
+void qemu_del_vm_change_state_handler(VMChangeStateEntry *e)
+{
+ /* Nothing to do. */
+}
diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093
index 2ed393a548..ef3997206b 100755
--- a/tests/qemu-iotests/093
+++ b/tests/qemu-iotests/093
@@ -133,6 +133,10 @@ class ThrottleTestCase(iotests.QMPTestCase):
self.assertTrue(check_limit(params['iops_rd'], rd_iops))
self.assertTrue(check_limit(params['iops_wr'], wr_iops))
+ # Allow remaining requests to finish. We submitted twice as many to
+ # ensure the throttle limit is reached.
+ self.vm.qtest("clock_step %d" % ns)
+
# Connect N drives to a VM and test I/O in all of them
def test_all(self):
params = {"bps": 4096,
diff --git a/tests/qemu-iotests/192 b/tests/qemu-iotests/192
new file mode 100755
index 0000000000..b50a2c0c8e
--- /dev/null
+++ b/tests/qemu-iotests/192
@@ -0,0 +1,63 @@
+#!/bin/bash
+#
+# Test NBD export with -incoming (non-shared storage migration use case from
+# libvirt)
+#
+# Copyright (C) 2017 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 <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=famz@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+status=1 # failure is the default!
+
+_cleanup()
+{
+ _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt generic
+_supported_proto file
+_supported_os Linux
+
+if [ "$QEMU_DEFAULT_MACHINE" != "pc" ]; then
+ _notrun "Requires a PC machine"
+fi
+
+size=64M
+_make_test_img $size
+
+{
+echo "nbd_server_start unix:$TEST_DIR/nbd"
+echo "nbd_server_add -w drive0"
+echo "q"
+} | $QEMU -nodefaults -display none -monitor stdio \
+ -drive format=$IMGFMT,file=$TEST_IMG,if=ide,id=drive0 \
+ -incoming defer 2>&1 | _filter_testdir | _filter_qemu | _filter_hmp
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/192.out b/tests/qemu-iotests/192.out
new file mode 100644
index 0000000000..1e0be4c4d7
--- /dev/null
+++ b/tests/qemu-iotests/192.out
@@ -0,0 +1,7 @@
+QA output created by 192
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) nbd_server_start unix:TEST_DIR/nbd
+(qemu) nbd_server_add -w drive0
+(qemu) q
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 1848077932..afbdc427ea 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -186,3 +186,4 @@
188 rw auto quick
189 rw auto
190 rw auto quick
+192 rw auto quick