qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
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?

  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).