From: Markus Armbruster <armbru@redhat.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jiri Pirko <jiri@resnulli.us>, Jason Wang <jasowang@redhat.com>,
Cao jin <caoj.fnst@cn.fujitsu.com>,
qemu-devel@nongnu.org, Dmitry Fleytman <dmitry@daynix.com>,
Alex Williamson <alex.williamson@redhat.com>,
Hannes Reinecke <hare@suse.de>,
Marcel Apfelbaum <marcel@redhat.com>,
Paolo Bonzini <pbonzini@redhat.com>,
Gerd Hoffmann <kraxel@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v7 00/10] Convert msix_init() to error
Date: Tue, 10 Jan 2017 17:19:07 +0100 [thread overview]
Message-ID: <87pojuc2h0.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <20170110162151-mutt-send-email-mst@kernel.org> (Michael S. Tsirkin's message of "Tue, 10 Jan 2017 16:38:00 +0200")
"Michael S. Tsirkin" <mst@redhat.com> writes:
> On Tue, Jan 10, 2017 at 11:06:08AM +0100, Markus Armbruster wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>>
>> > On Mon, Nov 14, 2016 at 03:25:30PM +0800, Cao jin wrote:
>> >> v7 changelog:
>> >> 1. fix the segfaut bug in patch 2. So drop the all the R-b of it,
>> >> please take a look, there is detailed description in the patch.
>> >> 2. add the R-b from Hannes Reinecke
>> >>
>> >> Test:
>> >> 1. make check: pass
>> >> 2. After applied all the patch, command line test for all the
>> >> affected devices, just make sure device realize process is ok,
>> >> no crash, but no further use of device.
>> >
>> > Consider the megasas device for example, don't you
>> > need to test that the change actually does what
>> > it's intended to do?
>>
>> For better or worse, that's a higher bar than we commonly require for
>> refactorings.
>>
>> [...]
>
> Well the patch says that it's addressing a TODO. If no one can
> be bothered to test the functionality, maybe we shouldn't bother
> with the change.
>
> Generally this patchset is at v7. It brings a very limited benefit to
> the project. It better be perfect otherwise I don't see why bother.
We obviously disagree on the benefit. Before this series, error
reporting is *broken* for QMP. Fixing that is definitely not "why
bother" material.
I certainly don't object to tightening our testing habits. I was merely
pointing out that you're doing that.
Testing patches that modernize interfaces often isn't easy for the
person doing the work. Fortunately, megasas has a maintainer: Hannes.
Who gave his R-by. For me, that would suffice, but you may see things
differently. Would a Tested-by from him satisfy you? If not, what
would satisfy you?
next prev parent reply other threads:[~2017-01-10 16:19 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-14 7:25 [Qemu-devel] [PATCH v7 00/10] Convert msix_init() to error Cao jin
2016-11-14 7:25 ` [Qemu-devel] [PATCH v7 01/10] msix: Follow CODING_STYLE Cao jin
2016-11-14 7:25 ` [Qemu-devel] [PATCH v7 02/10] hcd-xhci: check & correct param before using it Cao jin
2016-11-17 13:48 ` Markus Armbruster
2016-11-14 7:25 ` [Qemu-devel] [PATCH v7 03/10] pci: Convert msix_init() to Error and fix callers to check it Cao jin
2017-01-10 17:54 ` Michael S. Tsirkin
2017-01-10 17:58 ` Paolo Bonzini
2017-01-10 19:21 ` Michael S. Tsirkin
2017-01-11 9:55 ` Markus Armbruster
2017-01-12 15:08 ` Michael S. Tsirkin
2017-01-12 14:58 ` Michael S. Tsirkin
2016-11-14 7:25 ` [Qemu-devel] [PATCH v7 04/10] megasas: change behaviour of msix switch Cao jin
2016-11-14 7:25 ` [Qemu-devel] [PATCH v7 05/10] hcd-xhci: " Cao jin
2016-11-14 7:25 ` [Qemu-devel] [PATCH v7 06/10] megasas: remove unnecessary megasas_use_msix() Cao jin
2016-11-14 7:25 ` [Qemu-devel] [PATCH v7 07/10] megasas: undo the overwrites of msi user configuration Cao jin
2016-11-14 7:25 ` [Qemu-devel] [PATCH v7 08/10] vmxnet3: fix reference leak issue Cao jin
2016-11-14 7:25 ` [Qemu-devel] [PATCH v7 09/10] vmxnet3: remove unnecessary internal msix flag Cao jin
2016-11-14 7:25 ` [Qemu-devel] [PATCH v7 10/10] msi_init: convert assert to return -errno Cao jin
2016-11-14 23:22 ` [Qemu-devel] [PATCH v7 00/10] Convert msix_init() to error Michael S. Tsirkin
2016-12-21 6:16 ` Cao jin
2016-12-21 10:43 ` Marcel Apfelbaum
2017-01-09 21:45 ` Michael S. Tsirkin
2017-01-10 10:06 ` Markus Armbruster
2017-01-10 14:38 ` Michael S. Tsirkin
2017-01-10 15:38 ` Paolo Bonzini
2017-01-10 16:19 ` Markus Armbruster [this message]
2017-01-10 16:28 ` Paolo Bonzini
2017-01-11 15:25 ` Cao jin
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=87pojuc2h0.fsf@dusky.pond.sub.org \
--to=armbru@redhat.com \
--cc=alex.williamson@redhat.com \
--cc=caoj.fnst@cn.fujitsu.com \
--cc=dmitry@daynix.com \
--cc=hare@suse.de \
--cc=jasowang@redhat.com \
--cc=jiri@resnulli.us \
--cc=kraxel@redhat.com \
--cc=marcel@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).