On 05.10.23 16:49, Eric Blake wrote:
On Wed, Oct 04, 2023 at 04:55:02PM -0500, Eric Blake wrote:
+static int
+nbd_co_block_status_payload_read(NBDClient *client, NBDRequest *request,
+ Error **errp)
[..]
+ for (i = 0; i < count; i++) {
+ id = ldl_be_p(buf + sizeof(NBDBlockStatusPayload) + sizeof(id) * i);
+ if (id == NBD_META_ID_BASE_ALLOCATION) {
+ if (request->contexts->base_allocation) {
+ goto skip;
+ }
should we also check that base_allocation is negotiated?
Oh, good point. Without that check, the client can pass in random id
numbers that it never negotiated. I've queued 1-11 and will probably
send a pull request for those this week, while respinning this patch
to fix the remaining issues you pointed out.
I'm squashing in the following. If you can review it today, I'll
include it in my pull request this afternoon; if not, we still have
time before soft freeze to get it in the next batch.
diff --git i/nbd/server.c w/nbd/server.c
index 30816b42386..62654579cbc 100644
--- i/nbd/server.c
+++ w/nbd/server.c
@@ -2478,19 +2478,22 @@ nbd_co_block_status_payload_read(NBDClient *client,
NBDRequest *request,
for (i = 0; i < count; i++) {
id = ldl_be_p(buf + sizeof(NBDBlockStatusPayload) + sizeof(id) * i);
if (id == NBD_META_ID_BASE_ALLOCATION) {
- if (request->contexts->base_allocation) {
+ if (!client->contexts.base_allocation ||
+ request->contexts->base_allocation) {
goto skip;
}
request->contexts->base_allocation = true;
} else if (id == NBD_META_ID_ALLOCATION_DEPTH) {
- if (request->contexts->allocation_depth) {
+ if (!client->contexts.allocation_depth ||
+ request->contexts->allocation_depth) {
goto skip;
}
request->contexts->allocation_depth = true;
} else {
- int idx = id - NBD_META_ID_DIRTY_BITMAP;
+ unsigned idx = id - NBD_META_ID_DIRTY_BITMAP;
- if (idx > nr_bitmaps || request->contexts->bitmaps[idx]) {
+ if (idx > nr_bitmaps || !client->contexts.bitmaps[idx] ||
Oops, I didn't notice: s/>/>=/, as nr_bitmaps is length of array.
+ request->contexts->bitmaps[idx]) {
goto skip;
}
request->contexts->bitmaps[idx] = true;
diff --git i/nbd/trace-events w/nbd/trace-events
index 3cf2d00e458..00ae3216a11 100644
--- i/nbd/trace-events
+++ w/nbd/trace-events
@@ -70,7 +70,7 @@ nbd_co_send_chunk_read(uint64_t cookie, uint64_t offset, void
*data, uint64_t si
nbd_co_send_chunk_read_hole(uint64_t cookie, uint64_t offset, uint64_t size) "Send structured read
hole reply: cookie = %" PRIu64 ", offset = %" PRIu64 ", len = %" PRIu64
nbd_co_send_extents(uint64_t cookie, unsigned int extents, uint32_t id, uint64_t length, int last)
"Send block status reply: cookie = %" PRIu64 ", extents = %u, context = %d (extents cover
%" PRIu64 " bytes, last chunk = %d)"
nbd_co_send_chunk_error(uint64_t cookie, int err, const char *errname, const char *msg)
"Send structured error reply: cookie = %" PRIu64 ", error = %d (%s), msg = '%s'"
-nbd_co_receive_block_status_payload_compliance(uint64_t from, int len) "client sent unusable
block status payload: from=0x%" PRIx64 ", len=0x%x"
+nbd_co_receive_block_status_payload_compliance(uint64_t from, uint64_t len) "client sent
unusable block status payload: from=0x%" PRIx64 ", len=0x%" PRIx64
nbd_co_receive_request_decode_type(uint64_t cookie, uint16_t type, const char *name) "Decoding type:
cookie = %" PRIu64 ", type = %" PRIu16 " (%s)"
nbd_co_receive_request_payload_received(uint64_t cookie, uint64_t len) "Payload received:
cookie = %" PRIu64 ", len = %" PRIu64
nbd_co_receive_ext_payload_compliance(uint64_t from, uint64_t len) "client sent
non-compliant write without payload flag: from=0x%" PRIx64 ", len=0x%" PRIx64
--
Best regards,
Vladimir
