qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Zhi Hui Li <zhihuili@linux.vnet.ibm.com>
Cc: "Paolo Bonzini" <pbonzini@redhat.com>,
	zhihuili@cn.ibm.com, "Hervé Poussineau" <hpoussin@reactos.org>,
	qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 2/3 v6] Replace bdrv_* to bdrv_aio_* functions in DMA mode in fdc.c
Date: Wed, 16 May 2012 12:59:12 +0200	[thread overview]
Message-ID: <4FB38880.4000401@redhat.com> (raw)
In-Reply-To: <4FB363F7.5080601@linux.vnet.ibm.com>

Am 16.05.2012 10:23, schrieb Zhi Hui Li:
> On 2012年05月15日 17:38, Paolo Bonzini wrote:
>> Il 15/05/2012 11:33, Kevin Wolf ha scritto:
>>>>> which blindly overwrites status2.  Hence the new code was not written
>>>>> based on it.  However, the new code is untested as far as I know.
>>> In the thread of an earlier version of this series, I said that a qtest
>>> for floppy is required. This only confirms it.
>>
>> The problem with writing a qtest is that the spec is incredibly complex
>> and obscure.  It's probably even better to rip out code that cannot be
>> tested properly, so you don't have to test it at all...
>>
>> (Mostly tongue-in-cheek of course.  A qtest for basic read/write in PIO
>> and DMA modes is indeed a very good idea).
>>
>> Paolo
>>
>>
> 
> Yes , I think maybe Paolo is right.
> 
> Because the spec is incredibly complex and obscure and I am newer.
> To write the whole code's qtest beyond my ability. I am afraid I can't 
> finish it. so I want only do a qtest about basic read/write in PIO
> and DMA modes. I don't know whether it is OK.

Don't worry, any test is better than no test. We should try to add a
qtest for basic operation to fdc-test.c. More detailed tests can be
added later, or maybe we find good additions during review.

I know that the floppy controller spec is hard to read. Writing test
cases basically means translating it into clearer requirements.

> (I don't know whether we can use qtest to replace the real test, 
> especially on PIO mode 's test.)

In theory yes, qtest can do everything if you have a complete set of
test cases to cover the whole spec. In practice it will just help to
find regressions earlier (floppy isn't tested very often manually).

Kevin

  parent reply	other threads:[~2012-05-16 10:59 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-15  9:17 [Qemu-devel] [PATCH 0/3 v6] Replace bdrv_* to bdrv_aio_* functions in DMA mode in fdc.c Li Zhi Hui
2012-05-15  9:17 ` [Qemu-devel] [PATCH 1/3 v6] add function DMA_set_return and DMA_set_channel_async in dma.c Li Zhi Hui
2012-05-15  9:17 ` [Qemu-devel] [PATCH 2/3 v6] Replace bdrv_* to bdrv_aio_* functions in DMA mode in fdc.c Li Zhi Hui
2012-05-15  9:27   ` Paolo Bonzini
2012-05-15  9:33     ` Kevin Wolf
2012-05-15  9:38       ` Paolo Bonzini
2012-05-16  8:23         ` Zhi Hui Li
2012-05-16  9:11           ` Paolo Bonzini
2012-05-16 10:59           ` Kevin Wolf [this message]
2012-05-15 20:49     ` Hervé Poussineau
2012-05-16  7:58     ` Zhi Hui Li
2012-05-15  9:17 ` [Qemu-devel] [PATCH 3/3 v6] fdc.c: add tracing Li Zhi Hui

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=4FB38880.4000401@redhat.com \
    --to=kwolf@redhat.com \
    --cc=hpoussin@reactos.org \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=zhihuili@cn.ibm.com \
    --cc=zhihuili@linux.vnet.ibm.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).