All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eduardo Habkost <ehabkost@redhat.com>
To: Igor Mammedov <imammedo@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>,
	peter.maydell@linaro.org, pkrempa@redhat.com, cohuck@redhat.com,
	qemu-devel@nongnu.org, pbonzini@redhat.com,
	david@gibson.dropbear.id.au
Subject: Re: [Qemu-devel] [PATCH v4 0/9] enable numa configuration before machine_init() from QMP
Date: Thu, 19 Apr 2018 16:42:51 -0300	[thread overview]
Message-ID: <20180419194251.GZ29865@localhost.localdomain> (raw)
In-Reply-To: <20180419100004.0b560c85@redhat.com>

On Thu, Apr 19, 2018 at 10:00:04AM +0200, Igor Mammedov wrote:
> On Wed, 18 Apr 2018 09:08:30 +0200
> Markus Armbruster <armbru@redhat.com> wrote:
> 
> > Eduardo Habkost <ehabkost@redhat.com> writes:
> > 
> > > On Tue, Apr 17, 2018 at 05:41:10PM +0200, Igor Mammedov wrote:  
> > >> On Tue, 17 Apr 2018 11:27:39 -0300
> > >> Eduardo Habkost <ehabkost@redhat.com> wrote:
> > >>   
> > >> > On Tue, Apr 17, 2018 at 04:13:34PM +0200, Markus Armbruster wrote:  
> > >> > > Igor Mammedov <imammedo@redhat.com> writes:
> > >> > > 
> > >> > > [...]    
> > >> > > > Series allows to configure NUMA mapping at runtime using QMP
> > >> > > > interface. For that to happen it introduces a new '-preconfig' CLI option
> > >> > > > which allows to pause QEMU before machine_init() is run and
> > >> > > > adds new set-numa-node QMP command which in conjunction with
> > >> > > > query-hotpluggable-cpus allows to configure NUMA mapping for cpus.
> > >> > > >
> > >> > > > Later we can modify other commands to run early, for example device_add.
> > >> > > > I recall SPAPR had problem when libvirt started QEMU with -S and, while it's
> > >> > > > paused, added CPUs with device_add. Intent was to coldplug CPUs (but at that
> > >> > > > stage it's considered hotplug already), so SPAPR had to work around the issue.    
> > >> > > 
> > >> > > That instance is just stupidity / laziness, I think: we consider any
> > >> > > plug after machine creation a hot plug.  Real machines remain cold until
> > >> > > you press the power button.  Our virtual machines should remain cold
> > >> > > until they start running, i.e. with -S until the first "cont".  
> > >> It probably would be too risky to change semantics of -S from hotplug to coldplug.
> > >> But even if we were easy it won't matter in case if dynamic configuration
> > >> done properly. More on it below.
> > >>   
> > >> > > I vaguely remember me asking this before, but your answer didn't make it
> > >> > > into this cover letter, which gives me a pretext to ask again instead of
> > >> > > looking it up in the archives: what exactly prevents us from keeping the
> > >> > > machine cold enough for numa configuration until the first "cont"?    
> > >> > 
> > >> > I also think this would be better, but it seems to be difficult
> > >> > in practice, see:
> > >> > http://mid.mail-archive.com/20180323210532.GD28161@localhost.localdomain  
> > >> 
> > >> In addition to Eduardo's reply, here is what I've answered back
> > >> when you've asked question the 1st time (v2 late at -S pause point reconfig):
> > >> https://www.mail-archive.com/qemu-devel@nongnu.org/msg504140.html
> > >> 
> > >> In short:
> > >> I think it's wrong in general doing fixups after machine is build
> > >> instead of getting correct configuration before building machine.
> > >> That's going to be complex and fragile and might be hard to do at
> > >> all depending on what we are fixing up.  
> > >
> > > What "building the machine" should mean, exactly, for external
> > > users?
> under "building machine", I've meant machine_run_board_init()
> and all follow up steps to machine_done stage.
> 
> > > The main question I'd like to see answered is: why exactly we
> > > must "build" the machine before the first "cont" is issued when
> > > using -S?  Why can't we delay everything to "cont" when using -S?  
> Nor sure what question is about,
> Did you mean if it were possible to postpone machine_run_board_init()
> and all later steps to -S/cont time?

Exactly.  In other words, what exactly must be done before the
monitor is available when using -S, and what exactly can be
postponed after "cont" when using -S?

>  
> > > Is it just because it's a long and complex task?  Does that mean
> > > we might still do that eventually, and eliminate the
> > > prelaunch/preconfig distinction in the distant future?  
> > 
> > Why would anyone want to use -S going forward?  For reasons other "we've
> > always used -S, and can't be bothered to change".
> We should be able to deprecate/remove -S once we can do all
> initial configuration that's possible to do there at
> preconfig time.

If the plan is to deprecate -S, what are the important
user-visible differences between -S and -preconfig today?  Do we
plan to eliminate all those differences before
deprecating/removing -S?

>  
> > > Even if we follow your approach, we need to answer these
> > > questions.  I'm sure we will try to reorder initialization steps
> > > between the preconfig/prelaunch states in the future, and we
> > > shouldn't break any expectations from external users when doing
> > > that.
> As minimum I expect -preconfig to be a runtime equivalent to CLI,
> with difference that it will be interactive and use QMP interface.
> As long as it sits between CLI parsing and the rest of initialization
> it shouldn't break that.

What prevents us from making -S a runtime equivalent to CLI?

> 
> > Moreover, the questions need to be answered in Git.  Commit message,
> > comments, docs/, use your judgement.
> I've thought that commit messages/patches were describing introduced
> changes sufficiently. But I've been sitting on these patches for
> a long time and what's obvious to me might be not so clear to others.
> I might just not see what's missing. Any suggestions to improve it
> are welcome.

I miss something that documents why both -S and -preconfig need
to exist, what are the differences between them today, and what
we plan to do about the differences between them in the future.

-- 
Eduardo

  reply	other threads:[~2018-04-19 19:43 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-12 13:11 [Qemu-devel] [PATCH v4 0/9] enable numa configuration before machine_init() from QMP Igor Mammedov
2018-03-12 13:11 ` [Qemu-devel] [PATCH v4 1/9] numa: postpone options post-processing till machine_run_board_init() Igor Mammedov
2018-03-23 20:34   ` Eduardo Habkost
2018-03-12 13:11 ` [Qemu-devel] [PATCH v4 2/9] numa: split out NumaOptions parsing into parse_NumaOptions() Igor Mammedov
2018-03-23 20:42   ` Eduardo Habkost
2018-03-23 20:49     ` Eric Blake
2018-03-23 21:09       ` Eduardo Habkost
2018-03-26  8:38       ` Laurent Vivier
2018-03-26 14:33         ` Eric Blake
2018-03-27 13:08     ` Igor Mammedov
2018-03-28 18:54       ` Eduardo Habkost
2018-03-29 13:05         ` Igor Mammedov
2018-03-29 16:31           ` Eduardo Habkost
2018-04-03 13:55             ` Igor Mammedov
2018-03-12 13:11 ` [Qemu-devel] [PATCH v4 3/9] cli: add -preconfig option Igor Mammedov
2018-03-23 21:02   ` Eric Blake
2018-03-23 21:05   ` Eduardo Habkost
2018-03-23 21:25   ` Eduardo Habkost
2018-03-27 15:05     ` Igor Mammedov
2018-03-28 11:48       ` Igor Mammedov
2018-03-28 19:21         ` Eduardo Habkost
2018-03-29 11:43           ` Igor Mammedov
2018-03-29 16:24             ` Eduardo Habkost
2018-04-03 14:32               ` Igor Mammedov
2018-04-03 15:31                 ` Eduardo Habkost
2018-04-04  8:51                   ` Igor Mammedov
2018-03-28 19:17       ` Eduardo Habkost
2018-03-29 13:01         ` Igor Mammedov
2018-03-29 16:57           ` Eduardo Habkost
2018-04-03 10:41             ` Peter Krempa
2018-04-03 13:49             ` Igor Mammedov
2018-04-03 13:52               ` Eduardo Habkost
2018-04-30 19:12                 ` Dr. David Alan Gilbert
2018-03-12 13:11 ` [Qemu-devel] [PATCH v4 4/9] hmp: disable monitor in preconfig state Igor Mammedov
2018-03-23 21:27   ` Eduardo Habkost
2018-03-28 11:16     ` Igor Mammedov
2018-03-28 18:55       ` Eduardo Habkost
2018-03-12 13:11 ` [Qemu-devel] [PATCH v4 5/9] qapi: introduce new cmd option "allowed-in-preconfig" Igor Mammedov
2018-03-23 21:11   ` Eric Blake
2018-03-28 15:23     ` Igor Mammedov
2018-03-23 21:28   ` Eduardo Habkost
2018-03-28 12:29     ` Igor Mammedov
2018-03-28 19:30       ` Eduardo Habkost
2018-03-29  9:53         ` Igor Mammedov
2018-03-29 12:21           ` Eduardo Habkost
2018-03-12 13:11 ` [Qemu-devel] [PATCH v4 6/9] tests: extend qmp test with preconfig checks Igor Mammedov
2018-03-12 13:11 ` [Qemu-devel] [PATCH v4 7/9] qmp: permit query-hotpluggable-cpus in preconfig state Igor Mammedov
2018-03-12 13:11 ` [Qemu-devel] [PATCH v4 8/9] qmp: add set-numa-node command Igor Mammedov
2018-03-12 13:11 ` [Qemu-devel] [PATCH v4 9/9] tests: functional tests for QMP command set-numa-node Igor Mammedov
2018-04-17 14:13 ` [Qemu-devel] [PATCH v4 0/9] enable numa configuration before machine_init() from QMP Markus Armbruster
2018-04-17 14:27   ` Eduardo Habkost
2018-04-17 15:41     ` Igor Mammedov
2018-04-17 20:41       ` Eduardo Habkost
2018-04-18  7:08         ` Markus Armbruster
2018-04-19  8:00           ` Igor Mammedov
2018-04-19 19:42             ` Eduardo Habkost [this message]
2018-04-20  6:31               ` Markus Armbruster
2018-04-23  9:50                 ` Igor Mammedov
2018-04-23 13:05                   ` Eduardo Habkost
2018-04-23 16:55                     ` Igor Mammedov
2018-04-23 20:45                       ` Eduardo Habkost
2018-04-26 14:39                         ` Igor Mammedov
2018-04-26 14:55                           ` Eric Blake
2018-04-27 12:19                             ` Igor Mammedov
2018-04-20  5:23   ` David Gibson

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=20180419194251.GZ29865@localhost.localdomain \
    --to=ehabkost@redhat.com \
    --cc=armbru@redhat.com \
    --cc=cohuck@redhat.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=imammedo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=pkrempa@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 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.