From 95667c3be0c9f5fc62f58fe845879250f63f7d32 Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Wed, 10 Jul 2019 16:57:44 +0200 Subject: nvme: Set number of queues later in nvme_init() When creating the admin queue in nvme_init() the variable that holds the number of queues created is modified before actual queue creation. This is a problem because if creating the queue fails then the variable is left in inconsistent state. This was actually observed when I tried to hotplug a nvme disk. The control got to nvme_file_open() which called nvme_init() which failed and thus nvme_close() was called which in turn called nvme_free_queue_pair() with queue being NULL. This lead to an instant crash: #0 0x000055d9507ec211 in nvme_free_queue_pair (bs=0x55d952ddb880, q=0x0) at block/nvme.c:164 #1 0x000055d9507ee180 in nvme_close (bs=0x55d952ddb880) at block/nvme.c:729 #2 0x000055d9507ee3d5 in nvme_file_open (bs=0x55d952ddb880, options=0x55d952bb1410, flags=147456, errp=0x7ffd8e19e200) at block/nvme.c:781 #3 0x000055d9507629f3 in bdrv_open_driver (bs=0x55d952ddb880, drv=0x55d95109c1e0 , node_name=0x0, options=0x55d952bb1410, open_flags=147456, errp=0x7ffd8e19e310) at block.c:1291 #4 0x000055d9507633d6 in bdrv_open_common (bs=0x55d952ddb880, file=0x0, options=0x55d952bb1410, errp=0x7ffd8e19e310) at block.c:1551 #5 0x000055d950766881 in bdrv_open_inherit (filename=0x0, reference=0x0, options=0x55d952bb1410, flags=32768, parent=0x55d9538ce420, child_role=0x55d950eaade0 , errp=0x7ffd8e19e510) at block.c:3063 #6 0x000055d950765ae4 in bdrv_open_child_bs (filename=0x0, options=0x55d9541cdff0, bdref_key=0x55d950af33aa "file", parent=0x55d9538ce420, child_role=0x55d950eaade0 , allow_none=true, errp=0x7ffd8e19e510) at block.c:2712 #7 0x000055d950766633 in bdrv_open_inherit (filename=0x0, reference=0x0, options=0x55d9541cdff0, flags=0, parent=0x0, child_role=0x0, errp=0x7ffd8e19e908) at block.c:3011 #8 0x000055d950766dba in bdrv_open (filename=0x0, reference=0x0, options=0x55d953d00390, flags=0, errp=0x7ffd8e19e908) at block.c:3156 #9 0x000055d9507cb635 in blk_new_open (filename=0x0, reference=0x0, options=0x55d953d00390, flags=0, errp=0x7ffd8e19e908) at block/block-backend.c:389 #10 0x000055d950465ec5 in blockdev_init (file=0x0, bs_opts=0x55d953d00390, errp=0x7ffd8e19e908) at blockdev.c:602 Signed-off-by: Michal Privoznik Message-id: 927aae40b617ba7d4b6c7ffe74e6d7a2595f8e86.1562770546.git.mprivozn@redhat.com Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé Reviewed-by: Maxim Levitsky Signed-off-by: Max Reitz --- block/nvme.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'block') diff --git a/block/nvme.c b/block/nvme.c index 73ed5fa75f..9896b7f7c6 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -613,12 +613,12 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, /* Set up admin queue. */ s->queues = g_new(NVMeQueuePair *, 1); - s->nr_queues = 1; s->queues[0] = nvme_create_queue_pair(bs, 0, NVME_QUEUE_SIZE, errp); if (!s->queues[0]) { ret = -EINVAL; goto out; } + s->nr_queues = 1; QEMU_BUILD_BUG_ON(NVME_QUEUE_SIZE & 0xF000); s->regs->aqa = cpu_to_le32((NVME_QUEUE_SIZE << 16) | NVME_QUEUE_SIZE); s->regs->asq = cpu_to_le64(s->queues[0]->sq.iova); -- cgit v1.2.3-55-g7522 From e5182c1c57ac9aa0e9c399b9c60af3c41cff35b4 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 3 Jul 2019 19:28:02 +0200 Subject: block: Add BDS.never_freeze The commit and the mirror block job must be able to drop their filter node at any point. However, this will not be possible if any of the BdrvChild links to them is frozen. Therefore, we need to prevent them from ever becoming frozen. Signed-off-by: Max Reitz Reviewed-by: Andrey Shinkevich Reviewed-by: Alberto Garcia Message-id: 20190703172813.6868-2-mreitz@redhat.com Signed-off-by: Max Reitz --- block.c | 8 ++++++++ block/commit.c | 4 ++++ block/mirror.c | 4 ++++ include/block/block_int.h | 3 +++ 4 files changed, 19 insertions(+) (limited to 'block') diff --git a/block.c b/block.c index c139540f2b..6565192b91 100644 --- a/block.c +++ b/block.c @@ -4416,6 +4416,14 @@ int bdrv_freeze_backing_chain(BlockDriverState *bs, BlockDriverState *base, return -EPERM; } + for (i = bs; i != base; i = backing_bs(i)) { + if (i->backing && backing_bs(i)->never_freeze) { + error_setg(errp, "Cannot freeze '%s' link to '%s'", + i->backing->name, backing_bs(i)->node_name); + return -EPERM; + } + } + for (i = bs; i != base; i = backing_bs(i)) { if (i->backing) { i->backing->frozen = true; diff --git a/block/commit.c b/block/commit.c index ca7e408b26..2c5a6d4ebc 100644 --- a/block/commit.c +++ b/block/commit.c @@ -298,6 +298,10 @@ void commit_start(const char *job_id, BlockDriverState *bs, if (!filter_node_name) { commit_top_bs->implicit = true; } + + /* So that we can always drop this node */ + commit_top_bs->never_freeze = true; + commit_top_bs->total_sectors = top->total_sectors; bdrv_append(commit_top_bs, top, &local_err); diff --git a/block/mirror.c b/block/mirror.c index 2fcec70e35..8cb75fb409 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -1551,6 +1551,10 @@ static BlockJob *mirror_start_job( if (!filter_node_name) { mirror_top_bs->implicit = true; } + + /* So that we can always drop this node */ + mirror_top_bs->never_freeze = true; + mirror_top_bs->total_sectors = bs->total_sectors; mirror_top_bs->supported_write_flags = BDRV_REQ_WRITE_UNCHANGED; mirror_top_bs->supported_zero_flags = BDRV_REQ_WRITE_UNCHANGED | diff --git a/include/block/block_int.h b/include/block/block_int.h index d6415b53c1..50902531b7 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -885,6 +885,9 @@ struct BlockDriverState { /* Only read/written by whoever has set active_flush_req to true. */ unsigned int flushed_gen; /* Flushed write generation */ + + /* BdrvChild links to this node may never be frozen */ + bool never_freeze; }; struct BlockBackendRootState { -- cgit v1.2.3-55-g7522 From 17a7c39248fc629469d6f66c6122db841b736bc7 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 3 Jul 2019 19:28:03 +0200 Subject: block/stream: Fix error path As of commit c624b015bf14fe01f1e6452a36e63b3ea1ae4998, the stream job only freezes the chain until the overlay of the base node. The error path must consider this. Fixes: c624b015bf14fe01f1e6452a36e63b3ea1ae4998 Signed-off-by: Max Reitz Message-id: 20190703172813.6868-3-mreitz@redhat.com Signed-off-by: Max Reitz --- block/stream.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'block') diff --git a/block/stream.c b/block/stream.c index cd5e2ba9b0..b27e61625d 100644 --- a/block/stream.c +++ b/block/stream.c @@ -284,5 +284,5 @@ fail: if (bs_read_only) { bdrv_reopen_set_read_only(bs, true, NULL); } - bdrv_unfreeze_backing_chain(bs, base); + bdrv_unfreeze_backing_chain(bs, bottom); } -- cgit v1.2.3-55-g7522 From 8441d82d51e25c6a7d1ca92cecc42168f20af72a Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 3 Jul 2019 19:28:04 +0200 Subject: block/stream: Swap backing file change order bdrv_change_backing_file() can result in yields. Therefore, @base may no longer be the the backing_bs() of s->bottom afterwards. Just swap the order of the two calls to fix this. Signed-off-by: Max Reitz Message-id: 20190703172813.6868-4-mreitz@redhat.com Signed-off-by: Max Reitz --- block/stream.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'block') diff --git a/block/stream.c b/block/stream.c index b27e61625d..6ac1e7bec4 100644 --- a/block/stream.c +++ b/block/stream.c @@ -78,8 +78,8 @@ static int stream_prepare(Job *job) base_fmt = base->drv->format_name; } } - ret = bdrv_change_backing_file(bs, base_id, base_fmt); bdrv_set_backing_hd(bs, base, &local_err); + ret = bdrv_change_backing_file(bs, base_id, base_fmt); if (local_err) { error_report_err(local_err); return -EPERM; -- cgit v1.2.3-55-g7522 From 0b1847bbc2b4f50e7497cb05c4540bf7b016c9c6 Mon Sep 17 00:00:00 2001 From: Stefano Garzarella Date: Mon, 15 Jul 2019 15:28:44 +0200 Subject: gluster: fix .bdrv_reopen_prepare when backing file is a JSON object When the backing_file is specified as a JSON object, the qemu_gluster_reopen_prepare() fails with this message: invalid URI json:{"server.0.host": ...} In this case, we should call qemu_gluster_init() using the QDict 'state->options' that contains the JSON parameters already parsed. Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1542445 Signed-off-by: Stefano Garzarella Message-id: 20190715132844.506584-1-sgarzare@redhat.com Signed-off-by: Max Reitz --- block/gluster.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 'block') diff --git a/block/gluster.c b/block/gluster.c index 62f8ff2147..f64dc5b01e 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -931,7 +931,17 @@ static int qemu_gluster_reopen_prepare(BDRVReopenState *state, gconf->has_debug = true; gconf->logfile = g_strdup(s->logfile); gconf->has_logfile = true; - reop_s->glfs = qemu_gluster_init(gconf, state->bs->filename, NULL, errp); + + /* + * If 'state->bs->exact_filename' is empty, 'state->options' should contain + * the JSON parameters already parsed. + */ + if (state->bs->exact_filename[0] != '\0') { + reop_s->glfs = qemu_gluster_init(gconf, state->bs->exact_filename, NULL, + errp); + } else { + reop_s->glfs = qemu_gluster_init(gconf, NULL, state->options, errp); + } if (reop_s->glfs == NULL) { ret = -errno; goto exit; -- cgit v1.2.3-55-g7522