qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Max Reitz <mreitz@redhat.com>, Markus Armbruster <armbru@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	qemu-devel@nongnu.org, Anthony Liguori <aliguori@amazon.com>,
	"Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: [Qemu-devel] [PATCH] virtio-scsi: Allocate op blocker reason before blocking
Date: Tue, 24 Feb 2015 09:24:08 -0700	[thread overview]
Message-ID: <54ECA5A8.3070304@redhat.com> (raw)
In-Reply-To: <54EC849C.4000400@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 1691 bytes --]

On 02/24/2015 07:03 AM, Max Reitz wrote:
> On 2015-02-24 at 04:43, Markus Armbruster wrote:
>> Max Reitz <mreitz@redhat.com> writes:
>>
>>> s->blocker is really only used in hw/scsi/virtio-scsi.c; the only places
>>> where it is used in hw/scsi/virtio-scsi-dataplane.c is when it is
>>> allocated and when it is freed. That does not make a whole lot of sense
>>> (and is actually wrong because this leads to s->blocker potentially
>>> being NULL when blk_op_block_all() is called in virtio-scsi.c), so move
>>> the allocation and destruction of s->blocker to blk_op_block_all() and
>>> blk_op_unblock_all() in virtio-scsi.c, respectively.
>>>
>> Why do you put your nice reproducer below the --- divider?  I rather
>> like bug fixing commits come with reproducers in the commit message.

As do I :)

>>
>> [...]
> 
> Because then I'm afraid that Eric complains because I used echo -e
> instead of printf.

I might point out the non-portability, but for a commit message, I'm
perfectly fine leaving a non-portable construct in place.  I'm only
going to ask for a respin if 'echo -e' is used in the patch body.

> 
> Seriously speaking, I don't mind putting it into the commit message.
> I'll wait for reviews on the change itself, and then either send a v2
> with the reproducer included in the commit message or hope for a
> maintainer to fix it up himself (which I'd be totally fine with *hint
> hint*).

Up to the maintainer on that front, but if it helps, then with the
amended commit message:

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

  reply	other threads:[~2015-02-24 16:24 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-23 22:58 [Qemu-devel] [PATCH] virtio-scsi: Allocate op blocker reason before blocking Max Reitz
2015-02-24  9:43 ` Markus Armbruster
2015-02-24 14:03   ` Max Reitz
2015-02-24 16:24     ` Eric Blake [this message]
2015-02-25  6:22 ` Fam Zheng

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=54ECA5A8.3070304@redhat.com \
    --to=eblake@redhat.com \
    --cc=aliguori@amazon.com \
    --cc=armbru@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.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).