qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Peter Xu <peterx@redhat.com>
To: Eric Blake <eblake@redhat.com>
Cc: qemu-devel@nongnu.org, Stefan Hajnoczi <shajnocz@redhat.com>,
	"Daniel P . Berrange" <berrange@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>, Fam Zheng <famz@redhat.com>,
	Juan Quintela <quintela@redhat.com>,
	mdroth@linux.vnet.ibm.com, Laurent Vivier <lvivier@redhat.com>,
	Markus Armbruster <armbru@redhat.com>,
	marcandre.lureau@redhat.com,
	"Dr . David Alan Gilbert" <dgilbert@redhat.com>
Subject: Re: [Qemu-devel] [RFC v6 04/27] qobject: let object_property_get_str() use new API
Date: Thu, 11 Jan 2018 16:17:13 +0800	[thread overview]
Message-ID: <20180111081713.GA2551@xz-mi> (raw)
In-Reply-To: <bfc93abf-3d79-1c8b-ab64-372c0e50999a@redhat.com>

On Wed, Jan 10, 2018 at 06:59:13AM -0600, Eric Blake wrote:
> On 01/10/2018 01:57 AM, Peter Xu wrote:
> > On Tue, Jan 09, 2018 at 04:53:40PM -0600, Eric Blake wrote:
> >> On 12/19/2017 02:45 AM, Peter Xu wrote:
> >>> We can simplify object_property_get_str() using the new
> >>> qobject_get_try_str().
> >>>
> >>> Reviewed-by: Fam Zheng <famz@redhat.com>
> >>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> >>> Signed-off-by: Peter Xu <peterx@redhat.com>
> >>> ---
> >>>  qom/object.c | 9 +++------
> >>>  1 file changed, 3 insertions(+), 6 deletions(-)
> >>
> >> Reviewed-by: Eric Blake <eblake@redhat.com>
> >>
> >> I'm not opposed to your patch split (particularly since it makes
> >> backports easier if it just needs the new function and then your later
> >> uses of the new function, without touching existing uses); but I might
> >> have merged this with the previous patch so that the new API has a
> >> client right away, proving why the new API is worthwhile as part of its
> >> introduction.
> > 
> > Sure, I'll follow the rule next time.
> > 
> > (I can simply squash it but I got a few r-bs for separate patches
> >  already, so I'll keep them separated for now)
> 
> Keeping it separate is fine, especially since you have r-b.
> 
> Another factor in my personal workflow - if I'm going to be converting
> lots of users, but the users span over multiple areas of the code, then
> introducing the helper in one patch, and then a series of conversions to
> start using it, makes sense.  But if I'm converting just one existing
> user, that's a refactoring, and doing it all at once is easier (prior to
> adding the new code that becomes the second user).  I guess the moral of
> the story is that patch splitting is an art form, and there's more than
> one way to do things that still end up nice and reviewable.

Agreed.  I started to learn how to split patches since when I started
to work on QEMU, and obvious now I'm still learning. :-)

Thanks Eric.

-- 
Peter Xu

  reply	other threads:[~2018-01-11  8:17 UTC|newest]

