qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Frediano Ziglio <freddy77@gmail.com>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [RFC PATCH 0/5] qcow: coroutines cleanup
Date: Fri, 22 Jul 2011 12:10:13 +0200	[thread overview]
Message-ID: <4E294C85.2020005@redhat.com> (raw)
In-Reply-To: <CAHt6W4fdv4bF1A1xM_RZ1RnF=CXfmxPD-zPaZq_yUo90QAJSNQ@mail.gmail.com>

Am 22.07.2011 11:26, schrieb Frediano Ziglio:
> 2011/7/22 Kevin Wolf <kwolf@redhat.com>:
>> Am 20.07.2011 15:56, schrieb Frediano Ziglio:
>>> These patches mostly cleanup some AIO code using coroutines.
>>> These patches apply to Kevin's repository, branch coroutine-block.
>>> Mostly they use stack instead of allocated AIO structure.
>>>
>>> Frediano Ziglio (5):
>>>   qcow: allocate QCowAIOCB structure using stack
>>>   qcow: QCowAIOCB field cleanup
>>>   qcow: move some blocks of code to avoid useless variable
>>>     initialization
>>>   avoid dandling pointers
>>>   qcow: small optimization initializing QCowAIOCB
>>>
>>>  block/qcow.c  |  210 +++++++++++++++++++++++++--------------------------------
>>>  block/qcow2.c |   38 +++-------
>>>  2 files changed, 102 insertions(+), 146 deletions(-)
>>
>> Most of it looks good now. Did you include the "RFC" in the subject just
>> because the coroutine work is in RFC state, too, or did you intend to
>> tell me that I shouldn't merge yet?
>>
>> Kevin
>>
> 
> As these patches are first quite big patches I send (typo or small
> fixes do not counts) I just want to mark that I could write something
> really wrong. Just a way to avoid somebody having to send more patches
> and get more attention. Some projects are quite prone to merge even
> not that fine ones. I prefer to have some (a bit) pedantic comments
> and a real fix/improve.
> 
> Now I removed the RFC from last update. The main reason is that I
> found your qemu-iotests repository which, I think should be merged to
> main repository, but it's just my opinion.
> Oh... qcow fails 004 test (even origin/coroutines-block) with a I/O error.

Yup, you're right, I must have messed it up. Care to fix it or should I
look into it?

> I must say there are a lot of small hidden things that a developer
> should know about Qemu, for instance
> - mailing list follow some LKML rules as CC ML and send to maintainer
> to get more attention
> - you can use scripts/checkpatch.pl to check your patches before send
> 
> I still have also to understand how to use git format-patch/send-email
> correctly and "fluently" :)

That's true. Maybe you can update the wiki page with your findings?

The one suggestion I have for your use of git format-patch is
--subject-prefix="PATCH v3", so I don't get confused by the different
versions ;-)

Kevin

  reply	other threads:[~2011-07-22 10:07 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-20 13:56 [Qemu-devel] [RFC PATCH 0/5] qcow: coroutines cleanup Frediano Ziglio
2011-07-20 13:56 ` [Qemu-devel] [RFC PATCH 1/5] qcow: allocate QCowAIOCB structure using stack Frediano Ziglio
2011-07-20 13:56 ` [Qemu-devel] [RFC PATCH 2/5] qcow: QCowAIOCB field cleanup Frediano Ziglio
2011-07-22  7:01   ` Kevin Wolf
2011-07-20 13:56 ` [Qemu-devel] [RFC PATCH 3/5] qcow: move some blocks of code to avoid useless variable initialization Frediano Ziglio
2011-07-20 13:56 ` [Qemu-devel] [RFC PATCH 4/5] avoid dandling pointers Frediano Ziglio
2011-07-22  7:02   ` Kevin Wolf
2011-07-22  9:29     ` Frediano Ziglio
2011-07-20 13:56 ` [Qemu-devel] [RFC PATCH 5/5] qcow: small optimization initializing QCowAIOCB Frediano Ziglio
2011-07-22  7:04 ` [Qemu-devel] [RFC PATCH 0/5] qcow: coroutines cleanup Kevin Wolf
2011-07-22  9:26   ` Frediano Ziglio
2011-07-22 10:10     ` Kevin Wolf [this message]
2011-07-22 11:00       ` Stefan Hajnoczi
2011-07-22 13:24         ` Frediano Ziglio
2011-07-22 13:39           ` Kevin Wolf
2011-07-22 13:48           ` Gerd Hoffmann
2011-07-22 20:09       ` Frediano Ziglio
2011-07-25  7:53         ` Kevin Wolf

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=4E294C85.2020005@redhat.com \
    --to=kwolf@redhat.com \
    --cc=freddy77@gmail.com \
    --cc=qemu-devel@nongnu.org \
    /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).