search for: nbd_flag_send_write_zero

...uct connection *conn, uint16_t *flags) +{ + uint16_t eflags = NBD_FLAG_HAS_FLAGS; + int fl; + + fl = plugin_can_write (conn); + if (fl == -1) + return -1; + if (readonly || !fl) { + eflags |= NBD_FLAG_READ_ONLY; + conn->readonly = 1; + } + if (!conn->readonly) { + eflags |= NBD_FLAG_SEND_WRITE_ZEROES; + } + + fl = plugin_can_flush (conn); + if (fl == -1) + return -1; + if (fl) { + eflags |= NBD_FLAG_SEND_FLUSH | NBD_FLAG_SEND_FUA; + conn->can_flush = 1; + } + + fl = plugin_is_rotational (conn); + if (fl == -1) + return -1; + if (fl) { + eflags |= NBD_FLAG_ROTATIONAL;...

...1191,7 @@ nbd_can_trim (void *handle) { struct handle *h = handle; - return h->flags & NBD_FLAG_SEND_TRIM; + return !!(h->flags & NBD_FLAG_SEND_TRIM); } static int @@ -1199,7 +1199,7 @@ nbd_can_zero (void *handle) { struct handle *h = handle; - return h->flags & NBD_FLAG_SEND_WRITE_ZEROES; + return !!(h->flags & NBD_FLAG_SEND_WRITE_ZEROES); } static int @@ -1215,7 +1215,7 @@ nbd_can_multi_conn (void *handle) { struct handle *h = handle; - return h->flags & NBD_FLAG_CAN_MULTI_CONN; + return !!(h->flags & NBD_FLAG_CAN_MULTI_CONN); } static int --...

...D_FLAG_SEND_FAST_ZERO`: allow clients to detect whether > + `NBD_CMD_WRITE_ZEROES` is efficient. The server MUST set this > + transmission flag to 1 if the `NBD_CMD_WRITE_ZEROES` request > + supports the `NBD_CMD_FLAG_FAST_ZERO` flag, and MUST set this > + transmission flag to 0 if `NBD_FLAG_SEND_WRITE_ZEROES` is not > + set. Servers SHOULD NOT set this transmission flag if there is no > + quick way to determine whether a particular write zeroes request > + will be efficient, but the lack of an efficient write zero > I think we should use "fast" instead of "efficient&quo...

...SEND_FAST_ZERO`: allow clients to detect whether + `NBD_CMD_WRITE_ZEROES` is faster than a corresponding write. The + server MUST set this transmission flag to 1 if the + `NBD_CMD_WRITE_ZEROES` request supports the `NBD_CMD_FLAG_FAST_ZERO` + flag, and MUST set this transmission flag to 0 if + `NBD_FLAG_SEND_WRITE_ZEROES` is not set. Servers MAY set this this + transmission flag even if it will always use `NBD_ENOTSUP` failures for + requests with `NBD_CMD_FLAG_FAST_ZERO` set (such as if the server + cannot quickly determine whether a particular write zeroes request + will be faster than a regular write). Cli...

...from no-op to .pread or from .pread to > no-op. Be thinking about that as you review the rest of the series. I'm worried that we end up in the same situation that we do with .zero: because we emulate it, we end up with a slow zero support, whereas clients [well, Nir S. mainly] actually want NBD_FLAG_SEND_WRITE_ZEROES == *fast* zeroing is available. In particular in the NBD_CMD_CACHE case: for some plugins emulating this behind the scenes with pread makes some sense. eg. It works for nbdkit-file-plugin because it will prefetch the data into Linux's page cache. However if a plugin makes a network access,...

...diff --git a/lib/flags.c b/lib/flags.c index 2bcacb8..d55d10a 100644 --- a/lib/flags.c +++ b/lib/flags.c @@ -47,6 +47,12 @@ nbd_internal_set_size_and_flags (struct nbd_handle *h, eflags &= ~NBD_FLAG_SEND_DF; } + if (eflags & NBD_FLAG_SEND_FAST_ZERO && + !(eflags & NBD_FLAG_SEND_WRITE_ZEROES)) { + debug (h, "server lacks write zeroes, ignoring claim of fast zero"); + eflags &= ~NBD_FLAG_SEND_FAST_ZERO; + } + h->exportsize = exportsize; h->eflags = eflags; return 0; @@ -100,6 +106,12 @@ nbd_unlocked_can_zero (struct nbd_handle *h) return get_flag...

...le *h = handle; > > - return h->flags & NBD_FLAG_SEND_TRIM; > + return !!(h->flags & NBD_FLAG_SEND_TRIM); > } > > static int > @@ -1199,7 +1199,7 @@ nbd_can_zero (void *handle) > { > struct handle *h = handle; > > - return h->flags & NBD_FLAG_SEND_WRITE_ZEROES; > + return !!(h->flags & NBD_FLAG_SEND_WRITE_ZEROES); > } > > static int > @@ -1215,7 +1215,7 @@ nbd_can_multi_conn (void *handle) > { > struct handle *h = handle; > > - return h->flags & NBD_FLAG_CAN_MULTI_CONN; > + return !!(h->flags &a...

...D_CMD_WRITE_ZEROES. SHOULD be set to 1 if the client wants to ensure that the server does not create a hole. The client MAY send NBD_CMD_FLAG_NO_HOLE even if NBD_FLAG_SEND_TRIM was not set in the transmission flags field. The server MUST support the use of this flag if it advertises NBD_FLAG_SEND_WRITE_ZEROES. * qemu-img convert uses NBD_CMD_WRITE_ZEROES and does NOT set this flag (hence in the plugin we see may_trim=1), and I believe that qemu-img is correct because it doesn't want to force preallocation. Rich. * https://github.com/NetworkBlockDevice/nbd/blob/master/doc/proto.md -- Richard J...

The upstream protocol recently promoted NBD_CMD_WRITE_ZEROES from experimental to a documented extension. Exposing support for this allows plugin writers to create sparse files when driven by a client that knows how to use the extension; meanwhile, even if a plugin does not support this extension, the server benefits from less network traffic from the client. Eric Blake (5): protocol: Support

...pact. +- bit 11, `NBD_FLAG_SEND_FAST_ZERO`: allow clients to detect whether + `NBD_CMD_WRITE_ZEROES` is efficient. The server MUST set this + transmission flag to 1 if the `NBD_CMD_WRITE_ZEROES` request + supports the `NBD_CMD_FLAG_FAST_ZERO` flag, and MUST set this + transmission flag to 0 if `NBD_FLAG_SEND_WRITE_ZEROES` is not + set. Servers SHOULD NOT set this transmission flag if there is no + quick way to determine whether a particular write zeroes request + will be efficient, but the lack of an efficient write zero + implementation SHOULD NOT prevent a server from setting this + flag. Clients MUST NOT...

....h b/common/protocol/protocol.h index a7de2f0..4334be4 100644 --- a/common/protocol/protocol.h +++ b/common/protocol/protocol.h @@ -94,6 +94,7 @@ extern const char *name_of_nbd_flag (int); #define NBD_FLAG_ROTATIONAL (1 << 4) #define NBD_FLAG_SEND_TRIM (1 << 5) #define NBD_FLAG_SEND_WRITE_ZEROES (1 << 6) +#define NBD_FLAG_SEND_DF (1 << 7) #define NBD_FLAG_CAN_MULTI_CONN (1 << 8) /* NBD options (new style handshake only). */ @@ -217,6 +218,7 @@ extern const char *name_of_nbd_cmd (int); extern const char *name_of_nbd_cmd_flag (int); #define NBD_CMD_FLAG_...

...nnection { int can_flush; int is_rotational; int can_trim; + int can_zero; + int can_fua; int using_tls; int sockin, sockout; @@ -426,7 +428,13 @@ compute_eflags (struct connection *conn, uint16_t *flags) conn->readonly = 1; } if (!conn->readonly) { - eflags |= NBD_FLAG_SEND_WRITE_ZEROES; + fl = backend->can_zero (backend, conn); + if (fl == -1) + return -1; + if (fl) { + eflags |= NBD_FLAG_SEND_WRITE_ZEROES; + conn->can_zero = 1; + } fl = backend->can_trim (backend, conn); if (fl == -1) @@ -435,13 +443,21 @@ compute_eflags (struct c...

...changed, 8 insertions(+), 8 deletions(-) diff --git a/src/connections.c b/src/connections.c index 8e110a5..75c2c2d 100644 --- a/src/connections.c +++ b/src/connections.c @@ -427,6 +427,14 @@ compute_eflags (struct connection *conn, uint16_t *flags) } if (!conn->readonly) { eflags |= NBD_FLAG_SEND_WRITE_ZEROES; + + fl = backend->can_trim (backend, conn); + if (fl == -1) + return -1; + if (fl) { + eflags |= NBD_FLAG_SEND_TRIM; + conn->can_trim = 1; + } } fl = backend->can_flush (backend, conn); @@ -445,14 +453,6 @@ compute_eflags (struct connection *conn, uint1...

...SHOULD be set to 1 if the client wants to > ensure that the server does not create a hole. The client MAY send > NBD_CMD_FLAG_NO_HOLE even if NBD_FLAG_SEND_TRIM was not set in the > transmission flags field. The server MUST support the use of this > flag if it advertises NBD_FLAG_SEND_WRITE_ZEROES. * > > qemu-img convert uses NBD_CMD_WRITE_ZEROES and does NOT set this flag > (hence in the plugin we see may_trim=1), and I believe that qemu-img > is correct because it doesn't want to force preallocation. Yes, the flag usage is correct, and you are also correct that the '...

...lugins/nbd/nbd.c index dd84b04..51de178 100644 --- a/plugins/nbd/nbd.c +++ b/plugins/nbd/nbd.c @@ -618,6 +618,14 @@ nbd_can_trim (void *handle) return h->flags & NBD_FLAG_SEND_TRIM; } +static int +nbd_can_zero (void *handle) +{ + struct handle *h = handle; + + return h->flags & NBD_FLAG_SEND_WRITE_ZEROES; +} + static int nbd_can_fua (void *handle) { @@ -662,11 +670,7 @@ nbd_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags) int f = 0; assert (!(flags & ~(NBDKIT_FLAG_FUA | NBDKIT_FLAG_MAY_TRIM))); - if (!(h->flags & NBD_FLAG_SEND_WRITE_ZEROES)) { - /* Tri...

...replies) { + debug (h, "server lacks structured replies, ignoring claim of df"); + eflags &= ~NBD_FLAG_SEND_DF; + } + h->exportsize = exportsize; h->eflags = eflags; return 0; @@ -95,6 +100,12 @@ nbd_unlocked_can_zero (struct nbd_handle *h) return get_flag (h, NBD_FLAG_SEND_WRITE_ZEROES); } +int +nbd_unlocked_can_df (struct nbd_handle *h) +{ + return get_flag (h, NBD_FLAG_SEND_DF); +} + int nbd_unlocked_can_multi_conn (struct nbd_handle *h) { diff --git a/lib/rw.c b/lib/rw.c index feaf468..343c340 100644 --- a/lib/rw.c +++ b/lib/rw.c @@ -234,11 +234,17 @@ int64_t nbd_unlo...

...SHOULD be set to 1 if the client wants to > ensure that the server does not create a hole. The client MAY send > NBD_CMD_FLAG_NO_HOLE even if NBD_FLAG_SEND_TRIM was not set in the > transmission flags field. The server MUST support the use of this > flag if it advertises NBD_FLAG_SEND_WRITE_ZEROES. * > > qemu-img convert uses NBD_CMD_WRITE_ZEROES and does NOT set this flag > (hence in the plugin we see may_trim=1), and I believe that qemu-img > is correct because it doesn't want to force preallocation. > So once oVirt will support efficient zeroing, this flag may be tra...

I still need to add testsuite coverage. Perhaps it might be easier if I create a new '--filter=log logfile=foo' filter that produces a log of which commands a client sent, then compare the log using a known client that uses write_zeroes (qemu-io works well) both with and without --filter=nozero to prove that the change in advertisement changes the commands sent over the wire (that would

This is a cover letter to a series of patches being proposed in tandem to four different projects: - nbd: Document a new NBD_CMD_FLAG_FAST_ZERO command flag - qemu: Implement the flag for both clients and server - libnbd: Implement the flag for clients - nbdkit: Implement the flag for servers, including the nbd passthrough client If you want to test the patches together, I've pushed a

...NO_ZEROES 2 /* Per-export flags. */ +extern const char *name_of_nbd_flag (int); #define NBD_FLAG_HAS_FLAGS (1 << 0) #define NBD_FLAG_READ_ONLY (1 << 1) #define NBD_FLAG_SEND_FLUSH (1 << 2) @@ -94,6 +96,7 @@ struct fixed_new_option_reply { #define NBD_FLAG_SEND_WRITE_ZEROES (1 << 6) /* NBD options (new style handshake only). */ +extern const char *name_of_nbd_opt (int); #define NBD_OPT_EXPORT_NAME 1 #define NBD_OPT_ABORT 2 #define NBD_OPT_LIST 3 @@ -101,6 +104,7 @@ struct fixed_new_option_reply { #define NBD_OPT_INFO 6 #define N...