Thread overview: 114+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-19  8:45 [Qemu-devel] [RFC v6 00/27] QMP: out-of-band (OOB) execution support Peter Xu
2017-12-19  8:45 ` [Qemu-devel] [RFC v6 01/27] chardev: use backend chr context when watch for fe Peter Xu
2017-12-20 16:40   ` Stefan Hajnoczi
2017-12-25  2:58     ` Peter Xu
2017-12-20 16:40   ` Stefan Hajnoczi
2017-12-19  8:45 ` [Qemu-devel] [RFC v6 02/27] qobject: introduce qstring_get_try_str() Peter Xu
2018-01-09 22:47   ` Eric Blake
2017-12-19  8:45 ` [Qemu-devel] [RFC v6 03/27] qobject: introduce qobject_get_try_str() Peter Xu
2018-01-09 22:50   ` Eric Blake
2018-01-10  7:52     ` Peter Xu
2017-12-19  8:45 ` [Qemu-devel] [RFC v6 04/27] qobject: let object_property_get_str() use new API Peter Xu
2018-01-09 22:53   ` Eric Blake
2018-01-10  7:57     ` Peter Xu
2018-01-10 12:59       ` Eric Blake
2018-01-11  8:17         ` Peter Xu [this message]
2017-12-19  8:45 ` [Qemu-devel] [RFC v6 05/27] monitor: move skip_flush into monitor_data_init Peter Xu
2017-12-19  8:45 ` [Qemu-devel] [RFC v6 06/27] monitor: move the cur_mon hack deeper for QMP Peter Xu
2017-12-20 16:42   ` Stefan Hajnoczi
2018-01-09 22:57   ` Eric Blake
2017-12-19  8:45 ` [Qemu-devel] [RFC v6 07/27] monitor: unify global init Peter Xu
2017-12-20 16:42   ` Stefan Hajnoczi
2018-01-09 23:13   ` Eric Blake
2018-01-10  8:26     ` Peter Xu
2018-01-10 12:54       ` Eric Blake
2018-01-11  8:18         ` Peter Xu
2017-12-19  8:45 ` [Qemu-devel] [RFC v6 08/27] monitor: let mon_list be tail queue Peter Xu
2017-12-19  8:45 ` [Qemu-devel] [RFC v6 09/27] monitor: create monitor dedicate iothread Peter Xu
2017-12-21  6:18   ` Fam Zheng
2018-01-05 16:23   ` Stefan Hajnoczi
2018-01-09 23:31   ` Eric Blake
2018-01-10  8:34     ` Peter Xu
2017-12-19  8:45 ` [Qemu-devel] [RFC v6 10/27] monitor: allow to use IO thread for parsing Peter Xu
2017-12-21  9:34   ` Fam Zheng
2018-01-05 17:22   ` Stefan Hajnoczi
2018-01-12  3:22     ` Peter Xu
2018-08-23 10:55     ` Marc-André Lureau
2018-01-09 23:37   ` Eric Blake
2018-01-12  3:23     ` Peter Xu
2017-12-19  8:45 ` [Qemu-devel] [RFC v6 11/27] qmp: introduce QMPCapability Peter Xu
2017-12-21  9:56   ` Fam Zheng
2017-12-25  3:16     ` Peter Xu
2018-01-08 16:23   ` Stefan Hajnoczi
2018-01-11 23:07   ` Eric Blake
2018-01-12  4:28     ` Peter Xu
2018-01-12 14:10       ` Eric Blake
2017-12-19  8:45 ` [Qemu-devel] [RFC v6 12/27] qmp: negotiate QMP capabilities Peter Xu
2017-12-21 10:01   ` Fam Zheng
2017-12-25  3:18     ` Peter Xu
2018-01-08 16:23   ` Stefan Hajnoczi
2018-01-12 20:57   ` Eric Blake
2018-01-22  7:29     ` Peter Xu
2017-12-19  8:45 ` [Qemu-devel] [RFC v6 13/27] monitor: introduce monitor_qmp_respond() Peter Xu
2018-01-08 16:25   ` Stefan Hajnoczi
2017-12-19  8:45 ` [Qemu-devel] [RFC v6 14/27] monitor: let suspend_cnt be thread safe Peter Xu
2018-01-12 21:48   ` Eric Blake
2018-01-22  7:43     ` Peter Xu
2017-12-19  8:45 ` [Qemu-devel] [RFC v6 15/27] monitor: let suspend/resume work even with QMPs Peter Xu
2017-12-21 11:27   ` Fam Zheng
2017-12-25  3:26     ` Peter Xu
2018-01-08 16:49       ` Stefan Hajnoczi
2018-01-12  4:51         ` Peter Xu
2018-01-12 14:28           ` Stefan Hajnoczi
2018-01-22  7:56             ` Peter Xu
2017-12-19  8:45 ` [Qemu-devel] [RFC v6 16/27] monitor: separate QMP parser and dispatcher Peter Xu
2017-12-21 11:40   ` Fam Zheng
2017-12-25  5:14     ` Peter Xu
2018-01-08 17:09   ` Stefan Hajnoczi
2018-01-12  6:05     ` Peter Xu
2018-01-12 14:22       ` Stefan Hajnoczi
2017-12-19  8:45 ` [Qemu-devel] [RFC v6 17/27] qmp: add new event "command-dropped" Peter Xu
2017-12-21 11:29   ` Fam Zheng
2018-01-09 13:20   ` Stefan Hajnoczi
2018-01-12  6:09     ` Peter Xu
2017-12-19  8:45 ` [Qemu-devel] [RFC v6 18/27] monitor: send event when command queue full Peter Xu
2017-12-21 11:42   ` Fam Zheng
2017-12-25  5:18     ` Peter Xu
2017-12-25  5:55       ` Fam Zheng
2017-12-25  6:18         ` Peter Xu
2017-12-25  7:13           ` Fam Zheng
2017-12-25  7:22             ` Peter Xu
2018-01-09 13:30               ` Stefan Hajnoczi
2018-01-09 13:42   ` Stefan Hajnoczi
2018-01-12  4:59     ` Peter Xu
2017-12-19  8:45 ` [Qemu-devel] [RFC v6 19/27] qapi: introduce new cmd option "allow-oob" Peter Xu
2017-12-21 11:45   ` Fam Zheng
2017-12-19  8:45 ` [Qemu-devel] [RFC v6 20/27] qmp: export qmp_dispatch_check_obj and allow "id" Peter Xu
2017-12-21 11:46   ` Fam Zheng
2018-01-09 13:45   ` Stefan Hajnoczi
2018-01-12  6:16     ` Peter Xu
2018-01-12 14:20       ` Stefan Hajnoczi
2018-01-22  8:42         ` Peter Xu
2017-12-19  8:45 ` [Qemu-devel] [RFC v6 21/27] qmp: support out-of-band (oob) execution Peter Xu
2017-12-21 11:54   ` Fam Zheng
2018-01-09 14:08   ` Stefan Hajnoczi
2018-01-12  6:23     ` Peter Xu
2018-01-12 14:18       ` Stefan Hajnoczi
2017-12-19  8:45 ` [Qemu-devel] [RFC v6 22/27] qmp: isolate responses into io thread Peter Xu
2017-12-21 12:57   ` Fam Zheng
2017-12-25  5:20     ` Peter Xu
2018-01-09 14:24   ` Stefan Hajnoczi
2018-01-12  6:44     ` Peter Xu
2018-01-12 14:16       ` Stefan Hajnoczi
2017-12-19  8:45 ` [Qemu-devel] [RFC v6 23/27] monitor: enable IO thread for (qmp & !mux) typed Peter Xu
2017-12-21 12:57   ` Fam Zheng
2018-01-09 14:24   ` Stefan Hajnoczi
2017-12-19  8:45 ` [Qemu-devel] [RFC v6 24/27] qmp: add command "x-oob-test" Peter Xu
2017-12-21 12:58   ` Fam Zheng
2017-12-19  8:45 ` [Qemu-devel] [RFC v6 25/27] docs: update QMP documents for OOB commands Peter Xu
2018-01-09 14:52   ` Stefan Hajnoczi
2018-01-12  6:54     ` Peter Xu
2017-12-19  8:45 ` [Qemu-devel] [RFC v6 26/27] tests: qmp-test: verify command batching Peter Xu
2017-12-19  8:45 ` [Qemu-devel] [RFC v6 27/27] tests: qmp-test: add oob test Peter Xu
2018-01-09 14:52 ` [Qemu-devel] [RFC v6 00/27] QMP: out-of-band (OOB) execution support Stefan Hajnoczi
2018-01-10  4:48   ` Peter Xu

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=20180111081713.GA2551@xz-mi \
    --to=peterx@redhat.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=eblake@redhat.com \
    --cc=famz@redhat.com \
    --cc=lvivier@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    --cc=shajnocz@redhat.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).