On 10/13/2017 11:00 AM, Eric Blake wrote:
On 10/12/2017 04:53 AM, Vladimir Sementsov-Ogievskiy wrote:
Minimal implementation of structured read: one structured reply chunk,
no segmentation.
Minimal structured error implementation: no text message.
Support DF flag, but just ignore it, as there is no segmentation any
way.
@@ -1304,10 +1375,17 @@ static int nbd_co_receive_request(NBDRequestData *req,
NBDRequest *request,
(uint64_t)client->exp->size);
return request->type == NBD_CMD_WRITE ? -ENOSPC : -EINVAL;
}
- if (request->flags & ~(NBD_CMD_FLAG_FUA | NBD_CMD_FLAG_NO_HOLE)) {
+ if (request->flags & ~(NBD_CMD_FLAG_FUA | NBD_CMD_FLAG_NO_HOLE |
+ NBD_CMD_FLAG_DF))
This says we accept the client sending NBD_CMD_FLAG_DF, but where did we
advertise it? I see no reason to advertise it unless a client
negotiates structured replies, at which point the obvious place to set
it is when we reply to the negotiation (and NBD_OPT_INFO prior to that
point won't see the flag, but the NBD_OPT_GO will see it).
Oh, one more tweak: if we didn't advertise the flag, the client
shouldn't be sending it.
So, here's what I'm squashing, before adding:
Reviewed-by: Eric Blake <address@hidden>
I'm also adding:
diff --git i/nbd/server.c w/nbd/server.c
index b4966ed8b1..acad2ceb59 100644
--- i/nbd/server.c
+++ w/nbd/server.c
@@ -1315,6 +1315,7 @@ static int nbd_co_receive_request(NBDRequestData
*req, NBDRequest *request,
Error **errp)
{
NBDClient *client = req->client;
+ int valid_flags;
g_assert(qemu_in_coroutine());
assert(client->recv_coroutine == qemu_coroutine_self());
@@ -1376,20 +1377,15 @@ static int nbd_co_receive_request(NBDRequestData
*req, NBDRequest *request,
(uint64_t)client->exp->size);
return request->type == NBD_CMD_WRITE ? -ENOSPC : -EINVAL;
}
- if (request->flags & ~(NBD_CMD_FLAG_FUA | NBD_CMD_FLAG_NO_HOLE |
- NBD_CMD_FLAG_DF))
- {
- error_setg(errp, "unsupported flags (got 0x%x)", request->flags);
- return -EINVAL;
+ valid_flags = NBD_CMD_FLAG_FUA;
+ if (request->type == NBD_CMD_READ && client->structured_reply) {
+ valid_flags |= NBD_CMD_FLAG_DF;
+ } else if (request->type == NBD_CMD_WRITE_ZEROES) {
+ valid_flags |= NBD_CMD_FLAG_NO_HOLE;
}
- if (request->type != NBD_CMD_READ && (request->flags &
NBD_CMD_FLAG_DF)) {
- error_setg(errp, "DF flag used with command %d (%s) which is
not READ",
- request->type, nbd_cmd_lookup(request->type));
- return -EINVAL;
- }
- if (request->type != NBD_CMD_WRITE_ZEROES &&
- (request->flags & NBD_CMD_FLAG_NO_HOLE)) {
- error_setg(errp, "unexpected flags (got 0x%x)", request->flags);
+ if (request->flags & ~valid_flags) {
+ error_setg(errp, "unsupported flags for command %s (got 0x%x)",
+ nbd_cmd_lookup(request->type), request->flags);
return -EINVAL;
}