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
next prev parent 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.