[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH v3 09/13] nbd: Minimal structured r
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH v3 09/13] nbd: Minimal structured read for server |
Date: |
Fri, 13 Oct 2017 11:15:01 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 |
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;
}
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature