On 06/13/2016 06:19 AM, Paolo Bonzini wrote:
>> + /* Sanity checks, part 2. */
>> + if (request->from + request->len > client->exp->size) {
>> + LOG("operation past EOF; From: %" PRIu64 ", Len: %" PRIu32
>> + ", Size: %" PRIu64, request->from, request->len,
>> + (uint64_t)client->exp->size);
>> + rc = -EINVAL;
>
> For writes, this should be ENOSPC according to the spec.
Good call.
>> + if (nbd_co_send_reply(req, &reply, 0) < 0 || command ==
>> NBD_CMD_READ ||
>> + (command == NBD_CMD_WRITE && !req->complete)) {
>
> It's simpler to always set req->complete. Putting everything together:
>
> diff --git a/nbd/server.c b/nbd/server.c
> index 4743316..73505dc 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -1017,6 +1017,11 @@ static ssize_t nbd_co_receive_request(NBDRequest *req,
> TRACE("Decoding type");
>
> command = request->type & NBD_CMD_MASK_COMMAND;
> + if (command != NBD_CMD_WRITE) {
> + /* No payload, we are ready to read the next request. */
> + req->complete = true;
> + }
> +
Nice.
> @@ -1213,12 +1218,9 @@ static void nbd_trip(void *opaque)
> LOG("invalid request type (%" PRIu32 ") received", request.type);
> reply.error = EINVAL;
> error_reply:
> - /* We must disconnect after replying with an error to
> - * NBD_CMD_READ, since we choose not to send bogus filler
> - * data; likewise after NBD_CMD_WRITE if we did not read the
> - * payload. */
> - if (nbd_co_send_reply(req, &reply, 0) < 0 || command == NBD_CMD_READ
> ||
> - (command == NBD_CMD_WRITE && !req->complete)) {
> + /* We must disconnect after NBD_CMD_WRITE if we did not
> + * read the payload. */
> + if (nbd_co_send_reply(req, &reply, 0) < 0 || !req->complete)) {
I'm not sure I agree with your change on NBD_CMD_READ, but we can hash
that out with upstream NBD list on the correct protocol, and make any
further changes as a followup.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
