Since NBD is a big-endian protocol, the upstream spec was able to repurpose a 32-bit field with flags starting at (1<<16) OR'd into the command into two 16-bit fields (flags first, starting at 1<<0, then the command) for ease of documentation. Matching that split in our code base will also make it easier to implement smarter FUA flag support. This addresses one of the TODO in the nbd plugin. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- plugins/nbd/nbd.c | 39 ++++++++++++++++++++------------------- src/connections.c | 12 ++++++------ src/protocol.h | 10 +++++----- 3 files changed, 31 insertions(+), 30 deletions(-) diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c index 04147f1..c9727f7 100644 --- a/plugins/nbd/nbd.c +++ b/plugins/nbd/nbd.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2017 Red Hat Inc. + * Copyright (C) 2017-2018 Red Hat Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -249,13 +249,14 @@ find_trans_by_cookie (struct handle *h, uint64_t cookie) /* Send a request, return 0 on success or -1 on write failure. */ static int -nbd_request_raw (struct handle *h, uint32_t type, uint64_t offset, - uint32_t count, uint64_t cookie, const void *buf) +nbd_request_raw (struct handle *h, uint16_t flags, uint16_t type, + uint64_t offset, uint32_t count, uint64_t cookie, + const void *buf) { struct request req = { .magic = htobe32 (NBD_REQUEST_MAGIC), - /* TODO nbdkit should have a way to pass flags, separate from cmd type */ - .type = htobe32 (type), + .flags = htobe16 (flags), + .type = htobe16 (type), .handle = cookie, /* Opaque to server, so endianness doesn't matter */ .offset = htobe64 (offset), .count = htobe32 (count), @@ -275,8 +276,9 @@ nbd_request_raw (struct handle *h, uint32_t type, uint64_t offset, /* Perform the request half of a transaction. On success, return the non-negative fd for reading the reply; on error return -1. */ static int -nbd_request_full (struct handle *h, uint32_t type, uint64_t offset, - uint32_t count, const void *req_buf, void *rep_buf) +nbd_request_full (struct handle *h, uint16_t flags, uint16_t type, + uint64_t offset, uint32_t count, const void *req_buf, + void *rep_buf) { int err; struct transaction *trans; @@ -307,7 +309,7 @@ nbd_request_full (struct handle *h, uint32_t type, uint64_t offset, fd = trans->u.fds[0]; cookie = trans->u.cookie; nbd_unlock (h); - if (nbd_request_raw (h, type, offset, count, cookie, req_buf) == 0) + if (nbd_request_raw (h, flags, type, offset, count, cookie, req_buf) == 0) return fd; trans = find_trans_by_cookie (h, cookie); @@ -326,9 +328,10 @@ nbd_request_full (struct handle *h, uint32_t type, uint64_t offset, /* Shorthand for nbd_request_full when no extra buffers are involved. */ static int -nbd_request (struct handle *h, uint32_t type, uint64_t offset, uint32_t count) +nbd_request (struct handle *h, uint16_t flags, uint16_t type, uint64_t offset, + uint32_t count) { - return nbd_request_full (h, type, offset, count, NULL, NULL); + return nbd_request_full (h, flags, type, offset, count, NULL, NULL); } /* Read a reply, and look up the fd corresponding to the transaction. @@ -563,7 +566,7 @@ nbd_close (void *handle) struct handle *h = handle; if (!h->dead) - nbd_request_raw (h, NBD_CMD_DISC, 0, 0, 0, NULL); + nbd_request_raw (h, 0, NBD_CMD_DISC, 0, 0, 0, NULL); close (h->fd); if ((errno = pthread_join (h->reader, NULL))) nbdkit_debug ("failed to join reader thread: %m"); @@ -622,7 +625,7 @@ nbd_pread (void *handle, void *buf, uint32_t count, uint64_t offset) /* TODO Auto-fragment this if the client has a larger max transfer limit than the server */ - c = nbd_request_full (h, NBD_CMD_READ, offset, count, NULL, buf); + c = nbd_request_full (h, 0, NBD_CMD_READ, offset, count, NULL, buf); return c < 0 ? c : nbd_reply (h, c); } @@ -635,7 +638,7 @@ nbd_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset) /* TODO Auto-fragment this if the client has a larger max transfer limit than the server */ - c = nbd_request_full (h, NBD_CMD_WRITE, offset, count, buf, NULL); + c = nbd_request_full (h, 0, NBD_CMD_WRITE, offset, count, buf, NULL); return c < 0 ? c : nbd_reply (h, c); } @@ -644,7 +647,6 @@ static int nbd_zero (void *handle, uint32_t count, uint64_t offset, int may_trim) { struct handle *h = handle; - uint32_t cmd = NBD_CMD_WRITE_ZEROES; int c; if (!(h->flags & NBD_FLAG_SEND_WRITE_ZEROES)) { @@ -653,9 +655,8 @@ nbd_zero (void *handle, uint32_t count, uint64_t offset, int may_trim) return -1; } - if (!may_trim) - cmd |= NBD_CMD_FLAG_NO_HOLE; - c = nbd_request (h, cmd, offset, count); + c = nbd_request (h, may_trim ? 0 : NBD_CMD_FLAG_NO_HOLE, + NBD_CMD_WRITE_ZEROES, offset, count); return c < 0 ? c : nbd_reply (h, c); } @@ -666,7 +667,7 @@ nbd_trim (void *handle, uint32_t count, uint64_t offset) struct handle *h = handle; int c; - c = nbd_request (h, NBD_CMD_TRIM, offset, count); + c = nbd_request (h, 0, NBD_CMD_TRIM, offset, count); return c < 0 ? c : nbd_reply (h, c); } @@ -677,7 +678,7 @@ nbd_flush (void *handle) struct handle *h = handle; int c; - c = nbd_request (h, NBD_CMD_FLUSH, 0, 0); + c = nbd_request (h, 0, NBD_CMD_FLUSH, 0, 0); return c < 0 ? c : nbd_reply (h, c); } diff --git a/src/connections.c b/src/connections.c index a16c118..24df4b6 100644 --- a/src/connections.c +++ b/src/connections.c @@ -760,7 +760,7 @@ valid_range (struct connection *conn, uint64_t offset, uint32_t count) static bool validate_request (struct connection *conn, - uint32_t cmd, uint32_t flags, uint64_t offset, uint32_t count, + uint16_t cmd, uint16_t flags, uint64_t offset, uint32_t count, uint32_t *error) { /* Readonly connection? */ @@ -865,7 +865,7 @@ get_error (struct connection *conn) */ static uint32_t handle_request (struct connection *conn, - uint32_t cmd, uint32_t flags, uint64_t offset, uint32_t count, + uint16_t cmd, uint16_t flags, uint64_t offset, uint32_t count, void *buf) { bool flush_after_command; @@ -979,7 +979,8 @@ recv_request_send_reply (struct connection *conn) int r; struct request request; struct reply reply; - uint32_t magic, cmd, flags, count, error = 0; + uint16_t cmd, flags; + uint32_t magic, count, error = 0; uint64_t offset; CLEANUP_FREE char *buf = NULL; @@ -1005,9 +1006,8 @@ recv_request_send_reply (struct connection *conn) return set_status (conn, -1); } - cmd = be32toh (request.type); - flags = cmd & ~NBD_CMD_MASK_COMMAND; - cmd &= NBD_CMD_MASK_COMMAND; + flags = be16toh (request.flags); + cmd = be16toh (request.type); offset = be64toh (request.offset); count = be32toh (request.count); diff --git a/src/protocol.h b/src/protocol.h index 9d9dad9..aa458a0 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2013 Red Hat Inc. + * Copyright (C) 2013-2018 Red Hat Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -112,7 +112,8 @@ struct new_handshake_finish { /* Request (client -> server). */ struct request { uint32_t magic; /* NBD_REQUEST_MAGIC. */ - uint32_t type; /* Request type. */ + uint16_t flags; /* Request flags. */ + uint16_t type; /* Request type. */ uint64_t handle; /* Opaque handle. */ uint64_t offset; /* Request offset. */ uint32_t count; /* Request length. */ @@ -134,9 +135,8 @@ struct reply { #define NBD_CMD_FLUSH 3 #define NBD_CMD_TRIM 4 #define NBD_CMD_WRITE_ZEROES 6 -#define NBD_CMD_MASK_COMMAND 0xffff -#define NBD_CMD_FLAG_FUA (1<<16) -#define NBD_CMD_FLAG_NO_HOLE (2<<16) +#define NBD_CMD_FLAG_FUA (1<<0) +#define NBD_CMD_FLAG_NO_HOLE (1<<1) /* Error codes (previously errno). * See http://git.qemu.org/?p=qemu.git;a=commitdiff;h=ca4414804114fd0095b317785b... -- 2.14.3

The NBD protocol supports Forced Unit Access (FUA) as a more efficient way to wait for just one write to land in persistent storage, rather than all outstanding writes at the time of a flush; modeled after the kernel's block I/O flag of the same name. While we can emulate the proper semantics with a full-blown flush, there are some plugins that can properly pass the FUA flag on to the end storage and thereby avoid some overhead. This patch introduces new callbacks and documentations for those callbacks, although the actual implementation to take advantage of the new callbacks will be in later patches. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- docs/nbdkit-plugin.pod | 77 +++++++++++++++++++++++++++++++++++++++++++++++-- docs/nbdkit.pod | 9 ++++-- include/nbdkit-plugin.h | 8 ++++- 3 files changed, 88 insertions(+), 6 deletions(-) diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod index 9abf75f..cb936f7 100644 --- a/docs/nbdkit-plugin.pod +++ b/docs/nbdkit-plugin.pod @@ -359,7 +359,7 @@ If there is an error, C<.can_write> should call C<nbdkit_error> with an error message and return C<-1>. This callback is not required. If omitted, then we return true iff a -C<.pwrite> callback has been defined. +C<.pwrite> or C<.pwrite_fua> callback has been defined. =head2 C<.can_flush> @@ -400,7 +400,28 @@ If there is an error, C<.can_trim> should call C<nbdkit_error> with an error message and return C<-1>. This callback is not required. If omitted, then we return true iff a -C<.trim> callback has been defined. +C<.trim> or C<.trim_fua> callback has been defined. + +=head2 C<.can_fua> + + int can_fua (void *handle); + +This is called during the option negotiation phase to find out if the +plugin supports the Forced Unit Access (FUA) flag on write and trim +requests. + +If there is an error, C<.can_fua> should call C<nbdkit_error> with an +error message and return C<-1>. + +This callback is not required. If omitted, then we return true iff +either the C<.pwrite_fua> callback has been defined, or if C<.can_flush> +returns true (in the latter case, FUA semantics are emulated by nbdkit +calling C<.flush> before completing any write or trim operation with +the FUA flag set). + +Note that if this defaults to true and C<.can_trim> also returns true, +the plugin must provide either C<.flush> or C<.trim_fua> for correct +FUA semantics. =head2 C<.pread> @@ -442,6 +463,21 @@ recovered from), C<.pwrite> should call C<nbdkit_error> with an error message, and C<nbdkit_set_error> to record an appropriate error (unless C<errno> is sufficient), then return C<-1>. +If the plugin can provide efficient Forced Unit Access (FUA) semantics, +it should define C<.pwrite_fua> instead. + +=head2 C<.pwrite_fua> + + int pwrite_fua (void *handle, const void *buf, uint32_t count, uint64_t offset, int fua); + +This callback has the same requirements as C<.pwrite>, with the +additional parameter C<fua> set to a non-zero value if the client +wants FUA semantics (where the command must not return until the +actions of the write have landed in persistent storage). If the +plugin cannot provide efficient FUA, but C<.can_flush> returns true +and C<.can_fua> does not return false, then client requests for FUA +semantics are emulated by nbdkit calling C<.flush>. + =head2 C<.flush> int flush (void *handle); @@ -455,6 +491,11 @@ If there is an error, C<.flush> should call C<nbdkit_error> with an error message, and C<nbdkit_set_error> to record an appropriate error (unless C<errno> is sufficient), then return C<-1>. +Note that C<.flush> can be called both by the client doing an explicit +flush request, and by nbdkit when emulating Forced Unit Access (FUA) +semantics after a write or trim where the plugin did not provide FUA +callbacks (C<.pwrite_fua>, C<.zero_fua>, and C<.trim_fua>). + =head2 C<.trim> int trim (void *handle, uint32_t count, uint64_t offset); @@ -467,6 +508,21 @@ If there is an error, C<.trim> should call C<nbdkit_error> with an error message, and C<nbdkit_set_error> to record an appropriate error (unless C<errno> is sufficient), then return C<-1>. +If the plugin can provide efficient Forced Unit Access (FUA) semantics, +it should define C<.trim_fua> instead. + +=head2 C<.trim_fua> + + int trim_fua (void *handle, uint32_t count, uint64_t offset, int fua); + +This callback has the same requirements as C<.trim>, with the +additional parameter C<fua> set to a non-zero value if the client +wants FUA semantics (where the command must not return until the +actions of the trim have landed in persistent storage). If the plugin +cannot provide efficient FUA, but C<.can_flush> returns true and +C<.can_fua> does not return false, then client requests for FUA +semantics are emulated by nbdkit calling C<.flush>. + =head2 C<.zero> int zero (void *handle, uint32_t count, uint64_t offset, int may_trim); @@ -488,6 +544,21 @@ If there is an error, C<.zero> should call C<nbdkit_error> with an error message, and C<nbdkit_set_error> to record an appropriate error (unless C<errno> is sufficient), then return C<-1>. +If the plugin can provide efficient Forced Unit Access (FUA) semantics, +it should define C<.zero_fua> instead. + +=head2 C<.zero_fua> + + int zero_fua (void *handle, uint32_t count, uint64_t offset, int may_trim, int fua); + +This callback has the same requirements as C<.zero>, with the +additional parameter C<fua> set to a non-zero value if the client +wants FUA semantics (where the command must not return until the +actions of the write have landed in persistent storage). If the +plugin cannot provide efficient FUA, but C<.can_flush> returns true +and C<.can_fua> does not return false, then client requests for FUA +semantics are emulated by nbdkit calling C<.flush>. + =head1 THREADS Each nbdkit plugin must declare its thread safety model by defining @@ -711,7 +782,7 @@ Pino Toscano =head1 COPYRIGHT -Copyright (C) 2013-2017 Red Hat Inc. +Copyright (C) 2013-2018 Red Hat Inc. =head1 LICENSE diff --git a/docs/nbdkit.pod b/docs/nbdkit.pod index 1687ac9..2b868d9 100644 --- a/docs/nbdkit.pod +++ b/docs/nbdkit.pod @@ -785,7 +785,12 @@ information about that plugin, eg: [etc] Plugins which ship with nbdkit usually have the same version as the -corresponding nbdkit binary. +corresponding nbdkit binary. The nbdkit binary will always be able +to utilize plugins compiled against an older version of the header; +however, there are cases where a newer plugin may not be fully +supported by an older nbdkit binary (for example, a plugin that +supplies C<.pwrite_fua> but not C<.pwrite> may not support writes +when loaded by the older nbdkit). =head2 Detect if a plugin is installed @@ -895,7 +900,7 @@ Pino Toscano =head1 COPYRIGHT -Copyright (C) 2013-2017 Red Hat Inc. +Copyright (C) 2013-2018 Red Hat Inc. =head1 LICENSE diff --git a/include/nbdkit-plugin.h b/include/nbdkit-plugin.h index 2ec3b15..d3b0050 100644 --- a/include/nbdkit-plugin.h +++ b/include/nbdkit-plugin.h @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2013-2017 Red Hat Inc. + * Copyright (C) 2013-2018 Red Hat Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -96,6 +96,12 @@ struct nbdkit_plugin { void (*dump_plugin) (void); + int (*can_fua) (void *handle); + int (*pwrite_fua) (void *handle, const void *buf, uint32_t count, + uint64_t offset, int fua); + int (*zero_fua) (void *handle, uint32_t count, uint64_t offset, int may_trim, + int fua); + int (*trim_fua) (void *handle, uint32_t count, uint64_t offset, int fua); /* int (*set_exportname) (void *handle, const char *exportname); */ }; -- 2.14.3

If the target server supports FUA, then we should pass the client flag through rather than emulating things with a less-efficient flush. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- plugins/nbd/nbd.c | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c index c9727f7..09cefe5 100644 --- a/plugins/nbd/nbd.c +++ b/plugins/nbd/nbd.c @@ -616,6 +616,14 @@ nbd_can_trim (void *handle) return h->flags & NBD_FLAG_SEND_TRIM; } +static int +nbd_can_fua (void *handle) +{ + struct handle *h = handle; + + return h->flags & NBD_FLAG_SEND_FUA; +} + /* Read data from the file. */ static int nbd_pread (void *handle, void *buf, uint32_t count, uint64_t offset) @@ -631,23 +639,26 @@ nbd_pread (void *handle, void *buf, uint32_t count, uint64_t offset) /* Write data to the file. */ static int -nbd_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset) +nbd_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset, + int fua) { struct handle *h = handle; int c; /* TODO Auto-fragment this if the client has a larger max transfer limit than the server */ - c = nbd_request_full (h, 0, NBD_CMD_WRITE, offset, count, buf, NULL); + c = nbd_request_full (h, fua ? NBD_CMD_FLAG_FUA : 0, + NBD_CMD_WRITE, offset, count, buf, NULL); return c < 0 ? c : nbd_reply (h, c); } /* Write zeroes to the file. */ static int -nbd_zero (void *handle, uint32_t count, uint64_t offset, int may_trim) +nbd_zero (void *handle, uint32_t count, uint64_t offset, int may_trim, int fua) { struct handle *h = handle; int c; + int flags = 0; if (!(h->flags & NBD_FLAG_SEND_WRITE_ZEROES)) { /* Trigger a fall back to regular writing */ @@ -655,19 +666,22 @@ nbd_zero (void *handle, uint32_t count, uint64_t offset, int may_trim) return -1; } - c = nbd_request (h, may_trim ? 0 : NBD_CMD_FLAG_NO_HOLE, - NBD_CMD_WRITE_ZEROES, offset, count); + if (!may_trim) + flags |= NBD_CMD_FLAG_NO_HOLE; + if (fua) + flags |= NBD_CMD_FLAG_FUA; + c = nbd_request (h, flags, NBD_CMD_WRITE_ZEROES, offset, count); return c < 0 ? c : nbd_reply (h, c); } /* Trim a portion of the file. */ static int -nbd_trim (void *handle, uint32_t count, uint64_t offset) +nbd_trim (void *handle, uint32_t count, uint64_t offset, int fua) { struct handle *h = handle; int c; - c = nbd_request (h, 0, NBD_CMD_TRIM, offset, count); + c = nbd_request (h, fua ? NBD_CMD_FLAG_FUA : 0, NBD_CMD_TRIM, offset, count); return c < 0 ? c : nbd_reply (h, c); } @@ -697,11 +711,12 @@ static struct nbdkit_plugin plugin = { .can_flush = nbd_can_flush, .is_rotational = nbd_is_rotational, .can_trim = nbd_can_trim, + .can_fua = nbd_can_fua, .pread = nbd_pread, - .pwrite = nbd_pwrite, - .zero = nbd_zero, + .pwrite_fua = nbd_pwrite, + .zero_fua = nbd_zero, .flush = nbd_flush, - .trim = nbd_trim, + .trim_fua = nbd_trim, .errno_is_preserved = 1, }; -- 2.14.3