From 7423f417827146f956df820f172d0bf80a489495 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Thu, 21 Jul 2016 13:34:46 -0600 Subject: nbd: Limit nbdflags to 16 bits Rather than asserting that nbdflags is within range, just give it the correct type to begin with :) nbdflags corresponds to the per-export portion of NBD Protocol "transmission flags", which is 16 bits in response to NBD_OPT_EXPORT_NAME and NBD_OPT_GO. Furthermore, upstream NBD has never passed the global flags to the kernel via ioctl(NBD_SET_FLAGS) (the ioctl was first introduced in NBD 2.9.22; then a latent bug in NBD 3.1 actually tried to OR the global flags with the transmission flags, with the disaster that the addition of NBD_FLAG_NO_ZEROES in 3.9 caused all earlier NBD 3.x clients to treat every export as read-only; NBD 3.10 and later intentionally clip things to 16 bits to pass only transmission flags). Qemu should follow suit, since the current two global flags (NBD_FLAG_FIXED_NEWSTYLE and NBD_FLAG_NO_ZEROES) have no impact on the kernel's behavior during transmission. CC: qemu-stable@nongnu.org Signed-off-by: Eric Blake Message-Id: <1469129688-22848-3-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini --- block/nbd-client.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'block') diff --git a/block/nbd-client.h b/block/nbd-client.h index fa9817b7d7..044aca4530 100644 --- a/block/nbd-client.h +++ b/block/nbd-client.h @@ -20,7 +20,7 @@ typedef struct NbdClientSession { QIOChannelSocket *sioc; /* The master data channel */ QIOChannel *ioc; /* The current I/O channel which may differ (eg TLS) */ - uint32_t nbdflags; + uint16_t nbdflags; off_t size; CoMutex send_mutex; -- cgit v1.2.3 From b8d0a9804d0bd43c9b662a6917ae9cd514a54dff Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Thu, 21 Jul 2016 13:34:48 -0600 Subject: block: Cater to iscsi with non-power-of-2 discard Dell Equallogic iSCSI SANs have a very unusual advertised geometry: $ iscsi-inq -e 1 -c $((0xb0)) iscsi://XXX/0 wsnz:0 maximum compare and write length:1 optimal transfer length granularity:0 maximum transfer length:0 optimal transfer length:0 maximum prefetch xdread xdwrite transfer length:0 maximum unmap lba count:30720 maximum unmap block descriptor count:2 optimal unmap granularity:30720 ugavalid:1 unmap granularity alignment:0 maximum write same length:30720 which says that both the maximum and the optimal discard size is 15M. It is not immediately apparent if the device allows discard requests not aligned to the optimal size, nor if it allows discards at a finer granularity than the optimal size. I tried to find details in the SCSI Commands Reference Manual Rev. A on what valid values of maximum and optimal sizes are permitted, but while that document mentions a "Block Limits VPD Page", I couldn't actually find documentation of that page or what values it would have, or if a SCSI device has an advertisement of its minimal unmap granularity. So it is not obvious to me whether the Dell Equallogic device is compliance with the SCSI specification. Fortunately, it is easy enough to support non-power-of-2 sizing, even if it means we are less efficient than truly possible when targetting that device (for example, it means that we refuse to unmap anything that is not a multiple of 15M and aligned to a 15M boundary, even if the device truly does support a smaller granularity where unmapping actually works). Reported-by: Peter Lieven Signed-off-by: Eric Blake Message-Id: <1469129688-22848-5-git-send-email-eblake@redhat.com> Acked-by: Stefan Hajnoczi Signed-off-by: Paolo Bonzini --- block/io.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) (limited to 'block') diff --git a/block/io.c b/block/io.c index 7323f0fb7b..d5493ba349 100644 --- a/block/io.c +++ b/block/io.c @@ -1180,10 +1180,11 @@ static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs, int alignment = MAX(bs->bl.pwrite_zeroes_alignment, bs->bl.request_alignment); - assert(is_power_of_2(alignment)); - head = offset & (alignment - 1); - tail = (offset + count) & (alignment - 1); - max_write_zeroes &= ~(alignment - 1); + assert(alignment % bs->bl.request_alignment == 0); + head = offset % alignment; + tail = (offset + count) % alignment; + max_write_zeroes = QEMU_ALIGN_DOWN(max_write_zeroes, alignment); + assert(max_write_zeroes >= bs->bl.request_alignment); while (count > 0 && !ret) { int num = count; @@ -2429,9 +2430,10 @@ int coroutine_fn bdrv_co_pdiscard(BlockDriverState *bs, int64_t offset, /* Discard is advisory, so ignore any unaligned head or tail */ align = MAX(bs->bl.pdiscard_alignment, bs->bl.request_alignment); - assert(is_power_of_2(align)); - head = MIN(count, -offset & (align - 1)); + assert(align % bs->bl.request_alignment == 0); + head = offset % align; if (head) { + head = MIN(count, align - head); count -= head; offset += head; } @@ -2449,6 +2451,7 @@ int coroutine_fn bdrv_co_pdiscard(BlockDriverState *bs, int64_t offset, max_pdiscard = QEMU_ALIGN_DOWN(MIN_NON_ZERO(bs->bl.max_pdiscard, INT_MAX), align); + assert(max_pdiscard); while (count > 0) { int ret; -- cgit v1.2.3