search for: block_status

This is an API/ABI break, but we have not yet declared stable API. In order to use optional arguments in languages that support that, any optional arguments must be last. Let's be consistent in this by rearranging the arguments to block_status so that flags is always last. --- generator/generator | 10 ++++++---- interop/dirty-bitmap.c | 6 +++--- lib/rw.c | 12 +++++------- python/t/460-block-status.py | 8 ++++++-- tests/meta-base-allocation.c | 10 ++++------ 5 files changed, 24 insertions(+), 22...

We decided to not request the "base:allocation" context by default (if a client wants to use block_status on a different context, then they'd have to get any default request out of the way); however, block status is useless without at least one meta context. This adds a convenience knob for requesting that, and has the nice benefit of working with the --connect command line option (previously, if...

When the server sets NBD_REPLY_FLAG_DONE on a data or block status chunk, we can use that fact to pass (VALID|FREE) and avoid a separate callback later just for FREE. --- As I've been promising in other threads... generator/states-reply-structured.c | 33 +++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/generator/states-reply-structured.c

...ithout some is_zero infrastructure for QEMU, > it’s impossible to implement this flag in qemu’s NBD server. You're right that we may need some more infrastructure before being able to decide when to report this bit in all cases. But for raw files, that infrastructure already exists: does block_status at offset 0 and the entire image as length return status that the entire file is a hole. And for qcow2 files, it would not be that hard to teach a similar block_status request to report the entire image as a hole based on my proposed qcow2 autoclear bit tracking that the image still reads as ze...

Patches 1 & 2 are fine, ACK. I have a separate comment about patch 4 coming up. In libguestfs we handle optional args by putting them in a separate list (so instead of { args; ret } we have we have { args; optargs; ret }). This gets translated into a language like Python in the natural way. However for C it's translated into a varargs list with a rather complex system of flags, see:

...ents. for ((i=$4;i<$4+$3;i++)); do echo $i 1 $((i&1)) done ;; *) exit 2 ;; esac EOF $ ~/libnbd/run sh/nbdsh ... nbd> h.connect_tcp("localhost","10809") nbd> def f(data,metacontext,offset,e): ... print ("entries:%d" % len(e)) ... nbd> h.block_status(9*1024*1024,0,0,f) Traceback (most recent call last): File "/usr/lib64/python3.7/code.py", line 90, in runcode exec(code, self.locals) File "<console>", line 1, in <module> File "/home/eblake/libnbd/python/nbd.py", line 577, in block_status re...

...+994,73 @@ C<LIBNBD_CONTEXT_> for some well-known contexts, but you are free to pass in other contexts. Other metadata contexts are server-specific, but include -C<\"qemu:dirty-bitmap:...\"> for qemu-nbd -(see qemu-nbd I<-B> option)."; - see_also = [Link "block_status"]; +C<\"qemu:dirty-bitmap:...\"> and C<\"qemu:allocation-depth\"> for +qemu-nbd (see qemu-nbd I<-B> and I<-A> options)."; + see_also = [Link "block_status"; Link "can_meta_context"; + Link "get_nr_meta_...

Rather than moving the connection to DEAD, we can just ignore further contexts to the existing command handle, and fail the overall command with the same errno as the failed callback. Eric Blake (2): states: Track cmd->error as errno, not wire value api: Recover from block status callback failure generator/generator | 5 ++- generator/states-reply-simple.c | 2 +-

...Test (unittest.TestCase): self.connect ({"size": 512, "can_cache": "native"}) assert self.h.can_cache() - # Not yet implemented: can_extents. + # In theory we could use a test like this, but nbdkit can + # always synthesize base:allocation block_status responses + # even if the plugin doesn't support them. + # + #def test_can_extents_true (self): + # self.h.add_meta_context ("base:allocation") + # self.connect ({"size": 512, "can_extents": True}) + # assert self.h.can_meta_context (&quo...

...gt; self.connect ({"size": 512, "can_cache": "native"}) > assert self.h.can_cache() > > - # Not yet implemented: can_extents. > + # In theory we could use a test like this, but nbdkit can > + # always synthesize base:allocation block_status responses > + # even if the plugin doesn't support them. > + # > + #def test_can_extents_true (self): > + # self.h.add_meta_context ("base:allocation") > + # self.connect ({"size": 512, "can_extents": True}) > + # assert...

...gher granularity than the underlying plugin actually supports, and the client is trying to collect block status for the entire disk, a mid-block transition in the plugin can result in the filter rounding a request so small that it no longer makes progress, causing the client to see: nbd.Error: nbd_block_status: block-status: command failed: Invalid argument (EINVAL) Better is to round mid-block transitions up to the block size; even though the client can make read/write requests smaller than the block alignment, they should not see transitions in status at that granularity. Fixes: 2515532316 Signed-off...

...abyssmal lseek() >>> performance on tmpfs. But until qemu learns useful caching, you're >>> right that most existing NBD clients that request block status do so one >>> extent at a time (because I don't know of any other existing NBD clients >>> that use BLOCK_STATUS yet). >> >> Is it ever possible to cache block status results? What happens in >> the (admittedly unusual) case where two writers are hitting the same >> NBD server? For example if the server is implementing a cluster >> filesystem. > >For a read-only client: c...

