On 12/14/2016 12:55 PM, Alex Bligh wrote:
>
>> On 14 Dec 2016, at 18:51, Eric Blake <eblake@...696...> wrote:
>>
>> It's working well in qemu 2.8 without needing tweaks to the
>> documentation. Should we try and do some cross-implementation testing
>> today, before doing the actual merge?
>
> If you have a minute to do so, that would be great.
With qemu-io as the client and nbd-server as the server (a merge of the
master branch, extensions-write-zeroes-branch, and the patch below), I
was successfully able to validate that nbd-server correctly advertises
the bit when asked to, that it handles NBD_CMD_WRITE_ZEROES requests at
any alignment, and that it is okay with any combination of the flags
NBD_CMD_FLAG_FUA and NBD_CMD_FLAG_NO_HOLE, insofar as the client can
then re-read correct all-zero data. There's an oddity with the error
reported when attempting to write to a read-only export: NBD_CMD_WRITE
fails with EPERM, but NBD_CMD_WRITE_ZEROES fails with EINVAL. That's
probably a bug in nbd-server, based on the recommendations of the spec,
but one which qemu-io handled just fine as a client.
I tested with this nbd-server config file, and the mentioned command lines:
# Use with:
# ./nbd-server -C local -d
# ./qemu-io -f raw -t none nbd://localhost:6666/file2
[generic]
port = 6666
allowlist = true
[file2]
exportname = /home/eblake/qemu/file2
flush = true
fua = true
trim = true
# uncomment as needed
# readonly = true
[It would be nice if '/nbd-server -C file -d -r' would force read-only
mode on ALL exports, regardless of the readonly settings in the
individual export settings of the config file - but I suppose that's a
patch for another day]
A quick read of nbd-server.c shows that it never tries to punch holes
(as permitted when NBD_CMD_FLAG_NO_HOLE is omitted), so the behavior is
semantically correct even if not optimal when sparse files are supported.
qemu-io as a client correctly refuses to send NBD_CMD_WRITE_ZEROES if
the server doesn't advertise support, and does not allow me to easily
attempt to try a write beyond EOF (which means I was unable to test the
server response in the face of an ill-behaved client that sends the
command when not advertised, or which sends bad length/offset). I could
tweak the code to qemu's client to allow me to test these situations, if
you think further testing of ill-behaved clients is worthwhile, but I'm
at least glad that the positive testing was successful.
As promised, here's the patches I used, above the merge of the master
and extensions-write-zeroes branches (most of the merge conflicts were
in doc/proto.md, I didn't closely review the resolution to those merges,
but we'll have to do it correctly when we actually make the merge):
diff --git c/nbd-server.c w/nbd-server.c
index 9d031c3..312091a 100644
--- c/nbd-server.c
+++ w/nbd-server.c
@@ -2252,7 +2252,7 @@ int mainloop(CLIENT *client) {
ERROR(client, reply, errno);
continue;
}
- SEND(client->net, reply);
+ SEND(client, reply);
continue;
default:
@@ -2690,11 +2690,15 @@ handle_modern_connection(GArray *const servers,
const int sock, struct generic_c
msg(LOG_ERR, "Modern initial negotiation failed");
goto handler_err;
}
- len = strlen(client->server->servename);
- writeit(commsocket, &len, sizeof len);
- writeit(commsocket, client->server->servename, len);
- readit(commsocket, &acl, 1);
- close(commsocket);
+ if (dontfork) {
+ acl = 'Y';
+ } else {
+ len = strlen(client->server->servename);
+ writeit(commsocket, &len, sizeof len);
+ writeit(commsocket, client->server->servename, len);
+ readit(commsocket, &acl, 1);
+ close(commsocket);
+ }
switch(acl) {
case 'N':
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature