| Message ID | 1473283640-15756-1-git-send-email-eblake@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Eric,
Thanks a lot, it seems the patch works. The VM starting.
Unfortunately we run into the next issue. By the accessing the
megasas controller we got a SIGSEGV.
See the trave below.
Best regards
Holger
===========================================================================
Thread 4 "CPU 1/KVM" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fe6c622b700 (LWP 18444)]
0x0000559049a34965 in megasas_handle_frame (s=0x7fe6c5658010,
frame_addr=935342080, frame_count=0)
at /home/shl/Install/qemu-2.7.0/hw/scsi/megasas.c:1984
1984 cmd->frame->header.cmd_status = frame_status;
(gdb) bt
#0 0x0000559049a34965 in megasas_handle_frame (s=0x7fe6c5658010,
frame_addr=935342080, frame_count=0) at
/home/shl/Install/qemu-2.7.0/hw/scsi/megasas.c:1984
#1 0x0000559049a34f90 in megasas_mmio_write (opaque=0x7fe6c5658010,
addr=64, val=935342081, size=8) at
/home/shl/Install/qemu-2.7.0/hw/scsi/megasas.c:2125
#2 0x0000559049743cbc in memory_region_write_accessor
(mr=0x7fe6c56588a0, addr=64, value=0x7fe6c622a7b8, size=8, shift=0,
mask=18446744073709551615, attrs=...) at
/home/shl/Install/qemu-2.7.0/memory.c:525
#3 0x0000559049743f36 in access_with_adjusted_size (addr=64,
value=0x7fe6c622a7b8, size=4, access_size_min=8, access_size_max=8, access=
0x559049743bc5 <memory_region_write_accessor>, mr=0x7fe6c56588a0,
attrs=...) at /home/shl/Install/qemu-2.7.0/memory.c:591
#4 0x0000559049746e62 in memory_region_dispatch_write
(mr=0x7fe6c56588a0, addr=64, data=935342081, size=4, attrs=...)
at /home/shl/Install/qemu-2.7.0/memory.c:1262
#5 0x00005590496eb4f8 in address_space_write_continue
(as=0x55904a25bc00 <address_space_memory>, addr=4272095296, attrs=...,
buf=0x7fe6da415028 "\001\060\300\067", len=4, addr1=64, l=4,
mr=0x7fe6c56588a0)
at /home/shl/Install/qemu-2.7.0/exec.c:2544
#6 0x00005590496eb6d8 in address_space_write (as=0x55904a25bc00
<address_space_memory>, addr=4272095296, attrs=..., buf=0x7fe6da415028
"\001\060\300\067", len=4) at /home/shl/Install/qemu-2.7.0/exec.c:2601
#7 0x00005590496eba88 in address_space_rw (as=0x55904a25bc00
<address_space_memory>, addr=4272095296, attrs=..., buf=0x7fe6da415028
"\001\060\300\067", len=4, ---Type <return> to continue, or q <return>
to quit---q
Python Exception <type 'exceptions.KeyboardInterrupt'> Quit:
#8 0x000055904973fa2a in kvm_cpu_exec (cpu=0x55904c7df620)
at /home/shl/Install/qemu-2.7.0/kvm-all.c:1965
#9 0x0000559049723116 in qemu_kvm_cpu_thread_fn (arg=0x55904c7df620)
at /home/shl/Install/qemu-2.7.0/cpus.c:1078
#10 0x00007fe6d76440a4 in start_thread () at /lib64/libpthread.so.0
#11 0x00007fe6d335202d in clone () at /lib64/libc.so.6
(gdb) print cmd
$1 = (MegasasCmd *) 0x7fe6c5658c20
(gdb) print *cmd
$2 = {index = 0, flags = 1, count = 0, context = 0, pa = 0, pa_size = 1024,
frame = 0x0, req = 0x0, qsg = {sg = 0x0, nsg = 0, nalloc = 0, size = 0,
dev = 0x0, as = 0x0}, iov_buf = 0x0, iov_size = 512, iov_offset = 0,
state = 0x7fe6c5658010}
(gdb) print cmd->cmd
There is no member named cmd.
(gdb) print cmd->frame
$3 = (union mfi_frame *) 0x0
(gdb)
=====================================================
megasas:
(gdb) bt
#0 0x0000559049a34965 in megasas_handle_frame (s=0x7fe6c5658010,
frame_addr=935342080, frame_count=0) at
/home/shl/Install/qemu-2.7.0/hw/scsi/megasas.c:1984
#1 0x0000559049a34f90 in megasas_mmio_write (opaque=0x7fe6c5658010,
addr=64, val=935342081, size=8) at
/home/shl/Install/qemu-2.7.0/hw/scsi/megasas.c:2125
:
/home/kvm/SOURCES/qemu.git/qemu/hw/scsi> diff megasas.c
/home/kvm/SOURCES/qemu-2.6.1/hw/scsi/megasas.c
32c32
< #include "qapi/error.h"
---
>
51c51,55
:
413a422,423
>
> return bcd_time;
1984c1994,1998
< cmd->frame->header.cmd_status = frame_status;
---
> if (cmd->frame) {
> cmd->frame->header.cmd_status = frame_status;
> } else {
> megasas_frame_set_cmd_status(s, frame_addr, frame_status);
> }
2301c2315,2317
< msi_uninit(d);
---
:
Am 07.09.2016 um 23:27 schrieb Eric Blake:
> 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.
>
> Then, in a later patch, commit a5b8dd2c added an assertion to
> bdrv_open_common() that request_alignment is always non-zero;
> which was not true for SG mode. Rather than relax that assertion,
> we can just provide a sane value (we don't know of any SG device
> with a block size smaller than qemu's default sizing of 512 bytes).
>
> 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, while still using multiples of the original size
> where possible. Resulting limits may still be zero in SG mode
> (that is, we mostly only fix block_size used as a denominator
> or which affect assertions, not all uses).
>
> Reported-by: Holger Schranz <holger@fam-schranz.de>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> CC: qemu-stable@nongnu.org
>
> ---
> v2: avoid second assertion failure
> ---
> block/iscsi.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 95ce9e1..c01e955 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -1813,19 +1813,23 @@ static void iscsi_refresh_limits(BlockDriverState *bs, Error **errp)
>
> IscsiLun *iscsilun = bs->opaque;
> uint64_t max_xfer_len = iscsilun->use_16_for_rw ? 0xffffffff : 0xffff;
> + unsigned int block_size = MIN_NON_ZERO(BDRV_SECTOR_SIZE,
> + iscsilun->block_size);
>
> - bs->bl.request_alignment = iscsilun->block_size;
> + assert(iscsilun->block_size >= BDRV_SECTOR_SIZE || bs->sg);
> +
> + bs->bl.request_alignment = block_size;
>
> if (iscsilun->bl.max_xfer_len) {
> max_xfer_len = MIN(max_xfer_len, iscsilun->bl.max_xfer_len);
> }
>
> - if (max_xfer_len * iscsilun->block_size < INT_MAX) {
> + if (max_xfer_len * block_size < INT_MAX) {
> bs->bl.max_transfer = max_xfer_len * iscsilun->block_size;
> }
>
> if (iscsilun->lbp.lbpu) {
> - if (iscsilun->bl.max_unmap < 0xffffffff / iscsilun->block_size) {
> + if (iscsilun->bl.max_unmap < 0xffffffff / block_size) {
> bs->bl.max_pdiscard =
> iscsilun->bl.max_unmap * iscsilun->block_size;
> }
> @@ -1835,7 +1839,7 @@ static void iscsi_refresh_limits(BlockDriverState *bs, Error **errp)
> bs->bl.pdiscard_alignment = iscsilun->block_size;
> }
>
> - if (iscsilun->bl.max_ws_len < 0xffffffff / iscsilun->block_size) {
> + if (iscsilun->bl.max_ws_len < 0xffffffff / block_size) {
> bs->bl.max_pwrite_zeroes =
> iscsilun->bl.max_ws_len * iscsilun->block_size;
> }
> @@ -1846,7 +1850,7 @@ static void iscsi_refresh_limits(BlockDriverState *bs, Error **errp)
> bs->bl.pwrite_zeroes_alignment = iscsilun->block_size;
> }
> if (iscsilun->bl.opt_xfer_len &&
> - iscsilun->bl.opt_xfer_len < INT_MAX / iscsilun->block_size) {
> + iscsilun->bl.opt_xfer_len < INT_MAX / block_size) {
> bs->bl.opt_transfer = pow2floor(iscsilun->bl.opt_xfer_len *
> iscsilun->block_size);
> }
---
Diese E-Mail wurde von Avast Antivirus-Software auf Viren geprüft.
https://www.avast.com/antivirus
On 09/08/2016 09:27 AM, Holger Schranz wrote: > Hi Eric, > > Thanks a lot, it seems the patch works. The VM starting. Should I take that as a Tested-by: line? > Unfortunately we run into the next issue. By the accessing the > megasas controller we got a SIGSEGV. Seems unrelated, let's figure out who's responsible. > > /home/kvm/SOURCES/qemu.git/qemu/hw/scsi> diff megasas.c > /home/kvm/SOURCES/qemu-2.6.1/hw/scsi/megasas.c 'diff -u' is much easier to read than plain 'diff' (ed script diffs have no context); in fact, 'git diff' uses -u format by default. Also, it's traditional to 'diff old new', but you flipped the argument, so the diff looks backwards. > 32c32 > < #include "qapi/error.h" > --- >> > 51c51,55 > : > 413a422,423 >> >> return bcd_time; > 1984c1994,1998 > < cmd->frame->header.cmd_status = frame_status; > --- >> if (cmd->frame) { >> cmd->frame->header.cmd_status = frame_status; >> } else { >> megasas_frame_set_cmd_status(s, frame_addr, frame_status); >> } Here's a nicer way to get the above diff $ git diff v2.6.1 v2.7.0 hw/scsi/megasas.c at which point 'git gui blame hw/scsi/megasas.c' shows that it was commit 8cc46787, by Paolo, that claimed that cmd->frame was non-NULL.
On 08/09/2016 17:25, Eric Blake wrote: > On 09/08/2016 09:27 AM, Holger Schranz wrote: >> Hi Eric, >> >> Thanks a lot, it seems the patch works. The VM starting. > > Should I take that as a Tested-by: line? > >> Unfortunately we run into the next issue. By the accessing the >> megasas controller we got a SIGSEGV. > > Seems unrelated, let's figure out who's responsible. > > >> >> /home/kvm/SOURCES/qemu.git/qemu/hw/scsi> diff megasas.c >> /home/kvm/SOURCES/qemu-2.6.1/hw/scsi/megasas.c > > 'diff -u' is much easier to read than plain 'diff' (ed script diffs have > no context); in fact, 'git diff' uses -u format by default. Also, it's > traditional to 'diff old new', but you flipped the argument, so the diff > looks backwards. > >> 32c32 >> < #include "qapi/error.h" >> --- >>> >> 51c51,55 >> : >> 413a422,423 >>> >>> return bcd_time; >> 1984c1994,1998 >> < cmd->frame->header.cmd_status = frame_status; >> --- >>> if (cmd->frame) { >>> cmd->frame->header.cmd_status = frame_status; >>> } else { >>> megasas_frame_set_cmd_status(s, frame_addr, frame_status); >>> } > > Here's a nicer way to get the above diff > $ git diff v2.6.1 v2.7.0 hw/scsi/megasas.c > > at which point 'git gui blame hw/scsi/megasas.c' shows that it was > commit 8cc46787, by Paolo, that claimed that cmd->frame was non-NULL. Well, that's easy, let's just revert it. :) Paolo
On 09/08/2016 05:25 PM, Eric Blake wrote: > On 09/08/2016 09:27 AM, Holger Schranz wrote: >> Hi Eric, >> >> Thanks a lot, it seems the patch works. The VM starting. > > Should I take that as a Tested-by: line? > >> Unfortunately we run into the next issue. By the accessing the >> megasas controller we got a SIGSEGV. > > Seems unrelated, let's figure out who's responsible. > > >> >> /home/kvm/SOURCES/qemu.git/qemu/hw/scsi> diff megasas.c >> /home/kvm/SOURCES/qemu-2.6.1/hw/scsi/megasas.c > > 'diff -u' is much easier to read than plain 'diff' (ed script diffs have > no context); in fact, 'git diff' uses -u format by default. Also, it's > traditional to 'diff old new', but you flipped the argument, so the diff > looks backwards. > >> 32c32 >> < #include "qapi/error.h" >> --- >>> >> 51c51,55 >> : >> 413a422,423 >>> >>> return bcd_time; >> 1984c1994,1998 >> < cmd->frame->header.cmd_status = frame_status; >> --- >>> if (cmd->frame) { >>> cmd->frame->header.cmd_status = frame_status; >>> } else { >>> megasas_frame_set_cmd_status(s, frame_addr, frame_status); >>> } > > Here's a nicer way to get the above diff > $ git diff v2.6.1 v2.7.0 hw/scsi/megasas.c Sorry, try to make it better next time > > at which point 'git gui blame hw/scsi/megasas.c' shows that it was > commit 8cc46787, by Paolo, that claimed that cmd->frame was non-NULL. >
Hi Eric, Am 08.09.2016 um 17:25 schrieb Eric Blake: > On 09/08/2016 09:27 AM, Holger Schranz wrote: >> Hi Eric, >> >> Thanks a lot, it seems the patch works. The VM starting. > Should I take that as a Tested-by: line? The test was: 1) login as root 2) cd to workdir 3) start virt-viewer: "virt-viewer VCS82M3-IUP0 --wait &" 4) start VM with: "virsh create M3-IUP0.xml". At the first test: only an error message displayed (this was our first report) after execute the virsh command. Last test (with your Patch V2). Now, the virt-viewer shows the known dialog: BIOS ... System ... and closed the session with error (megasas-issue) Is this the information you wanted Eric? > >> Unfortunately we run into the next issue. By the accessing the >> megasas controller we got a SIGSEGV. > Seems unrelated, let's figure out who's responsible. > > >> /home/kvm/SOURCES/qemu.git/qemu/hw/scsi> diff megasas.c >> /home/kvm/SOURCES/qemu-2.6.1/hw/scsi/megasas.c > 'diff -u' is much easier to read than plain 'diff' (ed script diffs have > no context); in fact, 'git diff' uses -u format by default. Also, it's > traditional to 'diff old new', but you flipped the argument, so the diff > looks backwards. O.k. . Thanks for the hint. We will use 'diff -u' in the future. > >> 32c32 >> < #include "qapi/error.h" >> --- >> 51c51,55 >> : >> 413a422,423 >>> return bcd_time; >> 1984c1994,1998 >> < cmd->frame->header.cmd_status = frame_status; >> --- >>> if (cmd->frame) { >>> cmd->frame->header.cmd_status = frame_status; >>> } else { >>> megasas_frame_set_cmd_status(s, frame_addr, frame_status); >>> } > Here's a nicer way to get the above diff > $ git diff v2.6.1 v2.7.0 hw/scsi/megasas.c > > at which point 'git gui blame hw/scsi/megasas.c' shows that it was > commit 8cc46787, by Paolo, that claimed that cmd->frame was non-NULL. > --- Diese E-Mail wurde von Avast Antivirus-Software auf Viren geprüft. https://www.avast.com/antivirus
Hello Eric, Kevin, Paolo, Thank you very much for the very fast help. All works fine now!!! Best regards and have nice weekend Holger Am 08.09.2016 um 19:34 schrieb Holger Schranz: > Hi Eric, > > Am 08.09.2016 um 17:25 schrieb Eric Blake: >> On 09/08/2016 09:27 AM, Holger Schranz wrote: >>> Hi Eric, >>> >>> Thanks a lot, it seems the patch works. The VM starting. >> Should I take that as a Tested-by: line? > > The test was: > 1) login as root > 2) cd to workdir > 3) start virt-viewer: "virt-viewer VCS82M3-IUP0 --wait &" > 4) start VM with: "virsh create M3-IUP0.xml". > > At the first test: > only an error message displayed (this was our first report) after > execute the virsh command. > > Last test (with your Patch V2). > Now, the virt-viewer shows the known dialog: BIOS ... System ... and > closed > the session with error (megasas-issue) > > Is this the information you wanted Eric? > >> >>> Unfortunately we run into the next issue. By the accessing the >>> megasas controller we got a SIGSEGV. >> Seems unrelated, let's figure out who's responsible. >> >> >>> /home/kvm/SOURCES/qemu.git/qemu/hw/scsi> diff megasas.c >>> /home/kvm/SOURCES/qemu-2.6.1/hw/scsi/megasas.c >> 'diff -u' is much easier to read than plain 'diff' (ed script diffs have >> no context); in fact, 'git diff' uses -u format by default. Also, it's >> traditional to 'diff old new', but you flipped the argument, so the diff >> looks backwards. > O.k. . Thanks for the hint. We will use 'diff -u' in the future. >> >>> 32c32 >>> < #include "qapi/error.h" >>> --- >>> 51c51,55 >>> : >>> 413a422,423 >>>> return bcd_time; >>> 1984c1994,1998 >>> < cmd->frame->header.cmd_status = frame_status; >>> --- >>>> if (cmd->frame) { >>>> cmd->frame->header.cmd_status = frame_status; >>>> } else { >>>> megasas_frame_set_cmd_status(s, frame_addr, >>>> frame_status); >>>> } >> Here's a nicer way to get the above diff >> $ git diff v2.6.1 v2.7.0 hw/scsi/megasas.c >> >> at which point 'git gui blame hw/scsi/megasas.c' shows that it was >> commit 8cc46787, by Paolo, that claimed that cmd->frame was non-NULL. >> > > > --- > Diese E-Mail wurde von Avast Antivirus-Software auf Viren geprüft. > https://www.avast.com/antivirus > > --- Diese E-Mail wurde von Avast Antivirus-Software auf Viren geprüft. https://www.avast.com/antivirus
On 07/09/2016 23:27, Eric Blake wrote: > 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. > > Then, in a later patch, commit a5b8dd2c added an assertion to > bdrv_open_common() that request_alignment is always non-zero; > which was not true for SG mode. Rather than relax that assertion, > we can just provide a sane value (we don't know of any SG device > with a block size smaller than qemu's default sizing of 512 bytes). > > 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, while still using multiples of the original size > where possible. Resulting limits may still be zero in SG mode > (that is, we mostly only fix block_size used as a denominator > or which affect assertions, not all uses). > > Reported-by: Holger Schranz <holger@fam-schranz.de> > Signed-off-by: Eric Blake <eblake@redhat.com> > CC: qemu-stable@nongnu.org > > --- > v2: avoid second assertion failure > --- > block/iscsi.c | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/block/iscsi.c b/block/iscsi.c > index 95ce9e1..c01e955 100644 > --- a/block/iscsi.c > +++ b/block/iscsi.c > @@ -1813,19 +1813,23 @@ static void iscsi_refresh_limits(BlockDriverState *bs, Error **errp) > > IscsiLun *iscsilun = bs->opaque; > uint64_t max_xfer_len = iscsilun->use_16_for_rw ? 0xffffffff : 0xffff; > + unsigned int block_size = MIN_NON_ZERO(BDRV_SECTOR_SIZE, > + iscsilun->block_size); > > - bs->bl.request_alignment = iscsilun->block_size; > + assert(iscsilun->block_size >= BDRV_SECTOR_SIZE || bs->sg); > + > + bs->bl.request_alignment = block_size; > > if (iscsilun->bl.max_xfer_len) { > max_xfer_len = MIN(max_xfer_len, iscsilun->bl.max_xfer_len); > } > > - if (max_xfer_len * iscsilun->block_size < INT_MAX) { > + if (max_xfer_len * block_size < INT_MAX) { > bs->bl.max_transfer = max_xfer_len * iscsilun->block_size; > } > > if (iscsilun->lbp.lbpu) { > - if (iscsilun->bl.max_unmap < 0xffffffff / iscsilun->block_size) { > + if (iscsilun->bl.max_unmap < 0xffffffff / block_size) { > bs->bl.max_pdiscard = > iscsilun->bl.max_unmap * iscsilun->block_size; > } > @@ -1835,7 +1839,7 @@ static void iscsi_refresh_limits(BlockDriverState *bs, Error **errp) > bs->bl.pdiscard_alignment = iscsilun->block_size; > } > > - if (iscsilun->bl.max_ws_len < 0xffffffff / iscsilun->block_size) { > + if (iscsilun->bl.max_ws_len < 0xffffffff / block_size) { > bs->bl.max_pwrite_zeroes = > iscsilun->bl.max_ws_len * iscsilun->block_size; > } > @@ -1846,7 +1850,7 @@ static void iscsi_refresh_limits(BlockDriverState *bs, Error **errp) > bs->bl.pwrite_zeroes_alignment = iscsilun->block_size; > } > if (iscsilun->bl.opt_xfer_len && > - iscsilun->bl.opt_xfer_len < INT_MAX / iscsilun->block_size) { > + iscsilun->bl.opt_xfer_len < INT_MAX / block_size) { > bs->bl.opt_transfer = pow2floor(iscsilun->bl.opt_xfer_len * > iscsilun->block_size); > } > Queued, thanks. Paolo
diff --git a/block/iscsi.c b/block/iscsi.c index 95ce9e1..c01e955 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -1813,19 +1813,23 @@ static void iscsi_refresh_limits(BlockDriverState *bs, Error **errp) IscsiLun *iscsilun = bs->opaque; uint64_t max_xfer_len = iscsilun->use_16_for_rw ? 0xffffffff : 0xffff; + unsigned int block_size = MIN_NON_ZERO(BDRV_SECTOR_SIZE, + iscsilun->block_size); - bs->bl.request_alignment = iscsilun->block_size; + assert(iscsilun->block_size >= BDRV_SECTOR_SIZE || bs->sg); + + bs->bl.request_alignment = block_size; if (iscsilun->bl.max_xfer_len) { max_xfer_len = MIN(max_xfer_len, iscsilun->bl.max_xfer_len); } - if (max_xfer_len * iscsilun->block_size < INT_MAX) { + if (max_xfer_len * block_size < INT_MAX) { bs->bl.max_transfer = max_xfer_len * iscsilun->block_size; } if (iscsilun->lbp.lbpu) { - if (iscsilun->bl.max_unmap < 0xffffffff / iscsilun->block_size) { + if (iscsilun->bl.max_unmap < 0xffffffff / block_size) { bs->bl.max_pdiscard = iscsilun->bl.max_unmap * iscsilun->block_size; } @@ -1835,7 +1839,7 @@ static void iscsi_refresh_limits(BlockDriverState *bs, Error **errp) bs->bl.pdiscard_alignment = iscsilun->block_size; } - if (iscsilun->bl.max_ws_len < 0xffffffff / iscsilun->block_size) { + if (iscsilun->bl.max_ws_len < 0xffffffff / block_size) { bs->bl.max_pwrite_zeroes = iscsilun->bl.max_ws_len * iscsilun->block_size; } @@ -1846,7 +1850,7 @@ static void iscsi_refresh_limits(BlockDriverState *bs, Error **errp) bs->bl.pwrite_zeroes_alignment = iscsilun->block_size; } if (iscsilun->bl.opt_xfer_len && - iscsilun->bl.opt_xfer_len < INT_MAX / iscsilun->block_size) { + iscsilun->bl.opt_xfer_len < INT_MAX / block_size) { bs->bl.opt_transfer = pow2floor(iscsilun->bl.opt_xfer_len * iscsilun->block_size); }
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. Then, in a later patch, commit a5b8dd2c added an assertion to bdrv_open_common() that request_alignment is always non-zero; which was not true for SG mode. Rather than relax that assertion, we can just provide a sane value (we don't know of any SG device with a block size smaller than qemu's default sizing of 512 bytes). 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, while still using multiples of the original size where possible. Resulting limits may still be zero in SG mode (that is, we mostly only fix block_size used as a denominator or which affect assertions, not all uses). Reported-by: Holger Schranz <holger@fam-schranz.de> Signed-off-by: Eric Blake <eblake@redhat.com> CC: qemu-stable@nongnu.org --- v2: avoid second assertion failure --- block/iscsi.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)