...d_flags = { flag_prefix = "CMD_FLAG"; flags = [ @@ -1969,7 +1969,8 @@ are free to pass in other contexts." ^ non_blocking_test_call_description; see_also = [SectionLink "Flag calls"; Link "add_meta_context"; - Link "block_status"; Link "aio_block_status"]; + Link "block_status"; Link "aio_block_status"; + Link "get_init_flags"]; }; "get_protocol", { @@ -1990,6 +1991,30 @@ Most modern NBD servers use C<\"newstyle-fixed\"...

...aggravated by the kernel's abyssmal lseek() > performance on tmpfs. But until qemu learns useful caching, you're > right that most existing NBD clients that request block status do so one > extent at a time (because I don't know of any other existing NBD clients > that use BLOCK_STATUS yet). Is it ever possible to cache block status results? What happens in the (admittedly unusual) case where two writers are hitting the same NBD server? For example if the server is implementing a cluster filesystem. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat....

...once extended headers are negotiated, all transmission traffic (both from client to server and server to client) uses just one header size - add support for the client to pass a payload on commands to the server, and demonstrate it by further implementing a way to pass a flag with NBD_CMD_BLOCK_STATUS that says the client is passing a payload to request status of just a subset of the negotiated contexts, rather than all possible contexts that were earlier negotiated during NBD_OPT_SET_META_CONTEXT - tweaks to the header layouts: tweak block status to provide 64-bit flags values (althoug...

...e == NULL) Semantic change. In generator, you used CALLBACK_IS_NULL() for both Closure and OClosure. For OClosure, the new semantics are still correct. But for Closure, we now no longer return EFAULT when the callback itself is missing but a .free was provided. This changes pread_structured and block_status to accept NULL for the callback; which is probably not a good idea. In short, there are some points in the code that care only whether .callback is NULL, and others that care whether both pointers are NULL. > #define CALLBACK_IS_NOT_NULL(cb) (! CALLBACK_IS_NULL ((cb))) > -#define SET_CALL...

On Wed, Aug 02, 2023 at 08:50:21PM -0500, Eric Blake wrote: > Previously, we had not been doing any validation of server extent > responses, which means a client query at an offset near the end of the > export can result in a buggy server sending a response longer than the > export length and potentially confusing the client. The NBD spec also > says that an extent length should be

...free() rather than mixing malloc() in plugin and free() in nbdkit? Among other things, doing this would let us switch to a different allocation engine (pool-based, maybe?) without a super-close coupling where plugins and nbdkit have to share the same allocator. nbdkit plugin receive client BLOCK_STATUS call call plugin.extents call nbdkit_extents_array(N) allocate an array of N extents populate the array and return process the array The only other thing I can think of is that nbdkit could have a function that the plugin must call one or more times per extent, before...

...a/v2v/nbdkit.ml b/v2v/nbdkit.ml index 89fca337cbaa..77d2a506dc41 100644 --- a/v2v/nbdkit.ml +++ b/v2v/nbdkit.ml @@ -169,6 +169,13 @@ let common_create ?bandwidth plugin_name plugin_args plugin_env = ) else [] in + (* Caching extents speeds up qemu-img, especially its consecutive + block_status requests with req_one=1. + *) + if probe_filter "cacheextents" then ( + add_arg "--filter"; add_arg "cacheextents" + ); + (* Retry filter (if it exists) can be used to get around brief * interruptions in service. It must be closest to the plugin. *) -...

...2, 0) assert buf1 == buf2 diff --git a/python/t/460-block-status.py b/python/t/460-block-status.py index f511091..eeaa7b2 100644 --- a/python/t/460-block-status.py +++ b/python/t/460-block-status.py @@ -34,14 +34,14 @@ def f (data, metacontext, offset, e): return entries = e -h.block_status (65536, 0, 42, f, 0) +h.block_status (65536, 0, 42, f) assert entries == [ 8192, 0, 8192, 1, 16384, 3, 16384, 2, 16384, 0] -h.block_status (1024, 32256, 42, f, 0) +h.block_status (1024, 32256, 42, f) print ("e...