Re: [Qemu-devel] [PATCH] iscsi: Fix divide-by-zero regression on raw SG devices

[Kevin Wolf <kwolf@redhat.com>]

Am 06.09.2016 um 21:04 hat Eric Blake geschrieben:
> When qemu uses iscsi devices in sg mode, iscsilun->block_size
> is left at 0.  Prior to commits cf081fca and similar, when
> block limits were tracked in sectors, this did not matter:
> various block limits were just left at 0.  But when we started
> scaling by block size, this caused SIGFPE.
> 
> One possible solution for SG mode is to just blindly skip ALL
> of iscsi_refresh_limits(), since we already short circuit so
> many other things in sg mode.  But this patch takes a slightly
> more conservative approach, and merely guarantees that scaling
> will succeed (for SG devices, the scaling is done to the block
> layer default of 512 bytes, since we don't know of any iscsi
> devices with a smaller block size), while still using multiples
> of the original size.  Resulting limits may still be zero in SG
> mode (that is, we only fix block_size used as a denominator, not
> all uses).
> 
> Reported-by: Holger Schranz <holger@fam-schranz.de>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> CC: qemu-stable@nongnu.org
> ---
> 
> I would really appreciate Holger testing this patch. We could
> also go with the much shorter patch that just does
> if (bs->sg) { return; }
> at the top of iscsi_refresh_limits(), but I'm not sure if that
> would break anything else in the block layer (we had several,
> but not all, limits that were provably left alone at 0 for
> sg mode).

Hm, originally I thought that we could even just return for bs->sg in
bdrv_refresh_limits() like below because for SG devices the limits
should never be used, but with scsi-block they might actually be.

Paolo, what do you think?

Anyway, let's go with the above patch first, it's a conservative one
that qemu-stable and possibly downstream can safely backport. If we're
going to add anything else, that would be for 2.8 only.

Kevin

diff --git a/block/io.c b/block/io.c
index fdf7080..144ff65 100644
--- a/block/io.c
+++ b/block/io.c
@@ -84,7 +84,7 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error **errp)
 
     memset(&bs->bl, 0, sizeof(bs->bl));
 
-    if (!drv) {
+    if (!drv || bs->sg) {
         return;
     }