Patchwork fix scsi-generic

login
register
mail settings
Submitter adq
Date Aug. 8, 2010, 8:08 p.m.
Message ID <AANLkTikZY9cf9TBdH79+Kw9HrvUC6WM6JtyACqatorQD@mail.gmail.com>
Download mbox | patch
Permalink /patch/61232/
State New
Headers show

Comments

adq - Aug. 8, 2010, 8:08 p.m.
On 8 August 2010 14:11, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 07.08.2010 02:55, schrieb adq:
>> Hi, I've been tracking down why scsi generic devices (using SG_IO)
>> don't work any more. After adding debug, I can see that it actually
>> submits the scsi CDB in hw/scsi-generic.c/execute_command(), but that
>> the hw/scsi-generic.c/scsi_read_complete() callback is never called.
>>
>> This is because these are done with ioctls, and the posix async ioctl
>> code is, I think, broken right now. Some more debugging, led me to
>> posix-aio-compat.c/posix_aio_process_queue():
>>
>>             if (acb->async_context_id != async_context_id) {
>>
>> The async_context_ids don't match, so the request is never handled.
>> This is because the acb->async_context_id field is not initialised in
>> posix-aio-compat.c/paio_ioctl() (compare with
>> posix-aio-compat.c/paio_submit()). The attached patch adds the missing
>> line in.
>>
>> This seems to fix the problem. Of course, /now/ I'm getting other
>> weird problems (as I'm trying to see if I can get slysoft anydvd
>> working in a KVM winXP vm), but they need further investigation and
>> likely other fixes.
>>
>> Please forgive me if I'm mistaken in this, I've only just started
>> looking at the qemu code.
>
> The patch looks correct to me.
>
> Please use git format-patch to generate the patch, so that it contains a
> decent commit message and I can apply it with git am. Also, please don't
> forget the Signed-off-by line, otherwise we can't accept it.

Hi, please find it attached; I've not used format-patch before, hope
this is correct!
adq - Aug. 8, 2010, 10:13 p.m.
Hi, more information on the command that is (now) killing my
qemu+scsi-generic in case someone else can help... Its our old friend
the READ DVD STRUCTURE command:

CMD: 00 ad
CMD: 01 00
CMD: 02 00
CMD: 03 00
CMD: 04 00
CMD: 05 00
CMD: 06 00
CMD: 07 01
CMD: 08 00
CMD: 09 08
CMD: 0a 00
CMD: 0b 00

What seems to happen is that it is sent to the sg device, but
lsi_command_complete() function is never called for it. Instead, I get
a message 0xc, or BUS DEVICE RESET. Since that isn't actually
implemented, everything seems to screw up after that point.
Kevin Wolf - Aug. 30, 2010, 1:16 p.m.
Am 08.08.2010 22:08, schrieb adq:
> On 8 August 2010 14:11, Kevin Wolf <kwolf@redhat.com> wrote:
>> Am 07.08.2010 02:55, schrieb adq:
>>> Hi, I've been tracking down why scsi generic devices (using SG_IO)
>>> don't work any more. After adding debug, I can see that it actually
>>> submits the scsi CDB in hw/scsi-generic.c/execute_command(), but that
>>> the hw/scsi-generic.c/scsi_read_complete() callback is never called.
>>>
>>> This is because these are done with ioctls, and the posix async ioctl
>>> code is, I think, broken right now. Some more debugging, led me to
>>> posix-aio-compat.c/posix_aio_process_queue():
>>>
>>>             if (acb->async_context_id != async_context_id) {
>>>
>>> The async_context_ids don't match, so the request is never handled.
>>> This is because the acb->async_context_id field is not initialised in
>>> posix-aio-compat.c/paio_ioctl() (compare with
>>> posix-aio-compat.c/paio_submit()). The attached patch adds the missing
>>> line in.
>>>
>>> This seems to fix the problem. Of course, /now/ I'm getting other
>>> weird problems (as I'm trying to see if I can get slysoft anydvd
>>> working in a KVM winXP vm), but they need further investigation and
>>> likely other fixes.
>>>
>>> Please forgive me if I'm mistaken in this, I've only just started
>>> looking at the qemu code.
>>
>> The patch looks correct to me.
>>
>> Please use git format-patch to generate the patch, so that it contains a
>> decent commit message and I can apply it with git am. Also, please don't
>> forget the Signed-off-by line, otherwise we can't accept it.
> 
> Hi, please find it attached; I've not used format-patch before, hope
> this is correct!

Thanks, applied to the block branch. And sorry for the delay.

Kevin

Patch

From db3cfa4f12ca7e3ed300304ba8a4db15e49b4189 Mon Sep 17 00:00:00 2001
From: Andrew de Quincey <adq@lidskialf.net>
Date: Sun, 8 Aug 2010 21:04:50 +0100
Subject: [PATCH] Set the async_context_id field when queuing an async ioctl call

Signed-off-by: Andrew de Quincey <adq@lidskialf.net>
---
 posix-aio-compat.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/posix-aio-compat.c b/posix-aio-compat.c
index a67ffe3..efc5968 100644
--- a/posix-aio-compat.c
+++ b/posix-aio-compat.c
@@ -599,6 +599,7 @@  BlockDriverAIOCB *paio_ioctl(BlockDriverState *bs, int fd,
     acb->aio_type = QEMU_AIO_IOCTL;
     acb->aio_fildes = fd;
     acb->ev_signo = SIGUSR2;
+    acb->async_context_id = get_async_context_id();
     acb->aio_offset = 0;
     acb->aio_ioctl_buf = buf;
     acb->aio_ioctl_cmd = req;
-- 
1.7.2.1