* [PULL 01/15] block: Remove unused include
2020-11-09 17:38 [PULL 00/15] Block patches for 5.2.0-rc1 Max Reitz
@ 2020-11-09 17:38 ` Max Reitz
2020-11-09 17:38 ` [PULL 02/15] block: Move bdrv_drain_all_end_quiesce() to block_int.h Max Reitz
` (14 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Max Reitz @ 2020-11-09 17:38 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, Peter Maydell, qemu-devel, Max Reitz
From: AlexChen <alex.chen@huawei.com>
The "qemu-common.h" include is not used, remove it.
Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: AlexChen <alex.chen@huawei.com>
Message-Id: <5F8FFB94.3030209@huawei.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/dmg-lzfse.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/block/dmg-lzfse.c b/block/dmg-lzfse.c
index 19d25bc646..6798cf4fbf 100644
--- a/block/dmg-lzfse.c
+++ b/block/dmg-lzfse.c
@@ -22,7 +22,6 @@
* THE SOFTWARE.
*/
#include "qemu/osdep.h"
-#include "qemu-common.h"
#include "dmg.h"
#include <lzfse.h>
--
2.28.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PULL 02/15] block: Move bdrv_drain_all_end_quiesce() to block_int.h
2020-11-09 17:38 [PULL 00/15] Block patches for 5.2.0-rc1 Max Reitz
2020-11-09 17:38 ` [PULL 01/15] block: Remove unused include Max Reitz
@ 2020-11-09 17:38 ` Max Reitz
2020-11-09 17:38 ` [PULL 03/15] qcow2: Document and enforce the QCowL2Meta invariants Max Reitz
` (13 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Max Reitz @ 2020-11-09 17:38 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, Peter Maydell, qemu-devel, Max Reitz
From: Greg Kurz <groug@kaod.org>
This function is really an internal helper for bdrv_close(). Update its
doc comment to make this clear and make the function private.
Signed-off-by: Greg Kurz <groug@kaod.org>
Message-Id: <160387245480.131299.13430357162209598411.stgit@bahia>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
include/block/block.h | 6 ------
include/block/block_int.h | 9 +++++++++
2 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/include/block/block.h b/include/block/block.h
index 4bfe3b546b..c9d7c58765 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -781,12 +781,6 @@ void bdrv_drained_end(BlockDriverState *bs);
*/
void bdrv_drained_end_no_poll(BlockDriverState *bs, int *drained_end_counter);
-/**
- * End all quiescent sections started by bdrv_drain_all_begin(). This is
- * only needed when deleting a BDS before bdrv_drain_all_end() is called.
- */
-void bdrv_drain_all_end_quiesce(BlockDriverState *bs);
-
/**
* End a quiescent section started by bdrv_subtree_drained_begin().
*/
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 38cad9d15c..95d9333be1 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -1407,4 +1407,13 @@ static inline BlockDriverState *bdrv_primary_bs(BlockDriverState *bs)
return child_bs(bdrv_primary_child(bs));
}
+/**
+ * End all quiescent sections started by bdrv_drain_all_begin(). This is
+ * needed when deleting a BDS before bdrv_drain_all_end() is called.
+ *
+ * NOTE: this is an internal helper for bdrv_close() *only*. No one else
+ * should call it.
+ */
+void bdrv_drain_all_end_quiesce(BlockDriverState *bs);
+
#endif /* BLOCK_INT_H */
--
2.28.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PULL 03/15] qcow2: Document and enforce the QCowL2Meta invariants
2020-11-09 17:38 [PULL 00/15] Block patches for 5.2.0-rc1 Max Reitz
2020-11-09 17:38 ` [PULL 01/15] block: Remove unused include Max Reitz
2020-11-09 17:38 ` [PULL 02/15] block: Move bdrv_drain_all_end_quiesce() to block_int.h Max Reitz
@ 2020-11-09 17:38 ` Max Reitz
2020-11-09 17:38 ` [PULL 04/15] hw/block/nvme: fix null ns in register namespace Max Reitz
` (12 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Max Reitz @ 2020-11-09 17:38 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, Peter Maydell, qemu-devel, Max Reitz
From: Alberto Garcia <berto@igalia.com>
The QCowL2Meta structure is used to store information about a part of
a write request that touches clusters that need changes in their L2
entries. This happens with newly-allocated clusters or subclusters.
This structure has changed a bit since it was first created and its
current documentation is not quite up-to-date.
A write request can span a region consisting of a combination of
clusters of different types, and qcow2_alloc_host_offset() can
repeatedly call handle_copied() and handle_alloc() to add more
clusters to the mix as long as they all are contiguous on the image
file.
Because of this a write request has a list of QCowL2Meta structures,
one for each part of the request that needs changes in the L2
metadata.
Each one of them spans nb_clusters and has two copy-on-write regions
located immediately before and after the middle region touched by that
part of the write request. Even when those regions themselves are
empty their offsets must be correct because they are used to know the
location of the middle region.
This was not always the case but it is not a problem anymore
because the only two places where QCowL2Meta structures are created
(calculate_l2_meta() and qcow2_co_truncate()) ensure that the
copy-on-write regions are correctly defined, and so do assertions like
the ones in perform_cow().
The conditional initialization of the 'written_to' variable is
therefore unnecessary and is removed by this patch.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20201007161323.4667-1-berto@igalia.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/qcow2.h | 19 +++++++++++--------
block/qcow2-cluster.c | 5 +++--
block/qcow2.c | 19 +++++++++++++++----
3 files changed, 29 insertions(+), 14 deletions(-)
diff --git a/block/qcow2.h b/block/qcow2.h
index 125ea9679b..2e0272a7b8 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -435,17 +435,18 @@ typedef struct Qcow2COWRegion {
/**
* Describes an in-flight (part of a) write request that writes to clusters
- * that are not referenced in their L2 table yet.
+ * that need to have their L2 table entries updated (because they are
+ * newly allocated or need changes in their L2 bitmaps)
*/
typedef struct QCowL2Meta
{
- /** Guest offset of the first newly allocated cluster */
+ /** Guest offset of the first updated cluster */
uint64_t offset;
- /** Host offset of the first newly allocated cluster */
+ /** Host offset of the first updated cluster */
uint64_t alloc_offset;
- /** Number of newly allocated clusters */
+ /** Number of updated clusters */
int nb_clusters;
/** Do not free the old clusters */
@@ -458,14 +459,16 @@ typedef struct QCowL2Meta
CoQueue dependent_requests;
/**
- * The COW Region between the start of the first allocated cluster and the
- * area the guest actually writes to.
+ * The COW Region immediately before the area the guest actually
+ * writes to. This (part of the) write request starts at
+ * cow_start.offset + cow_start.nb_bytes.
*/
Qcow2COWRegion cow_start;
/**
- * The COW Region between the area the guest actually writes to and the
- * end of the last allocated cluster.
+ * The COW Region immediately after the area the guest actually
+ * writes to. This (part of the) write request ends at cow_end.offset
+ * (which must always be set even when cow_end.nb_bytes is 0).
*/
Qcow2COWRegion cow_end;
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index aa87d3e99b..485b4cb92e 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1049,6 +1049,8 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_slice);
assert(l2_index + m->nb_clusters <= s->l2_slice_size);
+ assert(m->cow_end.offset + m->cow_end.nb_bytes <=
+ m->nb_clusters << s->cluster_bits);
for (i = 0; i < m->nb_clusters; i++) {
uint64_t offset = cluster_offset + ((uint64_t)i << s->cluster_bits);
/* if two concurrent writes happen to the same unallocated cluster
@@ -1070,8 +1072,7 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
if (has_subclusters(s) && !m->prealloc) {
uint64_t l2_bitmap = get_l2_bitmap(s, l2_slice, l2_index + i);
unsigned written_from = m->cow_start.offset;
- unsigned written_to = m->cow_end.offset + m->cow_end.nb_bytes ?:
- m->nb_clusters << s->cluster_bits;
+ unsigned written_to = m->cow_end.offset + m->cow_end.nb_bytes;
int first_sc, last_sc;
/* Narrow written_from and written_to down to the current cluster */
written_from = MAX(written_from, i << s->cluster_bits);
diff --git a/block/qcow2.c b/block/qcow2.c
index 4274806a2a..1b0733238b 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2361,15 +2361,26 @@ static bool merge_cow(uint64_t offset, unsigned bytes,
continue;
}
- /* The data (middle) region must be immediately after the
- * start region */
+ /*
+ * The write request should start immediately after the first
+ * COW region. This does not always happen because the area
+ * touched by the request can be larger than the one defined
+ * by @m (a single request can span an area consisting of a
+ * mix of previously unallocated and allocated clusters, that
+ * is why @l2meta is a list).
+ */
if (l2meta_cow_start(m) + m->cow_start.nb_bytes != offset) {
+ /* In this case the request starts before this region */
+ assert(offset < l2meta_cow_start(m));
+ assert(m->cow_start.nb_bytes == 0);
continue;
}
- /* The end region must be immediately after the data (middle)
- * region */
+ /* The write request should end immediately before the second
+ * COW region (see above for why it does not always happen) */
if (m->offset + m->cow_end.offset != offset + bytes) {
+ assert(offset + bytes > m->offset + m->cow_end.offset);
+ assert(m->cow_end.nb_bytes == 0);
continue;
}
--
2.28.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PULL 04/15] hw/block/nvme: fix null ns in register namespace
2020-11-09 17:38 [PULL 00/15] Block patches for 5.2.0-rc1 Max Reitz
` (2 preceding siblings ...)
2020-11-09 17:38 ` [PULL 03/15] qcow2: Document and enforce the QCowL2Meta invariants Max Reitz
@ 2020-11-09 17:38 ` Max Reitz
2020-11-09 17:38 ` [PULL 05/15] hw/block/nvme: fix uint16_t use of uint32_t sgls member Max Reitz
` (11 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Max Reitz @ 2020-11-09 17:38 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, Peter Maydell, qemu-devel, Max Reitz
From: Klaus Jensen <k.jensen@samsung.com>
Fix dereference after NULL check.
Reported-by: Coverity (CID 1436128)
Fixes: b20804946bce ("hw/block/nvme: update nsid when registered")
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
Message-Id: <20201104102248.32168-2-its@irrelevant.dk>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
hw/block/nvme.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index fa2cba744b..080d782f1c 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -2562,8 +2562,7 @@ int nvme_register_namespace(NvmeCtrl *n, NvmeNamespace *ns, Error **errp)
if (!nsid) {
for (int i = 1; i <= n->num_namespaces; i++) {
- NvmeNamespace *ns = nvme_ns(n, i);
- if (!ns) {
+ if (!nvme_ns(n, i)) {
nsid = ns->params.nsid = i;
break;
}
--
2.28.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PULL 05/15] hw/block/nvme: fix uint16_t use of uint32_t sgls member
2020-11-09 17:38 [PULL 00/15] Block patches for 5.2.0-rc1 Max Reitz
` (3 preceding siblings ...)
2020-11-09 17:38 ` [PULL 04/15] hw/block/nvme: fix null ns in register namespace Max Reitz
@ 2020-11-09 17:38 ` Max Reitz
2020-11-09 17:38 ` [PULL 06/15] hw/block/nvme: fix free of array-typed value Max Reitz
` (10 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Max Reitz @ 2020-11-09 17:38 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, Peter Maydell, qemu-devel, Max Reitz
From: Klaus Jensen <k.jensen@samsung.com>
nvme_map_sgl_data erroneously uses the sgls member of NvmeIdNs as a
uint16_t.
Reported-by: Coverity (CID 1436129)
Fixes: cba0a8a344fe ("hw/block/nvme: add support for scatter gather lists")
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
Message-Id: <20201104102248.32168-3-its@irrelevant.dk>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
hw/block/nvme.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 080d782f1c..2bdc50eb6f 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -452,7 +452,7 @@ static uint16_t nvme_map_sgl_data(NvmeCtrl *n, QEMUSGList *qsg,
* segments and/or descriptors. The controller might accept
* ignoring the rest of the SGL.
*/
- uint16_t sgls = le16_to_cpu(n->id_ctrl.sgls);
+ uint32_t sgls = le32_to_cpu(n->id_ctrl.sgls);
if (sgls & NVME_CTRL_SGLS_EXCESS_LENGTH) {
break;
}
--
2.28.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PULL 06/15] hw/block/nvme: fix free of array-typed value
2020-11-09 17:38 [PULL 00/15] Block patches for 5.2.0-rc1 Max Reitz
` (4 preceding siblings ...)
2020-11-09 17:38 ` [PULL 05/15] hw/block/nvme: fix uint16_t use of uint32_t sgls member Max Reitz
@ 2020-11-09 17:38 ` Max Reitz
2020-11-09 17:38 ` [PULL 07/15] iotests: add filter_qmp_virtio_scsi function Max Reitz
` (9 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Max Reitz @ 2020-11-09 17:38 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, Peter Maydell, qemu-devel, Max Reitz
From: Klaus Jensen <k.jensen@samsung.com>
Since 7f0f1acedf15 ("hw/block/nvme: support multiple namespaces"), the
namespaces member of NvmeCtrl is no longer a dynamically allocated
array. Remove the free.
Fixes: 7f0f1acedf15 ("hw/block/nvme: support multiple namespaces")
Reported-by: Coverity (CID 1436131)
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
Message-Id: <20201104102248.32168-4-its@irrelevant.dk>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
hw/block/nvme.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 2bdc50eb6f..01b657b1c5 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -2799,7 +2799,6 @@ static void nvme_exit(PCIDevice *pci_dev)
NvmeCtrl *n = NVME(pci_dev);
nvme_clear_ctrl(n);
- g_free(n->namespaces);
g_free(n->cq);
g_free(n->sq);
g_free(n->aer_reqs);
--
2.28.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PULL 07/15] iotests: add filter_qmp_virtio_scsi function
2020-11-09 17:38 [PULL 00/15] Block patches for 5.2.0-rc1 Max Reitz
` (5 preceding siblings ...)
2020-11-09 17:38 ` [PULL 06/15] hw/block/nvme: fix free of array-typed value Max Reitz
@ 2020-11-09 17:38 ` Max Reitz
2020-11-09 17:38 ` [PULL 08/15] iotests: rewrite iotest 240 in python Max Reitz
` (8 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Max Reitz @ 2020-11-09 17:38 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, Peter Maydell, qemu-devel, Max Reitz
From: Maxim Levitsky <mlevitsk@redhat.com>
filter_qmp_virtio_scsi can be used to filter virtio-scsi-pci/ccw differences.
Note that this patch was only tested on x86.
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20201104185025.434703-2-mlevitsk@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/iotests.py | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 814804a4c6..bcd4fe5b6f 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -392,6 +392,16 @@ def filter_qmp_testfiles(qmsg):
return value
return filter_qmp(qmsg, _filter)
+def filter_virtio_scsi(output: str) -> str:
+ return re.sub(r'(virtio-scsi)-(ccw|pci)', r'\1', output)
+
+def filter_qmp_virtio_scsi(qmsg):
+ def _filter(_key, value):
+ if is_str(value):
+ return filter_virtio_scsi(value)
+ return value
+ return filter_qmp(qmsg, _filter)
+
def filter_generated_node_ids(msg):
return re.sub("#block[0-9]+", "NODE_NAME", msg)
--
2.28.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PULL 08/15] iotests: rewrite iotest 240 in python
2020-11-09 17:38 [PULL 00/15] Block patches for 5.2.0-rc1 Max Reitz
` (6 preceding siblings ...)
2020-11-09 17:38 ` [PULL 07/15] iotests: add filter_qmp_virtio_scsi function Max Reitz
@ 2020-11-09 17:38 ` Max Reitz
2020-11-09 17:38 ` [PULL 09/15] block: Fixes nfs compiling error on msys2/mingw Max Reitz
` (7 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Max Reitz @ 2020-11-09 17:38 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, Peter Maydell, qemu-devel, Max Reitz
From: Maxim Levitsky <mlevitsk@redhat.com>
The recent changes that brought RCU delayed device deletion,
broke few tests and this test breakage went unnoticed.
Fix this test by rewriting it in python
(which allows to wait for DEVICE_DELETED events before continuing).
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20201104185025.434703-3-mlevitsk@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/240 | 219 +++++++++++++++----------------------
tests/qemu-iotests/240.out | 76 +++++++------
2 files changed, 130 insertions(+), 165 deletions(-)
diff --git a/tests/qemu-iotests/240 b/tests/qemu-iotests/240
index 8b4337b58d..c0f71f0461 100755
--- a/tests/qemu-iotests/240
+++ b/tests/qemu-iotests/240
@@ -1,5 +1,5 @@
-#!/usr/bin/env bash
-#
+#!/usr/bin/env python3
+
# Test hot plugging and unplugging with iothreads
#
# Copyright (C) 2019 Igalia, S.L.
@@ -17,133 +17,90 @@
#
# 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=berto@igalia.com
-
-seq=`basename $0`
-echo "QA output created by $seq"
-
-status=1 # failure is the default!
-
-_cleanup()
-{
- rm -f "$SOCK_DIR/nbd"
-}
-trap "_cleanup; exit \$status" 0 1 2 3 15
-
-# get standard environment, filters and checks
-. ./common.rc
-. ./common.filter
-
-_supported_fmt generic
-_supported_proto generic
-
-do_run_qemu()
-{
- echo Testing: "$@"
- $QEMU -nographic -qmp stdio -serial none "$@"
- echo
-}
-
-# Remove QMP events from (pretty-printed) output. Doesn't handle
-# nested dicts correctly, but we don't get any of those in this test.
-_filter_qmp_events()
-{
- tr '\n' '\t' | sed -e \
- 's/{\s*"timestamp":\s*{[^}]*},\s*"event":[^,}]*\(,\s*"data":\s*{[^}]*}\)\?\s*}\s*//g' \
- | tr '\t' '\n'
-}
-
-run_qemu()
-{
- do_run_qemu "$@" 2>&1 | _filter_qmp | _filter_qmp_events
-}
-
-case "$QEMU_DEFAULT_MACHINE" in
- s390-ccw-virtio)
- virtio_scsi=virtio-scsi-ccw
- ;;
- *)
- virtio_scsi=virtio-scsi-pci
- ;;
-esac
-
-echo
-echo === Unplug a SCSI disk and then plug it again ===
-echo
-
-run_qemu <<EOF
-{ "execute": "qmp_capabilities" }
-{ "execute": "blockdev-add", "arguments": {"driver": "null-co", "read-zeroes": true, "node-name": "hd0"}}
-{ "execute": "object-add", "arguments": {"qom-type": "iothread", "id": "iothread0"}}
-{ "execute": "device_add", "arguments": {"id": "scsi0", "driver": "${virtio_scsi}", "iothread": "iothread0"}}
-{ "execute": "device_add", "arguments": {"id": "scsi-hd0", "driver": "scsi-hd", "drive": "hd0"}}
-{ "execute": "device_del", "arguments": {"id": "scsi-hd0"}}
-{ "execute": "device_add", "arguments": {"id": "scsi-hd0", "driver": "scsi-hd", "drive": "hd0"}}
-{ "execute": "device_del", "arguments": {"id": "scsi-hd0"}}
-{ "execute": "device_del", "arguments": {"id": "scsi0"}}
-{ "execute": "blockdev-del", "arguments": {"node-name": "hd0"}}
-{ "execute": "quit"}
-EOF
-
-echo
-echo === Attach two SCSI disks using the same block device and the same iothread ===
-echo
-
-run_qemu <<EOF
-{ "execute": "qmp_capabilities" }
-{ "execute": "blockdev-add", "arguments": {"driver": "null-co", "read-zeroes": true, "node-name": "hd0", "read-only": true}}
-{ "execute": "object-add", "arguments": {"qom-type": "iothread", "id": "iothread0"}}
-{ "execute": "device_add", "arguments": {"id": "scsi0", "driver": "${virtio_scsi}", "iothread": "iothread0"}}
-{ "execute": "device_add", "arguments": {"id": "scsi-hd0", "driver": "scsi-hd", "drive": "hd0"}}
-{ "execute": "device_add", "arguments": {"id": "scsi-hd1", "driver": "scsi-hd", "drive": "hd0"}}
-{ "execute": "device_del", "arguments": {"id": "scsi-hd0"}}
-{ "execute": "device_del", "arguments": {"id": "scsi-hd1"}}
-{ "execute": "device_del", "arguments": {"id": "scsi0"}}
-{ "execute": "blockdev-del", "arguments": {"node-name": "hd0"}}
-{ "execute": "quit"}
-EOF
-
-echo
-echo === Attach two SCSI disks using the same block device but different iothreads ===
-echo
-
-run_qemu <<EOF
-{ "execute": "qmp_capabilities" }
-{ "execute": "blockdev-add", "arguments": {"driver": "null-co", "read-zeroes": true, "node-name": "hd0", "read-only": true}}
-{ "execute": "object-add", "arguments": {"qom-type": "iothread", "id": "iothread0"}}
-{ "execute": "object-add", "arguments": {"qom-type": "iothread", "id": "iothread1"}}
-{ "execute": "device_add", "arguments": {"id": "scsi0", "driver": "${virtio_scsi}", "iothread": "iothread0"}}
-{ "execute": "device_add", "arguments": {"id": "scsi1", "driver": "${virtio_scsi}", "iothread": "iothread1"}}
-{ "execute": "device_add", "arguments": {"id": "scsi-hd0", "driver": "scsi-hd", "drive": "hd0", "bus": "scsi0.0"}}
-{ "execute": "device_add", "arguments": {"id": "scsi-hd1", "driver": "scsi-hd", "drive": "hd0", "bus": "scsi1.0"}}
-{ "execute": "device_del", "arguments": {"id": "scsi-hd0"}}
-{ "execute": "device_add", "arguments": {"id": "scsi-hd1", "driver": "scsi-hd", "drive": "hd0", "bus": "scsi1.0"}}
-{ "execute": "device_del", "arguments": {"id": "scsi-hd1"}}
-{ "execute": "device_del", "arguments": {"id": "scsi0"}}
-{ "execute": "device_del", "arguments": {"id": "scsi1"}}
-{ "execute": "blockdev-del", "arguments": {"node-name": "hd0"}}
-{ "execute": "quit"}
-EOF
-
-echo
-echo === Attach a SCSI disks using the same block device as a NBD server ===
-echo
-
-run_qemu <<EOF
-{ "execute": "qmp_capabilities" }
-{ "execute": "blockdev-add", "arguments": {"driver": "null-co", "read-zeroes": true, "node-name": "hd0", "read-only": true}}
-{ "execute": "nbd-server-start", "arguments": {"addr":{"type":"unix","data":{"path":"$SOCK_DIR/nbd"}}}}
-{ "execute": "nbd-server-add", "arguments": {"device":"hd0"}}
-{ "execute": "object-add", "arguments": {"qom-type": "iothread", "id": "iothread0"}}
-{ "execute": "device_add", "arguments": {"id": "scsi0", "driver": "${virtio_scsi}", "iothread": "iothread0"}}
-{ "execute": "device_add", "arguments": {"id": "scsi-hd0", "driver": "scsi-hd", "drive": "hd0", "bus": "scsi0.0"}}
-{ "execute": "quit"}
-EOF
-
-# success, all done
-echo "*** done"
-rm -f $seq.full
-status=0
+import iotests
+import os
+
+nbd_sock = iotests.file_path('nbd.sock', base_dir=iotests.sock_dir)
+
+class TestCase(iotests.QMPTestCase):
+ test_driver = "null-co"
+
+ def required_drivers(self):
+ return [self.test_driver]
+
+ @iotests.skip_if_unsupported(required_drivers)
+ def setUp(self):
+ self.vm = iotests.VM()
+ self.vm.launch()
+
+ def tearDown(self):
+ self.vm.shutdown()
+
+ def test1(self):
+ iotests.log('==Unplug a SCSI disk and then plug it again==')
+ self.vm.qmp_log('blockdev-add', driver='null-co', read_zeroes=True, node_name='hd0')
+ self.vm.qmp_log('object-add', qom_type='iothread', id="iothread0")
+ self.vm.qmp_log('device_add', id='scsi0', driver=iotests.get_virtio_scsi_device(), iothread='iothread0', filters=[iotests.filter_qmp_virtio_scsi])
+ self.vm.qmp_log('device_add', id='scsi-hd0', driver='scsi-hd', drive='hd0')
+ self.vm.qmp_log('device_del', id='scsi-hd0')
+ self.vm.event_wait('DEVICE_DELETED')
+ self.vm.qmp_log('device_add', id='scsi-hd0', driver='scsi-hd', drive='hd0')
+ self.vm.qmp_log('device_del', id='scsi-hd0')
+ self.vm.event_wait('DEVICE_DELETED')
+ self.vm.qmp_log('blockdev-del', node_name='hd0')
+
+ def test2(self):
+ iotests.log('==Attach two SCSI disks using the same block device and the same iothread==')
+ self.vm.qmp_log('blockdev-add', driver='null-co', read_zeroes=True, node_name='hd0', read_only=True)
+ self.vm.qmp_log('object-add', qom_type='iothread', id="iothread0")
+ self.vm.qmp_log('device_add', id='scsi0', driver=iotests.get_virtio_scsi_device(), iothread='iothread0', filters=[iotests.filter_qmp_virtio_scsi])
+
+ self.vm.qmp_log('device_add', id='scsi-hd0', driver='scsi-hd', drive='hd0')
+ self.vm.qmp_log('device_add', id='scsi-hd1', driver='scsi-hd', drive='hd0')
+ self.vm.qmp_log('device_del', id='scsi-hd0')
+ self.vm.event_wait('DEVICE_DELETED')
+ self.vm.qmp_log('device_del', id='scsi-hd1')
+ self.vm.event_wait('DEVICE_DELETED')
+ self.vm.qmp_log('blockdev-del', node_name='hd0')
+
+ def test3(self):
+ iotests.log('==Attach two SCSI disks using the same block device but different iothreads==')
+
+ self.vm.qmp_log('blockdev-add', driver='null-co', read_zeroes=True, node_name='hd0', read_only=True)
+
+ self.vm.qmp_log('object-add', qom_type='iothread', id="iothread0")
+ self.vm.qmp_log('object-add', qom_type='iothread', id="iothread1")
+
+ self.vm.qmp_log('device_add', id='scsi0', driver=iotests.get_virtio_scsi_device(), iothread='iothread0', filters=[iotests.filter_qmp_virtio_scsi])
+ self.vm.qmp_log('device_add', id='scsi1', driver=iotests.get_virtio_scsi_device(), iothread='iothread1', filters=[iotests.filter_qmp_virtio_scsi])
+
+ self.vm.qmp_log('device_add', id='scsi-hd0', driver='scsi-hd', drive='hd0', bus="scsi0.0")
+ self.vm.qmp_log('device_add', id='scsi-hd1', driver='scsi-hd', drive='hd0', bus="scsi1.0")
+
+ self.vm.qmp_log('device_del', id='scsi-hd0')
+ self.vm.event_wait('DEVICE_DELETED')
+ self.vm.qmp_log('device_add', id='scsi-hd1', driver='scsi-hd', drive='hd0', bus="scsi1.0")
+
+ self.vm.qmp_log('device_del', id='scsi-hd1')
+ self.vm.event_wait('DEVICE_DELETED')
+ self.vm.qmp_log('blockdev-del', node_name='hd0')
+
+ def test4(self):
+ iotests.log('==Attach a SCSI disks using the same block device as a NBD server==')
+
+ self.vm.qmp_log('blockdev-add', driver='null-co', read_zeroes=True, node_name='hd0', read_only=True)
+
+ self.vm.qmp_log('nbd-server-start',
+ filters=[iotests.filter_qmp_testfiles],
+ addr={'type':'unix', 'data':{'path':nbd_sock}})
+
+ self.vm.qmp_log('nbd-server-add', device='hd0')
+
+ self.vm.qmp_log('object-add', qom_type='iothread', id="iothread0")
+ self.vm.qmp_log('device_add', id='scsi0', driver=iotests.get_virtio_scsi_device(), iothread='iothread0', filters=[iotests.filter_qmp_virtio_scsi])
+ self.vm.qmp_log('device_add', id='scsi-hd0', driver='scsi-hd', drive='hd0')
+
+if __name__ == '__main__':
+ iotests.activate_logging()
+ iotests.main()
diff --git a/tests/qemu-iotests/240.out b/tests/qemu-iotests/240.out
index d00df50297..e0982831ae 100644
--- a/tests/qemu-iotests/240.out
+++ b/tests/qemu-iotests/240.out
@@ -1,67 +1,75 @@
-QA output created by 240
-
-=== Unplug a SCSI disk and then plug it again ===
-
-Testing:
-QMP_VERSION
-{"return": {}}
+==Unplug a SCSI disk and then plug it again==
+{"execute": "blockdev-add", "arguments": {"driver": "null-co", "node-name": "hd0", "read-zeroes": true}}
{"return": {}}
+{"execute": "object-add", "arguments": {"id": "iothread0", "qom-type": "iothread"}}
{"return": {}}
+{"execute": "device_add", "arguments": {"driver": "virtio-scsi", "id": "scsi0", "iothread": "iothread0"}}
{"return": {}}
+{"execute": "device_add", "arguments": {"drive": "hd0", "driver": "scsi-hd", "id": "scsi-hd0"}}
{"return": {}}
+{"execute": "device_del", "arguments": {"id": "scsi-hd0"}}
{"return": {}}
+{"execute": "device_add", "arguments": {"drive": "hd0", "driver": "scsi-hd", "id": "scsi-hd0"}}
{"return": {}}
+{"execute": "device_del", "arguments": {"id": "scsi-hd0"}}
{"return": {}}
+{"execute": "blockdev-del", "arguments": {"node-name": "hd0"}}
{"return": {}}
+==Attach two SCSI disks using the same block device and the same iothread==
+{"execute": "blockdev-add", "arguments": {"driver": "null-co", "node-name": "hd0", "read-only": true, "read-zeroes": true}}
{"return": {}}
+{"execute": "object-add", "arguments": {"id": "iothread0", "qom-type": "iothread"}}
{"return": {}}
-
-=== Attach two SCSI disks using the same block device and the same iothread ===
-
-Testing:
-QMP_VERSION
-{"return": {}}
+{"execute": "device_add", "arguments": {"driver": "virtio-scsi", "id": "scsi0", "iothread": "iothread0"}}
{"return": {}}
+{"execute": "device_add", "arguments": {"drive": "hd0", "driver": "scsi-hd", "id": "scsi-hd0"}}
{"return": {}}
+{"execute": "device_add", "arguments": {"drive": "hd0", "driver": "scsi-hd", "id": "scsi-hd1"}}
{"return": {}}
+{"execute": "device_del", "arguments": {"id": "scsi-hd0"}}
{"return": {}}
+{"execute": "device_del", "arguments": {"id": "scsi-hd1"}}
{"return": {}}
+{"execute": "blockdev-del", "arguments": {"node-name": "hd0"}}
{"return": {}}
+==Attach two SCSI disks using the same block device but different iothreads==
+{"execute": "blockdev-add", "arguments": {"driver": "null-co", "node-name": "hd0", "read-only": true, "read-zeroes": true}}
{"return": {}}
+{"execute": "object-add", "arguments": {"id": "iothread0", "qom-type": "iothread"}}
{"return": {}}
+{"execute": "object-add", "arguments": {"id": "iothread1", "qom-type": "iothread"}}
{"return": {}}
+{"execute": "device_add", "arguments": {"driver": "virtio-scsi", "id": "scsi0", "iothread": "iothread0"}}
{"return": {}}
-
-=== Attach two SCSI disks using the same block device but different iothreads ===
-
-Testing:
-QMP_VERSION
-{"return": {}}
-{"return": {}}
-{"return": {}}
-{"return": {}}
-{"return": {}}
+{"execute": "device_add", "arguments": {"driver": "virtio-scsi", "id": "scsi1", "iothread": "iothread1"}}
{"return": {}}
+{"execute": "device_add", "arguments": {"bus": "scsi0.0", "drive": "hd0", "driver": "scsi-hd", "id": "scsi-hd0"}}
{"return": {}}
+{"execute": "device_add", "arguments": {"bus": "scsi1.0", "drive": "hd0", "driver": "scsi-hd", "id": "scsi-hd1"}}
{"error": {"class": "GenericError", "desc": "Cannot change iothread of active block backend"}}
+{"execute": "device_del", "arguments": {"id": "scsi-hd0"}}
{"return": {}}
+{"execute": "device_add", "arguments": {"bus": "scsi1.0", "drive": "hd0", "driver": "scsi-hd", "id": "scsi-hd1"}}
{"return": {}}
+{"execute": "device_del", "arguments": {"id": "scsi-hd1"}}
{"return": {}}
+{"execute": "blockdev-del", "arguments": {"node-name": "hd0"}}
{"return": {}}
+==Attach a SCSI disks using the same block device as a NBD server==
+{"execute": "blockdev-add", "arguments": {"driver": "null-co", "node-name": "hd0", "read-only": true, "read-zeroes": true}}
{"return": {}}
+{"execute": "nbd-server-start", "arguments": {"addr": {"data": {"path": "SOCK_DIR/PID-nbd.sock"}, "type": "unix"}}}
{"return": {}}
+{"execute": "nbd-server-add", "arguments": {"device": "hd0"}}
{"return": {}}
-
-=== Attach a SCSI disks using the same block device as a NBD server ===
-
-Testing:
-QMP_VERSION
-{"return": {}}
-{"return": {}}
-{"return": {}}
-{"return": {}}
+{"execute": "object-add", "arguments": {"id": "iothread0", "qom-type": "iothread"}}
{"return": {}}
+{"execute": "device_add", "arguments": {"driver": "virtio-scsi", "id": "scsi0", "iothread": "iothread0"}}
{"return": {}}
+{"execute": "device_add", "arguments": {"drive": "hd0", "driver": "scsi-hd", "id": "scsi-hd0"}}
{"return": {}}
-{"return": {}}
-*** done
+....
+----------------------------------------------------------------------
+Ran 4 tests
+
+OK
--
2.28.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PULL 09/15] block: Fixes nfs compiling error on msys2/mingw
2020-11-09 17:38 [PULL 00/15] Block patches for 5.2.0-rc1 Max Reitz
` (7 preceding siblings ...)
2020-11-09 17:38 ` [PULL 08/15] iotests: rewrite iotest 240 in python Max Reitz
@ 2020-11-09 17:38 ` Max Reitz
2020-11-09 17:38 ` [PULL 10/15] block: enable libnfs on msys2/mingw in cirrus.yml Max Reitz
` (6 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Max Reitz @ 2020-11-09 17:38 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, Peter Maydell, qemu-devel, Max Reitz
From: Yonggang Luo <luoyonggang@gmail.com>
These compiling errors are fixed:
../block/nfs.c:27:10: fatal error: poll.h: No such file or directory
27 | #include <poll.h>
| ^~~~~~~~
compilation terminated.
../block/nfs.c:63:5: error: unknown type name 'blkcnt_t'
63 | blkcnt_t st_blocks;
| ^~~~~~~~
../block/nfs.c: In function 'nfs_client_open':
../block/nfs.c:550:27: error: 'struct _stat64' has no member named 'st_blocks'
550 | client->st_blocks = st.st_blocks;
| ^
../block/nfs.c: In function 'nfs_get_allocated_file_size':
../block/nfs.c:751:41: error: 'struct _stat64' has no member named 'st_blocks'
751 | return (task.ret < 0 ? task.ret : st.st_blocks * 512);
| ^
../block/nfs.c: In function 'nfs_reopen_prepare':
../block/nfs.c:805:31: error: 'struct _stat64' has no member named 'st_blocks'
805 | client->st_blocks = st.st_blocks;
| ^
../block/nfs.c: In function 'nfs_get_allocated_file_size':
../block/nfs.c:752:1: error: control reaches end of non-void function [-Werror=return-type]
752 | }
| ^
On msys2/mingw, there is no st_blocks in struct _stat64 yet, we disable the usage of it
on msys2/mingw, and create a typedef long long blkcnt_t; for further implementation
Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
Message-Id: <20201105123116.674-2-luoyonggang@gmail.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/nfs.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/block/nfs.c b/block/nfs.c
index f86e660374..77905f516d 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -24,7 +24,9 @@
#include "qemu/osdep.h"
+#if !defined(_WIN32)
#include <poll.h>
+#endif
#include "qemu/config-file.h"
#include "qemu/error-report.h"
#include "qapi/error.h"
@@ -58,7 +60,7 @@ typedef struct NFSClient {
bool has_zero_init;
AioContext *aio_context;
QemuMutex mutex;
- blkcnt_t st_blocks;
+ uint64_t st_blocks;
bool cache_used;
NFSServer *server;
char *path;
@@ -545,7 +547,9 @@ static int64_t nfs_client_open(NFSClient *client, BlockdevOptionsNfs *opts,
}
ret = DIV_ROUND_UP(st.st_size, BDRV_SECTOR_SIZE);
+#if !defined(_WIN32)
client->st_blocks = st.st_blocks;
+#endif
client->has_zero_init = S_ISREG(st.st_mode);
*strp = '/';
goto out;
@@ -706,6 +710,7 @@ static int nfs_has_zero_init(BlockDriverState *bs)
return client->has_zero_init;
}
+#if !defined(_WIN32)
/* Called (via nfs_service) with QemuMutex held. */
static void
nfs_get_allocated_file_size_cb(int ret, struct nfs_context *nfs, void *data,
@@ -748,6 +753,7 @@ static int64_t nfs_get_allocated_file_size(BlockDriverState *bs)
return (task.ret < 0 ? task.ret : st.st_blocks * 512);
}
+#endif
static int coroutine_fn
nfs_file_co_truncate(BlockDriverState *bs, int64_t offset, bool exact,
@@ -800,7 +806,9 @@ static int nfs_reopen_prepare(BDRVReopenState *state,
nfs_get_error(client->context));
return ret;
}
+#if !defined(_WIN32)
client->st_blocks = st.st_blocks;
+#endif
}
return 0;
@@ -869,7 +877,10 @@ static BlockDriver bdrv_nfs = {
.create_opts = &nfs_create_opts,
.bdrv_has_zero_init = nfs_has_zero_init,
+/* libnfs does not provide the allocated filesize of a file on win32. */
+#if !defined(_WIN32)
.bdrv_get_allocated_file_size = nfs_get_allocated_file_size,
+#endif
.bdrv_co_truncate = nfs_file_co_truncate,
.bdrv_file_open = nfs_file_open,
--
2.28.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PULL 10/15] block: enable libnfs on msys2/mingw in cirrus.yml
2020-11-09 17:38 [PULL 00/15] Block patches for 5.2.0-rc1 Max Reitz
` (8 preceding siblings ...)
2020-11-09 17:38 ` [PULL 09/15] block: Fixes nfs compiling error on msys2/mingw Max Reitz
@ 2020-11-09 17:38 ` Max Reitz
2020-11-09 17:38 ` [PULL 11/15] block: Fix integer promotion error in bdrv_getlength() Max Reitz
` (5 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Max Reitz @ 2020-11-09 17:38 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, Peter Maydell, qemu-devel, Max Reitz
From: Yonggang Luo <luoyonggang@gmail.com>
Initially, libnfs has not been enabled, and now it's fixed, so enable it
on cirrus.
Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
Message-Id: <20201105123116.674-3-luoyonggang@gmail.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
.cirrus.yml | 1 +
1 file changed, 1 insertion(+)
diff --git a/.cirrus.yml b/.cirrus.yml
index 900437dd2a..f0209b7a3e 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -109,6 +109,7 @@ windows_msys2_task:
mingw-w64-x86_64-cyrus-sasl \
mingw-w64-x86_64-curl \
mingw-w64-x86_64-gnutls \
+ mingw-w64-x86_64-libnfs \
"
bitsadmin /transfer msys_download /dynamic /download /priority FOREGROUND `
https://repo.msys2.org/mingw/x86_64/mingw-w64-x86_64-python-sphinx-2.3.1-1-any.pkg.tar.xz `
--
2.28.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PULL 11/15] block: Fix integer promotion error in bdrv_getlength()
2020-11-09 17:38 [PULL 00/15] Block patches for 5.2.0-rc1 Max Reitz
` (9 preceding siblings ...)
2020-11-09 17:38 ` [PULL 10/15] block: enable libnfs on msys2/mingw in cirrus.yml Max Reitz
@ 2020-11-09 17:38 ` Max Reitz
2020-11-09 17:38 ` [PULL 12/15] block: Fix some code style problems, "foo* bar" should be "foo *bar" Max Reitz
` (4 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Max Reitz @ 2020-11-09 17:38 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, Peter Maydell, qemu-devel, Max Reitz
From: Eric Blake <eblake@redhat.com>
Back in 2015, we attempted to fix error reporting for images that
claimed to have more than INT64_MAX/512 sectors, but due to the type
promotions caused by BDRV_SECTOR_SIZE being unsigned, this
inadvertently forces all negative ret values to be slammed into -EFBIG
rather than the original error. While we're at it, we can avoid the
confusing ?: by spelling the logic more directly.
Fixes: 4a9c9ea0d3
Reported-by: Guoyi Tu <tu.guoyi@h3c.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20201105155122.60943-1-eblake@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/block.c b/block.c
index 56bacc9e9f..2fd932154e 100644
--- a/block.c
+++ b/block.c
@@ -5091,8 +5091,13 @@ int64_t bdrv_getlength(BlockDriverState *bs)
{
int64_t ret = bdrv_nb_sectors(bs);
- ret = ret > INT64_MAX / BDRV_SECTOR_SIZE ? -EFBIG : ret;
- return ret < 0 ? ret : ret * BDRV_SECTOR_SIZE;
+ if (ret < 0) {
+ return ret;
+ }
+ if (ret > INT64_MAX / BDRV_SECTOR_SIZE) {
+ return -EFBIG;
+ }
+ return ret * BDRV_SECTOR_SIZE;
}
/* return 0 as number of sectors if no device present or error */
--
2.28.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PULL 12/15] block: Fix some code style problems, "foo* bar" should be "foo *bar"
2020-11-09 17:38 [PULL 00/15] Block patches for 5.2.0-rc1 Max Reitz
` (10 preceding siblings ...)
2020-11-09 17:38 ` [PULL 11/15] block: Fix integer promotion error in bdrv_getlength() Max Reitz
@ 2020-11-09 17:38 ` Max Reitz
2020-11-09 17:38 ` [PULL 13/15] block: add forgotten bdrv_abort_perm_update() to bdrv_co_invalidate_cache() Max Reitz
` (3 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Max Reitz @ 2020-11-09 17:38 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, Peter Maydell, qemu-devel, Max Reitz
From: shiliyang <shiliyang@huawei.com>
There have some code style problems be found when read the block driver code.
So I fixes some problems of this error, ERROR: "foo* bar" should be "foo *bar".
Signed-off-by: Liyang Shi <shiliyang@huawei.com>
Reported-by: Euler Robot <euler.robot@huawei.com>
---
block/qcow2.h | 6 +++---
block/blkdebug.c | 2 +-
block/dmg.c | 2 +-
block/qcow2.c | 4 ++--
block/vpc.c | 10 +++++-----
5 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/block/qcow2.h b/block/qcow2.h
index 2e0272a7b8..0678073b74 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -343,8 +343,8 @@ typedef struct BDRVQcow2State {
uint64_t l1_table_offset;
uint64_t *l1_table;
- Qcow2Cache* l2_table_cache;
- Qcow2Cache* refcount_block_cache;
+ Qcow2Cache *l2_table_cache;
+ Qcow2Cache *refcount_block_cache;
QEMUTimer *cache_clean_timer;
unsigned cache_clean_interval;
@@ -394,7 +394,7 @@ typedef struct BDRVQcow2State {
uint64_t autoclear_features;
size_t unknown_header_fields_size;
- void* unknown_header_fields;
+ void *unknown_header_fields;
QLIST_HEAD(, Qcow2UnknownHeaderExtension) unknown_header_ext;
QTAILQ_HEAD (, Qcow2DiscardRegion) discards;
bool cache_discards;
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 54da719dd1..5fe6172da9 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -173,7 +173,7 @@ static int add_rule(void *opaque, QemuOpts *opts, Error **errp)
{
struct add_rule_data *d = opaque;
BDRVBlkdebugState *s = d->s;
- const char* event_name;
+ const char *event_name;
int event;
struct BlkdebugRule *rule;
int64_t sector;
diff --git a/block/dmg.c b/block/dmg.c
index 0d6c317296..ef35a505f2 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -559,7 +559,7 @@ static void dmg_refresh_limits(BlockDriverState *bs, Error **errp)
bs->bl.request_alignment = BDRV_SECTOR_SIZE; /* No sub-sector I/O */
}
-static inline int is_sector_in_chunk(BDRVDMGState* s,
+static inline int is_sector_in_chunk(BDRVDMGState *s,
uint32_t chunk_num, uint64_t sector_num)
{
if (chunk_num >= s->n_chunks || s->sectors[chunk_num] > sector_num ||
diff --git a/block/qcow2.c b/block/qcow2.c
index 1b0733238b..3a90ef2786 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -269,7 +269,7 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
case QCOW2_EXT_MAGIC_FEATURE_TABLE:
if (p_feature_table != NULL) {
- void* feature_table = g_malloc0(ext.len + 2 * sizeof(Qcow2Feature));
+ void *feature_table = g_malloc0(ext.len + 2 * sizeof(Qcow2Feature));
ret = bdrv_pread(bs->file, offset , feature_table, ext.len);
if (ret < 0) {
error_setg_errno(errp, -ret, "ERROR: ext_feature_table: "
@@ -3388,7 +3388,7 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
size_t cluster_size;
int version;
int refcount_order;
- uint64_t* refcount_table;
+ uint64_t *refcount_table;
int ret;
uint8_t compression_type = QCOW2_COMPRESSION_TYPE_ZLIB;
diff --git a/block/vpc.c b/block/vpc.c
index 890554277e..1ab55f9287 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -172,7 +172,7 @@ static QemuOptsList vpc_runtime_opts = {
static QemuOptsList vpc_create_opts;
-static uint32_t vpc_checksum(uint8_t* buf, size_t size)
+static uint32_t vpc_checksum(uint8_t *buf, size_t size)
{
uint32_t res = 0;
int i;
@@ -528,7 +528,7 @@ static inline int64_t get_image_offset(BlockDriverState *bs, uint64_t offset,
*
* Returns 0 on success and < 0 on error
*/
-static int rewrite_footer(BlockDriverState* bs)
+static int rewrite_footer(BlockDriverState *bs)
{
int ret;
BDRVVPCState *s = bs->opaque;
@@ -548,7 +548,7 @@ static int rewrite_footer(BlockDriverState* bs)
*
* Returns the sectors' offset in the image file on success and < 0 on error
*/
-static int64_t alloc_block(BlockDriverState* bs, int64_t offset)
+static int64_t alloc_block(BlockDriverState *bs, int64_t offset)
{
BDRVVPCState *s = bs->opaque;
int64_t bat_offset;
@@ -781,8 +781,8 @@ static int coroutine_fn vpc_co_block_status(BlockDriverState *bs,
* the hardware EIDE and ATA-2 limit of 16 heads (max disk size of 127 GB)
* and instead allow up to 255 heads.
*/
-static int calculate_geometry(int64_t total_sectors, uint16_t* cyls,
- uint8_t* heads, uint8_t* secs_per_cyl)
+static int calculate_geometry(int64_t total_sectors, uint16_t *cyls,
+ uint8_t *heads, uint8_t *secs_per_cyl)
{
uint32_t cyls_times_heads;
--
2.28.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PULL 13/15] block: add forgotten bdrv_abort_perm_update() to bdrv_co_invalidate_cache()
2020-11-09 17:38 [PULL 00/15] Block patches for 5.2.0-rc1 Max Reitz
` (11 preceding siblings ...)
2020-11-09 17:38 ` [PULL 12/15] block: Fix some code style problems, "foo* bar" should be "foo *bar" Max Reitz
@ 2020-11-09 17:38 ` Max Reitz
2020-11-09 17:38 ` [PULL 14/15] block: add bdrv_replace_node_common() Max Reitz
` (2 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Max Reitz @ 2020-11-09 17:38 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, Peter Maydell, qemu-devel, Max Reitz
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20201106124241.16950-2-vsementsov@virtuozzo.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/block.c b/block.c
index 2fd932154e..1e68bd2f5f 100644
--- a/block.c
+++ b/block.c
@@ -5787,6 +5787,7 @@ int coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, Error **errp)
bdrv_get_cumulative_perm(bs, &perm, &shared_perm);
ret = bdrv_check_perm(bs, NULL, perm, shared_perm, NULL, NULL, errp);
if (ret < 0) {
+ bdrv_abort_perm_update(bs);
bs->open_flags |= BDRV_O_INACTIVE;
return ret;
}
--
2.28.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PULL 14/15] block: add bdrv_replace_node_common()
2020-11-09 17:38 [PULL 00/15] Block patches for 5.2.0-rc1 Max Reitz
` (12 preceding siblings ...)
2020-11-09 17:38 ` [PULL 13/15] block: add forgotten bdrv_abort_perm_update() to bdrv_co_invalidate_cache() Max Reitz
@ 2020-11-09 17:38 ` Max Reitz
2020-11-09 17:38 ` [PULL 15/15] block: make bdrv_drop_intermediate() less wrong Max Reitz
2020-11-09 17:49 ` [PULL 00/15] Block patches for 5.2.0-rc1 Max Reitz
15 siblings, 0 replies; 17+ messages in thread
From: Max Reitz @ 2020-11-09 17:38 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, Peter Maydell, qemu-devel, Max Reitz
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Add new parameter to bdrv_replace_node(): auto_skip. With
auto_skip=false we'll have stricter behavior: update _all_ from
parents or fail. New behaviour will be used in the following commit in
block.c, so keep original function name as public interface.
Note: new error message is a bit funny in contrast with further
"Cannot" in case of frozen child, but we'd better keep some difference
to make it possible to distinguish one from another on failure. Still,
actually we'd better refactor should_update_child() call to distinguish
also different kinds of "should not". Let's do it later.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20201106124241.16950-3-vsementsov@virtuozzo.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 25 ++++++++++++++++++++++---
1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/block.c b/block.c
index 1e68bd2f5f..9a945a058d 100644
--- a/block.c
+++ b/block.c
@@ -4563,8 +4563,16 @@ static bool should_update_child(BdrvChild *c, BlockDriverState *to)
return ret;
}
-void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
- Error **errp)
+/*
+ * With auto_skip=true bdrv_replace_node_common skips updating from parents
+ * if it creates a parent-child relation loop or if parent is block-job.
+ *
+ * With auto_skip=false the error is returned if from has a parent which should
+ * not be updated.
+ */
+static void bdrv_replace_node_common(BlockDriverState *from,
+ BlockDriverState *to,
+ bool auto_skip, Error **errp)
{
BdrvChild *c, *next;
GSList *list = NULL, *p;
@@ -4583,7 +4591,12 @@ void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
QLIST_FOREACH_SAFE(c, &from->parents, next_parent, next) {
assert(c->bs == from);
if (!should_update_child(c, to)) {
- continue;
+ if (auto_skip) {
+ continue;
+ }
+ error_setg(errp, "Should not change '%s' link to '%s'",
+ c->name, from->node_name);
+ goto out;
}
if (c->frozen) {
error_setg(errp, "Cannot change '%s' link to '%s'",
@@ -4623,6 +4636,12 @@ out:
bdrv_unref(from);
}
+void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
+ Error **errp)
+{
+ return bdrv_replace_node_common(from, to, true, errp);
+}
+
/*
* Add new bs contents at the top of an image chain while the chain is
* live, while keeping required fields on the top layer.
--
2.28.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PULL 15/15] block: make bdrv_drop_intermediate() less wrong
2020-11-09 17:38 [PULL 00/15] Block patches for 5.2.0-rc1 Max Reitz
` (13 preceding siblings ...)
2020-11-09 17:38 ` [PULL 14/15] block: add bdrv_replace_node_common() Max Reitz
@ 2020-11-09 17:38 ` Max Reitz
2020-11-09 17:49 ` [PULL 00/15] Block patches for 5.2.0-rc1 Max Reitz
15 siblings, 0 replies; 17+ messages in thread
From: Max Reitz @ 2020-11-09 17:38 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, Peter Maydell, qemu-devel, Max Reitz
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
First, permission update loop tries to do iterations transactionally,
but the whole update is not transactional: nobody roll-back successful
loop iterations when some iteration fails.
Second, in the iteration we have nested permission update:
c->klass->update_filename may point to bdrv_child_cb_update_filename()
which calls bdrv_backing_update_filename(), which may do node reopen to
RW.
Permission update system is not prepared to nested updates, at least it
has intermediate permission-update state stored in BdrvChild
structures: has_backup_perm, backup_perm and backup_shared_perm.
So, let's first do bdrv_replace_node_common() (which is more
transactional than open-coded update in bdrv_drop_intermediate()) and
then call update_filename() in separate. We still do not rollback
changes in case of update_filename() failure but it's not much worse
than pre-patch behavior.
Note that bdrv_replace_node_common() does check for frozen children,
so corresponding check is dropped in bdrv_drop_intermediate().
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20201106124241.16950-4-vsementsov@virtuozzo.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 54 ++++++++++++++++++++++++------------------------------
1 file changed, 24 insertions(+), 30 deletions(-)
diff --git a/block.c b/block.c
index 9a945a058d..f1cedac362 100644
--- a/block.c
+++ b/block.c
@@ -4910,9 +4910,11 @@ int bdrv_drop_intermediate(BlockDriverState *top, BlockDriverState *base,
{
BlockDriverState *explicit_top = top;
bool update_inherits_from;
- BdrvChild *c, *next;
+ BdrvChild *c;
Error *local_err = NULL;
int ret = -EIO;
+ g_autoptr(GSList) updated_children = NULL;
+ GSList *p;
bdrv_ref(top);
bdrv_subtree_drained_begin(top);
@@ -4926,14 +4928,6 @@ int bdrv_drop_intermediate(BlockDriverState *top, BlockDriverState *base,
goto exit;
}
- /* This function changes all links that point to top and makes
- * them point to base. Check that none of them is frozen. */
- QLIST_FOREACH(c, &top->parents, next_parent) {
- if (c->frozen) {
- goto exit;
- }
- }
-
/* If 'base' recursively inherits from 'top' then we should set
* base->inherits_from to top->inherits_from after 'top' and all
* other intermediate nodes have been dropped.
@@ -4950,36 +4944,36 @@ int bdrv_drop_intermediate(BlockDriverState *top, BlockDriverState *base,
backing_file_str = base->filename;
}
- QLIST_FOREACH_SAFE(c, &top->parents, next_parent, next) {
- /* Check whether we are allowed to switch c from top to base */
- GSList *ignore_children = g_slist_prepend(NULL, c);
- ret = bdrv_check_update_perm(base, NULL, c->perm, c->shared_perm,
- ignore_children, NULL, &local_err);
- g_slist_free(ignore_children);
- if (ret < 0) {
- error_report_err(local_err);
- goto exit;
- }
+ QLIST_FOREACH(c, &top->parents, next_parent) {
+ updated_children = g_slist_prepend(updated_children, c);
+ }
+
+ bdrv_replace_node_common(top, base, false, &local_err);
+ if (local_err) {
+ error_report_err(local_err);
+ goto exit;
+ }
+
+ for (p = updated_children; p; p = p->next) {
+ c = p->data;
- /* If so, update the backing file path in the image file */
if (c->klass->update_filename) {
ret = c->klass->update_filename(c, base, backing_file_str,
&local_err);
if (ret < 0) {
- bdrv_abort_perm_update(base);
+ /*
+ * TODO: Actually, we want to rollback all previous iterations
+ * of this loop, and (which is almost impossible) previous
+ * bdrv_replace_node()...
+ *
+ * Note, that c->klass->update_filename may lead to permission
+ * update, so it's a bad idea to call it inside permission
+ * update transaction of bdrv_replace_node.
+ */
error_report_err(local_err);
goto exit;
}
}
-
- /*
- * Do the actual switch in the in-memory graph.
- * Completes bdrv_check_update_perm() transaction internally.
- * c->frozen is false, we have checked that above.
- */
- bdrv_ref(base);
- bdrv_replace_child(c, base);
- bdrv_unref(top);
}
if (update_inherits_from) {
--
2.28.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PULL 00/15] Block patches for 5.2.0-rc1
2020-11-09 17:38 [PULL 00/15] Block patches for 5.2.0-rc1 Max Reitz
` (14 preceding siblings ...)
2020-11-09 17:38 ` [PULL 15/15] block: make bdrv_drop_intermediate() less wrong Max Reitz
@ 2020-11-09 17:49 ` Max Reitz
15 siblings, 0 replies; 17+ messages in thread
From: Max Reitz @ 2020-11-09 17:49 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, Peter Maydell, qemu-devel
On 09.11.20 18:38, Max Reitz wrote:
> The following changes since commit 2a190a7256a3e0563b29ffd67e0164097b4a6dac:
>
> Merge remote-tracking branch 'remotes/philmd-gitlab/tags/renesas-fixes-20201109' into staging (2020-11-09 11:20:25 +0000)
>
> are available in the Git repository at:
>
> https://github.com/XanClic/qemu.git tags/pull-block-2020-11-09
>
> for you to fetch changes up to 8f88cb77b06be0c2e45ed31c9fd6817d2ce3836a:
>
> block: make bdrv_drop_intermediate() less wrong (2020-11-09 15:44:21 +0100)
>
> ----------------------------------------------------------------
> Block patches for 5.2.0-rc1:
> - Some nvme fixes (addressing problems spotted by Coverity)
> - Fix nfs compiling on mingw (and enable it in Cirrus)
> - Fix an error path in bdrv_co_invalidate_cache() (permission update
> was initiated, but not aborted)
> - Fix (on-error) roll back in bdrv_drop_intermediate(): Instead of
> inlining bdrv_replace_node() (wrongly), call that function
> - Fix for iotest 240
> - Fix error handling in bdrv_getlength()
> - Be more explicit about how QCowL2Meta objects are handled
> - Cleanups
Sorry, NAK. I applied patch 12 the wrong way (namely, just using
git-am) and so it’s missing the Message-Id and my Signed-off-by.
v2 is on the way.
Max
^ permalink raw reply [flat|nested] 17+ messages in thread