On 10/19/2017 05:26 PM, Eric Blake wrote: > From: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> > > Minimal implementation: for structured error only error_report error > message. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> > Signed-off-by: Eric Blake <ebl...@redhat.com> >
> +static int nbd_parse_offset_hole_payload(NBDStructuredReplyChunk *chunk, > + uint8_t *payload, QEMUIOVector > *qiov, > + Error **errp) > +{ > + uint64_t offset; > + uint32_t hole_size; > + > + if (chunk->length != sizeof(offset) + sizeof(hole_size)) { > + error_setg(errp, "Protocol error: invalid payload for " > + "NBD_REPLY_TYPE_OFFSET_HOLE"); > + return -EINVAL; > + } > + > + offset = payload_advance64(&payload); > + hole_size = payload_advance32(&payload); > + > + if (offset > qiov->size - hole_size) { > + error_setg(errp, "Protocol error: server sent chunk exceeding > requested" > + " region"); > + return -EINVAL; > + } > + > + qemu_iovec_memset(qiov, offset, 0, hole_size); Ouch. We have a discrepancy here, that needs clarification in the NBD spec (cc'd). In your server implementation, you are returning the offset as sent by the client (that is, all offsets are absolute to the size of the export). But in this client implementation, you are computing where to decode the zeroes by treating offset as though it were relative to the request, rather than absolute to the export. Or, in numbers, if I request a read of 2k starting at an offset of 4k, the server implementation returns an offset of 4k, and the client rejects the message because 4k is larger than the 2k request. I think that absolute numbers are better to work with than request-relative, but don't see anything in the proposed spec that states one way or the other, so this is your chance to agree with my preference or else argue why request-relative offsets are nicer, before wordsmithing a change to the spec to make it obvious which semantics are intended. Then I can change either the server to match (if we want request-relative) or the client to remember the original offset it sent in order to turn absolute answers from the server back into request-relative offsets for where to place the zeroes (if we go with absolute offsets). -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature