qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Evgeny Yakovlev <eyakovlev@virtuozzo.com>
To: Paolo Bonzini <pbonzini@redhat.com>,
	"Denis V. Lunev" <den@openvz.org>,
	qemu-block@nongnu.org, qemu-devel@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, Fam Zheng <famz@redhat.com>,
	Max Reitz <mreitz@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	John Snow <jsnow@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 2/3] ide: ignore retry_unit check for non-retry operations
Date: Wed, 29 Jun 2016 11:35:04 +0300	[thread overview]
Message-ID: <57738838.5050708@virtuozzo.com> (raw)
In-Reply-To: <9cafd60f-ac23-863d-0225-636499904398@redhat.com>



On 28.06.2016 23:56, Paolo Bonzini wrote:
>
> On 24/06/2016 17:06, Denis V. Lunev wrote:
>> When doing DMA request ide/core.c will set s->retry_unit to s->unit in
>> ide_start_dma. When dma completes ide_set_inactive sets retry_unit to -1.
>> After that ide_flush_cache runs and fails thanks to blkdebug.
>> ide_flush_cb calls ide_handle_rw_error which asserts that s->retry_unit
>> == s->unit. But s->retry_unit is still -1 after previous DMA completion
>> and flush does not use anything related to retry.
> Wouldn't the assertion fail for a PIO read/write too?  Perhaps
> retry_unit should be set to s->unit in ide_transfer_start too.

If PIO follows DMA and fails then yes, it looks like it will trigger an 
assert. I am not sure about setting retry_unit in ide_transfer_start. It 
looks like currently only DMA I/O entries touch retry_unit at all. Does 
that mean that PIO, flush, etc do not support retries by design and we 
need to add more exceptions to assert check or is it a real bug in how 
retries are initialized?

>
> Paolo

  reply	other threads:[~2016-06-29 10:07 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-24 15:06 [Qemu-devel] [PATCH 0/3] block: ignore flush requests when storage is clean Denis V. Lunev
2016-06-24 15:06 ` [Qemu-devel] [PATCH 1/3] " Denis V. Lunev
2016-06-24 15:31   ` Eric Blake
2016-06-24 15:54     ` Evgeny Yakovlev
2016-06-28 21:01   ` Paolo Bonzini
2016-06-29  7:36     ` Paolo Bonzini
2016-06-29  8:32       ` Denis V. Lunev
2016-06-24 15:06 ` [Qemu-devel] [PATCH 2/3] ide: ignore retry_unit check for non-retry operations Denis V. Lunev
2016-06-28 20:56   ` Paolo Bonzini
2016-06-29  8:35     ` Evgeny Yakovlev [this message]
2016-06-29 10:41       ` Paolo Bonzini
2016-06-24 15:06 ` [Qemu-devel] [PATCH 3/3] tests: in IDE and AHCI tests perform DMA write before flushing Denis V. Lunev
2016-06-24 15:22 ` [Qemu-devel] [PATCH 0/3] block: ignore flush requests when storage is clean Eric Blake

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=57738838.5050708@virtuozzo.com \
    --to=eyakovlev@virtuozzo.com \
    --cc=den@openvz.org \
    --cc=famz@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).