* [Qemu-devel] [PATCH] scsi-block: Handle error from host devices
@ 2018-04-02 3:23 Fam Zheng
2018-04-03 20:41 ` Daniel Henrique Barboza
0 siblings, 1 reply; 3+ messages in thread
From: Fam Zheng @ 2018-04-02 3:23 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Fam Zheng
The callback of blk_aio_ioctl is not sensible to SCSI errors, so
werror=stop doesn't work if ioctl returns 0 but the scsi status is
error.
Peek at the sg_io_hdr_t fields and amend ret to fix that.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
hw/scsi/scsi-disk.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index f5ab767ab5..2c43830586 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2651,10 +2651,26 @@ typedef struct SCSIBlockReq {
/* Selected bytes of the original CDB, copied into our own CDB. */
uint8_t cmd, cdb1, group_number;
+ BlockCompletionFunc *cb;
+ void *cb_opaque;
+
/* CDB passed to SG_IO. */
uint8_t cdb[16];
} SCSIBlockReq;
+static void scsi_block_sgio_cb(void *opaque, int ret)
+{
+ SCSIBlockReq *req = opaque;
+
+ if (!ret &&
+ (req->io_header.status ||
+ req->io_header.host_status ||
+ req->io_header.driver_status)) {
+ ret = -EIO;
+ }
+ req->cb(req->cb_opaque, ret);
+}
+
static BlockAIOCB *scsi_block_do_sgio(SCSIBlockReq *req,
int64_t offset, QEMUIOVector *iov,
int direction,
@@ -2734,7 +2750,9 @@ static BlockAIOCB *scsi_block_do_sgio(SCSIBlockReq *req,
io_header->usr_ptr = r;
io_header->flags |= SG_FLAG_DIRECT_IO;
- aiocb = blk_aio_ioctl(s->qdev.conf.blk, SG_IO, io_header, cb, opaque);
+ req->cb_opaque = opaque;
+ aiocb = blk_aio_ioctl(s->qdev.conf.blk, SG_IO, io_header,
+ scsi_block_sgio_cb, req);
assert(aiocb != NULL);
return aiocb;
}
--
2.14.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] scsi-block: Handle error from host devices
2018-04-02 3:23 [Qemu-devel] [PATCH] scsi-block: Handle error from host devices Fam Zheng
@ 2018-04-03 20:41 ` Daniel Henrique Barboza
2018-04-04 1:42 ` Fam Zheng
0 siblings, 1 reply; 3+ messages in thread
From: Daniel Henrique Barboza @ 2018-04-03 20:41 UTC (permalink / raw)
To: Fam Zheng, qemu-devel; +Cc: Paolo Bonzini
Hi Fam,
I've tried this patch and found issues when booting a VM using SCSI
passthrough. This is the backtrace from gdb from the segfault that
happens in the middle of kernel boot:
Thread 1 "qemu-system-ppc" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7ffff7ff63a0 (LWP 16830)]
0x0000000000000000 in ?? ()
(gdb) bt
#0 0x0000000000000000 in ?? ()
#1 0x00000001007b8da8 in scsi_block_sgio_cb (opaque=0x10212e580, ret=0)
at /home/danielhb/qemu/hw/scsi/scsi-disk.c:2772
#2 0x0000000100993f68 in blk_aio_complete (acb=0x101909520) at
/home/danielhb/qemu/block/block-backend.c:1331
#3 0x0000000100994ccc in blk_aio_ioctl_entry (opaque=0x101909520) at
/home/danielhb/qemu/block/block-backend.c:1542
#4 0x0000000100ac0954 in coroutine_trampoline (i0=28666944, i1=1) at
/home/danielhb/qemu/util/coroutine-ucontext.c:116
#5 0x00007ffff789574c in makecontext () at
../sysdeps/unix/sysv/linux/powerpc/powerpc64/makecontext.S:136
#6 0x0000000000000000 in ?? ()
The segfault is happening at this line:
static void scsi_block_sgio_cb(void *opaque, int ret)
{
SCSIBlockReq *req = opaque;
if (!ret &&
(req->io_header.status ||
req->io_header.host_status ||
req->io_header.driver_status)) {
ret = -EIO;
}
req->cb(req->cb_opaque, ret); <-----------------
}
This is happening because inside scsi_block_do_sgio you're not setting
req->cb, just req->cb_opaque. Setting req->cb made the VM boot again:
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2851,6 +2851,7 @@ static BlockAIOCB *scsi_block_do_sgio(SCSIBlockReq
*req,
io_header->usr_ptr = r;
io_header->flags |= SG_FLAG_DIRECT_IO;
+ req->cb = cb;
req->cb_opaque = opaque;
aiocb = blk_aio_ioctl(s->qdev.conf.blk, SG_IO, io_header,
scsi_block_sgio_cb, req);
Thanks,
Daniel
On 04/02/2018 12:23 AM, Fam Zheng wrote:
> The callback of blk_aio_ioctl is not sensible to SCSI errors, so
> werror=stop doesn't work if ioctl returns 0 but the scsi status is
> error.
>
> Peek at the sg_io_hdr_t fields and amend ret to fix that.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
> hw/scsi/scsi-disk.c | 20 +++++++++++++++++++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index f5ab767ab5..2c43830586 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -2651,10 +2651,26 @@ typedef struct SCSIBlockReq {
> /* Selected bytes of the original CDB, copied into our own CDB. */
> uint8_t cmd, cdb1, group_number;
>
> + BlockCompletionFunc *cb;
> + void *cb_opaque;
> +
> /* CDB passed to SG_IO. */
> uint8_t cdb[16];
> } SCSIBlockReq;
>
> +static void scsi_block_sgio_cb(void *opaque, int ret)
> +{
> + SCSIBlockReq *req = opaque;
> +
> + if (!ret &&
> + (req->io_header.status ||
> + req->io_header.host_status ||
> + req->io_header.driver_status)) {
> + ret = -EIO;
> + }
> + req->cb(req->cb_opaque, ret);
> +}
> +
> static BlockAIOCB *scsi_block_do_sgio(SCSIBlockReq *req,
> int64_t offset, QEMUIOVector *iov,
> int direction,
> @@ -2734,7 +2750,9 @@ static BlockAIOCB *scsi_block_do_sgio(SCSIBlockReq *req,
> io_header->usr_ptr = r;
> io_header->flags |= SG_FLAG_DIRECT_IO;
>
> - aiocb = blk_aio_ioctl(s->qdev.conf.blk, SG_IO, io_header, cb, opaque);
> + req->cb_opaque = opaque;
> + aiocb = blk_aio_ioctl(s->qdev.conf.blk, SG_IO, io_header,
> + scsi_block_sgio_cb, req);
> assert(aiocb != NULL);
> return aiocb;
> }
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] scsi-block: Handle error from host devices
2018-04-03 20:41 ` Daniel Henrique Barboza
@ 2018-04-04 1:42 ` Fam Zheng
0 siblings, 0 replies; 3+ messages in thread
From: Fam Zheng @ 2018-04-04 1:42 UTC (permalink / raw)
To: Daniel Henrique Barboza; +Cc: qemu-devel, Paolo Bonzini
On Tue, 04/03 17:41, Daniel Henrique Barboza wrote:
> Hi Fam,
>
> I've tried this patch and found issues when booting a VM using SCSI
> passthrough. This is the backtrace from gdb from the segfault that happens
> in the middle of kernel boot:
>
> Thread 1 "qemu-system-ppc" received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 0x7ffff7ff63a0 (LWP 16830)]
> 0x0000000000000000 in ?? ()
> (gdb) bt
> #0 0x0000000000000000 in ?? ()
> #1 0x00000001007b8da8 in scsi_block_sgio_cb (opaque=0x10212e580, ret=0) at
> /home/danielhb/qemu/hw/scsi/scsi-disk.c:2772
> #2 0x0000000100993f68 in blk_aio_complete (acb=0x101909520) at
> /home/danielhb/qemu/block/block-backend.c:1331
> #3 0x0000000100994ccc in blk_aio_ioctl_entry (opaque=0x101909520) at
> /home/danielhb/qemu/block/block-backend.c:1542
> #4 0x0000000100ac0954 in coroutine_trampoline (i0=28666944, i1=1) at
> /home/danielhb/qemu/util/coroutine-ucontext.c:116
> #5 0x00007ffff789574c in makecontext () at
> ../sysdeps/unix/sysv/linux/powerpc/powerpc64/makecontext.S:136
> #6 0x0000000000000000 in ?? ()
>
> The segfault is happening at this line:
>
> static void scsi_block_sgio_cb(void *opaque, int ret)
> {
> SCSIBlockReq *req = opaque;
>
> if (!ret &&
> (req->io_header.status ||
> req->io_header.host_status ||
> req->io_header.driver_status)) {
> ret = -EIO;
> }
> req->cb(req->cb_opaque, ret); <-----------------
> }
>
>
> This is happening because inside scsi_block_do_sgio you're not setting
> req->cb, just req->cb_opaque. Setting req->cb made the VM boot again:
Oops, thanks. Apparently I failed to test my patch, sorry. :(
Fam
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-04-04 1:42 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-04-02 3:23 [Qemu-devel] [PATCH] scsi-block: Handle error from host devices Fam Zheng
2018-04-03 20:41 ` Daniel Henrique Barboza
2018-04-04 1:42 ` Fam Zheng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).