Hi,
thanks - i applied the patch to the latest master. I hope that this will
solve my issue. Will this one get integrated in 1.4 final?
Greets,
Stefan
Am 11.02.2013 15:42, schrieb Paolo Bonzini:
> Il 11/02/2013 15:18, Stefan Priebe - Profihost AG ha scritto:
>>>> Some trace that a request was actually cancelled, but I think I
>>>> believe
>> Ah but that must be in guest not on host right? How to grab that from
>> client when it is crashing?
>
> Serial console could have something like "sda: aborting command". It is
> actually interesting to see what is causing commands to be aborted (typically
> a timeout, but what causes the timeout? :).
>
>>>> that. This seems to be the same issue as commits
>>>> 1bd075f29ea6d11853475c7c42734595720c3ac6 (iSCSI) and
>>>> 473c7f0255920bcaf37411990a3725898772817f (rbd), where the "cancelled"
>>>> callback is called before the "complete" callback.
>> If there is the same code in virtio-scsi it might be.
>
> No, virtio-scsi is relying on the backends (including scsi-disk)
> doing it correctly. The RBD code looks okay, so it's still my
> fault :) but not virtio-scsi's.
>
> I think this happens when a request is split into multiple parts,
> and one of them is canceled. Then the next part is fired, but
> virtio-scsi's cancellation callbacks have fired already.
>
> You can test this patch:
>
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index 07220e4..1d8289c 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -221,6 +221,10 @@ static void scsi_write_do_fua(SCSIDiskReq *r)
> {
> SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
>
> + if (r->req.io_canceled) {
> + return;
> + }
> +
> if (scsi_is_cmd_fua(&r->req.cmd)) {
> bdrv_acct_start(s->qdev.conf.bs, &r->acct, 0, BDRV_ACCT_FLUSH);
> r->req.aiocb = bdrv_aio_flush(s->qdev.conf.bs, scsi_aio_complete, r);
> @@ -352,6 +356,10 @@ static void scsi_read_data(SCSIRequest *req)
> /* No data transfer may already be in progress */
> assert(r->req.aiocb == NULL);
>
> + if (r->req.io_canceled) {
> + return;
> + }
> +
> /* The request is used as the AIO opaque value, so add a ref. */
> scsi_req_ref(&r->req);
> if (r->req.cmd.mode == SCSI_XFER_TO_DEV) {
> @@ -455,6 +463,10 @@ static void scsi_write_data(SCSIRequest *req)
> /* No data transfer may already be in progress */
> assert(r->req.aiocb == NULL);
>
> + if (r->req.io_canceled) {
> + return;
> + }
> +
> /* The request is used as the AIO opaque value, so add a ref. */
> scsi_req_ref(&r->req);
> if (r->req.cmd.mode != SCSI_XFER_TO_DEV) {
>
> Paolo
>
