* [PULL 01/31] iotests: Drop deprecated 'props' from object-add
2021-03-05 16:54 [PULL 00/31] Block layer patches Kevin Wolf
@ 2021-03-05 16:54 ` Kevin Wolf
2021-03-05 16:54 ` [PULL 02/31] backup: Remove nodes from job in .clean() Kevin Wolf
` (30 subsequent siblings)
31 siblings, 0 replies; 35+ messages in thread
From: Kevin Wolf @ 2021-03-05 16:54 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Alberto Garcia <berto@igalia.com>
Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-Id: <20210222115737.2993-1-berto@igalia.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qemu-iotests/087 | 8 ++------
tests/qemu-iotests/184 | 18 ++++++------------
tests/qemu-iotests/218 | 2 +-
tests/qemu-iotests/235 | 2 +-
tests/qemu-iotests/245 | 4 ++--
tests/qemu-iotests/258 | 6 +++---
tests/qemu-iotests/258.out | 4 ++--
tests/qemu-iotests/295 | 2 +-
tests/qemu-iotests/296 | 2 +-
9 files changed, 19 insertions(+), 29 deletions(-)
diff --git a/tests/qemu-iotests/087 b/tests/qemu-iotests/087
index edd43f1a28..d8e0e384cd 100755
--- a/tests/qemu-iotests/087
+++ b/tests/qemu-iotests/087
@@ -143,9 +143,7 @@ run_qemu <<EOF
"arguments": {
"qom-type": "secret",
"id": "sec0",
- "props": {
- "data": "123456"
- }
+ "data": "123456"
}
}
{ "execute": "blockdev-add",
@@ -176,9 +174,7 @@ run_qemu <<EOF
"arguments": {
"qom-type": "secret",
"id": "sec0",
- "props": {
- "data": "123456"
- }
+ "data": "123456"
}
}
{ "execute": "blockdev-add",
diff --git a/tests/qemu-iotests/184 b/tests/qemu-iotests/184
index 513d167098..e4cbcd8634 100755
--- a/tests/qemu-iotests/184
+++ b/tests/qemu-iotests/184
@@ -67,10 +67,8 @@ run_qemu <<EOF
"arguments": {
"qom-type": "throttle-group",
"id": "group0",
- "props": {
- "limits" : {
- "iops-total": 1000
- }
+ "limits" : {
+ "iops-total": 1000
}
}
}
@@ -96,10 +94,8 @@ run_qemu <<EOF
"arguments": {
"qom-type": "throttle-group",
"id": "group0",
- "props" : {
- "limits": {
- "iops-total": 1000
- }
+ "limits": {
+ "iops-total": 1000
}
}
}
@@ -136,10 +132,8 @@ run_qemu <<EOF
"arguments": {
"qom-type": "throttle-group",
"id": "group0",
- "props" : {
- "limits": {
- "iops-total": 1000
- }
+ "limits": {
+ "iops-total": 1000
}
}
}
diff --git a/tests/qemu-iotests/218 b/tests/qemu-iotests/218
index ae7c4fb187..325d8244fb 100755
--- a/tests/qemu-iotests/218
+++ b/tests/qemu-iotests/218
@@ -152,7 +152,7 @@ with iotests.VM() as vm, \
vm.launch()
ret = vm.qmp('object-add', qom_type='throttle-group', id='tg',
- props={'x-bps-read': 4096})
+ limits={'bps-read': 4096})
assert ret['return'] == {}
ret = vm.qmp('blockdev-add',
diff --git a/tests/qemu-iotests/235 b/tests/qemu-iotests/235
index 20d16dbf38..8aed45f9a7 100755
--- a/tests/qemu-iotests/235
+++ b/tests/qemu-iotests/235
@@ -57,7 +57,7 @@ vm.add_args('-drive', 'id=src,file=' + disk)
vm.launch()
log(vm.qmp('object-add', qom_type='throttle-group', id='tg0',
- props={ 'x-bps-total': size }))
+ limits={'bps-total': size}))
log(vm.qmp('blockdev-add',
**{ 'node-name': 'target',
diff --git a/tests/qemu-iotests/245 b/tests/qemu-iotests/245
index cfdeb902be..30b1d7b22d 100755
--- a/tests/qemu-iotests/245
+++ b/tests/qemu-iotests/245
@@ -644,12 +644,12 @@ class TestBlockdevReopen(iotests.QMPTestCase):
###### throttle ######
######################
opts = { 'qom-type': 'throttle-group', 'id': 'group0',
- 'props': { 'limits': { 'iops-total': 1000 } } }
+ 'limits': { 'iops-total': 1000 } }
result = self.vm.qmp('object-add', conv_keys = False, **opts)
self.assert_qmp(result, 'return', {})
opts = { 'qom-type': 'throttle-group', 'id': 'group1',
- 'props': { 'limits': { 'iops-total': 2000 } } }
+ 'limits': { 'iops-total': 2000 } }
result = self.vm.qmp('object-add', conv_keys = False, **opts)
self.assert_qmp(result, 'return', {})
diff --git a/tests/qemu-iotests/258 b/tests/qemu-iotests/258
index 9a2d33ae5e..a6618208a8 100755
--- a/tests/qemu-iotests/258
+++ b/tests/qemu-iotests/258
@@ -103,9 +103,9 @@ def test_concurrent_finish(write_to_stream_node):
vm.qmp_log('object-add',
qom_type='throttle-group',
id='tg',
- props={
- 'x-iops-write': 1,
- 'x-iops-write-max': 1
+ limits={
+ 'iops-write': 1,
+ 'iops-write-max': 1
})
vm.qmp_log('blockdev-add',
diff --git a/tests/qemu-iotests/258.out b/tests/qemu-iotests/258.out
index ce6e9ba3e5..c3a003d3e3 100644
--- a/tests/qemu-iotests/258.out
+++ b/tests/qemu-iotests/258.out
@@ -2,7 +2,7 @@ Running tests:
=== Commit and stream finish concurrently (letting stream write) ===
-{"execute": "object-add", "arguments": {"id": "tg", "props": {"x-iops-write": 1, "x-iops-write-max": 1}, "qom-type": "throttle-group"}}
+{"execute": "object-add", "arguments": {"id": "tg", "limits": {"iops-write": 1, "iops-write-max": 1}, "qom-type": "throttle-group"}}
{"return": {}}
{"execute": "blockdev-add", "arguments": {"backing": {"backing": {"backing": {"backing": {"driver": "raw", "file": {"driver": "file", "filename": "TEST_DIR/PID-node0.img"}, "node-name": "node0"}, "driver": "IMGFMT", "file": {"driver": "file", "filename": "TEST_DIR/PID-node1.img"}, "node-name": "node1"}, "driver": "IMGFMT", "file": {"driver": "file", "filename": "TEST_DIR/PID-node2.img"}, "node-name": "node2"}, "driver": "IMGFMT", "file": {"driver": "file", "filename": "TEST_DIR/PID-node3.img"}, "node-name": "node3"}, "driver": "IMGFMT", "file": {"driver": "throttle", "file": {"driver": "file", "filename": "TEST_DIR/PID-node4.img"}, "throttle-group": "tg"}, "node-name": "node4"}}
{"return": {}}
@@ -18,7 +18,7 @@ Running tests:
=== Commit and stream finish concurrently (letting commit write) ===
-{"execute": "object-add", "arguments": {"id": "tg", "props": {"x-iops-write": 1, "x-iops-write-max": 1}, "qom-type": "throttle-group"}}
+{"execute": "object-add", "arguments": {"id": "tg", "limits": {"iops-write": 1, "iops-write-max": 1}, "qom-type": "throttle-group"}}
{"return": {}}
{"execute": "blockdev-add", "arguments": {"backing": {"backing": {"backing": {"backing": {"driver": "raw", "file": {"driver": "throttle", "file": {"driver": "file", "filename": "TEST_DIR/PID-node0.img"}, "throttle-group": "tg"}, "node-name": "node0"}, "driver": "IMGFMT", "file": {"driver": "file", "filename": "TEST_DIR/PID-node1.img"}, "node-name": "node1"}, "driver": "IMGFMT", "file": {"driver": "file", "filename": "TEST_DIR/PID-node2.img"}, "node-name": "node2"}, "driver": "IMGFMT", "file": {"driver": "file", "filename": "TEST_DIR/PID-node3.img"}, "node-name": "node3"}, "driver": "IMGFMT", "file": {"driver": "file", "filename": "TEST_DIR/PID-node4.img"}, "node-name": "node4"}}
{"return": {}}
diff --git a/tests/qemu-iotests/295 b/tests/qemu-iotests/295
index 01a6c0b31f..270ad3999f 100755
--- a/tests/qemu-iotests/295
+++ b/tests/qemu-iotests/295
@@ -43,7 +43,7 @@ class Secret:
def to_qmp_object(self):
return { "qom_type" : "secret", "id": self.id(),
- "props": { "data": self.secret() } }
+ "data": self.secret() }
################################################################################
class EncryptionSetupTestCase(iotests.QMPTestCase):
diff --git a/tests/qemu-iotests/296 b/tests/qemu-iotests/296
index 0bc3c6c7d7..7c65e987a1 100755
--- a/tests/qemu-iotests/296
+++ b/tests/qemu-iotests/296
@@ -43,7 +43,7 @@ class Secret:
def to_qmp_object(self):
return { "qom_type" : "secret", "id": self.id(),
- "props": { "data": self.secret() } }
+ "data": self.secret() }
################################################################################
--
2.29.2
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PULL 02/31] backup: Remove nodes from job in .clean()
2021-03-05 16:54 [PULL 00/31] Block layer patches Kevin Wolf
2021-03-05 16:54 ` [PULL 01/31] iotests: Drop deprecated 'props' from object-add Kevin Wolf
@ 2021-03-05 16:54 ` Kevin Wolf
2021-03-05 16:54 ` [PULL 03/31] backup-top: Refuse I/O in inactive state Kevin Wolf
` (29 subsequent siblings)
31 siblings, 0 replies; 35+ messages in thread
From: Kevin Wolf @ 2021-03-05 16:54 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Max Reitz <mreitz@redhat.com>
The block job holds a reference to the backup-top node (because it is
passed as the main job BDS to block_job_create()). Therefore,
bdrv_backup_top_drop() cannot delete the backup-top node (replacing it
by its child does not affect the job parent, because that has
.stay_at_node set). That is a problem, because all of its I/O functions
assume the BlockCopyState (s->bcs) to be valid and that it has a
filtered child; but after bdrv_backup_top_drop(), neither of those
things are true.
It does not make sense to add new parents to backup-top after
backup_clean(), so we should detach it from the job before
bdrv_backup_top_drop(). Because there is no function to do that for a
single node, just detach all of the job's nodes -- the job does not do
anything past backup_clean() anyway.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20210219153348.41861-2-mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/backup.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/block/backup.c b/block/backup.c
index 94e6dcd72e..6cf2f974aa 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -103,6 +103,7 @@ static void backup_abort(Job *job)
static void backup_clean(Job *job)
{
BackupBlockJob *s = container_of(job, BackupBlockJob, common.job);
+ block_job_remove_all_bdrv(&s->common);
bdrv_backup_top_drop(s->backup_top);
}
--
2.29.2
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PULL 03/31] backup-top: Refuse I/O in inactive state
2021-03-05 16:54 [PULL 00/31] Block layer patches Kevin Wolf
2021-03-05 16:54 ` [PULL 01/31] iotests: Drop deprecated 'props' from object-add Kevin Wolf
2021-03-05 16:54 ` [PULL 02/31] backup: Remove nodes from job in .clean() Kevin Wolf
@ 2021-03-05 16:54 ` Kevin Wolf
2021-03-05 16:54 ` [PULL 04/31] iotests/283: Check that finalize drops backup-top Kevin Wolf
` (28 subsequent siblings)
31 siblings, 0 replies; 35+ messages in thread
From: Kevin Wolf @ 2021-03-05 16:54 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Max Reitz <mreitz@redhat.com>
When the backup-top node transitions from active to inactive in
bdrv_backup_top_drop(), the BlockCopyState is freed and the filtered
child is removed, so the node effectively becomes unusable.
However, noone told its I/O functions this, so they will happily
continue accessing bs->backing and s->bcs. Prevent that by aborting
early when s->active is false.
(After the preceding patch, the node should be gone after
bdrv_backup_top_drop(), so this should largely be a theoretical problem.
But still, better to be safe than sorry, and also I think it just makes
sense to check s->active in the I/O functions.)
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20210219153348.41861-3-mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/backup-top.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/block/backup-top.c b/block/backup-top.c
index d1253e1aa6..589e8b651d 100644
--- a/block/backup-top.c
+++ b/block/backup-top.c
@@ -45,6 +45,12 @@ static coroutine_fn int backup_top_co_preadv(
BlockDriverState *bs, uint64_t offset, uint64_t bytes,
QEMUIOVector *qiov, int flags)
{
+ BDRVBackupTopState *s = bs->opaque;
+
+ if (!s->active) {
+ return -EIO;
+ }
+
return bdrv_co_preadv(bs->backing, offset, bytes, qiov, flags);
}
@@ -54,6 +60,10 @@ static coroutine_fn int backup_top_cbw(BlockDriverState *bs, uint64_t offset,
BDRVBackupTopState *s = bs->opaque;
uint64_t off, end;
+ if (!s->active) {
+ return -EIO;
+ }
+
if (flags & BDRV_REQ_WRITE_UNCHANGED) {
return 0;
}
--
2.29.2
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PULL 04/31] iotests/283: Check that finalize drops backup-top
2021-03-05 16:54 [PULL 00/31] Block layer patches Kevin Wolf
` (2 preceding siblings ...)
2021-03-05 16:54 ` [PULL 03/31] backup-top: Refuse I/O in inactive state Kevin Wolf
@ 2021-03-05 16:54 ` Kevin Wolf
2021-03-05 16:54 ` [PULL 05/31] iotests: Fix up python style in 300 Kevin Wolf
` (27 subsequent siblings)
31 siblings, 0 replies; 35+ messages in thread
From: Kevin Wolf @ 2021-03-05 16:54 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Max Reitz <mreitz@redhat.com>
Without any of HEAD^ or HEAD^^ applied, qemu will most likely crash on
the qemu-io invocation, for a variety of immediate reasons. The
underlying problem is generally a use-after-free access into
backup-top's BlockCopyState.
With only HEAD^ applied, qemu-io will run into an EIO (which is not
capture by the output, but you can see that the qemu-io invocation will
be accepted (i.e., qemu-io will run) in contrast to the reference
output, where the node name cannot be found), and qemu will then crash
in query-named-block-nodes: bdrv_get_allocated_file_size() detects
backup-top to be a filter and passes the request through to its child.
However, after bdrv_backup_top_drop(), that child is NULL, so the
recursive call crashes.
With HEAD^^ applied, this test should pass.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20210219153348.41861-4-mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qemu-iotests/283 | 53 ++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/283.out | 15 +++++++++++
2 files changed, 68 insertions(+)
diff --git a/tests/qemu-iotests/283 b/tests/qemu-iotests/283
index 79643e375b..010c22f0a2 100755
--- a/tests/qemu-iotests/283
+++ b/tests/qemu-iotests/283
@@ -97,3 +97,56 @@ vm.qmp_log('blockdev-add', **{
vm.qmp_log('blockdev-backup', sync='full', device='source', target='target')
vm.shutdown()
+
+
+print('\n=== backup-top should be gone after job-finalize ===\n')
+
+# Check that the backup-top node is gone after job-finalize.
+#
+# During finalization, the node becomes inactive and can no longer
+# function. If it is still present, new parents might be attached, and
+# there would be no meaningful way to handle their I/O requests.
+
+vm = iotests.VM()
+vm.launch()
+
+vm.qmp_log('blockdev-add', **{
+ 'node-name': 'source',
+ 'driver': 'null-co',
+})
+
+vm.qmp_log('blockdev-add', **{
+ 'node-name': 'target',
+ 'driver': 'null-co',
+})
+
+vm.qmp_log('blockdev-backup',
+ job_id='backup',
+ device='source',
+ target='target',
+ sync='full',
+ filter_node_name='backup-filter',
+ auto_finalize=False,
+ auto_dismiss=False)
+
+vm.event_wait('BLOCK_JOB_PENDING', 5.0)
+
+# The backup-top filter should still be present prior to finalization
+assert vm.node_info('backup-filter') is not None
+
+vm.qmp_log('job-finalize', id='backup')
+vm.event_wait('BLOCK_JOB_COMPLETED', 5.0)
+
+# The filter should be gone now. Check that by trying to access it
+# with qemu-io (which will most likely crash qemu if it is still
+# there.).
+vm.qmp_log('human-monitor-command',
+ command_line='qemu-io backup-filter "write 0 1M"')
+
+# (Also, do an explicit check.)
+assert vm.node_info('backup-filter') is None
+
+vm.qmp_log('job-dismiss', id='backup')
+vm.event_wait('JOB_STATUS_CHANGE', 5.0, {'data': {'status': 'null'}})
+
+vm.shutdown()
diff --git a/tests/qemu-iotests/283.out b/tests/qemu-iotests/283.out
index d8cff22cc1..7e9cd9a7d4 100644
--- a/tests/qemu-iotests/283.out
+++ b/tests/qemu-iotests/283.out
@@ -6,3 +6,18 @@
{"return": {}}
{"execute": "blockdev-backup", "arguments": {"device": "source", "sync": "full", "target": "target"}}
{"error": {"class": "GenericError", "desc": "Cannot set permissions for backup-top filter: Conflicts with use by other as 'image', which uses 'write' on base"}}
+
+=== backup-top should be gone after job-finalize ===
+
+{"execute": "blockdev-add", "arguments": {"driver": "null-co", "node-name": "source"}}
+{"return": {}}
+{"execute": "blockdev-add", "arguments": {"driver": "null-co", "node-name": "target"}}
+{"return": {}}
+{"execute": "blockdev-backup", "arguments": {"auto-dismiss": false, "auto-finalize": false, "device": "source", "filter-node-name": "backup-filter", "job-id": "backup", "sync": "full", "target": "target"}}
+{"return": {}}
+{"execute": "job-finalize", "arguments": {"id": "backup"}}
+{"return": {}}
+{"execute": "human-monitor-command", "arguments": {"command-line": "qemu-io backup-filter \"write 0 1M\""}}
+{"return": "Error: Cannot find device= nor node_name=backup-filter\r\n"}
+{"execute": "job-dismiss", "arguments": {"id": "backup"}}
+{"return": {}}
--
2.29.2
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PULL 05/31] iotests: Fix up python style in 300
2021-03-05 16:54 [PULL 00/31] Block layer patches Kevin Wolf
` (3 preceding siblings ...)
2021-03-05 16:54 ` [PULL 04/31] iotests/283: Check that finalize drops backup-top Kevin Wolf
@ 2021-03-05 16:54 ` Kevin Wolf
2021-03-05 16:54 ` [PULL 06/31] blockjob: report a better error message Kevin Wolf
` (26 subsequent siblings)
31 siblings, 0 replies; 35+ messages in thread
From: Kevin Wolf @ 2021-03-05 16:54 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Eric Blake <eblake@redhat.com>
Break some long lines, and relax our type hints to be more generic to
any JSON, in order to more easily permit the additional JSON depth now
possible in migration parameters. Detected by iotest 297.
Fixes: ca4bfec41d56
(qemu-iotests: 300: Add test case for modifying persistence of bitmap)
Reported-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20210215220518.1745469-1-eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qemu-iotests/300 | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/tests/qemu-iotests/300 b/tests/qemu-iotests/300
index 63036f6a6e..adb9276297 100755
--- a/tests/qemu-iotests/300
+++ b/tests/qemu-iotests/300
@@ -22,7 +22,7 @@
import os
import random
import re
-from typing import Dict, List, Optional, Union
+from typing import Dict, List, Optional
import iotests
@@ -30,7 +30,7 @@ import iotests
# pylint: disable=wrong-import-order
import qemu
-BlockBitmapMapping = List[Dict[str, Union[str, List[Dict[str, str]]]]]
+BlockBitmapMapping = List[Dict[str, object]]
mig_sock = os.path.join(iotests.sock_dir, 'mig_sock')
@@ -602,7 +602,8 @@ class TestCrossAliasMigration(TestDirtyBitmapMigration):
class TestAliasTransformMigration(TestDirtyBitmapMigration):
"""
- Tests the 'transform' option which modifies bitmap persistence on migration.
+ Tests the 'transform' option which modifies bitmap persistence on
+ migration.
"""
src_node_name = 'node-a'
@@ -674,7 +675,8 @@ class TestAliasTransformMigration(TestDirtyBitmapMigration):
bitmaps = self.vm_b.query_bitmaps()
for node in bitmaps:
- bitmaps[node] = sorted(((bmap['name'], bmap['persistent']) for bmap in bitmaps[node]))
+ bitmaps[node] = sorted(((bmap['name'], bmap['persistent'])
+ for bmap in bitmaps[node]))
self.assertEqual(bitmaps,
{'node-a': [('bmap-a', True), ('bmap-b', False)],
--
2.29.2
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PULL 06/31] blockjob: report a better error message
2021-03-05 16:54 [PULL 00/31] Block layer patches Kevin Wolf
` (4 preceding siblings ...)
2021-03-05 16:54 ` [PULL 05/31] iotests: Fix up python style in 300 Kevin Wolf
@ 2021-03-05 16:54 ` Kevin Wolf
2021-03-05 16:54 ` [PULL 07/31] storage-daemon: report unexpected arguments on the fly Kevin Wolf
` (25 subsequent siblings)
31 siblings, 0 replies; 35+ messages in thread
From: Kevin Wolf @ 2021-03-05 16:54 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Stefano Garzarella <sgarzare@redhat.com>
When a block job fails, we report strerror(-job->job.ret) error
message, also if the job set an error object.
Let's report a better error message using error_get_pretty(job->job.err).
If an error object was not set, strerror(-job->ret) is used as fallback,
as explained in include/qemu/job.h:
typedef struct Job {
...
/**
* Error object for a failed job.
* If job->ret is nonzero and an error object was not set, it will be set
* to strerror(-job->ret) during job_completed.
*/
Error *err;
}
In block_job_query() there can be a transient where 'job.err' is not set
by a scheduled bottom half. In that case we use strerror(-job->ret) as it
was before.
Suggested-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Message-Id: <20210225103633.76746-1-sgarzare@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
blockjob.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/blockjob.c b/blockjob.c
index f2feff051d..ef968017a2 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -318,8 +318,12 @@ BlockJobInfo *block_job_query(BlockJob *job, Error **errp)
info->status = job->job.status;
info->auto_finalize = job->job.auto_finalize;
info->auto_dismiss = job->job.auto_dismiss;
- info->has_error = job->job.ret != 0;
- info->error = job->job.ret ? g_strdup(strerror(-job->job.ret)) : NULL;
+ if (job->job.ret) {
+ info->has_error = true;
+ info->error = job->job.err ?
+ g_strdup(error_get_pretty(job->job.err)) :
+ g_strdup(strerror(-job->job.ret));
+ }
return info;
}
@@ -356,7 +360,7 @@ static void block_job_event_completed(Notifier *n, void *opaque)
}
if (job->job.ret < 0) {
- msg = strerror(-job->job.ret);
+ msg = error_get_pretty(job->job.err);
}
qapi_event_send_block_job_completed(job_type(&job->job),
--
2.29.2
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PULL 07/31] storage-daemon: report unexpected arguments on the fly
2021-03-05 16:54 [PULL 00/31] Block layer patches Kevin Wolf
` (5 preceding siblings ...)
2021-03-05 16:54 ` [PULL 06/31] blockjob: report a better error message Kevin Wolf
@ 2021-03-05 16:54 ` Kevin Wolf
2021-03-05 16:54 ` [PULL 08/31] storage-daemon: include current command line option in the errors Kevin Wolf
` (24 subsequent siblings)
31 siblings, 0 replies; 35+ messages in thread
From: Kevin Wolf @ 2021-03-05 16:54 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Paolo Bonzini <pbonzini@redhat.com>
If the first character of optstring is '-', then each nonoption argv
element is handled as if it were the argument of an option with character
code 1. This removes the reordering of the argv array, and enables usage
of loc_set_cmdline to provide better error messages.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20210301152844.291799-2-pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
storage-daemon/qemu-storage-daemon.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c
index 9021a46b3a..b7e1b90fb1 100644
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -174,7 +174,7 @@ static void process_options(int argc, char *argv[])
* they are given on the command lines. This means that things must be
* defined first before they can be referenced in another option.
*/
- while ((c = getopt_long(argc, argv, "hT:V", long_options, NULL)) != -1) {
+ while ((c = getopt_long(argc, argv, "-hT:V", long_options, NULL)) != -1) {
switch (c) {
case '?':
exit(EXIT_FAILURE);
@@ -275,14 +275,13 @@ static void process_options(int argc, char *argv[])
qobject_unref(args);
break;
}
+ case 1:
+ error_report("Unexpected argument: %s", optarg);
+ exit(EXIT_FAILURE);
default:
g_assert_not_reached();
}
}
- if (optind != argc) {
- error_report("Unexpected argument: %s", argv[optind]);
- exit(EXIT_FAILURE);
- }
}
int main(int argc, char *argv[])
--
2.29.2
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PULL 08/31] storage-daemon: include current command line option in the errors
2021-03-05 16:54 [PULL 00/31] Block layer patches Kevin Wolf
` (6 preceding siblings ...)
2021-03-05 16:54 ` [PULL 07/31] storage-daemon: report unexpected arguments on the fly Kevin Wolf
@ 2021-03-05 16:54 ` Kevin Wolf
2021-03-05 16:54 ` [PULL 09/31] qemu-storage-daemon: add --pidfile option Kevin Wolf
` (23 subsequent siblings)
31 siblings, 0 replies; 35+ messages in thread
From: Kevin Wolf @ 2021-03-05 16:54 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Paolo Bonzini <pbonzini@redhat.com>
Use the location management facilities that the emulator uses, so that
the current command line option appears in the error message.
Before:
$ storage-daemon/qemu-storage-daemon --nbd key..=
qemu-storage-daemon: Invalid parameter 'key..'
After:
$ storage-daemon/qemu-storage-daemon --nbd key..=
qemu-storage-daemon: --nbd key..=: Invalid parameter 'key..'
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20210301152844.291799-3-pbonzini@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
storage-daemon/qemu-storage-daemon.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c
index b7e1b90fb1..78ddf619d4 100644
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -152,6 +152,20 @@ static void init_qmp_commands(void)
qmp_marshal_qmp_capabilities, QCO_ALLOW_PRECONFIG);
}
+static int getopt_set_loc(int argc, char **argv, const char *optstring,
+ const struct option *longopts)
+{
+ int c, save_index;
+
+ optarg = NULL;
+ save_index = optind;
+ c = getopt_long(argc, argv, optstring, longopts, NULL);
+ if (optarg) {
+ loc_set_cmdline(argv, save_index, MAX(1, optind - save_index));
+ }
+ return c;
+}
+
static void process_options(int argc, char *argv[])
{
int c;
@@ -174,7 +188,7 @@ static void process_options(int argc, char *argv[])
* they are given on the command lines. This means that things must be
* defined first before they can be referenced in another option.
*/
- while ((c = getopt_long(argc, argv, "-hT:V", long_options, NULL)) != -1) {
+ while ((c = getopt_set_loc(argc, argv, "-hT:V", long_options)) != -1) {
switch (c) {
case '?':
exit(EXIT_FAILURE);
@@ -276,12 +290,13 @@ static void process_options(int argc, char *argv[])
break;
}
case 1:
- error_report("Unexpected argument: %s", optarg);
+ error_report("Unexpected argument");
exit(EXIT_FAILURE);
default:
g_assert_not_reached();
}
}
+ loc_set_none();
}
int main(int argc, char *argv[])
--
2.29.2
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PULL 09/31] qemu-storage-daemon: add --pidfile option
2021-03-05 16:54 [PULL 00/31] Block layer patches Kevin Wolf
` (7 preceding siblings ...)
2021-03-05 16:54 ` [PULL 08/31] storage-daemon: include current command line option in the errors Kevin Wolf
@ 2021-03-05 16:54 ` Kevin Wolf
2021-03-05 16:54 ` [PULL 10/31] docs: show how to spawn qemu-storage-daemon with fd passing Kevin Wolf
` (22 subsequent siblings)
31 siblings, 0 replies; 35+ messages in thread
From: Kevin Wolf @ 2021-03-05 16:54 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Stefan Hajnoczi <stefanha@redhat.com>
Daemons often have a --pidfile option where the pid is written to a file
so that scripts can stop the daemon by sending a signal.
The pid file also acts as a lock to prevent multiple instances of the
daemon from launching for a given pid file.
QEMU, qemu-nbd, qemu-ga, virtiofsd, and qemu-pr-helper all support the
--pidfile option. Add it to qemu-storage-daemon too.
Reported-by: Richard W.M. Jones <rjones@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20210302142746.170535-1-stefanha@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
docs/tools/qemu-storage-daemon.rst | 14 +++++++++++
storage-daemon/qemu-storage-daemon.c | 36 ++++++++++++++++++++++++++++
2 files changed, 50 insertions(+)
diff --git a/docs/tools/qemu-storage-daemon.rst b/docs/tools/qemu-storage-daemon.rst
index c05b3d3811..6ce85f2f7d 100644
--- a/docs/tools/qemu-storage-daemon.rst
+++ b/docs/tools/qemu-storage-daemon.rst
@@ -118,6 +118,20 @@ Standard options:
List object properties with ``<type>,help``. See the :manpage:`qemu(1)`
manual page for a description of the object properties.
+.. option:: --pidfile PATH
+
+ is the path to a file where the daemon writes its pid. This allows scripts to
+ stop the daemon by sending a signal::
+
+ $ kill -SIGTERM $(<path/to/qsd.pid)
+
+ A file lock is applied to the file so only one instance of the daemon can run
+ with a given pid file path. The daemon unlinks its pid file when terminating.
+
+ The pid file is written after chardevs, exports, and NBD servers have been
+ created but before accepting connections. The daemon has started successfully
+ when the pid file is written and clients may begin connecting.
+
Examples
--------
Launch the daemon with QMP monitor socket ``qmp.sock`` so clients can execute
diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c
index 78ddf619d4..23756fc8e5 100644
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -59,6 +59,7 @@
#include "sysemu/runstate.h"
#include "trace/control.h"
+static const char *pid_file;
static volatile bool exit_requested = false;
void qemu_system_killed(int signal, pid_t pid)
@@ -115,6 +116,8 @@ static void help(void)
" See the qemu(1) man page for documentation of the\n"
" objects that can be added.\n"
"\n"
+" --pidfile <path> write process ID to a file after startup\n"
+"\n"
QEMU_HELP_BOTTOM "\n",
error_get_progname());
}
@@ -126,6 +129,7 @@ enum {
OPTION_MONITOR,
OPTION_NBD_SERVER,
OPTION_OBJECT,
+ OPTION_PIDFILE,
};
extern QemuOptsList qemu_chardev_opts;
@@ -178,6 +182,7 @@ static void process_options(int argc, char *argv[])
{"monitor", required_argument, NULL, OPTION_MONITOR},
{"nbd-server", required_argument, NULL, OPTION_NBD_SERVER},
{"object", required_argument, NULL, OPTION_OBJECT},
+ {"pidfile", required_argument, NULL, OPTION_PIDFILE},
{"trace", required_argument, NULL, 'T'},
{"version", no_argument, NULL, 'V'},
{0, 0, 0, 0}
@@ -289,6 +294,9 @@ static void process_options(int argc, char *argv[])
qobject_unref(args);
break;
}
+ case OPTION_PIDFILE:
+ pid_file = optarg;
+ break;
case 1:
error_report("Unexpected argument");
exit(EXIT_FAILURE);
@@ -299,6 +307,27 @@ static void process_options(int argc, char *argv[])
loc_set_none();
}
+static void pid_file_cleanup(void)
+{
+ unlink(pid_file);
+}
+
+static void pid_file_init(void)
+{
+ Error *err = NULL;
+
+ if (!pid_file) {
+ return;
+ }
+
+ if (!qemu_write_pidfile(pid_file, &err)) {
+ error_reportf_err(err, "cannot create PID file: ");
+ exit(EXIT_FAILURE);
+ }
+
+ atexit(pid_file_cleanup);
+}
+
int main(int argc, char *argv[])
{
#ifdef CONFIG_POSIX
@@ -326,6 +355,13 @@ int main(int argc, char *argv[])
qemu_init_main_loop(&error_fatal);
process_options(argc, argv);
+ /*
+ * Write the pid file after creating chardevs, exports, and NBD servers but
+ * before accepting connections. This ordering is documented. Do not change
+ * it.
+ */
+ pid_file_init();
+
while (!exit_requested) {
main_loop_wait(false);
}
--
2.29.2
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PULL 10/31] docs: show how to spawn qemu-storage-daemon with fd passing
2021-03-05 16:54 [PULL 00/31] Block layer patches Kevin Wolf
` (8 preceding siblings ...)
2021-03-05 16:54 ` [PULL 09/31] qemu-storage-daemon: add --pidfile option Kevin Wolf
@ 2021-03-05 16:54 ` Kevin Wolf
2021-03-05 16:54 ` [PULL 11/31] docs: replace insecure /tmp examples in qsd docs Kevin Wolf
` (21 subsequent siblings)
31 siblings, 0 replies; 35+ messages in thread
From: Kevin Wolf @ 2021-03-05 16:54 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Stefan Hajnoczi <stefanha@redhat.com>
The QMP monitor, NBD server, and vhost-user-blk export all support file
descriptor passing. This is a useful technique because it allows the
parent process to spawn and wait for qemu-storage-daemon without busy
waiting, which may delay startup due to arbitrary sleep() calls.
This Python example is inspired by the test case written for libnbd by
Richard W.M. Jones <rjones@redhat.com>:
https://gitlab.com/nbdkit/libnbd/-/commit/89113f484effb0e6c322314ba75c1cbe07a04543
Thanks to Daniel P. Berrangé <berrange@redhat.com> for suggestions on
how to get this working. Now let's document it!
Reported-by: Richard W.M. Jones <rjones@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20210301172728.135331-2-stefanha@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
docs/tools/qemu-storage-daemon.rst | 42 ++++++++++++++++++++++++++++--
1 file changed, 40 insertions(+), 2 deletions(-)
diff --git a/docs/tools/qemu-storage-daemon.rst b/docs/tools/qemu-storage-daemon.rst
index 6ce85f2f7d..5714794775 100644
--- a/docs/tools/qemu-storage-daemon.rst
+++ b/docs/tools/qemu-storage-daemon.rst
@@ -101,10 +101,12 @@ Standard options:
.. option:: --nbd-server addr.type=inet,addr.host=<host>,addr.port=<port>[,tls-creds=<id>][,tls-authz=<id>][,max-connections=<n>]
--nbd-server addr.type=unix,addr.path=<path>[,tls-creds=<id>][,tls-authz=<id>][,max-connections=<n>]
+ --nbd-server addr.type=fd,addr.str=<fd>[,tls-creds=<id>][,tls-authz=<id>][,max-connections=<n>]
is a server for NBD exports. Both TCP and UNIX domain sockets are supported.
- TLS encryption can be configured using ``--object`` tls-creds-* and authz-*
- secrets (see below).
+ A listen socket can be provided via file descriptor passing (see Examples
+ below). TLS encryption can be configured using ``--object`` tls-creds-* and
+ authz-* secrets (see below).
To configure an NBD server on UNIX domain socket path ``/tmp/nbd.sock``::
@@ -141,6 +143,42 @@ QMP commands::
--chardev socket,path=qmp.sock,server=on,wait=off,id=char1 \
--monitor chardev=char1
+Launch the daemon from Python with a QMP monitor socket using file descriptor
+passing so there is no need to busy wait for the QMP monitor to become
+available::
+
+ #!/usr/bin/env python3
+ import subprocess
+ import socket
+
+ sock_path = '/var/run/qmp.sock'
+
+ with socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) as listen_sock:
+ listen_sock.bind(sock_path)
+ listen_sock.listen()
+
+ fd = listen_sock.fileno()
+
+ subprocess.Popen(
+ ['qemu-storage-daemon',
+ '--chardev', f'socket,fd={fd},server=on,id=char1',
+ '--monitor', 'chardev=char1'],
+ pass_fds=[fd],
+ )
+
+ # listen_sock was automatically closed when leaving the 'with' statement
+ # body. If the daemon process terminated early then the following connect()
+ # will fail with "Connection refused" because no process has the listen
+ # socket open anymore. Launch errors can be detected this way.
+
+ qmp_sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
+ qmp_sock.connect(sock_path)
+ ...QMP interaction...
+
+The same socket spawning approach also works with the ``--nbd-server
+addr.type=fd,addr.str=<fd>`` and ``--export
+type=vhost-user-blk,addr.type=fd,addr.str=<fd>`` options.
+
Export raw image file ``disk.img`` over NBD UNIX domain socket ``nbd.sock``::
$ qemu-storage-daemon \
--
2.29.2
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PULL 11/31] docs: replace insecure /tmp examples in qsd docs
2021-03-05 16:54 [PULL 00/31] Block layer patches Kevin Wolf
` (9 preceding siblings ...)
2021-03-05 16:54 ` [PULL 10/31] docs: show how to spawn qemu-storage-daemon with fd passing Kevin Wolf
@ 2021-03-05 16:54 ` Kevin Wolf
2021-03-05 16:54 ` [PULL 12/31] vhost-user-blk: fix blkcfg->num_queues endianness Kevin Wolf
` (20 subsequent siblings)
31 siblings, 0 replies; 35+ messages in thread
From: Kevin Wolf @ 2021-03-05 16:54 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Stefan Hajnoczi <stefanha@redhat.com>
World-writeable directories have security issues. Avoid showing them in
the documentation since someone might accidentally use them in
situations where they are insecure.
There tend to be 3 security problems:
1. Denial of service. An adversary may be able to create the file
beforehand, consume all space/inodes, etc to sabotage us.
2. Impersonation. An adversary may be able to create a listen socket and
accept incoming connections that were meant for us.
3. Unauthenticated client access. An adversary may be able to connect to
us if we did not set the uid/gid and permissions correctly.
These can be prevented or mitigated with private /tmp, carefully setting
the umask, etc but that requires special action and does not apply to
all situations. Just avoid using /tmp in examples.
Reported-by: Richard W.M. Jones <rjones@redhat.com>
Reported-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20210301172728.135331-3-stefanha@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
docs/tools/qemu-storage-daemon.rst | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/docs/tools/qemu-storage-daemon.rst b/docs/tools/qemu-storage-daemon.rst
index 5714794775..fe3042d609 100644
--- a/docs/tools/qemu-storage-daemon.rst
+++ b/docs/tools/qemu-storage-daemon.rst
@@ -69,7 +69,7 @@ Standard options:
a description of character device properties. A common character device
definition configures a UNIX domain socket::
- --chardev socket,id=char1,path=/tmp/qmp.sock,server=on,wait=off
+ --chardev socket,id=char1,path=/var/run/qsd-qmp.sock,server=on,wait=off
.. option:: --export [type=]nbd,id=<id>,node-name=<node-name>[,name=<export-name>][,writable=on|off][,bitmap=<name>]
--export [type=]vhost-user-blk,id=<id>,node-name=<node-name>,addr.type=unix,addr.path=<socket-path>[,writable=on|off][,logical-block-size=<block-size>][,num-queues=<num-queues>]
@@ -108,9 +108,10 @@ Standard options:
below). TLS encryption can be configured using ``--object`` tls-creds-* and
authz-* secrets (see below).
- To configure an NBD server on UNIX domain socket path ``/tmp/nbd.sock``::
+ To configure an NBD server on UNIX domain socket path
+ ``/var/run/qsd-nbd.sock``::
- --nbd-server addr.type=unix,addr.path=/tmp/nbd.sock
+ --nbd-server addr.type=unix,addr.path=/var/run/qsd-nbd.sock
.. option:: --object help
--object <type>,help
--
2.29.2
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PULL 12/31] vhost-user-blk: fix blkcfg->num_queues endianness
2021-03-05 16:54 [PULL 00/31] Block layer patches Kevin Wolf
` (10 preceding siblings ...)
2021-03-05 16:54 ` [PULL 11/31] docs: replace insecure /tmp examples in qsd docs Kevin Wolf
@ 2021-03-05 16:54 ` Kevin Wolf
2021-03-05 16:54 ` [PULL 13/31] libqtest: add qtest_socket_server() Kevin Wolf
` (19 subsequent siblings)
31 siblings, 0 replies; 35+ messages in thread
From: Kevin Wolf @ 2021-03-05 16:54 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Stefan Hajnoczi <stefanha@redhat.com>
Treat the num_queues field as virtio-endian. On big-endian hosts the
vhost-user-blk num_queues field was in the wrong endianness.
Move the blkcfg.num_queues store operation from realize to
vhost_user_blk_update_config() so feature negotiation has finished and
we know the endianness of the device. VIRTIO 1.0 devices are
little-endian, but in case someone wants to use legacy VIRTIO we support
all endianness cases.
Cc: qemu-stable@nongnu.org
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20210223144653.811468-2-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
hw/block/vhost-user-blk.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index da4fbf9084..b870a50e6b 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -54,6 +54,9 @@ static void vhost_user_blk_update_config(VirtIODevice *vdev, uint8_t *config)
{
VHostUserBlk *s = VHOST_USER_BLK(vdev);
+ /* Our num_queues overrides the device backend */
+ virtio_stw_p(vdev, &s->blkcfg.num_queues, s->num_queues);
+
memcpy(config, &s->blkcfg, sizeof(struct virtio_blk_config));
}
@@ -491,10 +494,6 @@ reconnect:
goto reconnect;
}
- if (s->blkcfg.num_queues != s->num_queues) {
- s->blkcfg.num_queues = s->num_queues;
- }
-
return;
virtio_err:
--
2.29.2
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PULL 13/31] libqtest: add qtest_socket_server()
2021-03-05 16:54 [PULL 00/31] Block layer patches Kevin Wolf
` (11 preceding siblings ...)
2021-03-05 16:54 ` [PULL 12/31] vhost-user-blk: fix blkcfg->num_queues endianness Kevin Wolf
@ 2021-03-05 16:54 ` Kevin Wolf
2021-03-05 16:54 ` [PULL 14/31] libqtest: add qtest_kill_qemu() Kevin Wolf
` (18 subsequent siblings)
31 siblings, 0 replies; 35+ messages in thread
From: Kevin Wolf @ 2021-03-05 16:54 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Stefan Hajnoczi <stefanha@redhat.com>
Add an API that returns a new UNIX domain socket in the listen state.
The code for this was already there but only used internally in
init_socket().
This new API will be used by vhost-user-blk-test.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
Message-Id: <20210223144653.811468-3-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qtest/libqos/libqtest.h | 8 +++++++
tests/qtest/libqtest.c | 40 ++++++++++++++++++++---------------
2 files changed, 31 insertions(+), 17 deletions(-)
diff --git a/tests/qtest/libqos/libqtest.h b/tests/qtest/libqos/libqtest.h
index 724f65aa94..e5f1ec590c 100644
--- a/tests/qtest/libqos/libqtest.h
+++ b/tests/qtest/libqos/libqtest.h
@@ -132,6 +132,14 @@ void qtest_qmp_send(QTestState *s, const char *fmt, ...)
void qtest_qmp_send_raw(QTestState *s, const char *fmt, ...)
GCC_FMT_ATTR(2, 3);
+/**
+ * qtest_socket_server:
+ * @socket_path: the UNIX domain socket path
+ *
+ * Create and return a listen socket file descriptor, or abort on failure.
+ */
+int qtest_socket_server(const char *socket_path);
+
/**
* qtest_vqmp_fds:
* @s: #QTestState instance to operate on.
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index fd043b0570..b19d2ebda0 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -81,24 +81,8 @@ static void qtest_client_set_rx_handler(QTestState *s, QTestRecvFn recv);
static int init_socket(const char *socket_path)
{
- struct sockaddr_un addr;
- int sock;
- int ret;
-
- sock = socket(PF_UNIX, SOCK_STREAM, 0);
- g_assert_cmpint(sock, !=, -1);
-
- addr.sun_family = AF_UNIX;
- snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", socket_path);
+ int sock = qtest_socket_server(socket_path);
qemu_set_cloexec(sock);
-
- do {
- ret = bind(sock, (struct sockaddr *)&addr, sizeof(addr));
- } while (ret == -1 && errno == EINTR);
- g_assert_cmpint(ret, !=, -1);
- ret = listen(sock, 1);
- g_assert_cmpint(ret, !=, -1);
-
return sock;
}
@@ -638,6 +622,28 @@ QDict *qtest_qmp_receive_dict(QTestState *s)
return qmp_fd_receive(s->qmp_fd);
}
+int qtest_socket_server(const char *socket_path)
+{
+ struct sockaddr_un addr;
+ int sock;
+ int ret;
+
+ sock = socket(PF_UNIX, SOCK_STREAM, 0);
+ g_assert_cmpint(sock, !=, -1);
+
+ addr.sun_family = AF_UNIX;
+ snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", socket_path);
+
+ do {
+ ret = bind(sock, (struct sockaddr *)&addr, sizeof(addr));
+ } while (ret == -1 && errno == EINTR);
+ g_assert_cmpint(ret, !=, -1);
+ ret = listen(sock, 1);
+ g_assert_cmpint(ret, !=, -1);
+
+ return sock;
+}
+
/**
* Allow users to send a message without waiting for the reply,
* in the case that they choose to discard all replies up until
--
2.29.2
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PULL 14/31] libqtest: add qtest_kill_qemu()
2021-03-05 16:54 [PULL 00/31] Block layer patches Kevin Wolf
` (12 preceding siblings ...)
2021-03-05 16:54 ` [PULL 13/31] libqtest: add qtest_socket_server() Kevin Wolf
@ 2021-03-05 16:54 ` Kevin Wolf
2021-03-05 16:54 ` [PULL 15/31] libqtest: add qtest_remove_abrt_handler() Kevin Wolf
` (17 subsequent siblings)
31 siblings, 0 replies; 35+ messages in thread
From: Kevin Wolf @ 2021-03-05 16:54 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Stefan Hajnoczi <stefanha@redhat.com>
Tests that manage multiple processes may wish to kill QEMU before
destroying the QTestState. Expose a function to do that.
The vhost-user-blk-test testcase will need this.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
Message-Id: <20210223144653.811468-4-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qtest/libqos/libqtest.h | 11 +++++++++++
tests/qtest/libqtest.c | 7 ++++---
2 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/tests/qtest/libqos/libqtest.h b/tests/qtest/libqos/libqtest.h
index e5f1ec590c..51287b9276 100644
--- a/tests/qtest/libqos/libqtest.h
+++ b/tests/qtest/libqos/libqtest.h
@@ -74,6 +74,17 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args);
*/
QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd);
+/**
+ * qtest_kill_qemu:
+ * @s: #QTestState instance to operate on.
+ *
+ * Kill the QEMU process and wait for it to terminate. It is safe to call this
+ * function multiple times. Normally qtest_quit() is used instead because it
+ * also frees QTestState. Use qtest_kill_qemu() when you just want to kill QEMU
+ * and qtest_quit() will be called later.
+ */
+void qtest_kill_qemu(QTestState *s);
+
/**
* qtest_quit:
* @s: #QTestState instance to operate on.
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index b19d2ebda0..2a98de2907 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -133,7 +133,7 @@ void qtest_set_expected_status(QTestState *s, int status)
s->expected_status = status;
}
-static void kill_qemu(QTestState *s)
+void qtest_kill_qemu(QTestState *s)
{
pid_t pid = s->qemu_pid;
int wstatus;
@@ -143,6 +143,7 @@ static void kill_qemu(QTestState *s)
kill(pid, SIGTERM);
TFR(pid = waitpid(s->qemu_pid, &s->wstatus, 0));
assert(pid == s->qemu_pid);
+ s->qemu_pid = -1;
}
/*
@@ -169,7 +170,7 @@ static void kill_qemu(QTestState *s)
static void kill_qemu_hook_func(void *s)
{
- kill_qemu(s);
+ qtest_kill_qemu(s);
}
static void sigabrt_handler(int signo)
@@ -373,7 +374,7 @@ void qtest_quit(QTestState *s)
/* Uninstall SIGABRT handler on last instance */
cleanup_sigabrt_handler();
- kill_qemu(s);
+ qtest_kill_qemu(s);
close(s->fd);
close(s->qmp_fd);
g_string_free(s->rx, true);
--
2.29.2
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PULL 15/31] libqtest: add qtest_remove_abrt_handler()
2021-03-05 16:54 [PULL 00/31] Block layer patches Kevin Wolf
` (13 preceding siblings ...)
2021-03-05 16:54 ` [PULL 14/31] libqtest: add qtest_kill_qemu() Kevin Wolf
@ 2021-03-05 16:54 ` Kevin Wolf
2021-03-05 16:54 ` [PULL 16/31] test: new qTest case to test the vhost-user-blk-server Kevin Wolf
` (16 subsequent siblings)
31 siblings, 0 replies; 35+ messages in thread
From: Kevin Wolf @ 2021-03-05 16:54 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Stefan Hajnoczi <stefanha@redhat.com>
Add a function to remove previously-added abrt handler functions.
Now that a symmetric pair of add/remove functions exists we can also
balance the SIGABRT handler installation. The signal handler was
installed each time qtest_add_abrt_handler() was called. Now it is
installed when the abrt handler list becomes non-empty and removed again
when the list becomes empty.
The qtest_remove_abrt_handler() function will be used by
vhost-user-blk-test.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
Message-Id: <20210223144653.811468-5-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qtest/libqos/libqtest.h | 18 ++++++++++++++++++
tests/qtest/libqtest.c | 35 +++++++++++++++++++++++++++++------
2 files changed, 47 insertions(+), 6 deletions(-)
diff --git a/tests/qtest/libqos/libqtest.h b/tests/qtest/libqos/libqtest.h
index 51287b9276..a68dcd79d4 100644
--- a/tests/qtest/libqos/libqtest.h
+++ b/tests/qtest/libqos/libqtest.h
@@ -649,8 +649,26 @@ void qtest_add_data_func_full(const char *str, void *data,
g_free(path); \
} while (0)
+/**
+ * qtest_add_abrt_handler:
+ * @fn: Handler function
+ * @data: Argument that is passed to the handler
+ *
+ * Add a handler function that is invoked on SIGABRT. This can be used to
+ * terminate processes and perform other cleanup. The handler can be removed
+ * with qtest_remove_abrt_handler().
+ */
void qtest_add_abrt_handler(GHookFunc fn, const void *data);
+/**
+ * qtest_remove_abrt_handler:
+ * @data: Argument previously passed to qtest_add_abrt_handler()
+ *
+ * Remove an abrt handler that was previously added with
+ * qtest_add_abrt_handler().
+ */
+void qtest_remove_abrt_handler(void *data);
+
/**
* qtest_qmp_assert_success:
* @qts: QTestState instance to operate on
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 2a98de2907..71e359efcd 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -196,15 +196,30 @@ static void cleanup_sigabrt_handler(void)
sigaction(SIGABRT, &sigact_old, NULL);
}
+static bool hook_list_is_empty(GHookList *hook_list)
+{
+ GHook *hook = g_hook_first_valid(hook_list, TRUE);
+
+ if (!hook) {
+ return false;
+ }
+
+ g_hook_unref(hook_list, hook);
+ return true;
+}
+
void qtest_add_abrt_handler(GHookFunc fn, const void *data)
{
GHook *hook;
- /* Only install SIGABRT handler once */
if (!abrt_hooks.is_setup) {
g_hook_list_init(&abrt_hooks, sizeof(GHook));
}
- setup_sigabrt_handler();
+
+ /* Only install SIGABRT handler once */
+ if (hook_list_is_empty(&abrt_hooks)) {
+ setup_sigabrt_handler();
+ }
hook = g_hook_alloc(&abrt_hooks);
hook->func = fn;
@@ -213,6 +228,17 @@ void qtest_add_abrt_handler(GHookFunc fn, const void *data)
g_hook_prepend(&abrt_hooks, hook);
}
+void qtest_remove_abrt_handler(void *data)
+{
+ GHook *hook = g_hook_find_data(&abrt_hooks, TRUE, data);
+ g_hook_destroy_link(&abrt_hooks, hook);
+
+ /* Uninstall SIGABRT handler on last instance */
+ if (hook_list_is_empty(&abrt_hooks)) {
+ cleanup_sigabrt_handler();
+ }
+}
+
static const char *qtest_qemu_binary(void)
{
const char *qemu_bin;
@@ -369,10 +395,7 @@ QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd)
void qtest_quit(QTestState *s)
{
- g_hook_destroy_link(&abrt_hooks, g_hook_find_data(&abrt_hooks, TRUE, s));
-
- /* Uninstall SIGABRT handler on last instance */
- cleanup_sigabrt_handler();
+ qtest_remove_abrt_handler(s);
qtest_kill_qemu(s);
close(s->fd);
--
2.29.2
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PULL 16/31] test: new qTest case to test the vhost-user-blk-server
2021-03-05 16:54 [PULL 00/31] Block layer patches Kevin Wolf
` (14 preceding siblings ...)
2021-03-05 16:54 ` [PULL 15/31] libqtest: add qtest_remove_abrt_handler() Kevin Wolf
@ 2021-03-05 16:54 ` Kevin Wolf
2021-03-05 16:54 ` [PULL 17/31] tests/qtest: add multi-queue test case to vhost-user-blk-test Kevin Wolf
` (15 subsequent siblings)
31 siblings, 0 replies; 35+ messages in thread
From: Kevin Wolf @ 2021-03-05 16:54 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Coiby Xu <coiby.xu@gmail.com>
This test case has the same tests as tests/virtio-blk-test.c except for
tests have block_resize. Since the vhost-user-blk export only serves one
client one time, two exports are started by qemu-storage-daemon for the
hotplug test.
Suggested-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20210223144653.811468-6-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qtest/libqos/vhost-user-blk.h | 48 ++
tests/qtest/libqos/vhost-user-blk.c | 130 +++++
tests/qtest/vhost-user-blk-test.c | 788 ++++++++++++++++++++++++++++
MAINTAINERS | 2 +
tests/qtest/libqos/meson.build | 1 +
tests/qtest/meson.build | 4 +
6 files changed, 973 insertions(+)
create mode 100644 tests/qtest/libqos/vhost-user-blk.h
create mode 100644 tests/qtest/libqos/vhost-user-blk.c
create mode 100644 tests/qtest/vhost-user-blk-test.c
diff --git a/tests/qtest/libqos/vhost-user-blk.h b/tests/qtest/libqos/vhost-user-blk.h
new file mode 100644
index 0000000000..2a03456a45
--- /dev/null
+++ b/tests/qtest/libqos/vhost-user-blk.h
@@ -0,0 +1,48 @@
+/*
+ * libqos driver framework
+ *
+ * Based on tests/qtest/libqos/virtio-blk.c
+ *
+ * Copyright (c) 2020 Coiby Xu <coiby.xu@gmail.com>
+ *
+ * Copyright (c) 2018 Emanuele Giuseppe Esposito <e.emanuelegiuseppe@gmail.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License version 2 as published by the Free Software Foundation.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>
+ */
+
+#ifndef TESTS_LIBQOS_VHOST_USER_BLK_H
+#define TESTS_LIBQOS_VHOST_USER_BLK_H
+
+#include "qgraph.h"
+#include "virtio.h"
+#include "virtio-pci.h"
+
+typedef struct QVhostUserBlk QVhostUserBlk;
+typedef struct QVhostUserBlkPCI QVhostUserBlkPCI;
+typedef struct QVhostUserBlkDevice QVhostUserBlkDevice;
+
+struct QVhostUserBlk {
+ QVirtioDevice *vdev;
+};
+
+struct QVhostUserBlkPCI {
+ QVirtioPCIDevice pci_vdev;
+ QVhostUserBlk blk;
+};
+
+struct QVhostUserBlkDevice {
+ QOSGraphObject obj;
+ QVhostUserBlk blk;
+};
+
+#endif
diff --git a/tests/qtest/libqos/vhost-user-blk.c b/tests/qtest/libqos/vhost-user-blk.c
new file mode 100644
index 0000000000..568c3426ed
--- /dev/null
+++ b/tests/qtest/libqos/vhost-user-blk.c
@@ -0,0 +1,130 @@
+/*
+ * libqos driver framework
+ *
+ * Based on tests/qtest/libqos/virtio-blk.c
+ *
+ * Copyright (c) 2020 Coiby Xu <coiby.xu@gmail.com>
+ *
+ * Copyright (c) 2018 Emanuele Giuseppe Esposito <e.emanuelegiuseppe@gmail.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License version 2.1 as published by the Free Software Foundation.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "qemu/module.h"
+#include "standard-headers/linux/virtio_blk.h"
+#include "vhost-user-blk.h"
+
+#define PCI_SLOT 0x04
+#define PCI_FN 0x00
+
+/* virtio-blk-device */
+static void *qvhost_user_blk_get_driver(QVhostUserBlk *v_blk,
+ const char *interface)
+{
+ if (!g_strcmp0(interface, "vhost-user-blk")) {
+ return v_blk;
+ }
+ if (!g_strcmp0(interface, "virtio")) {
+ return v_blk->vdev;
+ }
+
+ fprintf(stderr, "%s not present in vhost-user-blk-device\n", interface);
+ g_assert_not_reached();
+}
+
+static void *qvhost_user_blk_device_get_driver(void *object,
+ const char *interface)
+{
+ QVhostUserBlkDevice *v_blk = object;
+ return qvhost_user_blk_get_driver(&v_blk->blk, interface);
+}
+
+static void *vhost_user_blk_device_create(void *virtio_dev,
+ QGuestAllocator *t_alloc,
+ void *addr)
+{
+ QVhostUserBlkDevice *vhost_user_blk = g_new0(QVhostUserBlkDevice, 1);
+ QVhostUserBlk *interface = &vhost_user_blk->blk;
+
+ interface->vdev = virtio_dev;
+
+ vhost_user_blk->obj.get_driver = qvhost_user_blk_device_get_driver;
+
+ return &vhost_user_blk->obj;
+}
+
+/* virtio-blk-pci */
+static void *qvhost_user_blk_pci_get_driver(void *object, const char *interface)
+{
+ QVhostUserBlkPCI *v_blk = object;
+ if (!g_strcmp0(interface, "pci-device")) {
+ return v_blk->pci_vdev.pdev;
+ }
+ return qvhost_user_blk_get_driver(&v_blk->blk, interface);
+}
+
+static void *vhost_user_blk_pci_create(void *pci_bus, QGuestAllocator *t_alloc,
+ void *addr)
+{
+ QVhostUserBlkPCI *vhost_user_blk = g_new0(QVhostUserBlkPCI, 1);
+ QVhostUserBlk *interface = &vhost_user_blk->blk;
+ QOSGraphObject *obj = &vhost_user_blk->pci_vdev.obj;
+
+ virtio_pci_init(&vhost_user_blk->pci_vdev, pci_bus, addr);
+ interface->vdev = &vhost_user_blk->pci_vdev.vdev;
+
+ g_assert_cmphex(interface->vdev->device_type, ==, VIRTIO_ID_BLOCK);
+
+ obj->get_driver = qvhost_user_blk_pci_get_driver;
+
+ return obj;
+}
+
+static void vhost_user_blk_register_nodes(void)
+{
+ /*
+ * FIXME: every test using these two nodes needs to setup a
+ * -drive,id=drive0 otherwise QEMU is not going to start.
+ * Therefore, we do not include "produces" edge for virtio
+ * and pci-device yet.
+ */
+
+ char *arg = g_strdup_printf("id=drv0,chardev=char1,addr=%x.%x",
+ PCI_SLOT, PCI_FN);
+
+ QPCIAddress addr = {
+ .devfn = QPCI_DEVFN(PCI_SLOT, PCI_FN),
+ };
+
+ QOSGraphEdgeOptions opts = { };
+
+ /* virtio-blk-device */
+ /** opts.extra_device_opts = "drive=drive0"; */
+ qos_node_create_driver("vhost-user-blk-device",
+ vhost_user_blk_device_create);
+ qos_node_consumes("vhost-user-blk-device", "virtio-bus", &opts);
+ qos_node_produces("vhost-user-blk-device", "vhost-user-blk");
+
+ /* virtio-blk-pci */
+ opts.extra_device_opts = arg;
+ add_qpci_address(&opts, &addr);
+ qos_node_create_driver("vhost-user-blk-pci", vhost_user_blk_pci_create);
+ qos_node_consumes("vhost-user-blk-pci", "pci-bus", &opts);
+ qos_node_produces("vhost-user-blk-pci", "vhost-user-blk");
+
+ g_free(arg);
+}
+
+libqos_init(vhost_user_blk_register_nodes);
diff --git a/tests/qtest/vhost-user-blk-test.c b/tests/qtest/vhost-user-blk-test.c
new file mode 100644
index 0000000000..f0fb09893e
--- /dev/null
+++ b/tests/qtest/vhost-user-blk-test.c
@@ -0,0 +1,788 @@
+/*
+ * QTest testcase for Vhost-user Block Device
+ *
+ * Based on tests/qtest//virtio-blk-test.c
+
+ * Copyright (c) 2014 SUSE LINUX Products GmbH
+ * Copyright (c) 2014 Marc Marí
+ * Copyright (c) 2020 Coiby Xu
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest-single.h"
+#include "qemu/bswap.h"
+#include "qemu/module.h"
+#include "standard-headers/linux/virtio_blk.h"
+#include "standard-headers/linux/virtio_pci.h"
+#include "libqos/qgraph.h"
+#include "libqos/vhost-user-blk.h"
+#include "libqos/libqos-pc.h"
+
+#define TEST_IMAGE_SIZE (64 * 1024 * 1024)
+#define QVIRTIO_BLK_TIMEOUT_US (30 * 1000 * 1000)
+#define PCI_SLOT_HP 0x06
+
+typedef struct {
+ pid_t pid;
+} QemuStorageDaemonState;
+
+typedef struct QVirtioBlkReq {
+ uint32_t type;
+ uint32_t ioprio;
+ uint64_t sector;
+ char *data;
+ uint8_t status;
+} QVirtioBlkReq;
+
+#ifdef HOST_WORDS_BIGENDIAN
+static const bool host_is_big_endian = true;
+#else
+static const bool host_is_big_endian; /* false */
+#endif
+
+static inline void virtio_blk_fix_request(QVirtioDevice *d, QVirtioBlkReq *req)
+{
+ if (qvirtio_is_big_endian(d) != host_is_big_endian) {
+ req->type = bswap32(req->type);
+ req->ioprio = bswap32(req->ioprio);
+ req->sector = bswap64(req->sector);
+ }
+}
+
+static inline void virtio_blk_fix_dwz_hdr(QVirtioDevice *d,
+ struct virtio_blk_discard_write_zeroes *dwz_hdr)
+{
+ if (qvirtio_is_big_endian(d) != host_is_big_endian) {
+ dwz_hdr->sector = bswap64(dwz_hdr->sector);
+ dwz_hdr->num_sectors = bswap32(dwz_hdr->num_sectors);
+ dwz_hdr->flags = bswap32(dwz_hdr->flags);
+ }
+}
+
+static uint64_t virtio_blk_request(QGuestAllocator *alloc, QVirtioDevice *d,
+ QVirtioBlkReq *req, uint64_t data_size)
+{
+ uint64_t addr;
+ uint8_t status = 0xFF;
+ QTestState *qts = global_qtest;
+
+ switch (req->type) {
+ case VIRTIO_BLK_T_IN:
+ case VIRTIO_BLK_T_OUT:
+ g_assert_cmpuint(data_size % 512, ==, 0);
+ break;
+ case VIRTIO_BLK_T_DISCARD:
+ case VIRTIO_BLK_T_WRITE_ZEROES:
+ g_assert_cmpuint(data_size %
+ sizeof(struct virtio_blk_discard_write_zeroes), ==, 0);
+ break;
+ default:
+ g_assert_cmpuint(data_size, ==, 0);
+ }
+
+ addr = guest_alloc(alloc, sizeof(*req) + data_size);
+
+ virtio_blk_fix_request(d, req);
+
+ qtest_memwrite(qts, addr, req, 16);
+ qtest_memwrite(qts, addr + 16, req->data, data_size);
+ qtest_memwrite(qts, addr + 16 + data_size, &status, sizeof(status));
+
+ return addr;
+}
+
+/* Returns the request virtqueue so the caller can perform further tests */
+static QVirtQueue *test_basic(QVirtioDevice *dev, QGuestAllocator *alloc)
+{
+ QVirtioBlkReq req;
+ uint64_t req_addr;
+ uint64_t capacity;
+ uint64_t features;
+ uint32_t free_head;
+ uint8_t status;
+ char *data;
+ QTestState *qts = global_qtest;
+ QVirtQueue *vq;
+
+ features = qvirtio_get_features(dev);
+ features = features & ~(QVIRTIO_F_BAD_FEATURE |
+ (1u << VIRTIO_RING_F_INDIRECT_DESC) |
+ (1u << VIRTIO_RING_F_EVENT_IDX) |
+ (1u << VIRTIO_BLK_F_SCSI));
+ qvirtio_set_features(dev, features);
+
+ capacity = qvirtio_config_readq(dev, 0);
+ g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
+
+ vq = qvirtqueue_setup(dev, alloc, 0);
+
+ qvirtio_set_driver_ok(dev);
+
+ /* Write and read with 3 descriptor layout */
+ /* Write request */
+ req.type = VIRTIO_BLK_T_OUT;
+ req.ioprio = 1;
+ req.sector = 0;
+ req.data = g_malloc0(512);
+ strcpy(req.data, "TEST");
+
+ req_addr = virtio_blk_request(alloc, dev, &req, 512);
+
+ g_free(req.data);
+
+ free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
+ qvirtqueue_add(qts, vq, req_addr + 16, 512, false, true);
+ qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
+
+ qvirtqueue_kick(qts, dev, vq, free_head);
+
+ qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+ QVIRTIO_BLK_TIMEOUT_US);
+ status = readb(req_addr + 528);
+ g_assert_cmpint(status, ==, 0);
+
+ guest_free(alloc, req_addr);
+
+ /* Read request */
+ req.type = VIRTIO_BLK_T_IN;
+ req.ioprio = 1;
+ req.sector = 0;
+ req.data = g_malloc0(512);
+
+ req_addr = virtio_blk_request(alloc, dev, &req, 512);
+
+ g_free(req.data);
+
+ free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
+ qvirtqueue_add(qts, vq, req_addr + 16, 512, true, true);
+ qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
+
+ qvirtqueue_kick(qts, dev, vq, free_head);
+
+ qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+ QVIRTIO_BLK_TIMEOUT_US);
+ status = readb(req_addr + 528);
+ g_assert_cmpint(status, ==, 0);
+
+ data = g_malloc0(512);
+ qtest_memread(qts, req_addr + 16, data, 512);
+ g_assert_cmpstr(data, ==, "TEST");
+ g_free(data);
+
+ guest_free(alloc, req_addr);
+
+ if (features & (1u << VIRTIO_BLK_F_WRITE_ZEROES)) {
+ struct virtio_blk_discard_write_zeroes dwz_hdr;
+ void *expected;
+
+ /*
+ * WRITE_ZEROES request on the same sector of previous test where
+ * we wrote "TEST".
+ */
+ req.type = VIRTIO_BLK_T_WRITE_ZEROES;
+ req.data = (char *) &dwz_hdr;
+ dwz_hdr.sector = 0;
+ dwz_hdr.num_sectors = 1;
+ dwz_hdr.flags = 0;
+
+ virtio_blk_fix_dwz_hdr(dev, &dwz_hdr);
+
+ req_addr = virtio_blk_request(alloc, dev, &req, sizeof(dwz_hdr));
+
+ free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
+ qvirtqueue_add(qts, vq, req_addr + 16, sizeof(dwz_hdr), false, true);
+ qvirtqueue_add(qts, vq, req_addr + 16 + sizeof(dwz_hdr), 1, true,
+ false);
+
+ qvirtqueue_kick(qts, dev, vq, free_head);
+
+ qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+ QVIRTIO_BLK_TIMEOUT_US);
+ status = readb(req_addr + 16 + sizeof(dwz_hdr));
+ g_assert_cmpint(status, ==, 0);
+
+ guest_free(alloc, req_addr);
+
+ /* Read request to check if the sector contains all zeroes */
+ req.type = VIRTIO_BLK_T_IN;
+ req.ioprio = 1;
+ req.sector = 0;
+ req.data = g_malloc0(512);
+
+ req_addr = virtio_blk_request(alloc, dev, &req, 512);
+
+ g_free(req.data);
+
+ free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
+ qvirtqueue_add(qts, vq, req_addr + 16, 512, true, true);
+ qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
+
+ qvirtqueue_kick(qts, dev, vq, free_head);
+
+ qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+ QVIRTIO_BLK_TIMEOUT_US);
+ status = readb(req_addr + 528);
+ g_assert_cmpint(status, ==, 0);
+
+ data = g_malloc(512);
+ expected = g_malloc0(512);
+ qtest_memread(qts, req_addr + 16, data, 512);
+ g_assert_cmpmem(data, 512, expected, 512);
+ g_free(expected);
+ g_free(data);
+
+ guest_free(alloc, req_addr);
+ }
+
+ if (features & (1u << VIRTIO_BLK_F_DISCARD)) {
+ struct virtio_blk_discard_write_zeroes dwz_hdr;
+
+ req.type = VIRTIO_BLK_T_DISCARD;
+ req.data = (char *) &dwz_hdr;
+ dwz_hdr.sector = 0;
+ dwz_hdr.num_sectors = 1;
+ dwz_hdr.flags = 0;
+
+ virtio_blk_fix_dwz_hdr(dev, &dwz_hdr);
+
+ req_addr = virtio_blk_request(alloc, dev, &req, sizeof(dwz_hdr));
+
+ free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
+ qvirtqueue_add(qts, vq, req_addr + 16, sizeof(dwz_hdr), false, true);
+ qvirtqueue_add(qts, vq, req_addr + 16 + sizeof(dwz_hdr),
+ 1, true, false);
+
+ qvirtqueue_kick(qts, dev, vq, free_head);
+
+ qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+ QVIRTIO_BLK_TIMEOUT_US);
+ status = readb(req_addr + 16 + sizeof(dwz_hdr));
+ g_assert_cmpint(status, ==, 0);
+
+ guest_free(alloc, req_addr);
+ }
+
+ if (features & (1u << VIRTIO_F_ANY_LAYOUT)) {
+ /* Write and read with 2 descriptor layout */
+ /* Write request */
+ req.type = VIRTIO_BLK_T_OUT;
+ req.ioprio = 1;
+ req.sector = 1;
+ req.data = g_malloc0(512);
+ strcpy(req.data, "TEST");
+
+ req_addr = virtio_blk_request(alloc, dev, &req, 512);
+
+ g_free(req.data);
+
+ free_head = qvirtqueue_add(qts, vq, req_addr, 528, false, true);
+ qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
+ qvirtqueue_kick(qts, dev, vq, free_head);
+
+ qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+ QVIRTIO_BLK_TIMEOUT_US);
+ status = readb(req_addr + 528);
+ g_assert_cmpint(status, ==, 0);
+
+ guest_free(alloc, req_addr);
+
+ /* Read request */
+ req.type = VIRTIO_BLK_T_IN;
+ req.ioprio = 1;
+ req.sector = 1;
+ req.data = g_malloc0(512);
+
+ req_addr = virtio_blk_request(alloc, dev, &req, 512);
+
+ g_free(req.data);
+
+ free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
+ qvirtqueue_add(qts, vq, req_addr + 16, 513, true, false);
+
+ qvirtqueue_kick(qts, dev, vq, free_head);
+
+ qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+ QVIRTIO_BLK_TIMEOUT_US);
+ status = readb(req_addr + 528);
+ g_assert_cmpint(status, ==, 0);
+
+ data = g_malloc0(512);
+ qtest_memread(qts, req_addr + 16, data, 512);
+ g_assert_cmpstr(data, ==, "TEST");
+ g_free(data);
+
+ guest_free(alloc, req_addr);
+ }
+
+ return vq;
+}
+
+static void basic(void *obj, void *data, QGuestAllocator *t_alloc)
+{
+ QVhostUserBlk *blk_if = obj;
+ QVirtQueue *vq;
+
+ vq = test_basic(blk_if->vdev, t_alloc);
+ qvirtqueue_cleanup(blk_if->vdev->bus, vq, t_alloc);
+
+}
+
+static void indirect(void *obj, void *u_data, QGuestAllocator *t_alloc)
+{
+ QVirtQueue *vq;
+ QVhostUserBlk *blk_if = obj;
+ QVirtioDevice *dev = blk_if->vdev;
+ QVirtioBlkReq req;
+ QVRingIndirectDesc *indirect;
+ uint64_t req_addr;
+ uint64_t capacity;
+ uint64_t features;
+ uint32_t free_head;
+ uint8_t status;
+ char *data;
+ QTestState *qts = global_qtest;
+
+ features = qvirtio_get_features(dev);
+ g_assert_cmphex(features & (1u << VIRTIO_RING_F_INDIRECT_DESC), !=, 0);
+ features = features & ~(QVIRTIO_F_BAD_FEATURE |
+ (1u << VIRTIO_RING_F_EVENT_IDX) |
+ (1u << VIRTIO_BLK_F_SCSI));
+ qvirtio_set_features(dev, features);
+
+ capacity = qvirtio_config_readq(dev, 0);
+ g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
+
+ vq = qvirtqueue_setup(dev, t_alloc, 0);
+ qvirtio_set_driver_ok(dev);
+
+ /* Write request */
+ req.type = VIRTIO_BLK_T_OUT;
+ req.ioprio = 1;
+ req.sector = 0;
+ req.data = g_malloc0(512);
+ strcpy(req.data, "TEST");
+
+ req_addr = virtio_blk_request(t_alloc, dev, &req, 512);
+
+ g_free(req.data);
+
+ indirect = qvring_indirect_desc_setup(qts, dev, t_alloc, 2);
+ qvring_indirect_desc_add(dev, qts, indirect, req_addr, 528, false);
+ qvring_indirect_desc_add(dev, qts, indirect, req_addr + 528, 1, true);
+ free_head = qvirtqueue_add_indirect(qts, vq, indirect);
+ qvirtqueue_kick(qts, dev, vq, free_head);
+
+ qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+ QVIRTIO_BLK_TIMEOUT_US);
+ status = readb(req_addr + 528);
+ g_assert_cmpint(status, ==, 0);
+
+ g_free(indirect);
+ guest_free(t_alloc, req_addr);
+
+ /* Read request */
+ req.type = VIRTIO_BLK_T_IN;
+ req.ioprio = 1;
+ req.sector = 0;
+ req.data = g_malloc0(512);
+ strcpy(req.data, "TEST");
+
+ req_addr = virtio_blk_request(t_alloc, dev, &req, 512);
+
+ g_free(req.data);
+
+ indirect = qvring_indirect_desc_setup(qts, dev, t_alloc, 2);
+ qvring_indirect_desc_add(dev, qts, indirect, req_addr, 16, false);
+ qvring_indirect_desc_add(dev, qts, indirect, req_addr + 16, 513, true);
+ free_head = qvirtqueue_add_indirect(qts, vq, indirect);
+ qvirtqueue_kick(qts, dev, vq, free_head);
+
+ qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+ QVIRTIO_BLK_TIMEOUT_US);
+ status = readb(req_addr + 528);
+ g_assert_cmpint(status, ==, 0);
+
+ data = g_malloc0(512);
+ qtest_memread(qts, req_addr + 16, data, 512);
+ g_assert_cmpstr(data, ==, "TEST");
+ g_free(data);
+
+ g_free(indirect);
+ guest_free(t_alloc, req_addr);
+ qvirtqueue_cleanup(dev->bus, vq, t_alloc);
+}
+
+static void idx(void *obj, void *u_data, QGuestAllocator *t_alloc)
+{
+ QVirtQueue *vq;
+ QVhostUserBlkPCI *blk = obj;
+ QVirtioPCIDevice *pdev = &blk->pci_vdev;
+ QVirtioDevice *dev = &pdev->vdev;
+ QVirtioBlkReq req;
+ uint64_t req_addr;
+ uint64_t capacity;
+ uint64_t features;
+ uint32_t free_head;
+ uint32_t write_head;
+ uint32_t desc_idx;
+ uint8_t status;
+ char *data;
+ QOSGraphObject *blk_object = obj;
+ QPCIDevice *pci_dev = blk_object->get_driver(blk_object, "pci-device");
+ QTestState *qts = global_qtest;
+
+ if (qpci_check_buggy_msi(pci_dev)) {
+ return;
+ }
+
+ qpci_msix_enable(pdev->pdev);
+ qvirtio_pci_set_msix_configuration_vector(pdev, t_alloc, 0);
+
+ features = qvirtio_get_features(dev);
+ features = features & ~(QVIRTIO_F_BAD_FEATURE |
+ (1u << VIRTIO_RING_F_INDIRECT_DESC) |
+ (1u << VIRTIO_F_NOTIFY_ON_EMPTY) |
+ (1u << VIRTIO_BLK_F_SCSI));
+ qvirtio_set_features(dev, features);
+
+ capacity = qvirtio_config_readq(dev, 0);
+ g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
+
+ vq = qvirtqueue_setup(dev, t_alloc, 0);
+ qvirtqueue_pci_msix_setup(pdev, (QVirtQueuePCI *)vq, t_alloc, 1);
+
+ qvirtio_set_driver_ok(dev);
+
+ /* Write request */
+ req.type = VIRTIO_BLK_T_OUT;
+ req.ioprio = 1;
+ req.sector = 0;
+ req.data = g_malloc0(512);
+ strcpy(req.data, "TEST");
+
+ req_addr = virtio_blk_request(t_alloc, dev, &req, 512);
+
+ g_free(req.data);
+
+ free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
+ qvirtqueue_add(qts, vq, req_addr + 16, 512, false, true);
+ qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
+ qvirtqueue_kick(qts, dev, vq, free_head);
+
+ qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+ QVIRTIO_BLK_TIMEOUT_US);
+
+ /* Write request */
+ req.type = VIRTIO_BLK_T_OUT;
+ req.ioprio = 1;
+ req.sector = 1;
+ req.data = g_malloc0(512);
+ strcpy(req.data, "TEST");
+
+ req_addr = virtio_blk_request(t_alloc, dev, &req, 512);
+
+ g_free(req.data);
+
+ /* Notify after processing the third request */
+ qvirtqueue_set_used_event(qts, vq, 2);
+ free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
+ qvirtqueue_add(qts, vq, req_addr + 16, 512, false, true);
+ qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
+ qvirtqueue_kick(qts, dev, vq, free_head);
+ write_head = free_head;
+
+ /* No notification expected */
+ status = qvirtio_wait_status_byte_no_isr(qts, dev,
+ vq, req_addr + 528,
+ QVIRTIO_BLK_TIMEOUT_US);
+ g_assert_cmpint(status, ==, 0);
+
+ guest_free(t_alloc, req_addr);
+
+ /* Read request */
+ req.type = VIRTIO_BLK_T_IN;
+ req.ioprio = 1;
+ req.sector = 1;
+ req.data = g_malloc0(512);
+
+ req_addr = virtio_blk_request(t_alloc, dev, &req, 512);
+
+ g_free(req.data);
+
+ free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
+ qvirtqueue_add(qts, vq, req_addr + 16, 512, true, true);
+ qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
+
+ qvirtqueue_kick(qts, dev, vq, free_head);
+
+ /* We get just one notification for both requests */
+ qvirtio_wait_used_elem(qts, dev, vq, write_head, NULL,
+ QVIRTIO_BLK_TIMEOUT_US);
+ g_assert(qvirtqueue_get_buf(qts, vq, &desc_idx, NULL));
+ g_assert_cmpint(desc_idx, ==, free_head);
+
+ status = readb(req_addr + 528);
+ g_assert_cmpint(status, ==, 0);
+
+ data = g_malloc0(512);
+ qtest_memread(qts, req_addr + 16, data, 512);
+ g_assert_cmpstr(data, ==, "TEST");
+ g_free(data);
+
+ guest_free(t_alloc, req_addr);
+
+ /* End test */
+ qpci_msix_disable(pdev->pdev);
+
+ qvirtqueue_cleanup(dev->bus, vq, t_alloc);
+}
+
+static void pci_hotplug(void *obj, void *data, QGuestAllocator *t_alloc)
+{
+ QVirtioPCIDevice *dev1 = obj;
+ QVirtioPCIDevice *dev;
+ QTestState *qts = dev1->pdev->bus->qts;
+
+ /* plug secondary disk */
+ qtest_qmp_device_add(qts, "vhost-user-blk-pci", "drv1",
+ "{'addr': %s, 'chardev': 'char2'}",
+ stringify(PCI_SLOT_HP) ".0");
+
+ dev = virtio_pci_new(dev1->pdev->bus,
+ &(QPCIAddress) { .devfn = QPCI_DEVFN(PCI_SLOT_HP, 0)
+ });
+ g_assert_nonnull(dev);
+ g_assert_cmpint(dev->vdev.device_type, ==, VIRTIO_ID_BLOCK);
+ qvirtio_pci_device_disable(dev);
+ qos_object_destroy((QOSGraphObject *)dev);
+
+ /* unplug secondary disk */
+ qpci_unplug_acpi_device_test(qts, "drv1", PCI_SLOT_HP);
+}
+
+/*
+ * Check that setting the vring addr on a non-existent virtqueue does
+ * not crash.
+ */
+static void test_nonexistent_virtqueue(void *obj, void *data,
+ QGuestAllocator *t_alloc)
+{
+ QVhostUserBlkPCI *blk = obj;
+ QVirtioPCIDevice *pdev = &blk->pci_vdev;
+ QPCIBar bar0;
+ QPCIDevice *dev;
+
+ dev = qpci_device_find(pdev->pdev->bus, QPCI_DEVFN(4, 0));
+ g_assert(dev != NULL);
+ qpci_device_enable(dev);
+
+ bar0 = qpci_iomap(dev, 0, NULL);
+
+ qpci_io_writeb(dev, bar0, VIRTIO_PCI_QUEUE_SEL, 2);
+ qpci_io_writel(dev, bar0, VIRTIO_PCI_QUEUE_PFN, 1);
+
+ g_free(dev);
+}
+
+static const char *qtest_qemu_storage_daemon_binary(void)
+{
+ const char *qemu_storage_daemon_bin;
+
+ qemu_storage_daemon_bin = getenv("QTEST_QEMU_STORAGE_DAEMON_BINARY");
+ if (!qemu_storage_daemon_bin) {
+ fprintf(stderr, "Environment variable "
+ "QTEST_QEMU_STORAGE_DAEMON_BINARY required\n");
+ exit(0);
+ }
+
+ return qemu_storage_daemon_bin;
+}
+
+/* g_test_queue_destroy() cleanup function for files */
+static void destroy_file(void *path)
+{
+ unlink(path);
+ g_free(path);
+ qos_invalidate_command_line();
+}
+
+static char *drive_create(void)
+{
+ int fd, ret;
+ /** vhost-user-blk won't recognize drive located in /tmp */
+ char *t_path = g_strdup("qtest.XXXXXX");
+
+ /** Create a temporary raw image */
+ fd = mkstemp(t_path);
+ g_assert_cmpint(fd, >=, 0);
+ ret = ftruncate(fd, TEST_IMAGE_SIZE);
+ g_assert_cmpint(ret, ==, 0);
+ close(fd);
+
+ g_test_queue_destroy(destroy_file, t_path);
+ return t_path;
+}
+
+static char *create_listen_socket(int *fd)
+{
+ int tmp_fd;
+ char *path;
+
+ /* No race because our pid makes the path unique */
+ path = g_strdup_printf("/tmp/qtest-%d-sock.XXXXXX", getpid());
+ tmp_fd = mkstemp(path);
+ g_assert_cmpint(tmp_fd, >=, 0);
+ close(tmp_fd);
+ unlink(path);
+
+ *fd = qtest_socket_server(path);
+ g_test_queue_destroy(destroy_file, path);
+ return path;
+}
+
+/*
+ * g_test_queue_destroy() and qtest_add_abrt_handler() cleanup function for
+ * qemu-storage-daemon.
+ */
+static void quit_storage_daemon(void *data)
+{
+ QemuStorageDaemonState *qsd = data;
+ int wstatus;
+ pid_t pid;
+
+ /*
+ * If we were invoked as a g_test_queue_destroy() cleanup function we need
+ * to remove the abrt handler to avoid being called again if the code below
+ * aborts. Also, we must not leave the abrt handler installed after
+ * cleanup.
+ */
+ qtest_remove_abrt_handler(data);
+
+ /* Before quitting storage-daemon, quit qemu to avoid dubious messages */
+ qtest_kill_qemu(global_qtest);
+
+ kill(qsd->pid, SIGTERM);
+ pid = waitpid(qsd->pid, &wstatus, 0);
+ g_assert_cmpint(pid, ==, qsd->pid);
+ if (!WIFEXITED(wstatus)) {
+ fprintf(stderr, "%s: expected qemu-storage-daemon to exit\n",
+ __func__);
+ abort();
+ }
+ if (WEXITSTATUS(wstatus) != 0) {
+ fprintf(stderr, "%s: expected qemu-storage-daemon to exit "
+ "successfully, got %d\n",
+ __func__, WEXITSTATUS(wstatus));
+ abort();
+ }
+
+ g_free(data);
+}
+
+static void start_vhost_user_blk(GString *cmd_line, int vus_instances)
+{
+ const char *vhost_user_blk_bin = qtest_qemu_storage_daemon_binary();
+ int i;
+ gchar *img_path;
+ GString *storage_daemon_command = g_string_new(NULL);
+ QemuStorageDaemonState *qsd;
+
+ g_string_append_printf(storage_daemon_command,
+ "exec %s ",
+ vhost_user_blk_bin);
+
+ g_string_append_printf(cmd_line,
+ " -object memory-backend-memfd,id=mem,size=256M,share=on "
+ " -M memory-backend=mem -m 256M ");
+
+ for (i = 0; i < vus_instances; i++) {
+ int fd;
+ char *sock_path = create_listen_socket(&fd);
+
+ /* create image file */
+ img_path = drive_create();
+ g_string_append_printf(storage_daemon_command,
+ "--blockdev driver=file,node-name=disk%d,filename=%s "
+ "--export type=vhost-user-blk,id=disk%d,addr.type=unix,addr.path=%s,"
+ "node-name=disk%i,writable=on ",
+ i, img_path, i, sock_path, i);
+
+ g_string_append_printf(cmd_line, "-chardev socket,id=char%d,path=%s ",
+ i + 1, sock_path);
+ }
+
+ g_test_message("starting vhost-user backend: %s",
+ storage_daemon_command->str);
+ pid_t pid = fork();
+ if (pid == 0) {
+ /*
+ * Close standard file descriptors so tap-driver.pl pipe detects when
+ * our parent terminates.
+ */
+ close(0);
+ close(1);
+ open("/dev/null", O_RDONLY);
+ open("/dev/null", O_WRONLY);
+
+ execlp("/bin/sh", "sh", "-c", storage_daemon_command->str, NULL);
+ exit(1);
+ }
+ g_string_free(storage_daemon_command, true);
+
+ qsd = g_new(QemuStorageDaemonState, 1);
+ qsd->pid = pid;
+
+ /* Make sure qemu-storage-daemon is stopped */
+ qtest_add_abrt_handler(quit_storage_daemon, qsd);
+ g_test_queue_destroy(quit_storage_daemon, qsd);
+}
+
+static void *vhost_user_blk_test_setup(GString *cmd_line, void *arg)
+{
+ start_vhost_user_blk(cmd_line, 1);
+ return arg;
+}
+
+/*
+ * Setup for hotplug.
+ *
+ * Since vhost-user server only serves one vhost-user client one time,
+ * another exprot
+ *
+ */
+static void *vhost_user_blk_hotplug_test_setup(GString *cmd_line, void *arg)
+{
+ /* "-chardev socket,id=char2" is used for pci_hotplug*/
+ start_vhost_user_blk(cmd_line, 2);
+ return arg;
+}
+
+static void register_vhost_user_blk_test(void)
+{
+ QOSGraphTestOptions opts = {
+ .before = vhost_user_blk_test_setup,
+ };
+
+ /*
+ * tests for vhost-user-blk and vhost-user-blk-pci
+ * The tests are borrowed from tests/virtio-blk-test.c. But some tests
+ * regarding block_resize don't work for vhost-user-blk.
+ * vhost-user-blk device doesn't have -drive, so tests containing
+ * block_resize are also abandoned,
+ * - config
+ * - resize
+ */
+ qos_add_test("basic", "vhost-user-blk", basic, &opts);
+ qos_add_test("indirect", "vhost-user-blk", indirect, &opts);
+ qos_add_test("idx", "vhost-user-blk-pci", idx, &opts);
+ qos_add_test("nxvirtq", "vhost-user-blk-pci",
+ test_nonexistent_virtqueue, &opts);
+
+ opts.before = vhost_user_blk_hotplug_test_setup;
+ qos_add_test("hotplug", "vhost-user-blk-pci", pci_hotplug, &opts);
+}
+
+libqos_init(register_vhost_user_blk_test);
diff --git a/MAINTAINERS b/MAINTAINERS
index 26c9454823..4249acc62e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3203,6 +3203,8 @@ F: block/export/vhost-user-blk-server.c
F: block/export/vhost-user-blk-server.h
F: include/qemu/vhost-user-server.h
F: tests/qtest/libqos/vhost-user-blk.c
+F: tests/qtest/libqos/vhost-user-blk.h
+F: tests/qtest/vhost-user-blk-test.c
F: util/vhost-user-server.c
FUSE block device exports
diff --git a/tests/qtest/libqos/meson.build b/tests/qtest/libqos/meson.build
index 1cddf5bdaa..1f5c8f1053 100644
--- a/tests/qtest/libqos/meson.build
+++ b/tests/qtest/libqos/meson.build
@@ -32,6 +32,7 @@ libqos_srcs = files('../libqtest.c',
'virtio-9p.c',
'virtio-balloon.c',
'virtio-blk.c',
+ 'vhost-user-blk.c',
'virtio-mmio.c',
'virtio-net.c',
'virtio-pci.c',
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index ba6ecaed32..1aeaf58d96 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -210,6 +210,9 @@ if have_virtfs
qos_test_ss.add(files('virtio-9p-test.c'))
endif
qos_test_ss.add(when: 'CONFIG_VHOST_USER', if_true: files('vhost-user-test.c'))
+if have_vhost_user_blk_server
+ qos_test_ss.add(files('vhost-user-blk-test.c'))
+endif
tpmemu_files = ['tpm-emu.c', 'tpm-util.c', 'tpm-tests.c']
@@ -248,6 +251,7 @@ foreach dir : target_dirs
endif
qtest_env.set('G_TEST_DBUS_DAEMON', meson.source_root() / 'tests/dbus-vmstate-daemon.sh')
qtest_env.set('QTEST_QEMU_BINARY', './qemu-system-' + target_base)
+ qtest_env.set('QTEST_QEMU_STORAGE_DAEMON_BINARY', './storage-daemon/qemu-storage-daemon')
foreach test : target_qtests
# Executables are shared across targets, declare them only the first time we
--
2.29.2
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PULL 17/31] tests/qtest: add multi-queue test case to vhost-user-blk-test
2021-03-05 16:54 [PULL 00/31] Block layer patches Kevin Wolf
` (15 preceding siblings ...)
2021-03-05 16:54 ` [PULL 16/31] test: new qTest case to test the vhost-user-blk-server Kevin Wolf
@ 2021-03-05 16:54 ` Kevin Wolf
2021-03-05 16:54 ` [PULL 18/31] block/export: fix blk_size double byteswap Kevin Wolf
` (14 subsequent siblings)
31 siblings, 0 replies; 35+ messages in thread
From: Kevin Wolf @ 2021-03-05 16:54 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20210223144653.811468-7-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qtest/vhost-user-blk-test.c | 81 +++++++++++++++++++++++++++++--
1 file changed, 76 insertions(+), 5 deletions(-)
diff --git a/tests/qtest/vhost-user-blk-test.c b/tests/qtest/vhost-user-blk-test.c
index f0fb09893e..61beee52d3 100644
--- a/tests/qtest/vhost-user-blk-test.c
+++ b/tests/qtest/vhost-user-blk-test.c
@@ -563,6 +563,67 @@ static void pci_hotplug(void *obj, void *data, QGuestAllocator *t_alloc)
qpci_unplug_acpi_device_test(qts, "drv1", PCI_SLOT_HP);
}
+static void multiqueue(void *obj, void *data, QGuestAllocator *t_alloc)
+{
+ QVirtioPCIDevice *pdev1 = obj;
+ QVirtioDevice *dev1 = &pdev1->vdev;
+ QVirtioPCIDevice *pdev8;
+ QVirtioDevice *dev8;
+ QTestState *qts = pdev1->pdev->bus->qts;
+ uint64_t features;
+ uint16_t num_queues;
+
+ /*
+ * The primary device has 1 queue and VIRTIO_BLK_F_MQ is not enabled. The
+ * VIRTIO specification allows VIRTIO_BLK_F_MQ to be enabled when there is
+ * only 1 virtqueue, but --device vhost-user-blk-pci doesn't do this (which
+ * is also spec-compliant).
+ */
+ features = qvirtio_get_features(dev1);
+ g_assert_cmpint(features & (1u << VIRTIO_BLK_F_MQ), ==, 0);
+ features = features & ~(QVIRTIO_F_BAD_FEATURE |
+ (1u << VIRTIO_RING_F_INDIRECT_DESC) |
+ (1u << VIRTIO_F_NOTIFY_ON_EMPTY) |
+ (1u << VIRTIO_BLK_F_SCSI));
+ qvirtio_set_features(dev1, features);
+
+ /* Hotplug a secondary device with 8 queues */
+ qtest_qmp_device_add(qts, "vhost-user-blk-pci", "drv1",
+ "{'addr': %s, 'chardev': 'char2', 'num-queues': 8}",
+ stringify(PCI_SLOT_HP) ".0");
+
+ pdev8 = virtio_pci_new(pdev1->pdev->bus,
+ &(QPCIAddress) {
+ .devfn = QPCI_DEVFN(PCI_SLOT_HP, 0)
+ });
+ g_assert_nonnull(pdev8);
+ g_assert_cmpint(pdev8->vdev.device_type, ==, VIRTIO_ID_BLOCK);
+
+ qos_object_start_hw(&pdev8->obj);
+
+ dev8 = &pdev8->vdev;
+ features = qvirtio_get_features(dev8);
+ g_assert_cmpint(features & (1u << VIRTIO_BLK_F_MQ),
+ ==,
+ (1u << VIRTIO_BLK_F_MQ));
+ features = features & ~(QVIRTIO_F_BAD_FEATURE |
+ (1u << VIRTIO_RING_F_INDIRECT_DESC) |
+ (1u << VIRTIO_F_NOTIFY_ON_EMPTY) |
+ (1u << VIRTIO_BLK_F_SCSI) |
+ (1u << VIRTIO_BLK_F_MQ));
+ qvirtio_set_features(dev8, features);
+
+ num_queues = qvirtio_config_readw(dev8,
+ offsetof(struct virtio_blk_config, num_queues));
+ g_assert_cmpint(num_queues, ==, 8);
+
+ qvirtio_pci_device_disable(pdev8);
+ qos_object_destroy(&pdev8->obj);
+
+ /* unplug secondary disk */
+ qpci_unplug_acpi_device_test(qts, "drv1", PCI_SLOT_HP);
+}
+
/*
* Check that setting the vring addr on a non-existent virtqueue does
* not crash.
@@ -682,7 +743,8 @@ static void quit_storage_daemon(void *data)
g_free(data);
}
-static void start_vhost_user_blk(GString *cmd_line, int vus_instances)
+static void start_vhost_user_blk(GString *cmd_line, int vus_instances,
+ int num_queues)
{
const char *vhost_user_blk_bin = qtest_qemu_storage_daemon_binary();
int i;
@@ -707,8 +769,8 @@ static void start_vhost_user_blk(GString *cmd_line, int vus_instances)
g_string_append_printf(storage_daemon_command,
"--blockdev driver=file,node-name=disk%d,filename=%s "
"--export type=vhost-user-blk,id=disk%d,addr.type=unix,addr.path=%s,"
- "node-name=disk%i,writable=on ",
- i, img_path, i, sock_path, i);
+ "node-name=disk%i,writable=on,num-queues=%d ",
+ i, img_path, i, sock_path, i, num_queues);
g_string_append_printf(cmd_line, "-chardev socket,id=char%d,path=%s ",
i + 1, sock_path);
@@ -742,7 +804,7 @@ static void start_vhost_user_blk(GString *cmd_line, int vus_instances)
static void *vhost_user_blk_test_setup(GString *cmd_line, void *arg)
{
- start_vhost_user_blk(cmd_line, 1);
+ start_vhost_user_blk(cmd_line, 1, 1);
return arg;
}
@@ -756,7 +818,13 @@ static void *vhost_user_blk_test_setup(GString *cmd_line, void *arg)
static void *vhost_user_blk_hotplug_test_setup(GString *cmd_line, void *arg)
{
/* "-chardev socket,id=char2" is used for pci_hotplug*/
- start_vhost_user_blk(cmd_line, 2);
+ start_vhost_user_blk(cmd_line, 2, 1);
+ return arg;
+}
+
+static void *vhost_user_blk_multiqueue_test_setup(GString *cmd_line, void *arg)
+{
+ start_vhost_user_blk(cmd_line, 2, 8);
return arg;
}
@@ -783,6 +851,9 @@ static void register_vhost_user_blk_test(void)
opts.before = vhost_user_blk_hotplug_test_setup;
qos_add_test("hotplug", "vhost-user-blk-pci", pci_hotplug, &opts);
+
+ opts.before = vhost_user_blk_multiqueue_test_setup;
+ qos_add_test("multiqueue", "vhost-user-blk-pci", multiqueue, &opts);
}
libqos_init(register_vhost_user_blk_test);
--
2.29.2
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PULL 18/31] block/export: fix blk_size double byteswap
2021-03-05 16:54 [PULL 00/31] Block layer patches Kevin Wolf
` (16 preceding siblings ...)
2021-03-05 16:54 ` [PULL 17/31] tests/qtest: add multi-queue test case to vhost-user-blk-test Kevin Wolf
@ 2021-03-05 16:54 ` Kevin Wolf
2021-03-05 16:54 ` [PULL 19/31] block/export: use VIRTIO_BLK_SECTOR_BITS Kevin Wolf
` (13 subsequent siblings)
31 siblings, 0 replies; 35+ messages in thread
From: Kevin Wolf @ 2021-03-05 16:54 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Stefan Hajnoczi <stefanha@redhat.com>
The config->blk_size field is little-endian. Use the native-endian
blk_size variable to avoid double byteswapping.
Fixes: 11f60f7eaee2630dd6fa0c3a8c49f792e46c4cf1 ("block/export: make vhost-user-blk config space little-endian")
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20210223144653.811468-8-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/export/vhost-user-blk-server.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/export/vhost-user-blk-server.c b/block/export/vhost-user-blk-server.c
index ab2c4d44c4..7aea132f69 100644
--- a/block/export/vhost-user-blk-server.c
+++ b/block/export/vhost-user-blk-server.c
@@ -356,7 +356,7 @@ vu_blk_initialize_config(BlockDriverState *bs,
config->num_queues = cpu_to_le16(num_queues);
config->max_discard_sectors = cpu_to_le32(32768);
config->max_discard_seg = cpu_to_le32(1);
- config->discard_sector_alignment = cpu_to_le32(config->blk_size >> 9);
+ config->discard_sector_alignment = cpu_to_le32(blk_size >> 9);
config->max_write_zeroes_sectors = cpu_to_le32(32768);
config->max_write_zeroes_seg = cpu_to_le32(1);
}
--
2.29.2
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PULL 19/31] block/export: use VIRTIO_BLK_SECTOR_BITS
2021-03-05 16:54 [PULL 00/31] Block layer patches Kevin Wolf
` (17 preceding siblings ...)
2021-03-05 16:54 ` [PULL 18/31] block/export: fix blk_size double byteswap Kevin Wolf
@ 2021-03-05 16:54 ` Kevin Wolf
2021-03-05 16:54 ` [PULL 20/31] block/export: fix vhost-user-blk export sector number calculation Kevin Wolf
` (12 subsequent siblings)
31 siblings, 0 replies; 35+ messages in thread
From: Kevin Wolf @ 2021-03-05 16:54 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Stefan Hajnoczi <stefanha@redhat.com>
Use VIRTIO_BLK_SECTOR_BITS and VIRTIO_BLK_SECTOR_SIZE when dealing with
virtio-blk sector numbers. Although the values happen to be the same as
BDRV_SECTOR_BITS and BDRV_SECTOR_SIZE, they are conceptually different.
This makes it clearer when we are dealing with virtio-blk sector units.
Use VIRTIO_BLK_SECTOR_BITS in vu_blk_initialize_config(). Later patches
will use it the new constants the virtqueue request processing code
path.
Suggested-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20210223144653.811468-9-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/export/vhost-user-blk-server.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/block/export/vhost-user-blk-server.c b/block/export/vhost-user-blk-server.c
index 7aea132f69..2614a63791 100644
--- a/block/export/vhost-user-blk-server.c
+++ b/block/export/vhost-user-blk-server.c
@@ -20,6 +20,13 @@
#include "sysemu/block-backend.h"
#include "util/block-helpers.h"
+/*
+ * Sector units are 512 bytes regardless of the
+ * virtio_blk_config->blk_size value.
+ */
+#define VIRTIO_BLK_SECTOR_BITS 9
+#define VIRTIO_BLK_SECTOR_SIZE (1ull << VIRTIO_BLK_SECTOR_BITS)
+
enum {
VHOST_USER_BLK_NUM_QUEUES_DEFAULT = 1,
};
@@ -347,7 +354,8 @@ vu_blk_initialize_config(BlockDriverState *bs,
uint32_t blk_size,
uint16_t num_queues)
{
- config->capacity = cpu_to_le64(bdrv_getlength(bs) >> BDRV_SECTOR_BITS);
+ config->capacity =
+ cpu_to_le64(bdrv_getlength(bs) >> VIRTIO_BLK_SECTOR_BITS);
config->blk_size = cpu_to_le32(blk_size);
config->size_max = cpu_to_le32(0);
config->seg_max = cpu_to_le32(128 - 2);
@@ -356,7 +364,8 @@ vu_blk_initialize_config(BlockDriverState *bs,
config->num_queues = cpu_to_le16(num_queues);
config->max_discard_sectors = cpu_to_le32(32768);
config->max_discard_seg = cpu_to_le32(1);
- config->discard_sector_alignment = cpu_to_le32(blk_size >> 9);
+ config->discard_sector_alignment =
+ cpu_to_le32(blk_size >> VIRTIO_BLK_SECTOR_BITS);
config->max_write_zeroes_sectors = cpu_to_le32(32768);
config->max_write_zeroes_seg = cpu_to_le32(1);
}
@@ -383,7 +392,7 @@ static int vu_blk_exp_create(BlockExport *exp, BlockExportOptions *opts,
if (vu_opts->has_logical_block_size) {
logical_block_size = vu_opts->logical_block_size;
} else {
- logical_block_size = BDRV_SECTOR_SIZE;
+ logical_block_size = VIRTIO_BLK_SECTOR_SIZE;
}
check_block_size(exp->id, "logical-block-size", logical_block_size,
&local_err);
--
2.29.2
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PULL 20/31] block/export: fix vhost-user-blk export sector number calculation
2021-03-05 16:54 [PULL 00/31] Block layer patches Kevin Wolf
` (18 preceding siblings ...)
2021-03-05 16:54 ` [PULL 19/31] block/export: use VIRTIO_BLK_SECTOR_BITS Kevin Wolf
@ 2021-03-05 16:54 ` Kevin Wolf
2021-03-05 16:54 ` [PULL 21/31] block/export: port virtio-blk discard/write zeroes input validation Kevin Wolf
` (11 subsequent siblings)
31 siblings, 0 replies; 35+ messages in thread
From: Kevin Wolf @ 2021-03-05 16:54 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Stefan Hajnoczi <stefanha@redhat.com>
The driver is supposed to honor the blk_size field but the protocol
still uses 512-byte sector numbers. It is incorrect to multiply
req->sector_num by blk_size.
VIRTIO 1.1 5.2.5 Device Initialization says:
blk_size can be read to determine the optimal sector size for the
driver to use. This does not affect the units used in the protocol
(always 512 bytes), but awareness of the correct value can affect
performance.
Fixes: 3578389bcf76c824a5d82e6586a6f0c71e56f2aa ("block/export: vhost-user block device backend server")
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20210223144653.811468-10-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/export/vhost-user-blk-server.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/export/vhost-user-blk-server.c b/block/export/vhost-user-blk-server.c
index 2614a63791..f74796241c 100644
--- a/block/export/vhost-user-blk-server.c
+++ b/block/export/vhost-user-blk-server.c
@@ -144,7 +144,7 @@ static void coroutine_fn vu_blk_virtio_process_req(void *opaque)
break;
}
- int64_t offset = req->sector_num * vexp->blk_size;
+ int64_t offset = req->sector_num << VIRTIO_BLK_SECTOR_BITS;
QEMUIOVector qiov;
if (is_write) {
qemu_iovec_init_external(&qiov, out_iov, out_num);
--
2.29.2
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PULL 21/31] block/export: port virtio-blk discard/write zeroes input validation
2021-03-05 16:54 [PULL 00/31] Block layer patches Kevin Wolf
` (19 preceding siblings ...)
2021-03-05 16:54 ` [PULL 20/31] block/export: fix vhost-user-blk export sector number calculation Kevin Wolf
@ 2021-03-05 16:54 ` Kevin Wolf
2021-03-05 16:54 ` [PULL 22/31] vhost-user-blk-test: test discard/write zeroes invalid inputs Kevin Wolf
` (10 subsequent siblings)
31 siblings, 0 replies; 35+ messages in thread
From: Kevin Wolf @ 2021-03-05 16:54 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Stefan Hajnoczi <stefanha@redhat.com>
Validate discard/write zeroes the same way we do for virtio-blk. Some of
these checks are mandated by the VIRTIO specification, others are
internal to QEMU.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20210223144653.811468-11-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/export/vhost-user-blk-server.c | 116 +++++++++++++++++++++------
1 file changed, 93 insertions(+), 23 deletions(-)
diff --git a/block/export/vhost-user-blk-server.c b/block/export/vhost-user-blk-server.c
index f74796241c..04044228d4 100644
--- a/block/export/vhost-user-blk-server.c
+++ b/block/export/vhost-user-blk-server.c
@@ -29,6 +29,8 @@
enum {
VHOST_USER_BLK_NUM_QUEUES_DEFAULT = 1,
+ VHOST_USER_BLK_MAX_DISCARD_SECTORS = 32768,
+ VHOST_USER_BLK_MAX_WRITE_ZEROES_SECTORS = 32768,
};
struct virtio_blk_inhdr {
unsigned char status;
@@ -65,30 +67,102 @@ static void vu_blk_req_complete(VuBlkReq *req)
free(req);
}
+static bool vu_blk_sect_range_ok(VuBlkExport *vexp, uint64_t sector,
+ size_t size)
+{
+ uint64_t nb_sectors = size >> BDRV_SECTOR_BITS;
+ uint64_t total_sectors;
+
+ if (nb_sectors > BDRV_REQUEST_MAX_SECTORS) {
+ return false;
+ }
+ if ((sector << VIRTIO_BLK_SECTOR_BITS) % vexp->blk_size) {
+ return false;
+ }
+ blk_get_geometry(vexp->export.blk, &total_sectors);
+ if (sector > total_sectors || nb_sectors > total_sectors - sector) {
+ return false;
+ }
+ return true;
+}
+
static int coroutine_fn
-vu_blk_discard_write_zeroes(BlockBackend *blk, struct iovec *iov,
+vu_blk_discard_write_zeroes(VuBlkExport *vexp, struct iovec *iov,
uint32_t iovcnt, uint32_t type)
{
+ BlockBackend *blk = vexp->export.blk;
struct virtio_blk_discard_write_zeroes desc;
- ssize_t size = iov_to_buf(iov, iovcnt, 0, &desc, sizeof(desc));
+ ssize_t size;
+ uint64_t sector;
+ uint32_t num_sectors;
+ uint32_t max_sectors;
+ uint32_t flags;
+ int bytes;
+
+ /* Only one desc is currently supported */
+ if (unlikely(iov_size(iov, iovcnt) > sizeof(desc))) {
+ return VIRTIO_BLK_S_UNSUPP;
+ }
+
+ size = iov_to_buf(iov, iovcnt, 0, &desc, sizeof(desc));
if (unlikely(size != sizeof(desc))) {
- error_report("Invalid size %zd, expect %zu", size, sizeof(desc));
- return -EINVAL;
+ error_report("Invalid size %zd, expected %zu", size, sizeof(desc));
+ return VIRTIO_BLK_S_IOERR;
}
- uint64_t range[2] = { le64_to_cpu(desc.sector) << 9,
- le32_to_cpu(desc.num_sectors) << 9 };
- if (type == VIRTIO_BLK_T_DISCARD) {
- if (blk_co_pdiscard(blk, range[0], range[1]) == 0) {
- return 0;
+ sector = le64_to_cpu(desc.sector);
+ num_sectors = le32_to_cpu(desc.num_sectors);
+ flags = le32_to_cpu(desc.flags);
+ max_sectors = (type == VIRTIO_BLK_T_WRITE_ZEROES) ?
+ VHOST_USER_BLK_MAX_WRITE_ZEROES_SECTORS :
+ VHOST_USER_BLK_MAX_DISCARD_SECTORS;
+
+ /* This check ensures that 'bytes' fits in an int */
+ if (unlikely(num_sectors > max_sectors)) {
+ return VIRTIO_BLK_S_IOERR;
+ }
+
+ bytes = num_sectors << VIRTIO_BLK_SECTOR_BITS;
+
+ if (unlikely(!vu_blk_sect_range_ok(vexp, sector, bytes))) {
+ return VIRTIO_BLK_S_IOERR;
+ }
+
+ /*
+ * The device MUST set the status byte to VIRTIO_BLK_S_UNSUPP for discard
+ * and write zeroes commands if any unknown flag is set.
+ */
+ if (unlikely(flags & ~VIRTIO_BLK_WRITE_ZEROES_FLAG_UNMAP)) {
+ return VIRTIO_BLK_S_UNSUPP;
+ }
+
+ if (type == VIRTIO_BLK_T_WRITE_ZEROES) {
+ int blk_flags = 0;
+
+ if (flags & VIRTIO_BLK_WRITE_ZEROES_FLAG_UNMAP) {
+ blk_flags |= BDRV_REQ_MAY_UNMAP;
+ }
+
+ if (blk_co_pwrite_zeroes(blk, sector << VIRTIO_BLK_SECTOR_BITS,
+ bytes, blk_flags) == 0) {
+ return VIRTIO_BLK_S_OK;
}
- } else if (type == VIRTIO_BLK_T_WRITE_ZEROES) {
- if (blk_co_pwrite_zeroes(blk, range[0], range[1], 0) == 0) {
- return 0;
+ } else if (type == VIRTIO_BLK_T_DISCARD) {
+ /*
+ * The device MUST set the status byte to VIRTIO_BLK_S_UNSUPP for
+ * discard commands if the unmap flag is set.
+ */
+ if (unlikely(flags & VIRTIO_BLK_WRITE_ZEROES_FLAG_UNMAP)) {
+ return VIRTIO_BLK_S_UNSUPP;
+ }
+
+ if (blk_co_pdiscard(blk, sector << VIRTIO_BLK_SECTOR_BITS,
+ bytes) == 0) {
+ return VIRTIO_BLK_S_OK;
}
}
- return -EINVAL;
+ return VIRTIO_BLK_S_IOERR;
}
static void coroutine_fn vu_blk_virtio_process_req(void *opaque)
@@ -177,19 +251,13 @@ static void coroutine_fn vu_blk_virtio_process_req(void *opaque)
}
case VIRTIO_BLK_T_DISCARD:
case VIRTIO_BLK_T_WRITE_ZEROES: {
- int rc;
-
if (!vexp->writable) {
req->in->status = VIRTIO_BLK_S_IOERR;
break;
}
- rc = vu_blk_discard_write_zeroes(blk, &elem->out_sg[1], out_num, type);
- if (rc == 0) {
- req->in->status = VIRTIO_BLK_S_OK;
- } else {
- req->in->status = VIRTIO_BLK_S_IOERR;
- }
+ req->in->status = vu_blk_discard_write_zeroes(vexp, out_iov, out_num,
+ type);
break;
}
default:
@@ -362,11 +430,13 @@ vu_blk_initialize_config(BlockDriverState *bs,
config->min_io_size = cpu_to_le16(1);
config->opt_io_size = cpu_to_le32(1);
config->num_queues = cpu_to_le16(num_queues);
- config->max_discard_sectors = cpu_to_le32(32768);
+ config->max_discard_sectors =
+ cpu_to_le32(VHOST_USER_BLK_MAX_DISCARD_SECTORS);
config->max_discard_seg = cpu_to_le32(1);
config->discard_sector_alignment =
cpu_to_le32(blk_size >> VIRTIO_BLK_SECTOR_BITS);
- config->max_write_zeroes_sectors = cpu_to_le32(32768);
+ config->max_write_zeroes_sectors
+ = cpu_to_le32(VHOST_USER_BLK_MAX_WRITE_ZEROES_SECTORS);
config->max_write_zeroes_seg = cpu_to_le32(1);
}
--
2.29.2
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PULL 22/31] vhost-user-blk-test: test discard/write zeroes invalid inputs
2021-03-05 16:54 [PULL 00/31] Block layer patches Kevin Wolf
` (20 preceding siblings ...)
2021-03-05 16:54 ` [PULL 21/31] block/export: port virtio-blk discard/write zeroes input validation Kevin Wolf
@ 2021-03-05 16:54 ` Kevin Wolf
2021-03-05 16:54 ` [PULL 23/31] block/export: port virtio-blk read/write range check Kevin Wolf
` (9 subsequent siblings)
31 siblings, 0 replies; 35+ messages in thread
From: Kevin Wolf @ 2021-03-05 16:54 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Stefan Hajnoczi <stefanha@redhat.com>
Exercise input validation code paths in
block/export/vhost-user-blk-server.c.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20210223144653.811468-12-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qtest/vhost-user-blk-test.c | 124 ++++++++++++++++++++++++++++++
1 file changed, 124 insertions(+)
diff --git a/tests/qtest/vhost-user-blk-test.c b/tests/qtest/vhost-user-blk-test.c
index 61beee52d3..dc9d7a31ae 100644
--- a/tests/qtest/vhost-user-blk-test.c
+++ b/tests/qtest/vhost-user-blk-test.c
@@ -94,6 +94,124 @@ static uint64_t virtio_blk_request(QGuestAllocator *alloc, QVirtioDevice *d,
return addr;
}
+static void test_invalid_discard_write_zeroes(QVirtioDevice *dev,
+ QGuestAllocator *alloc,
+ QTestState *qts,
+ QVirtQueue *vq,
+ uint32_t type)
+{
+ QVirtioBlkReq req;
+ struct virtio_blk_discard_write_zeroes dwz_hdr;
+ struct virtio_blk_discard_write_zeroes dwz_hdr2[2];
+ uint64_t req_addr;
+ uint32_t free_head;
+ uint8_t status;
+
+ /* More than one dwz is not supported */
+ req.type = type;
+ req.data = (char *) dwz_hdr2;
+ dwz_hdr2[0].sector = 0;
+ dwz_hdr2[0].num_sectors = 1;
+ dwz_hdr2[0].flags = 0;
+ dwz_hdr2[1].sector = 1;
+ dwz_hdr2[1].num_sectors = 1;
+ dwz_hdr2[1].flags = 0;
+
+ virtio_blk_fix_dwz_hdr(dev, &dwz_hdr2[0]);
+ virtio_blk_fix_dwz_hdr(dev, &dwz_hdr2[1]);
+
+ req_addr = virtio_blk_request(alloc, dev, &req, sizeof(dwz_hdr2));
+
+ free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
+ qvirtqueue_add(qts, vq, req_addr + 16, sizeof(dwz_hdr2), false, true);
+ qvirtqueue_add(qts, vq, req_addr + 16 + sizeof(dwz_hdr2), 1, true,
+ false);
+
+ qvirtqueue_kick(qts, dev, vq, free_head);
+
+ qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+ QVIRTIO_BLK_TIMEOUT_US);
+ status = readb(req_addr + 16 + sizeof(dwz_hdr2));
+ g_assert_cmpint(status, ==, VIRTIO_BLK_S_UNSUPP);
+
+ guest_free(alloc, req_addr);
+
+ /* num_sectors must be less than config->max_write_zeroes_sectors */
+ req.type = type;
+ req.data = (char *) &dwz_hdr;
+ dwz_hdr.sector = 0;
+ dwz_hdr.num_sectors = 0xffffffff;
+ dwz_hdr.flags = 0;
+
+ virtio_blk_fix_dwz_hdr(dev, &dwz_hdr);
+
+ req_addr = virtio_blk_request(alloc, dev, &req, sizeof(dwz_hdr));
+
+ free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
+ qvirtqueue_add(qts, vq, req_addr + 16, sizeof(dwz_hdr), false, true);
+ qvirtqueue_add(qts, vq, req_addr + 16 + sizeof(dwz_hdr), 1, true,
+ false);
+
+ qvirtqueue_kick(qts, dev, vq, free_head);
+
+ qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+ QVIRTIO_BLK_TIMEOUT_US);
+ status = readb(req_addr + 16 + sizeof(dwz_hdr));
+ g_assert_cmpint(status, ==, VIRTIO_BLK_S_IOERR);
+
+ guest_free(alloc, req_addr);
+
+ /* sector must be less than the device capacity */
+ req.type = type;
+ req.data = (char *) &dwz_hdr;
+ dwz_hdr.sector = TEST_IMAGE_SIZE / 512 + 1;
+ dwz_hdr.num_sectors = 1;
+ dwz_hdr.flags = 0;
+
+ virtio_blk_fix_dwz_hdr(dev, &dwz_hdr);
+
+ req_addr = virtio_blk_request(alloc, dev, &req, sizeof(dwz_hdr));
+
+ free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
+ qvirtqueue_add(qts, vq, req_addr + 16, sizeof(dwz_hdr), false, true);
+ qvirtqueue_add(qts, vq, req_addr + 16 + sizeof(dwz_hdr), 1, true,
+ false);
+
+ qvirtqueue_kick(qts, dev, vq, free_head);
+
+ qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+ QVIRTIO_BLK_TIMEOUT_US);
+ status = readb(req_addr + 16 + sizeof(dwz_hdr));
+ g_assert_cmpint(status, ==, VIRTIO_BLK_S_IOERR);
+
+ guest_free(alloc, req_addr);
+
+ /* reserved flag bits must be zero */
+ req.type = type;
+ req.data = (char *) &dwz_hdr;
+ dwz_hdr.sector = 0;
+ dwz_hdr.num_sectors = 1;
+ dwz_hdr.flags = ~VIRTIO_BLK_WRITE_ZEROES_FLAG_UNMAP;
+
+ virtio_blk_fix_dwz_hdr(dev, &dwz_hdr);
+
+ req_addr = virtio_blk_request(alloc, dev, &req, sizeof(dwz_hdr));
+
+ free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
+ qvirtqueue_add(qts, vq, req_addr + 16, sizeof(dwz_hdr), false, true);
+ qvirtqueue_add(qts, vq, req_addr + 16 + sizeof(dwz_hdr), 1, true,
+ false);
+
+ qvirtqueue_kick(qts, dev, vq, free_head);
+
+ qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+ QVIRTIO_BLK_TIMEOUT_US);
+ status = readb(req_addr + 16 + sizeof(dwz_hdr));
+ g_assert_cmpint(status, ==, VIRTIO_BLK_S_UNSUPP);
+
+ guest_free(alloc, req_addr);
+}
+
/* Returns the request virtqueue so the caller can perform further tests */
static QVirtQueue *test_basic(QVirtioDevice *dev, QGuestAllocator *alloc)
{
@@ -235,6 +353,9 @@ static QVirtQueue *test_basic(QVirtioDevice *dev, QGuestAllocator *alloc)
g_free(data);
guest_free(alloc, req_addr);
+
+ test_invalid_discard_write_zeroes(dev, alloc, qts, vq,
+ VIRTIO_BLK_T_WRITE_ZEROES);
}
if (features & (1u << VIRTIO_BLK_F_DISCARD)) {
@@ -263,6 +384,9 @@ static QVirtQueue *test_basic(QVirtioDevice *dev, QGuestAllocator *alloc)
g_assert_cmpint(status, ==, 0);
guest_free(alloc, req_addr);
+
+ test_invalid_discard_write_zeroes(dev, alloc, qts, vq,
+ VIRTIO_BLK_T_DISCARD);
}
if (features & (1u << VIRTIO_F_ANY_LAYOUT)) {
--
2.29.2
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PULL 23/31] block/export: port virtio-blk read/write range check
2021-03-05 16:54 [PULL 00/31] Block layer patches Kevin Wolf
` (21 preceding siblings ...)
2021-03-05 16:54 ` [PULL 22/31] vhost-user-blk-test: test discard/write zeroes invalid inputs Kevin Wolf
@ 2021-03-05 16:54 ` Kevin Wolf
2021-03-05 16:54 ` [PULL 24/31] qcow2-bitmap: make bytes_covered_by_bitmap_cluster() public Kevin Wolf
` (8 subsequent siblings)
31 siblings, 0 replies; 35+ messages in thread
From: Kevin Wolf @ 2021-03-05 16:54 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Stefan Hajnoczi <stefanha@redhat.com>
Check that the sector number and byte count are valid.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20210223144653.811468-13-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/export/vhost-user-blk-server.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/block/export/vhost-user-blk-server.c b/block/export/vhost-user-blk-server.c
index 04044228d4..cb5d896b7b 100644
--- a/block/export/vhost-user-blk-server.c
+++ b/block/export/vhost-user-blk-server.c
@@ -209,6 +209,8 @@ static void coroutine_fn vu_blk_virtio_process_req(void *opaque)
switch (type & ~VIRTIO_BLK_T_BARRIER) {
case VIRTIO_BLK_T_IN:
case VIRTIO_BLK_T_OUT: {
+ QEMUIOVector qiov;
+ int64_t offset;
ssize_t ret = 0;
bool is_write = type & VIRTIO_BLK_T_OUT;
req->sector_num = le64_to_cpu(req->out.sector);
@@ -218,13 +220,24 @@ static void coroutine_fn vu_blk_virtio_process_req(void *opaque)
break;
}
- int64_t offset = req->sector_num << VIRTIO_BLK_SECTOR_BITS;
- QEMUIOVector qiov;
if (is_write) {
qemu_iovec_init_external(&qiov, out_iov, out_num);
- ret = blk_co_pwritev(blk, offset, qiov.size, &qiov, 0);
} else {
qemu_iovec_init_external(&qiov, in_iov, in_num);
+ }
+
+ if (unlikely(!vu_blk_sect_range_ok(vexp,
+ req->sector_num,
+ qiov.size))) {
+ req->in->status = VIRTIO_BLK_S_IOERR;
+ break;
+ }
+
+ offset = req->sector_num << VIRTIO_BLK_SECTOR_BITS;
+
+ if (is_write) {
+ ret = blk_co_pwritev(blk, offset, qiov.size, &qiov, 0);
+ } else {
ret = blk_co_preadv(blk, offset, qiov.size, &qiov, 0);
}
if (ret >= 0) {
--
2.29.2
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PULL 24/31] qcow2-bitmap: make bytes_covered_by_bitmap_cluster() public
2021-03-05 16:54 [PULL 00/31] Block layer patches Kevin Wolf
` (22 preceding siblings ...)
2021-03-05 16:54 ` [PULL 23/31] block/export: port virtio-blk read/write range check Kevin Wolf
@ 2021-03-05 16:54 ` Kevin Wolf
2021-03-05 16:54 ` [PULL 25/31] parallels.txt: fix bitmap L1 table description Kevin Wolf
` (7 subsequent siblings)
31 siblings, 0 replies; 35+ messages in thread
From: Kevin Wolf @ 2021-03-05 16:54 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Rename bytes_covered_by_bitmap_cluster() to
bdrv_dirty_bitmap_serialization_coverage() and make it public.
It is needed as we are going to share it with bitmap loading in
parallels format.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Message-Id: <20210224104707.88430-2-vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
include/block/dirty-bitmap.h | 2 ++
block/dirty-bitmap.c | 13 +++++++++++++
block/qcow2-bitmap.c | 16 ++--------------
3 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 36e8da4fc2..f581cf9fd7 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -57,6 +57,8 @@ void bdrv_dirty_iter_free(BdrvDirtyBitmapIter *iter);
uint64_t bdrv_dirty_bitmap_serialization_size(const BdrvDirtyBitmap *bitmap,
uint64_t offset, uint64_t bytes);
uint64_t bdrv_dirty_bitmap_serialization_align(const BdrvDirtyBitmap *bitmap);
+uint64_t bdrv_dirty_bitmap_serialization_coverage(int serialized_chunk_size,
+ const BdrvDirtyBitmap *bitmap);
void bdrv_dirty_bitmap_serialize_part(const BdrvDirtyBitmap *bitmap,
uint8_t *buf, uint64_t offset,
uint64_t bytes);
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 9b9cd71238..a0eaa28785 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -726,6 +726,19 @@ uint64_t bdrv_dirty_bitmap_serialization_align(const BdrvDirtyBitmap *bitmap)
return hbitmap_serialization_align(bitmap->bitmap);
}
+/* Return the disk size covered by a chunk of serialized bitmap data. */
+uint64_t bdrv_dirty_bitmap_serialization_coverage(int serialized_chunk_size,
+ const BdrvDirtyBitmap *bitmap)
+{
+ uint64_t granularity = bdrv_dirty_bitmap_granularity(bitmap);
+ uint64_t limit = granularity * (serialized_chunk_size << 3);
+
+ assert(QEMU_IS_ALIGNED(limit,
+ bdrv_dirty_bitmap_serialization_align(bitmap)));
+ return limit;
+}
+
+
void bdrv_dirty_bitmap_serialize_part(const BdrvDirtyBitmap *bitmap,
uint8_t *buf, uint64_t offset,
uint64_t bytes)
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index 5eef82fa55..42d81c44cd 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -278,18 +278,6 @@ static int free_bitmap_clusters(BlockDriverState *bs, Qcow2BitmapTable *tb)
return 0;
}
-/* Return the disk size covered by a single qcow2 cluster of bitmap data. */
-static uint64_t bytes_covered_by_bitmap_cluster(const BDRVQcow2State *s,
- const BdrvDirtyBitmap *bitmap)
-{
- uint64_t granularity = bdrv_dirty_bitmap_granularity(bitmap);
- uint64_t limit = granularity * (s->cluster_size << 3);
-
- assert(QEMU_IS_ALIGNED(limit,
- bdrv_dirty_bitmap_serialization_align(bitmap)));
- return limit;
-}
-
/* load_bitmap_data
* @bitmap_table entries must satisfy specification constraints.
* @bitmap must be cleared */
@@ -312,7 +300,7 @@ static int load_bitmap_data(BlockDriverState *bs,
}
buf = g_malloc(s->cluster_size);
- limit = bytes_covered_by_bitmap_cluster(s, bitmap);
+ limit = bdrv_dirty_bitmap_serialization_coverage(s->cluster_size, bitmap);
for (i = 0, offset = 0; i < tab_size; ++i, offset += limit) {
uint64_t count = MIN(bm_size - offset, limit);
uint64_t entry = bitmap_table[i];
@@ -1303,7 +1291,7 @@ static uint64_t *store_bitmap_data(BlockDriverState *bs,
}
buf = g_malloc(s->cluster_size);
- limit = bytes_covered_by_bitmap_cluster(s, bitmap);
+ limit = bdrv_dirty_bitmap_serialization_coverage(s->cluster_size, bitmap);
assert(DIV_ROUND_UP(bm_size, limit) == tb_size);
offset = 0;
--
2.29.2
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PULL 25/31] parallels.txt: fix bitmap L1 table description
2021-03-05 16:54 [PULL 00/31] Block layer patches Kevin Wolf
` (23 preceding siblings ...)
2021-03-05 16:54 ` [PULL 24/31] qcow2-bitmap: make bytes_covered_by_bitmap_cluster() public Kevin Wolf
@ 2021-03-05 16:54 ` Kevin Wolf
2021-03-05 16:54 ` [PULL 26/31] block/parallels: BDRVParallelsState: add cluster_size field Kevin Wolf
` (6 subsequent siblings)
31 siblings, 0 replies; 35+ messages in thread
From: Kevin Wolf @ 2021-03-05 16:54 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Actually L1 table entry offset is in 512 bytes sectors. Fix the spec.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210224104707.88430-3-vsementsov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
docs/interop/parallels.txt | 28 ++++++++++++++++------------
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/docs/interop/parallels.txt b/docs/interop/parallels.txt
index f15bf35bd1..bb3fadf369 100644
--- a/docs/interop/parallels.txt
+++ b/docs/interop/parallels.txt
@@ -208,21 +208,25 @@ of its data area are:
28 - 31: l1_size
The number of entries in the L1 table of the bitmap.
- variable: l1_table (8 * l1_size bytes)
- L1 offset table (in bytes)
+ variable: L1 offset table (l1_table), size: 8 * l1_size bytes
-A dirty bitmap is stored using a one-level structure for the mapping to host
-clusters - an L1 table.
+The dirty bitmap described by this feature extension is stored in a set of
+clusters inside the Parallels image file. The offsets of these clusters are
+saved in the L1 offset table specified by the feature extension. Each L1 table
+entry is a 64 bit integer as described below:
-Given an offset in bytes into the bitmap data, the offset in bytes into the
-image file can be obtained as follows:
+Given an offset in bytes into the bitmap data, corresponding L1 entry is
- offset = l1_table[offset / cluster_size] + (offset % cluster_size)
+ l1_table[offset / cluster_size]
-If an L1 table entry is 0, the corresponding cluster of the bitmap is assumed
-to be zero.
+If an L1 table entry is 0, all bits in the corresponding cluster of the bitmap
+are assumed to be 0.
-If an L1 table entry is 1, the corresponding cluster of the bitmap is assumed
-to have all bits set.
+If an L1 table entry is 1, all bits in the corresponding cluster of the bitmap
+are assumed to be 1.
-If an L1 table entry is not 0 or 1, it allocates a cluster from the data area.
+If an L1 table entry is not 0 or 1, it contains the corresponding cluster
+offset (in 512b sectors). Given an offset in bytes into the bitmap data the
+offset in bytes into the image file can be obtained as follows:
+
+ offset = l1_table[offset / cluster_size] * 512 + (offset % cluster_size)
--
2.29.2
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PULL 26/31] block/parallels: BDRVParallelsState: add cluster_size field
2021-03-05 16:54 [PULL 00/31] Block layer patches Kevin Wolf
` (24 preceding siblings ...)
2021-03-05 16:54 ` [PULL 25/31] parallels.txt: fix bitmap L1 table description Kevin Wolf
@ 2021-03-05 16:54 ` Kevin Wolf
2021-03-05 16:54 ` [PULL 27/31] parallels: support bitmap extension for read-only mode Kevin Wolf
` (5 subsequent siblings)
31 siblings, 0 replies; 35+ messages in thread
From: Kevin Wolf @ 2021-03-05 16:54 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
We are going to use it in more places, calculating
"s->tracks << BDRV_SECTOR_BITS" doesn't look good.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210224104707.88430-4-vsementsov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/parallels.h | 1 +
block/parallels.c | 8 ++++----
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/block/parallels.h b/block/parallels.h
index 5aa101cfc8..9a9209e320 100644
--- a/block/parallels.h
+++ b/block/parallels.h
@@ -79,6 +79,7 @@ typedef struct BDRVParallelsState {
ParallelsPreallocMode prealloc_mode;
unsigned int tracks;
+ unsigned int cluster_size;
unsigned int off_multiplier;
Error *migration_blocker;
diff --git a/block/parallels.c b/block/parallels.c
index 3c22dfdc9d..9594d84978 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -421,7 +421,6 @@ static int coroutine_fn parallels_co_check(BlockDriverState *bs,
int ret;
uint32_t i;
bool flush_bat = false;
- int cluster_size = s->tracks << BDRV_SECTOR_BITS;
size = bdrv_getlength(bs->file->bs);
if (size < 0) {
@@ -472,7 +471,7 @@ static int coroutine_fn parallels_co_check(BlockDriverState *bs,
high_off = off;
}
- if (prev_off != 0 && (prev_off + cluster_size) != off) {
+ if (prev_off != 0 && (prev_off + s->cluster_size) != off) {
res->bfi.fragmented_clusters++;
}
prev_off = off;
@@ -487,10 +486,10 @@ static int coroutine_fn parallels_co_check(BlockDriverState *bs,
}
}
- res->image_end_offset = high_off + cluster_size;
+ res->image_end_offset = high_off + s->cluster_size;
if (size > res->image_end_offset) {
int64_t count;
- count = DIV_ROUND_UP(size - res->image_end_offset, cluster_size);
+ count = DIV_ROUND_UP(size - res->image_end_offset, s->cluster_size);
fprintf(stderr, "%s space leaked at the end of the image %" PRId64 "\n",
fix & BDRV_FIX_LEAKS ? "Repairing" : "ERROR",
size - res->image_end_offset);
@@ -771,6 +770,7 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
ret = -EFBIG;
goto fail;
}
+ s->cluster_size = s->tracks << BDRV_SECTOR_BITS;
s->bat_size = le32_to_cpu(ph.bat_entries);
if (s->bat_size > INT_MAX / sizeof(uint32_t)) {
--
2.29.2
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PULL 27/31] parallels: support bitmap extension for read-only mode
2021-03-05 16:54 [PULL 00/31] Block layer patches Kevin Wolf
` (25 preceding siblings ...)
2021-03-05 16:54 ` [PULL 26/31] block/parallels: BDRVParallelsState: add cluster_size field Kevin Wolf
@ 2021-03-05 16:54 ` Kevin Wolf
2021-03-05 16:54 ` [PULL 28/31] iotests.py: add unarchive_sample_image() helper Kevin Wolf
` (4 subsequent siblings)
31 siblings, 0 replies; 35+ messages in thread
From: Kevin Wolf @ 2021-03-05 16:54 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210224104707.88430-5-vsementsov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/parallels.h | 6 +-
block/parallels-ext.c | 300 ++++++++++++++++++++++++++++++++++++++++++
block/parallels.c | 18 +++
block/meson.build | 3 +-
4 files changed, 325 insertions(+), 2 deletions(-)
create mode 100644 block/parallels-ext.c
diff --git a/block/parallels.h b/block/parallels.h
index 9a9209e320..f22f43f988 100644
--- a/block/parallels.h
+++ b/block/parallels.h
@@ -48,7 +48,8 @@ typedef struct ParallelsHeader {
uint64_t nb_sectors;
uint32_t inuse;
uint32_t data_off;
- char padding[12];
+ uint32_t flags;
+ uint64_t ext_off;
} QEMU_PACKED ParallelsHeader;
typedef enum ParallelsPreallocMode {
@@ -85,4 +86,7 @@ typedef struct BDRVParallelsState {
Error *migration_blocker;
} BDRVParallelsState;
+int parallels_read_format_extension(BlockDriverState *bs,
+ int64_t ext_off, Error **errp);
+
#endif
diff --git a/block/parallels-ext.c b/block/parallels-ext.c
new file mode 100644
index 0000000000..e0dd0975c6
--- /dev/null
+++ b/block/parallels-ext.c
@@ -0,0 +1,300 @@
+/*
+ * Support of Parallels Format Extension. It's a part of Parallels format
+ * driver.
+ *
+ * Copyright (c) 2021 Virtuozzo International GmbH
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "block/block_int.h"
+#include "parallels.h"
+#include "crypto/hash.h"
+#include "qemu/uuid.h"
+
+#define PARALLELS_FORMAT_EXTENSION_MAGIC 0xAB234CEF23DCEA87ULL
+
+#define PARALLELS_END_OF_FEATURES_MAGIC 0x0ULL
+#define PARALLELS_DIRTY_BITMAP_FEATURE_MAGIC 0x20385FAE252CB34AULL
+
+typedef struct ParallelsFormatExtensionHeader {
+ uint64_t magic; /* PARALLELS_FORMAT_EXTENSION_MAGIC */
+ uint8_t check_sum[16];
+} QEMU_PACKED ParallelsFormatExtensionHeader;
+
+typedef struct ParallelsFeatureHeader {
+ uint64_t magic;
+ uint64_t flags;
+ uint32_t data_size;
+ uint32_t _unused;
+} QEMU_PACKED ParallelsFeatureHeader;
+
+typedef struct ParallelsDirtyBitmapFeature {
+ uint64_t size;
+ uint8_t id[16];
+ uint32_t granularity;
+ uint32_t l1_size;
+ /* L1 table follows */
+} QEMU_PACKED ParallelsDirtyBitmapFeature;
+
+/* Given L1 table read bitmap data from the image and populate @bitmap */
+static int parallels_load_bitmap_data(BlockDriverState *bs,
+ const uint64_t *l1_table,
+ uint32_t l1_size,
+ BdrvDirtyBitmap *bitmap,
+ Error **errp)
+{
+ BDRVParallelsState *s = bs->opaque;
+ int ret = 0;
+ uint64_t offset, limit;
+ uint64_t bm_size = bdrv_dirty_bitmap_size(bitmap);
+ uint8_t *buf = NULL;
+ uint64_t i, tab_size =
+ DIV_ROUND_UP(bdrv_dirty_bitmap_serialization_size(bitmap, 0, bm_size),
+ s->cluster_size);
+
+ if (tab_size != l1_size) {
+ error_setg(errp, "Bitmap table size %" PRIu32 " does not correspond "
+ "to bitmap size and cluster size. Expected %" PRIu64,
+ l1_size, tab_size);
+ return -EINVAL;
+ }
+
+ buf = qemu_blockalign(bs, s->cluster_size);
+ limit = bdrv_dirty_bitmap_serialization_coverage(s->cluster_size, bitmap);
+ for (i = 0, offset = 0; i < tab_size; ++i, offset += limit) {
+ uint64_t count = MIN(bm_size - offset, limit);
+ uint64_t entry = l1_table[i];
+
+ if (entry == 0) {
+ /* No need to deserialize zeros because @bitmap is cleared. */
+ continue;
+ }
+
+ if (entry == 1) {
+ bdrv_dirty_bitmap_deserialize_ones(bitmap, offset, count, false);
+ } else {
+ ret = bdrv_pread(bs->file, entry << BDRV_SECTOR_BITS, buf,
+ s->cluster_size);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret,
+ "Failed to read bitmap data cluster");
+ goto finish;
+ }
+ bdrv_dirty_bitmap_deserialize_part(bitmap, buf, offset, count,
+ false);
+ }
+ }
+ ret = 0;
+
+ bdrv_dirty_bitmap_deserialize_finish(bitmap);
+
+finish:
+ qemu_vfree(buf);
+
+ return ret;
+}
+
+/*
+ * @data buffer (of @data_size size) is the Dirty bitmaps feature which
+ * consists of ParallelsDirtyBitmapFeature followed by L1 table.
+ */
+static BdrvDirtyBitmap *parallels_load_bitmap(BlockDriverState *bs,
+ uint8_t *data,
+ size_t data_size,
+ Error **errp)
+{
+ int ret;
+ ParallelsDirtyBitmapFeature bf;
+ g_autofree uint64_t *l1_table = NULL;
+ BdrvDirtyBitmap *bitmap;
+ QemuUUID uuid;
+ char uuidstr[UUID_FMT_LEN + 1];
+ int i;
+
+ if (data_size < sizeof(bf)) {
+ error_setg(errp, "Too small Bitmap Feature area in Parallels Format "
+ "Extension: %zu bytes, expected at least %zu bytes",
+ data_size, sizeof(bf));
+ return NULL;
+ }
+ memcpy(&bf, data, sizeof(bf));
+ bf.size = le64_to_cpu(bf.size);
+ bf.granularity = le32_to_cpu(bf.granularity) << BDRV_SECTOR_BITS;
+ bf.l1_size = le32_to_cpu(bf.l1_size);
+ data += sizeof(bf);
+ data_size -= sizeof(bf);
+
+ if (bf.size != bs->total_sectors) {
+ error_setg(errp, "Bitmap size (in sectors) %" PRId64 " differs from "
+ "disk size in sectors %" PRId64, bf.size, bs->total_sectors);
+ return NULL;
+ }
+
+ if (bf.l1_size * sizeof(uint64_t) > data_size) {
+ error_setg(errp, "Bitmaps feature corrupted: l1 table exceeds "
+ "extension data_size");
+ return NULL;
+ }
+
+ memcpy(&uuid, bf.id, sizeof(uuid));
+ qemu_uuid_unparse(&uuid, uuidstr);
+ bitmap = bdrv_create_dirty_bitmap(bs, bf.granularity, uuidstr, errp);
+ if (!bitmap) {
+ return NULL;
+ }
+
+ l1_table = g_new(uint64_t, bf.l1_size);
+ for (i = 0; i < bf.l1_size; i++, data += sizeof(uint64_t)) {
+ l1_table[i] = ldq_le_p(data);
+ }
+
+ ret = parallels_load_bitmap_data(bs, l1_table, bf.l1_size, bitmap, errp);
+ if (ret < 0) {
+ bdrv_release_dirty_bitmap(bitmap);
+ return NULL;
+ }
+
+ /* We support format extension only for RO parallels images. */
+ assert(!(bs->open_flags & BDRV_O_RDWR));
+ bdrv_dirty_bitmap_set_readonly(bitmap, true);
+
+ return bitmap;
+}
+
+static int parallels_parse_format_extension(BlockDriverState *bs,
+ uint8_t *ext_cluster, Error **errp)
+{
+ BDRVParallelsState *s = bs->opaque;
+ int ret;
+ int remaining = s->cluster_size;
+ uint8_t *pos = ext_cluster;
+ ParallelsFormatExtensionHeader eh;
+ g_autofree uint8_t *hash = NULL;
+ size_t hash_len = 0;
+ GSList *bitmaps = NULL, *el;
+
+ memcpy(&eh, pos, sizeof(eh));
+ eh.magic = le64_to_cpu(eh.magic);
+ pos += sizeof(eh);
+ remaining -= sizeof(eh);
+
+ if (eh.magic != PARALLELS_FORMAT_EXTENSION_MAGIC) {
+ error_setg(errp, "Wrong parallels Format Extension magic: 0x%" PRIx64
+ ", expected: 0x%llx", eh.magic,
+ PARALLELS_FORMAT_EXTENSION_MAGIC);
+ goto fail;
+ }
+
+ ret = qcrypto_hash_bytes(QCRYPTO_HASH_ALG_MD5, (char *)pos, remaining,
+ &hash, &hash_len, errp);
+ if (ret < 0) {
+ goto fail;
+ }
+
+ if (hash_len != sizeof(eh.check_sum) ||
+ memcmp(hash, eh.check_sum, sizeof(eh.check_sum)) != 0) {
+ error_setg(errp, "Wrong checksum in Format Extension header. Format "
+ "extension is corrupted.");
+ goto fail;
+ }
+
+ while (true) {
+ ParallelsFeatureHeader fh;
+ BdrvDirtyBitmap *bitmap;
+
+ if (remaining < sizeof(fh)) {
+ error_setg(errp, "Can not read feature header, as remaining bytes "
+ "(%d) in Format Extension is less than Feature header "
+ "size (%zu)", remaining, sizeof(fh));
+ goto fail;
+ }
+
+ memcpy(&fh, pos, sizeof(fh));
+ pos += sizeof(fh);
+ remaining -= sizeof(fh);
+
+ fh.magic = le64_to_cpu(fh.magic);
+ fh.flags = le64_to_cpu(fh.flags);
+ fh.data_size = le32_to_cpu(fh.data_size);
+
+ if (fh.flags) {
+ error_setg(errp, "Flags for extension feature are unsupported");
+ goto fail;
+ }
+
+ if (fh.data_size > remaining) {
+ error_setg(errp, "Feature data_size exceedes Format Extension "
+ "cluster");
+ goto fail;
+ }
+
+ switch (fh.magic) {
+ case PARALLELS_END_OF_FEATURES_MAGIC:
+ return 0;
+
+ case PARALLELS_DIRTY_BITMAP_FEATURE_MAGIC:
+ bitmap = parallels_load_bitmap(bs, pos, fh.data_size, errp);
+ if (!bitmap) {
+ goto fail;
+ }
+ bitmaps = g_slist_append(bitmaps, bitmap);
+ break;
+
+ default:
+ error_setg(errp, "Unknown feature: 0x%" PRIu64, fh.magic);
+ goto fail;
+ }
+
+ pos = ext_cluster + QEMU_ALIGN_UP(pos + fh.data_size - ext_cluster, 8);
+ }
+
+fail:
+ for (el = bitmaps; el; el = el->next) {
+ bdrv_release_dirty_bitmap(el->data);
+ }
+ g_slist_free(bitmaps);
+
+ return -EINVAL;
+}
+
+int parallels_read_format_extension(BlockDriverState *bs,
+ int64_t ext_off, Error **errp)
+{
+ BDRVParallelsState *s = bs->opaque;
+ int ret;
+ uint8_t *ext_cluster = qemu_blockalign(bs, s->cluster_size);
+
+ assert(ext_off > 0);
+
+ ret = bdrv_pread(bs->file, ext_off, ext_cluster, s->cluster_size);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "Failed to read Format Extension cluster");
+ goto out;
+ }
+
+ ret = parallels_parse_format_extension(bs, ext_cluster, errp);
+
+out:
+ qemu_vfree(ext_cluster);
+
+ return ret;
+}
diff --git a/block/parallels.c b/block/parallels.c
index 9594d84978..6ebad2a2bb 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -29,6 +29,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/error-report.h"
#include "qapi/error.h"
#include "block/block_int.h"
#include "block/qdict.h"
@@ -843,6 +844,23 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
goto fail_options;
}
+ if (ph.ext_off) {
+ if (flags & BDRV_O_RDWR) {
+ /*
+ * It's unsafe to open image RW if there is an extension (as we
+ * don't support it). But parallels driver in QEMU historically
+ * ignores the extension, so print warning and don't care.
+ */
+ warn_report("Format Extension ignored in RW mode");
+ } else {
+ ret = parallels_read_format_extension(
+ bs, le64_to_cpu(ph.ext_off) << BDRV_SECTOR_BITS, errp);
+ if (ret < 0) {
+ goto fail;
+ }
+ }
+ }
+
if ((flags & BDRV_O_RDWR) && !(flags & BDRV_O_INACTIVE)) {
s->header->inuse = cpu_to_le32(HEADER_INUSE_MAGIC);
ret = parallels_update_header(bs);
diff --git a/block/meson.build b/block/meson.build
index eeaefe5809..d21990ec95 100644
--- a/block/meson.build
+++ b/block/meson.build
@@ -57,7 +57,8 @@ block_ss.add(when: 'CONFIG_QED', if_true: files(
'qed-table.c',
'qed.c',
))
-block_ss.add(when: [libxml2, 'CONFIG_PARALLELS'], if_true: files('parallels.c'))
+block_ss.add(when: [libxml2, 'CONFIG_PARALLELS'],
+ if_true: files('parallels.c', 'parallels-ext.c'))
block_ss.add(when: 'CONFIG_WIN32', if_true: files('file-win32.c', 'win32-aio.c'))
block_ss.add(when: 'CONFIG_POSIX', if_true: [files('file-posix.c'), coref, iokit])
block_ss.add(when: libiscsi, if_true: files('iscsi-opts.c'))
--
2.29.2
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PULL 28/31] iotests.py: add unarchive_sample_image() helper
2021-03-05 16:54 [PULL 00/31] Block layer patches Kevin Wolf
` (26 preceding siblings ...)
2021-03-05 16:54 ` [PULL 27/31] parallels: support bitmap extension for read-only mode Kevin Wolf
@ 2021-03-05 16:54 ` Kevin Wolf
2021-03-05 16:54 ` [PULL 29/31] iotests: add parallels-read-bitmap test Kevin Wolf
` (3 subsequent siblings)
31 siblings, 0 replies; 35+ messages in thread
From: Kevin Wolf @ 2021-03-05 16:54 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210224104707.88430-6-vsementsov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Kevin Wolf <kwolf@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 4e758308f2..90d0b62523 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -17,6 +17,7 @@
#
import atexit
+import bz2
from collections import OrderedDict
import faulthandler
import io
@@ -24,6 +25,7 @@
import logging
import os
import re
+import shutil
import signal
import struct
import subprocess
@@ -96,6 +98,14 @@
os.environ.get('IMGKEYSECRET', '')
luks_default_key_secret_opt = 'key-secret=keysec0'
+sample_img_dir = os.environ['SAMPLE_IMG_DIR']
+
+
+def unarchive_sample_image(sample, fname):
+ sample_fname = os.path.join(sample_img_dir, sample + '.bz2')
+ with bz2.open(sample_fname) as f_in, open(fname, 'wb') as f_out:
+ shutil.copyfileobj(f_in, f_out)
+
def qemu_tool_pipe_and_status(tool: str, args: Sequence[str],
connect_stderr: bool = True) -> Tuple[str, int]:
--
2.29.2
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PULL 29/31] iotests: add parallels-read-bitmap test
2021-03-05 16:54 [PULL 00/31] Block layer patches Kevin Wolf
` (27 preceding siblings ...)
2021-03-05 16:54 ` [PULL 28/31] iotests.py: add unarchive_sample_image() helper Kevin Wolf
@ 2021-03-05 16:54 ` Kevin Wolf
2021-03-05 16:54 ` [PULL 30/31] MAINTAINERS: update parallels block driver Kevin Wolf
` (2 subsequent siblings)
31 siblings, 0 replies; 35+ messages in thread
From: Kevin Wolf @ 2021-03-05 16:54 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Test support for reading bitmap from parallels image format.
parallels-with-bitmap.bz2 is generated on Virtuozzo by
parallels-with-bitmap.sh
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210224104707.88430-7-vsementsov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
.../sample_images/parallels-with-bitmap.bz2 | Bin 0 -> 203 bytes
.../sample_images/parallels-with-bitmap.sh | 51 ++++++++++++++++
.../qemu-iotests/tests/parallels-read-bitmap | 55 ++++++++++++++++++
.../tests/parallels-read-bitmap.out | 6 ++
4 files changed, 112 insertions(+)
create mode 100644 tests/qemu-iotests/sample_images/parallels-with-bitmap.bz2
create mode 100755 tests/qemu-iotests/sample_images/parallels-with-bitmap.sh
create mode 100755 tests/qemu-iotests/tests/parallels-read-bitmap
create mode 100644 tests/qemu-iotests/tests/parallels-read-bitmap.out
diff --git a/tests/qemu-iotests/sample_images/parallels-with-bitmap.bz2 b/tests/qemu-iotests/sample_images/parallels-with-bitmap.bz2
new file mode 100644
index 0000000000000000000000000000000000000000..54892fd4d01bf743d395bd4f3d896494146ab5a9
GIT binary patch
literal 203
zcmV;+05tzXT4*^jL0KkKS@=;0bpT+Hf7|^?Km<xfFyKQJ7=Y^F-%vt;00~Ysa6|-=
zk&7Szk`SoS002EkfMftPG<ipnsiCK}K_sNmm}me3FiZr%Oaf_u5F8kD;mB_~cxD-r
z5P$(X{&Tq5C`<xK02D?NNdN+t$~z$m00O|zFh^ynq*yaCtkn+NZzWom<#OEoF`?zb
zv(i3x^K~wt!aLPcRBP+PckUsIh6*LgjYSh0`}#7hMC9NR5D)+W0d&8Mxgwk>NPH-R
Fx`3oHQ9u9y
literal 0
HcmV?d00001
diff --git a/tests/qemu-iotests/sample_images/parallels-with-bitmap.sh b/tests/qemu-iotests/sample_images/parallels-with-bitmap.sh
new file mode 100755
index 0000000000..30615aa6bd
--- /dev/null
+++ b/tests/qemu-iotests/sample_images/parallels-with-bitmap.sh
@@ -0,0 +1,51 @@
+#!/bin/bash
+#
+# Test parallels load bitmap
+#
+# Copyright (c) 2021 Virtuozzo International GmbH.
+#
+# 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/>.
+#
+
+CT=parallels-with-bitmap-ct
+DIR=$PWD/parallels-with-bitmap-dir
+IMG=$DIR/root.hds
+XML=$DIR/DiskDescriptor.xml
+TARGET=parallels-with-bitmap.bz2
+
+rm -rf $DIR
+
+prlctl create $CT --vmtype ct
+prlctl set $CT --device-add hdd --image $DIR --recreate --size 2G
+
+# cleanup the image
+qemu-img create -f parallels $IMG 64G
+
+# create bitmap
+prlctl backup $CT
+
+prlctl set $CT --device-del hdd1
+prlctl destroy $CT
+
+dev=$(ploop mount $XML | sed -n 's/^Adding delta dev=\(\/dev\/ploop[0-9]\+\).*/\1/p')
+dd if=/dev/zero of=$dev bs=64K seek=5 count=2 oflag=direct
+dd if=/dev/zero of=$dev bs=64K seek=30 count=1 oflag=direct
+dd if=/dev/zero of=$dev bs=64K seek=10 count=3 oflag=direct
+ploop umount $XML # bitmap name will be in the output
+
+bzip2 -z $IMG
+
+mv $IMG.bz2 $TARGET
+
+rm -rf $DIR
diff --git a/tests/qemu-iotests/tests/parallels-read-bitmap b/tests/qemu-iotests/tests/parallels-read-bitmap
new file mode 100755
index 0000000000..af6b9c5db3
--- /dev/null
+++ b/tests/qemu-iotests/tests/parallels-read-bitmap
@@ -0,0 +1,55 @@
+#!/usr/bin/env python3
+#
+# Test parallels load bitmap
+#
+# Copyright (c) 2021 Virtuozzo International GmbH.
+#
+# 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/>.
+#
+
+import json
+import iotests
+from iotests import qemu_nbd_popen, qemu_img_pipe, log, file_path
+
+iotests.script_initialize(supported_fmts=['parallels'])
+
+nbd_sock = file_path('nbd-sock', base_dir=iotests.sock_dir)
+disk = iotests.file_path('disk')
+bitmap = 'e4f2eed0-37fe-4539-b50b-85d2e7fd235f'
+nbd_opts = f'driver=nbd,server.type=unix,server.path={nbd_sock}' \
+ f',x-dirty-bitmap=qemu:dirty-bitmap:{bitmap}'
+
+
+iotests.unarchive_sample_image('parallels-with-bitmap', disk)
+
+
+with qemu_nbd_popen('--read-only', f'--socket={nbd_sock}',
+ f'--bitmap={bitmap}', '-f', iotests.imgfmt, disk):
+ out = qemu_img_pipe('map', '--output=json', '--image-opts', nbd_opts)
+ chunks = json.loads(out)
+ cluster = 64 * 1024
+
+ log('dirty clusters (cluster size is 64K):')
+ for c in chunks:
+ assert c['start'] % cluster == 0
+ assert c['length'] % cluster == 0
+ if c['data']:
+ continue
+
+ a = c['start'] // cluster
+ b = (c['start'] + c['length']) // cluster
+ if b - a > 1:
+ log(f'{a}-{b-1}')
+ else:
+ log(a)
diff --git a/tests/qemu-iotests/tests/parallels-read-bitmap.out b/tests/qemu-iotests/tests/parallels-read-bitmap.out
new file mode 100644
index 0000000000..e8f6bc9e96
--- /dev/null
+++ b/tests/qemu-iotests/tests/parallels-read-bitmap.out
@@ -0,0 +1,6 @@
+Start NBD server
+dirty clusters (cluster size is 64K):
+5-6
+10-12
+30
+Kill NBD server
--
2.29.2
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PULL 30/31] MAINTAINERS: update parallels block driver
2021-03-05 16:54 [PULL 00/31] Block layer patches Kevin Wolf
` (28 preceding siblings ...)
2021-03-05 16:54 ` [PULL 29/31] iotests: add parallels-read-bitmap test Kevin Wolf
@ 2021-03-05 16:54 ` Kevin Wolf
2021-03-05 16:54 ` [PULL 31/31] docs: qsd: Explain --export nbd,name=... default Kevin Wolf
2021-03-06 11:22 ` [PULL 00/31] Block layer patches Peter Maydell
31 siblings, 0 replies; 35+ messages in thread
From: Kevin Wolf @ 2021-03-05 16:54 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Add new parallels-ext.c and myself as co-maintainer.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210304095151.19358-1-vsementsov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
MAINTAINERS | 3 +++
1 file changed, 3 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 4249acc62e..81e7c3b5e5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3126,10 +3126,13 @@ F: block/dmg.c
parallels
M: Stefan Hajnoczi <stefanha@redhat.com>
M: Denis V. Lunev <den@openvz.org>
+M: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
L: qemu-block@nongnu.org
S: Supported
F: block/parallels.c
+F: block/parallels-ext.c
F: docs/interop/parallels.txt
+T: git https://src.openvz.org/scm/~vsementsov/qemu.git parallels
qed
M: Stefan Hajnoczi <stefanha@redhat.com>
--
2.29.2
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PULL 31/31] docs: qsd: Explain --export nbd,name=... default
2021-03-05 16:54 [PULL 00/31] Block layer patches Kevin Wolf
` (29 preceding siblings ...)
2021-03-05 16:54 ` [PULL 30/31] MAINTAINERS: update parallels block driver Kevin Wolf
@ 2021-03-05 16:54 ` Kevin Wolf
2021-03-06 11:22 ` [PULL 00/31] Block layer patches Peter Maydell
31 siblings, 0 replies; 35+ messages in thread
From: Kevin Wolf @ 2021-03-05 16:54 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
The 'name' option for NBD exports is optional. Add a note that the
default for the option is the node name (people could otherwise expect
that it's the empty string like for qemu-nbd).
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20210305094856.18964-1-kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
docs/tools/qemu-storage-daemon.rst | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/docs/tools/qemu-storage-daemon.rst b/docs/tools/qemu-storage-daemon.rst
index fe3042d609..086493ebb3 100644
--- a/docs/tools/qemu-storage-daemon.rst
+++ b/docs/tools/qemu-storage-daemon.rst
@@ -80,8 +80,9 @@ Standard options:
requests for modifying data (the default is off).
The ``nbd`` export type requires ``--nbd-server`` (see below). ``name`` is
- the NBD export name. ``bitmap`` is the name of a dirty bitmap reachable from
- the block node, so the NBD client can use NBD_OPT_SET_META_CONTEXT with the
+ the NBD export name (if not specified, it defaults to the given
+ ``node-name``). ``bitmap`` is the name of a dirty bitmap reachable from the
+ block node, so the NBD client can use NBD_OPT_SET_META_CONTEXT with the
metadata context name "qemu:dirty-bitmap:BITMAP" to inspect the bitmap.
The ``vhost-user-blk`` export type takes a vhost-user socket address on which
--
2.29.2
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PULL 00/31] Block layer patches
2021-03-05 16:54 [PULL 00/31] Block layer patches Kevin Wolf
` (30 preceding siblings ...)
2021-03-05 16:54 ` [PULL 31/31] docs: qsd: Explain --export nbd,name=... default Kevin Wolf
@ 2021-03-06 11:22 ` Peter Maydell
2021-03-08 11:08 ` Kevin Wolf
31 siblings, 1 reply; 35+ messages in thread
From: Peter Maydell @ 2021-03-06 11:22 UTC (permalink / raw)
To: Kevin Wolf; +Cc: QEMU Developers, Qemu-block
On Fri, 5 Mar 2021 at 16:55, Kevin Wolf <kwolf@redhat.com> wrote:
>
> The following changes since commit 9a7beaad3dbba982f7a461d676b55a5c3851d312:
>
> Merge remote-tracking branch 'remotes/alistair/tags/pull-riscv-to-apply-20210304' into staging (2021-03-05 10:47:46 +0000)
>
> are available in the Git repository at:
>
> git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to 67bedc3aed5c455b629c2cb5f523b536c46adff9:
>
> docs: qsd: Explain --export nbd,name=... default (2021-03-05 17:09:46 +0100)
>
> ----------------------------------------------------------------
> Block layer patches:
>
> - qemu-storage-daemon: add --pidfile option
> - qemu-storage-daemon: CLI error messages include the option name now
> - vhost-user-blk export: Misc fixes, added test cases
> - docs: Improvements for qemu-storage-daemon documentation
> - parallels: load bitmap extension
> - backup-top: Don't crash on post-finalize accesses
> - iotests improvements
This failed some of the gitlab CI jobs, like this:
https://gitlab.com/qemu-project/qemu/-/jobs/1077335781
Running test qtest-x86_64/test-hmp
Running test qtest-x86_64/qos-test
qemu-storage-daemon: vu_panic: Not implemented: memfd support is missing
qemu-storage-daemon: vu_panic: Failed to alloc vhost inflight area
qemu-system-x86_64: Failed to write msg. Wrote -1 instead of 20.
qemu-system-x86_64: vhost_set_features failed: Invalid argument (22)
qemu-system-x86_64: Error starting vhost: 22
qemu-system-x86_64: vhost-user-blk: vhost start failed: Invalid argument
**
ERROR:../tests/qtest/libqos/virtio.c:228:qvirtio_wait_used_elem:
assertion failed: (g_get_monotonic_time() - start_time <= timeout_us)
ERROR qtest-x86_64/qos-test - Bail out!
ERROR:../tests/qtest/libqos/virtio.c:228:qvirtio_wait_used_elem:
assertion failed: (g_get_monotonic_time() - start_time <= timeout_us)
make: *** [run-test-159] Error 1
I guess some test or other is assuming the presence of
a host feature that isn't guaranteed to be there ?
thanks
-- PMM
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PULL 00/31] Block layer patches
2021-03-06 11:22 ` [PULL 00/31] Block layer patches Peter Maydell
@ 2021-03-08 11:08 ` Kevin Wolf
2021-03-08 17:10 ` Stefan Hajnoczi
0 siblings, 1 reply; 35+ messages in thread
From: Kevin Wolf @ 2021-03-08 11:08 UTC (permalink / raw)
To: Peter Maydell; +Cc: stefanha, QEMU Developers, Qemu-block
Am 06.03.2021 um 12:22 hat Peter Maydell geschrieben:
> On Fri, 5 Mar 2021 at 16:55, Kevin Wolf <kwolf@redhat.com> wrote:
> >
> > The following changes since commit 9a7beaad3dbba982f7a461d676b55a5c3851d312:
> >
> > Merge remote-tracking branch 'remotes/alistair/tags/pull-riscv-to-apply-20210304' into staging (2021-03-05 10:47:46 +0000)
> >
> > are available in the Git repository at:
> >
> > git://repo.or.cz/qemu/kevin.git tags/for-upstream
> >
> > for you to fetch changes up to 67bedc3aed5c455b629c2cb5f523b536c46adff9:
> >
> > docs: qsd: Explain --export nbd,name=... default (2021-03-05 17:09:46 +0100)
> >
> > ----------------------------------------------------------------
> > Block layer patches:
> >
> > - qemu-storage-daemon: add --pidfile option
> > - qemu-storage-daemon: CLI error messages include the option name now
> > - vhost-user-blk export: Misc fixes, added test cases
> > - docs: Improvements for qemu-storage-daemon documentation
> > - parallels: load bitmap extension
> > - backup-top: Don't crash on post-finalize accesses
> > - iotests improvements
>
> This failed some of the gitlab CI jobs, like this:
>
> https://gitlab.com/qemu-project/qemu/-/jobs/1077335781
>
> Running test qtest-x86_64/test-hmp
> Running test qtest-x86_64/qos-test
> qemu-storage-daemon: vu_panic: Not implemented: memfd support is missing
> qemu-storage-daemon: vu_panic: Failed to alloc vhost inflight area
> qemu-system-x86_64: Failed to write msg. Wrote -1 instead of 20.
> qemu-system-x86_64: vhost_set_features failed: Invalid argument (22)
> qemu-system-x86_64: Error starting vhost: 22
> qemu-system-x86_64: vhost-user-blk: vhost start failed: Invalid argument
> **
> ERROR:../tests/qtest/libqos/virtio.c:228:qvirtio_wait_used_elem:
> assertion failed: (g_get_monotonic_time() - start_time <= timeout_us)
> ERROR qtest-x86_64/qos-test - Bail out!
> ERROR:../tests/qtest/libqos/virtio.c:228:qvirtio_wait_used_elem:
> assertion failed: (g_get_monotonic_time() - start_time <= timeout_us)
> make: *** [run-test-159] Error 1
>
> I guess some test or other is assuming the presence of
> a host feature that isn't guaranteed to be there ?
Stefan, can you have a look? This is from the new vhost-user-blk test
cases from your series.
If the fix isn't trivial, I'll resubmit v2 today with just the test case
dropped and then we can add it later.
Kevin
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PULL 00/31] Block layer patches
2021-03-08 11:08 ` Kevin Wolf
@ 2021-03-08 17:10 ` Stefan Hajnoczi
0 siblings, 0 replies; 35+ messages in thread
From: Stefan Hajnoczi @ 2021-03-08 17:10 UTC (permalink / raw)
To: Kevin Wolf; +Cc: Peter Maydell, QEMU Developers, Qemu-block
[-- Attachment #1: Type: text/plain, Size: 2722 bytes --]
On Mon, Mar 08, 2021 at 12:08:29PM +0100, Kevin Wolf wrote:
> Am 06.03.2021 um 12:22 hat Peter Maydell geschrieben:
> > On Fri, 5 Mar 2021 at 16:55, Kevin Wolf <kwolf@redhat.com> wrote:
> > >
> > > The following changes since commit 9a7beaad3dbba982f7a461d676b55a5c3851d312:
> > >
> > > Merge remote-tracking branch 'remotes/alistair/tags/pull-riscv-to-apply-20210304' into staging (2021-03-05 10:47:46 +0000)
> > >
> > > are available in the Git repository at:
> > >
> > > git://repo.or.cz/qemu/kevin.git tags/for-upstream
> > >
> > > for you to fetch changes up to 67bedc3aed5c455b629c2cb5f523b536c46adff9:
> > >
> > > docs: qsd: Explain --export nbd,name=... default (2021-03-05 17:09:46 +0100)
> > >
> > > ----------------------------------------------------------------
> > > Block layer patches:
> > >
> > > - qemu-storage-daemon: add --pidfile option
> > > - qemu-storage-daemon: CLI error messages include the option name now
> > > - vhost-user-blk export: Misc fixes, added test cases
> > > - docs: Improvements for qemu-storage-daemon documentation
> > > - parallels: load bitmap extension
> > > - backup-top: Don't crash on post-finalize accesses
> > > - iotests improvements
> >
> > This failed some of the gitlab CI jobs, like this:
> >
> > https://gitlab.com/qemu-project/qemu/-/jobs/1077335781
> >
> > Running test qtest-x86_64/test-hmp
> > Running test qtest-x86_64/qos-test
> > qemu-storage-daemon: vu_panic: Not implemented: memfd support is missing
> > qemu-storage-daemon: vu_panic: Failed to alloc vhost inflight area
> > qemu-system-x86_64: Failed to write msg. Wrote -1 instead of 20.
> > qemu-system-x86_64: vhost_set_features failed: Invalid argument (22)
> > qemu-system-x86_64: Error starting vhost: 22
> > qemu-system-x86_64: vhost-user-blk: vhost start failed: Invalid argument
> > **
> > ERROR:../tests/qtest/libqos/virtio.c:228:qvirtio_wait_used_elem:
> > assertion failed: (g_get_monotonic_time() - start_time <= timeout_us)
> > ERROR qtest-x86_64/qos-test - Bail out!
> > ERROR:../tests/qtest/libqos/virtio.c:228:qvirtio_wait_used_elem:
> > assertion failed: (g_get_monotonic_time() - start_time <= timeout_us)
> > make: *** [run-test-159] Error 1
> >
> > I guess some test or other is assuming the presence of
> > a host feature that isn't guaranteed to be there ?
>
> Stefan, can you have a look? This is from the new vhost-user-blk test
> cases from your series.
>
> If the fix isn't trivial, I'll resubmit v2 today with just the test case
> dropped and then we can add it later.
I'm testing the following commit:
https://gitlab.com/stefanha/qemu/-/pipelines/267172954
I'll look into it more tomorrow.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 35+ messages in thread