The new NBD_OPT_EXTENDED_HEADERS feature is worth using by default,
but there may be cases where the user explicitly wants to stick with
the older 32-bit headers. nbd_set_request_extended_headers() will let
the client override the default, nbd_get_request_extended_headers()
determines the current
Overcome the inherent 32-bit limitation of our existing
nbd_block_status command by adding a 64-bit variant. The command sent
to the server does not change, but the user's callback is now handed
64-bit information regardless of whether the server replies with 32-
or 64-bit extents.
Unit tests
This is the culmination of the previous patches preparation work for
using extended headers when possible. The new states in the state
machine are copied extensively from our handling of
OPT_STRUCTURED_REPLY.
At the same time I posted this patch, I had patches for qemu-nbd to
support extended
Add the magic numbers and new structs necessary to implement the NBD
protocol extension of extended headers providing 64-bit lengths.
---
lib/nbd-protocol.h | 61 ++
1 file changed, 51 insertions(+), 10 deletions(-)
diff --git a/lib/nbd-protocol.h
Prove that we can round-trip a block status request larger than 4G
through a new-enough qemu-nbd. Also serves as a unit test of our shim
for converting internal 64-bit representation back to the older 32-bit
nbd_block_status callback interface.
---
interop/Makefile.am | 6 ++
Support receiving headers for 64-bit replies if extended headers were
negotiated. We already insist that the server not send us too much
payload in one reply, so we can exploit that and merge the 64-bit
length back into a normalized 32-bit field for the rest of the payload
length calculations.
Support a server giving us a 64-bit extent. Note that the protocol
says a server should not give a 64-bit answer when extended headers
are not negotiated, but since the client's size is merely a hint, it
is possible for a server to have a 64-bit answer even when the
original query was 32 bits.
The existing nbd_block_status() callback is permanently stuck with an
array of uint32_t pairs (len/2 extents), and exposing 64-bit extents
requires a new API. Before we get there, we first need a way to
express a struct containing uint64_t length and uint32_t flags across
the various language
Support sending 64-bit requests if extended headers were negotiated.
At this point, h->extended_headers is permanently false (we can't
enable it until all other aspects of the protocol have likewise been
converted).
---
lib/internal.h | 12 ---
For 32-bit block status, we were able to cheat and use an array with
an odd number of elements, with array[0] holding the context id, and
passing [1] to the user's callback. But once we have 64-bit
extents, we can no longer abuse array element 0 like that. Split out
a new state to receive the
Because we use NBD_CMD_FLAG_REQ_ONE with NBD_CMD_BLOCK_STATUS, a
client in narrow mode should not be able to provoke a server into
sending a block status result larger than the client's 32-bit request.
But in extended mode, a 64-bit status request must be able to handle a
64-bit status result,
All the pieces are in place for a client to finally request extended
headers. Note that we must not request extended headers when qemu-nbd
is used to connect to the kernel module (as nbd.ko does not expect
them), but there is no harm in all other clients requesting them.
Extended headers do not
In the block status callback glue code, we need to copy a C uint32_t[]
into a golang []uint32. The copy is necessary since the lifetime of
the C array is not guaranteed to outlive whatever the Go callback may
have done with what it was handed; copying ensures that the user's Go
code doesn't have
When extended headers are in use, the server can send us 64-bit
extents, even for a 32-bit query (if the server knows the entire image
is data, for example). For maximum flexibility, we are thus better
off storing 64-bit lengths internally, even if we have to convert it
back to 32-bit lengths
Even though we don't allow the user to request NBD_CMD_READ with more
than 64M (and even if we did, our API signature caps us at SIZE_MAX,
which is 32 bits on a 32-bit machine), the NBD extension to allow
64-bit requests implies that for symmetry we have to be able to
support 64-bit holes over the
Available here: https://repo.or.cz/libnbd/ericb.git/shortlog/refs/tags/exthdr-v1
I also want to do followup patches to teach 'nbdinfo --map' and
'nbdcopy' to utilize 64-bit extents.
Eric Blake (13):
golang: Simplify nbd_block_status callback array copy
block_status: Refactor array storage
Since we cap NBD_CMD_READ requests to 32M, we never have a reason to
send a 64-bit chunk type for a hole; but it is worth producing these
for interoperability testing of clients that want extended headers.
---
nbd/server.c | 18 ++
1 file changed, 14 insertions(+), 4 deletions(-)
Update the client code to be able to send an extended request, and
parse an extended header from the server. Note that since we reject
any structured reply with a too-large payload, we can always normalize
a valid header back into the compact form, so that the caller need not
deal with two
Although our read requests are sized such that servers need not send
an extended hole chunk, we still have to be prepared for it to be
fully compliant if we request extended headers. We can also tolerate
a non-compliant server sending the new chunk even when it should not.
Signed-off-by: Eric
The previous patch handled extended headers by truncating large block
status requests from the client back to 32 bits. But this is not
ideal; for cases where we can truly determine the status of the entire
image quickly (for example, when reporting the entire image as
non-sparse because we lack
An upcoming NBD extension wants to add the ability to do 64-bit
requests. As part of that extension, the size of the reply headers
will change in order to permit a 64-bit length in the reply for
symmetry [*]. Additionally, where the reply header is currently 16
bytes for simple reply, and 20
Add the constants and structs necessary for later patches to start
implementing the NBD_OPT_EXTENDED_HEADERS extension in both the client
and server. This patch does not change any existing behavior, but
merely sets the stage.
This patch does not change the status quo that neither the client nor
We have no reason to send NBD_REPLY_TYPE_OFFSET_HOLE_EXT since we
already cap NBD_CMD_READ to 32M. For NBD_CMD_WRITE_ZEROES and
NBD_CMD_TRIM, the block layer already supports 64-bit operations
without any effort on our part. For NBD_CMD_BLOCK_STATUS, the
client's length is a hint; the easiest
Widen the length field of NBDRequest to 64-bits, although we can
assert that all current uses are still under 32 bits. Move the
request magic number to nbd.h, to live alongside the reply magic
number. Add a bool that will eventually track whether the client
successfully negotiated extended
Our existing use of structured replies either reads into a qiov capped
at 32M (NBD_CMD_READ) or caps allocation to 1000 bytes (see
NBD_MAX_MALLOC_PAYLOAD in block/nbd.c). But the existing length
checks are rather late; if we encounter a buggy (or malicious) server
that sends a super-large payload
Spelling fixes, grammar improvements and consistent spacing, noticed
while preparing other patches in this file.
Signed-off-by: Eric Blake
---
nbd/server.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index
Available at https://repo.or.cz/qemu/ericb.git/shortlog/refs/tags/exthdr-v1
Patch 14 is optional; I'm including it now because I tested with it,
but I'm also okay with dropping it based on RFC discussion.
Eric Blake (14):
nbd/server: Minor cleanups
qemu-io: Utilize 64-bit status during map
When writing zeroes can fall back to a slow write, permitting an
overly large request can become an amplification denial of service
attack in triggering a large amount of work from a small request. But
the whole point of the no fallback flag is to quickly determine if
writing an entire device to
The block layer has supported 64-bit block status from drivers since
commit 86a3d5c688 ("block: Add .bdrv_co_block_status() callback",
v2.12) and friends, with individual driver callbacks responsible for
capping things where necessary. Artificially capping things below 2G
in the qemu-io 'map'
Add a new negotiation feature where the client and server agree to use
larger packet headers on every packet sent during transmission phase.
This has two purposes: first, it makes it possible to perform
operations like trim, write zeroes, and block status on more than 2^32
bytes in a single
In response to this mail, I will be cross-posting a series of patches
to multiple projects as a proof-of-concept implementation and request
for comments on a new NBD protocol extension, called
NBD_OPT_EXTENDED_HEADERS. With this in place, it will be possible for
clients to request 64-bit zero,
03.12.2021 14:17, Peter Lieven wrote:
Am 19.05.21 um 18:48 schrieb Kevin Wolf:
Am 19.05.2021 um 15:24 hat Peter Lieven geschrieben:
Am 20.04.21 um 18:52 schrieb Vladimir Sementsov-Ogievskiy:
20.04.2021 18:04, Kevin Wolf wrote:
Am 20.04.2021 um 16:31 hat Vladimir Sementsov-Ogievskiy
Make the informal rules formal. In further commit we'll add
corresponding assertions.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
include/block/block.h | 42 ++
1 file changed, 42 insertions(+)
diff --git a/include/block/block.h
Now the indirection is not actually used, we can safely reduce it to
simple pointer.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
block/snapshot.c | 39 +--
1 file changed, 17 insertions(+), 22 deletions(-)
diff --git a/block/snapshot.c b/block/snapshot.c
Actually what we chose is a primary child. Let's stress it in the code.
We are going to drop indirect pointer logic here in future. Actually
this commit simplifies the future work: we drop use of indirection in
the assertion now.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
block/snapshot.c
bdrv_pass_through is used as filter, even all node variables has
corresponding names. We want to append it, so it should be
backing-child-based filter like mirror_top.
So, in test_update_perm_tree, first child should be DATA, as we don't
want filters with two filtered children.
That's a preparation to previously reverted
"block: Let replace_child_noperm free children". Drop it too, we don't
need it for a new approach.
This reverts commit be64bbb0149748f3999c49b13976aafb8330ea86.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
block.c | 23 +++
1
test_parallel_perm_update() does two things that we are going to
restrict in the near future:
1. It updates bs->file field by hand. bs->file will be managed
automatically by generic code (together with bs->children list).
Let's better refactor our "tricky" bds to have own state where one
We don't need to remove bs->file, generic layer takes care of it. No
other driver cares to remove bs->file on failure by hand.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
block/blklogwrites.c | 4
1 file changed, 4 deletions(-)
diff --git a/block/blklogwrites.c b/block/blklogwrites.c
Almost all drivers call bdrv_open_child() similarly. Let's create a
helper for this.
The only not updated driver that call bdrv_open_child() to set
bs->file is raw-format, as it sometimes want to have filtered child but
don't set drv->is_filter to true.
Possibly we should implement
bs->file and bs->backing are a kind of duplication of part of
bs->children. But very useful diplication, so let's not drop them at
all:)
We should manage bs->file and bs->backing in same place, where we
manage bs->children, to keep them in sync.
Moreover, generic io paths are unprepared to
We do add COW child to the node. In future we are going to forbid
adding COW child to the node that doesn't support backing. So, fix it
here now.
Don't worry about setting bs->backing itself: it further commit we'll
update the block-layer to automatically set/unset this field in generic
code.
That's a preparation to previously reverted
"block: Let replace_child_noperm free children". Drop it too, we don't
need it for a new approach.
This reverts commit 562bda8bb41879eeda0bd484dd3d55134579b28e.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
block.c | 21 +
1
We are going to reimplement this behavior (clear bs->file / bs->backing
pointers automatically when child->bs is cleared) in a nicer way.
This reverts commit b0a9f6fed3d80de610dcd04a7e66f9f30a04174f.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
block.c | 102
That's a preparation to previously reverted
"block: Let replace_child_noperm free children". Drop it too, we don't
need it for a new approach.
This reverts commit 82b54cf51656bf3cd5ed1ac549e8a1085a0e3290.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
block.c | 83
Hi all!
I started this as a follow-up to
"block: Attempt on fixing 030-reported errors" by Hanna.
In Hanna's series I really like how bs->children handling moved to
.attach/.detach handlers.
.file and .backing are kind of "shortcuts" or "links" to some elementes
of this list, they duplicate the
Unfortunately not all filters use .file child as filtered child. Two
exclusions are mirror_top and commit_top. Happily they both are private
filters. Bad thing is that this inconsistency is observable through qmp
commands query-block / query-named-block-nodes. So, could we just
change mirror_top
We are going to add filtering in _qcow2_dump_header and want all tests
use it.
The patch is generated by commands:
cd tests/qemu-iotests
sed -ie 's/$PYTHON qcow2.py "$TEST_IMG" dump-header\($\|
\)/_qcow2_dump_header\1/' ??? tests/*
(the difficulty is to avoid converting dump-header-exts)
We are going to support IMGOPTS environment variable like in bash
tests. Corresponding global variable in iotests.py should be called
imgopts. So to not interfere with function argument, rename it in
advance.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Max Reitz
---
The test checks different options. It of course fails if set
IMGOPTS='compression_type=zstd'. So, let's be explicit in what
compression type we want and independent of IMGOPTS. Test both existing
compression types.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Hanna Reitz
---
Don't touch other incompatible bits, like compression-type. This makes
the test pass with IMGOPTS='compression_type=zstd'.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Max Reitz
---
tests/qemu-iotests/060 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git
The test-case "Corrupted size field in compressed cluster descriptor"
heavily depends on zlib compression type. So, make it explicit. This
way test passes with IMGOPTS='compression_type=zstd'.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Max Reitz
---
tests/qemu-iotests/214 | 2 +-
_qcow2_dump_header has filter for compression type, so this change
makes test pass with IMGOPTS='compression_type=zstd'.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Max Reitz
---
tests/qemu-iotests/039 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git
We want iotests pass with both the default zlib compression and with
IMGOPTS='compression_type=zstd'.
Actually the only test that is interested in real compression type in
test output is 287 (test for qcow2 compression type), so implement
specific option for it.
Signed-off-by: Vladimir
Move the logic to more generic qemu_img_pipe_and_status(). Also behave
better when we have several -o options. And reuse argument parser of
course.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Hanna Reitz
---
tests/qemu-iotests/iotests.py | 36 +--
1
We support IMGOPTS for python iotests now. Still a lot of tests are
unprepared to common IMGOPTS that are used with bash iotests. So we
should define corresponding unsupported_imgopts.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
tests/qemu-iotests/044 | 3 ++-
If image doesn't have any compressed cluster we can easily switch to
zlib compression, which may allow to downgrade the image.
That's mostly needed to support IMGOPTS='compression_type=zstd' in some
iotests which do qcow2 downgrade.
While being here also fix checkpatch complain against '#' in
We'll use it in tests instead of explicit qcow2.py. Then we are going
to add some filtering in _qcow2_dump_header.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Max Reitz
---
tests/qemu-iotests/common.rc | 10 ++
1 file changed, 10 insertions(+)
diff --git
Instead of qemu_img_log("info", ..) use generic helper img_info_log().
img_info_log() has smarter logic. For example it use filter_img_info()
to filter output, which in turns filter a compression type. So it will
help us in future when we implement a possibility to use zstd
compression by default
The only "feature" of this "Formatting ..." line is that we have to
update it every time we add new option. Let's drop it.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Hanna Reitz
---
tests/qemu-iotests/149.out| 21 -
tests/qemu-iotests/237.out| 3 ---
compression_type can't be used if we want to create image with
compat=0.10. So, skip these tests, not many of them.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Hanna Reitz
---
tests/qemu-iotests/031 | 5 +++--
tests/qemu-iotests/051 | 5 +++--
tests/qemu-iotests/061 | 6 +-
The test prints qcow2 header fields which depends on chosen compression
type. So, let's be explicit in what compression type we want and
independent of IMGOPTS. Test both existing compression types.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Max Reitz
---
tests/qemu-iotests/303
Adding support of IMGOPTS (like in bash tests) allows user to pass a
lot of different options. Still, some may require additional logic.
Now we want compression_type option, so add some smart logic around it:
ignore compression_type=zstd in IMGOPTS, if test want qcow2 in
compatibility mode. As
We want iotests pass with both the default zlib compression and with
IMGOPTS='compression_type=zstd'.
Actually the only test that is interested in real compression type in
test output is 287 (test for qcow2 compression type) and it's in bash.
So for now we can safely filter out compression type
These series makes tests pass with
IMGOPTS='compression_type=zstd'
Also, python iotests start to support IMGOPTS (they didn't before).
v4: 18,19: new: add unsupported_imgopts as suggested by Hanna.
Vladimir Sementsov-Ogievskiy (19):
iotests.py: img_info_log(): rename imgopts argument
We have added support for some addition IMGOPTS in python iotests like
in bash iotests. Similarly to bash iotests, we want a way to skip some
tests which can't work with specific IMGOPTS.
Globally for python iotests we now don't support things like
'data_file=$TEST_IMG.ext_data_file' in IMGOPTS,
qemu_img_verbose() has a drawback of not going through generic
qemu_img_pipe_and_status(). qemu_img_verbose() is not very popular, so
update the only two users to qemu_img_log() and drop qemu_img_verbose()
at all.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Hanna Reitz
---
We are going to use do_run_test() in multiprocessing environment, where
we'll not be able to change original runner object.
Happily, the only thing we change is that last_elapsed and it's simple
to do it in run_tests() instead. All other accesses to self in
do_runt_test() and in run_test() are
Add -j parameter, to run tests in several jobs simultaneously.
For realization - simply utilize multiprocessing.Pool class.
Notes:
1. Of course, tests can't run simultaneously in same TEST_DIR. So,
use subdirectories TEST_DIR/testname/ and SOCK_DIR/testname/
instead of simply TEST_DIR and
We are going to modify these methods and will add more documentation in
further commit. As a preparation add basic documentation.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
tests/qemu-iotests/testrunner.py | 13 +
1 file changed, 13 insertions(+)
diff --git
Hi all!
Finally, I can not stand it any longer. So, I'm happy to present
multiprocessing support for iotests test runner.
testing on tmpfs:
Before:
time check -qcow2
...
real12m28.095s
user9m53.398s
sys 2m55.548s
After:
time check -qcow2 -j 12
...
real2m12.136s
user
Am 19.05.21 um 18:48 schrieb Kevin Wolf:
> Am 19.05.2021 um 15:24 hat Peter Lieven geschrieben:
>> Am 20.04.21 um 18:52 schrieb Vladimir Sementsov-Ogievskiy:
>>> 20.04.2021 18:04, Kevin Wolf wrote:
Am 20.04.2021 um 16:31 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 15.04.2021 18:22,
72 matches
Mail list logo
