[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-stable] [PATCH] iscsi: Fix divide-by-zero regression on raw SG
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-stable] [PATCH] iscsi: Fix divide-by-zero regression on raw SG devices |
Date: |
Wed, 7 Sep 2016 10:31:07 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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 <address@hidden>
> Signed-off-by: Eric Blake <address@hidden>
> CC: address@hidden
> ---
>
> 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;
}