v3->v4:
  * replace 'runstates' list in  QMP command with a single
    boolean 'ption allowed-in-preconfig' like it's done with
    'allow-oob'. Which allows to simplify intrusive QAPI
    changes quite a lot. (Eric Blake <eblake@redhat.com>)
  * Make sure HMP is disbled for real, v3 was just printing
    error mesage but allowing command to be executed
    ("Dr. David Alan Gilbert" <dgilbert@redhat.com>)
  * Improve "cli: add -preconfig option" commit message,
    explain a bit more on semantics of new state/option.
  * ithe rest of minor fixups suggested at v3 review
    (Eric Blake <eblake@redhat.com>)
    PS:
       havn't impl. test for new option in
         tests/qapi-schema/qapi-schema-test.json yet,
       can do it on top if approach is acceptable.
v1->v3:
  * introduce PRECONFIG runstate with -preconfig option.
    it's cleaner to manage transitions and do checks
    than reusing existing PRELAUNCH state.
  * extend QAPI schema commands with 'runstates' keyword,
    so that it would be possible to specify in command definition
    when it is valid to execute.
    (python changes a bit hackery, since I have little to
     no idea how it should work)
  * add preconfig QMP and set-numa-node tests
  * make mutually exclusive -preconfig and -incoming options,
    for simplicity sake. Shouldn't be problem as target can
    be starter with pure CLI, since mapping on source is
    already known.
  * Drop HMP part and leave only QMP in preconfig state.


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.

Example of configuration session:
$QEMU -smp 2 -preconfig ...

QMP:
# get CPUs layout for current target/machine/CLI
-> {'execute': 'query-hotpluggable-cpus' }  
<- {'return': [
       {'props': {'core-id': 0, 'thread-id': 0, 'socket-id': 1}, ... },
       {'props': {'core-id': 0, 'thread-id': 0, 'socket-id': 0}, ... }
   ]}

# configure 1st node
-> {'execute': 'set-numa-node', 'arguments': { 'type': 'node', 'nodeid': 0 } }  
<- {'return': {}}
-> {'execute': 'set-numa-node', 'arguments': { 'type': 'cpu',   
       'node-id': 0, 'core-id': 0, 'thread-id': 0, 'socket-id': 1, }
   }
<- {'return': {}}

# configure 2nd node
-> {'execute': 'set-numa-node', 'arguments': { 'type': 'node', 'nodeid': 1 } }
-> {'execute': 'set-numa-node', 'arguments': { 'type': 'cpu',  
       'node-id': 1, 'core-id': 0, 'thread-id': 0, 'socket-id': 0 }
   }
<- {'return': {}}

# [optional] verify configuration
-> {'execute': 'query-hotpluggable-cpus' }  
<- {'return': [
       {'props': {'core-id': 0, 'thread-id': 0, 'node-id': 0, 'socket-id': 1}, ... },
       {'props': {'core-id': 0, 'thread-id': 0, 'node-id': 1, 'socket-id': 0}, ... }
   ]}


Git tree:
    https://github.com/imammedo/qemu.git qmp_preconfig_v3

Ref to v1:
    https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg03583.html
    Message-Id: <1508170976-96869-1-git-send-email-imammedo@redhat.com>
  
CC: eblake@redhat.com
CC: armbru@redhat.com
CC: ehabkost@redhat.com
CC: pkrempa@redhat.com
CC: david@gibson.dropbear.id.au
CC: peter.maydell@linaro.org
CC: pbonzini@redhat.com
CC: cohuck@redhat.com


Igor Mammedov (9):
  numa: postpone options post-processing till machine_run_board_init()
  numa: split out NumaOptions parsing into parse_NumaOptions()
  cli: add -preconfig option
  hmp: disable monitor in preconfig state
  qapi: introduce new cmd option "allowed-in-preconfig"
  tests: extend qmp test with preconfig checks
  qmp: permit query-hotpluggable-cpus in preconfig state
  qmp: add set-numa-node command
  tests: functional tests for QMP command set-numa-node

 include/qapi/qmp/dispatch.h    |  3 +-
 include/sysemu/numa.h          |  2 ++
 include/sysemu/sysemu.h        |  1 +
 tests/libqtest.h               |  9 ++++++
 docs/devel/qapi-code-gen.txt   | 10 ++++++-
 hw/core/machine.c              |  5 ++--
 monitor.c                      | 10 +++++--
 numa.c                         | 66 +++++++++++++++++++++++++++---------------
 qapi/introspect.json           |  6 +++-
 qapi/misc.json                 | 24 ++++++++++++---
 qapi/qmp-dispatch.c            |  8 +++++
 qapi/run-state.json            |  8 +++--
 qemu-options.hx                | 13 +++++++++
 qmp.c                          |  5 ++++
 scripts/qapi/commands.py       | 19 ++++++++----
 scripts/qapi/common.py         | 15 ++++++----
 scripts/qapi/doc.py            |  2 +-
 scripts/qapi/introspect.py     | 10 +++++--
 tests/libqtest.c               |  7 +++++
 tests/numa-test.c              | 61 ++++++++++++++++++++++++++++++++++++++
 tests/qapi-schema/test-qapi.py |  2 +-
 tests/qmp-test.c               | 37 +++++++++++++++++++++++
 vl.c                           | 35 +++++++++++++++++++++-
 23 files changed, 306 insertions(+), 52 deletions(-)

-- 
2.7.4

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

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"?

>
> Example of configuration session:
> $QEMU -smp 2 -preconfig ...
>
> QMP:
> # get CPUs layout for current target/machine/CLI
> -> {'execute': 'query-hotpluggable-cpus' }  
> <- {'return': [
>        {'props': {'core-id': 0, 'thread-id': 0, 'socket-id': 1}, ... },
>        {'props': {'core-id': 0, 'thread-id': 0, 'socket-id': 0}, ... }
>    ]}
>
> # configure 1st node
> -> {'execute': 'set-numa-node', 'arguments': { 'type': 'node', 'nodeid': 0 } }  
> <- {'return': {}}
> -> {'execute': 'set-numa-node', 'arguments': { 'type': 'cpu',   
>        'node-id': 0, 'core-id': 0, 'thread-id': 0, 'socket-id': 1, }
>    }
> <- {'return': {}}
>
> # configure 2nd node
> -> {'execute': 'set-numa-node', 'arguments': { 'type': 'node', 'nodeid': 1 } }
> -> {'execute': 'set-numa-node', 'arguments': { 'type': 'cpu',  
>        'node-id': 1, 'core-id': 0, 'thread-id': 0, 'socket-id': 0 }
>    }
> <- {'return': {}}
>
> # [optional] verify configuration
> -> {'execute': 'query-hotpluggable-cpus' }  
> <- {'return': [
>        {'props': {'core-id': 0, 'thread-id': 0, 'node-id': 0, 'socket-id': 1}, ... },
>        {'props': {'core-id': 0, 'thread-id': 0, 'node-id': 1, 'socket-id': 0}, ... }
>    ]}
>
>
> Git tree:
>     https://github.com/imammedo/qemu.git qmp_preconfig_v3
>
> Ref to v1:
>     https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg03583.html
>     Message-Id: <1508170976-96869-1-git-send-email-imammedo@redhat.com>

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

-- 
Eduardo

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.

BTW this is an outdated version of series and there is a newer one v5
https://patchwork.ozlabs.org/cover/895315/
so pleases review it.

Short diff vs 1:
 - only limited(minimum) set of commands is available at preconfig stage for now
 - use QAPI schema to mark commands as preconfig enabled,
   so mgmt could see when it can use commands.
 - added preconfig runstate state-machine instead of adding more global variables
   to cleanly keep track of where QEMU is paused and what it's allowed to do

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?

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?

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?

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.

> 
> BTW this is an outdated version of series and there is a newer one v5
> https://patchwork.ozlabs.org/cover/895315/
> so pleases review it.
> 
> Short diff vs 1:
>  - only limited(minimum) set of commands is available at preconfig stage for now
>  - use QAPI schema to mark commands as preconfig enabled,
>    so mgmt could see when it can use commands.
>  - added preconfig runstate state-machine instead of adding more global variables
>    to cleanly keep track of where QEMU is paused and what it's allowed to do

-- 
Eduardo

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?
>
> 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?

Exactly.

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

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

Moreover, the questions need to be answered in Git.  Commit message,
comments, docs/, use your judgement.

>> BTW this is an outdated version of series and there is a newer one v5
>> https://patchwork.ozlabs.org/cover/895315/

Sorry about that.  I'm drowning in a sea of two months worth of patches.

>> so pleases review it.
>> 
>> Short diff vs 1:
>>  - only limited(minimum) set of commands is available at preconfig stage for now
>>  - use QAPI schema to mark commands as preconfig enabled,
>>    so mgmt could see when it can use commands.
>>  - added preconfig runstate state-machine instead of adding more global variables
>>    to cleanly keep track of where QEMU is paused and what it's allowed to do

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

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

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

Eduardo Habkost <ehabkost@redhat.com> writes:

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

This sounds like there are things we can do with -S but can't
--preconfig now.  Is that correct?

If yes, I got another question.  If you want to configure NUMA, you need
to use --preconfig.  If you also want to configure one of the things you
can't with --preconfig, you need to use -S.  In other words, you need to
use both.  How would that work?

> 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?

Documentation (including -help) needs to provide clear guidance on what
to use when.

>> > > 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?

Good question.  Igor?

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

Keep in mind that your poor reviewers context-switch to your patches
from somewhere else entirely, then read (mostly) linearly, starting with
the cover letter.  If the cover letter leads to questions and confusion,
the reading likely stops.  Answers in the commit messages won't do you a
lick of good then.

It's really, really useful to state the case for the feature right in
the cover letter.  "What" and "why", not "how".  Write for your
audience!  The cover letter's audience knows nothing (first order
approximation).  Don't make them guess.  Especially not "why".  Keep it
high-level.  Pointers to (specific!) commit messages for additional
detail can make sense.

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

Par for the course, don't feel bad about it.

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

Yes, please.  Additionally, guidance for users on which of them to use.

On Fri, 20 Apr 2018 08:31:18 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > 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?  
(1)
As David said -S pause point is practically breakpoint on some
instruction of built/existing machine and current monitor commands
expect it to be valid. Moving -S before machine_run_board_init()
will break semantics of current -S pause point (i.e. user expectation
on existing machine) as well as most of the commands that evolved
in environment where machine already existed.

Hence a new -preconfig option and runstate to avoid breaking
exiting users and being able to cleanly handle configuration that
affects machine_run_board_init().

> > Exactly.  In other words, what exactly must be done before the
> > monitor is available when using -S,
for MUST, it should be commands that affect machine_run_board_init()
like being added set-numa-node

> > and what exactly can be postponed after "cont" when using -S?
hotplug configuration and various runtime query commands that
expect built machine. (today it's most of the commands)

wrt configuration commands we should split them into coldplug
and hotplug ones (some could be both).
   
> >> > > 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.  
> 
> This sounds like there are things we can do with -S but can't
> --preconfig now.  Is that correct?
yes, we can't do at --preconfig time anything that requires built machine.

> If yes, I got another question.  If you want to configure NUMA, you need
> to use --preconfig.  If you also want to configure one of the things you
> can't with --preconfig, you need to use -S.  In other words, you need to
> use both.  How would that work?
It works and described in commit message of
"[PATCH for-2.13 v5 03/11] cli: add --preconfig option"
https://patchwork.kernel.org/patch/10323879/

"
When early introspection/configuration is done, command 'cont' should
be used to exit RUN_STATE_PRECONFIG and transition to the next
requested state (i.e. if -S is used then QEMU will pause the second
time when board/device initialization is completed or start guest
execution if -S isn't provided on CLI)
"
> 
> > 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?
we probably won't be able to deprecate -S in foreseeable future,
for that we would need to be able to do everything starting from
machine_run_board_init() to current pause point.
But we can gradually move configuration commands to -preconfig time
and gradually add CLI equivalents for that aren't possible at -S time
(like Paolo suggested picking to be used machine model at runtime)

> Documentation (including -help) needs to provide clear guidance on what
> to use when.
for -help part the same '[PATCH for-2.13 v5 03/11] cli: add --preconfig option'
As for commands that are allowed to run at --preconfig time, it is
documented in QAPI schema
'[PATCH for-2.13 v5 05/11] qapi: introduce new cmd  option "allowed-in-preconfig"'
http://patchwork.ozlabs.org/patch/895314/

> >> > > 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?  
Possibly [1] earlier answers it.

> Good question.  Igor?
> 
> >> > 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.  
> 
> Keep in mind that your poor reviewers context-switch to your patches
> from somewhere else entirely, then read (mostly) linearly, starting with
> the cover letter.  If the cover letter leads to questions and confusion,
> the reading likely stops.  Answers in the commit messages won't do you a
> lick of good then.
> 
> It's really, really useful to state the case for the feature right in
> the cover letter.  "What" and "why", not "how".  Write for your
> audience!  The cover letter's audience knows nothing (first order
> approximation).  Don't make them guess.  Especially not "why".  Keep it
> high-level.  Pointers to (specific!) commit messages for additional
> detail can make sense.
I'll try to write better cover letter if I end up respinning v5

> >>                       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.  
> 
> Par for the course, don't feel bad about it.
> 
> >> 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.
Where would you prefer it being documented?

> Yes, please.  Additionally, guidance for users on which of them to use.

On Mon, Apr 23, 2018 at 11:50:16AM +0200, Igor Mammedov wrote:
> On Fri, 20 Apr 2018 08:31:18 +0200
> Markus Armbruster <armbru@redhat.com> wrote:
> 
> > Eduardo Habkost <ehabkost@redhat.com> writes:
> > 
> > > 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?  
> (1)
> As David said -S pause point is practically breakpoint on some
> instruction of built/existing machine and current monitor commands
> expect it to be valid. Moving -S before machine_run_board_init()
> will break semantics of current -S pause point (i.e. user expectation
> on existing machine) as well as most of the commands that evolved
> in environment where machine already existed.

OK, so what's missing here is a clear description what the user
can expect on -S.

> 
> Hence a new -preconfig option and runstate to avoid breaking
> exiting users and being able to cleanly handle configuration that
> affects machine_run_board_init().
> 
> > > Exactly.  In other words, what exactly must be done before the
> > > monitor is available when using -S,
> for MUST, it should be commands that affect machine_run_board_init()
> like being added set-numa-node
> 
> > > and what exactly can be postponed after "cont" when using -S?
> hotplug configuration and various runtime query commands that
> expect built machine. (today it's most of the commands)
> 
> wrt configuration commands we should split them into coldplug
> and hotplug ones (some could be both).
>    
> > >> > > 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.  
> > 
> > This sounds like there are things we can do with -S but can't
> > --preconfig now.  Is that correct?
> yes, we can't do at --preconfig time anything that requires built machine.

"built machine" is a very broad description.  We need to specify
more clearly what "built machine" means for an external user.
Does it mean having the QOM tree available?  Does it mean having
the VCPU threads created?  Without defining what -S really must
provide, we won't be able to deprecate and replace it.


> 
[...]
> > > 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?
> we probably won't be able to deprecate -S in foreseeable future,
> for that we would need to be able to do everything starting from
> machine_run_board_init() to current pause point.
> But we can gradually move configuration commands to -preconfig time
> and gradually add CLI equivalents for that aren't possible at -S time
> (like Paolo suggested picking to be used machine model at runtime)

This could be a good plan, if we can explain why exactly -S is
still needed.

> 
[...]
> > >>                       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.  
> > 
> > Par for the course, don't feel bad about it.
> > 
> > >> 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.
> Where would you prefer it being documented?

I suggest qemu-options.hx and/or qemu-doc.texi.

BTW, "cont" is documented as "Resume guest VCPU execution", which
is not true when using preconfig.  Maybe it's better to add a
separate QMP command for "create machine and devices" instead of
overloading the semantics of "cont"?

-- 
Eduardo

On Mon, 23 Apr 2018 10:05:54 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Mon, Apr 23, 2018 at 11:50:16AM +0200, Igor Mammedov wrote:
> > On Fri, 20 Apr 2018 08:31:18 +0200
> > Markus Armbruster <armbru@redhat.com> wrote:
> >   
> > > Eduardo Habkost <ehabkost@redhat.com> writes:
> > >   
> > > > 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?    
> > (1)
> > As David said -S pause point is practically breakpoint on some
> > instruction of built/existing machine and current monitor commands
> > expect it to be valid. Moving -S before machine_run_board_init()
> > will break semantics of current -S pause point (i.e. user expectation
> > on existing machine) as well as most of the commands that evolved
> > in environment where machine already existed.  
> 
> OK, so what's missing here is a clear description what the user
> can expect on -S.
Currently it's fully configured machine with all CLI options taken
in account in paused state in initial state or with state it is getting
from migration stream if -incoming were used in combination with -S.

> > Hence a new -preconfig option and runstate to avoid breaking
> > exiting users and being able to cleanly handle configuration that
> > affects machine_run_board_init().
> >   
> > > > Exactly.  In other words, what exactly must be done before the
> > > > monitor is available when using -S,  
> > for MUST, it should be commands that affect machine_run_board_init()
> > like being added set-numa-node
> >   
> > > > and what exactly can be postponed after "cont" when using -S?  
> > hotplug configuration and various runtime query commands that
> > expect built machine. (today it's most of the commands)
> > 
> > wrt configuration commands we should split them into coldplug
> > and hotplug ones (some could be both).
> >      
> > > >> > > 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.    
> > > 
> > > This sounds like there are things we can do with -S but can't
> > > --preconfig now.  Is that correct?  
> > yes, we can't do at --preconfig time anything that requires built machine.  
> 
> "built machine" is a very broad description.  We need to specify
> more clearly what "built machine" means for an external user.
> Does it mean having the QOM tree available?  Does it mean having
> the VCPU threads created?  Without defining what -S really must
> provide, we won't be able to deprecate and replace it.
(*2) how about s/built machine/machine ready to execute guest code/,
that's what it is now.


> > > > 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?  
> > we probably won't be able to deprecate -S in foreseeable future,
> > for that we would need to be able to do everything starting from
> > machine_run_board_init() to current pause point.
> > But we can gradually move configuration commands to -preconfig time
> > and gradually add CLI equivalents for that aren't possible at -S time
> > (like Paolo suggested picking to be used machine model at runtime)  
> 
> This could be a good plan, if we can explain why exactly -S is
> still needed.
For a while -S would be need at least for compat reasons, if we ever
get to point where at -preconfig time machine could be build up to the
point -S provides[2] then we can talk about deprecating it, for now it's
way too premature to do something about it /I mean documenting intent
which is not there yet and might never materialize as there is no real
demand to deprecate it/.

> [...]
> > > >>                       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.    
> > > 
> > > Par for the course, don't feel bad about it.
> > >   
> > > >> 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.  
> > Where would you prefer it being documented?  
> 
> I suggest qemu-options.hx and/or qemu-doc.texi.
Regarding qemu-options.hx patch
 "[PATCH for-2.13 v5 03/11] cli: add --preconfig option" 
adds doc text describing --preconfig option with explanation of how
'cont' could be used (including in combination with -S).

I'll try to come up with a text for qemu-doc.texi, not about
deprecating -S but about when --preconfig should be used vs -S
and where to get list of commands that could be used at preconfig state.

> BTW, "cont" is documented as "Resume guest VCPU execution", which
> is not true when using preconfig.  Maybe it's better to add a
> separate QMP command for "create machine and devices" instead of
> overloading the semantics of "cont"?
My bad, I've missed it, I can fixup 'cont' description to match
its behavior with --preconfig taken in account.

I'm not so sure about adding a new command is better though, I recall
Markus being against adding new commands unless we have to,
but I don't have strong inclination both ways so it's up to you.

I'm more inclined towards reusing 'cont', it seems logical 
(/me looking from the point if I were user).

On Mon, Apr 23, 2018 at 06:55:14PM +0200, Igor Mammedov wrote:
> On Mon, 23 Apr 2018 10:05:54 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > On Mon, Apr 23, 2018 at 11:50:16AM +0200, Igor Mammedov wrote:
> > > On Fri, 20 Apr 2018 08:31:18 +0200
> > > Markus Armbruster <armbru@redhat.com> wrote:
> > >   
> > > > Eduardo Habkost <ehabkost@redhat.com> writes:
> > > >   
> > > > > 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?    
> > > (1)
> > > As David said -S pause point is practically breakpoint on some
> > > instruction of built/existing machine and current monitor commands
> > > expect it to be valid. Moving -S before machine_run_board_init()
> > > will break semantics of current -S pause point (i.e. user expectation
> > > on existing machine) as well as most of the commands that evolved
> > > in environment where machine already existed.  
> > 
> > OK, so what's missing here is a clear description what the user
> > can expect on -S.
> Currently it's fully configured machine with all CLI options taken
> in account in paused state in initial state or with state it is getting
> from migration stream if -incoming were used in combination with -S.
> 
> > > Hence a new -preconfig option and runstate to avoid breaking
> > > exiting users and being able to cleanly handle configuration that
> > > affects machine_run_board_init().
> > >   
> > > > > Exactly.  In other words, what exactly must be done before the
> > > > > monitor is available when using -S,  
> > > for MUST, it should be commands that affect machine_run_board_init()
> > > like being added set-numa-node
> > >   
> > > > > and what exactly can be postponed after "cont" when using -S?  
> > > hotplug configuration and various runtime query commands that
> > > expect built machine. (today it's most of the commands)
> > > 
> > > wrt configuration commands we should split them into coldplug
> > > and hotplug ones (some could be both).
> > >      
> > > > >> > > 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.    
> > > > 
> > > > This sounds like there are things we can do with -S but can't
> > > > --preconfig now.  Is that correct?  
> > > yes, we can't do at --preconfig time anything that requires built machine.  
> > 
> > "built machine" is a very broad description.  We need to specify
> > more clearly what "built machine" means for an external user.
> > Does it mean having the QOM tree available?  Does it mean having
> > the VCPU threads created?  Without defining what -S really must
> > provide, we won't be able to deprecate and replace it.
> (*2) how about s/built machine/machine ready to execute guest code/,
> that's what it is now.

This is a bit better, we still need to be clear about what
"ready" means.  e.g.: can users expect the VCPU threads be
already running?

Anyway, the details don't need to be sorted out immediately.  IMO
the most important part is to describe the difference between
-preconfig and -S.

> 
> 
> > > > > 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?  
> > > we probably won't be able to deprecate -S in foreseeable future,
> > > for that we would need to be able to do everything starting from
> > > machine_run_board_init() to current pause point.
> > > But we can gradually move configuration commands to -preconfig time
> > > and gradually add CLI equivalents for that aren't possible at -S time
> > > (like Paolo suggested picking to be used machine model at runtime)  
> > 
> > This could be a good plan, if we can explain why exactly -S is
> > still needed.
> For a while -S would be need at least for compat reasons, if we ever
> get to point where at -preconfig time machine could be build up to the
> point -S provides[2] then we can talk about deprecating it, for now it's
> way too premature to do something about it /I mean documenting intent
> which is not there yet and might never materialize as there is no real
> demand to deprecate it/.

Yeah, compatibility is the main reason we can't simply deprecate
or remove -S immediately.  We just need to find out what exactly
is important on -S.


> 
> > [...]
> > > > >>                       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.    
> > > > 
> > > > Par for the course, don't feel bad about it.
> > > >   
> > > > >> 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.  
> > > Where would you prefer it being documented?  
> > 
> > I suggest qemu-options.hx and/or qemu-doc.texi.
> Regarding qemu-options.hx patch
>  "[PATCH for-2.13 v5 03/11] cli: add --preconfig option" 
> adds doc text describing --preconfig option with explanation of how
> 'cont' could be used (including in combination with -S).
> 
> I'll try to come up with a text for qemu-doc.texi, not about
> deprecating -S but about when --preconfig should be used vs -S
> and where to get list of commands that could be used at preconfig state.

Sounds good to me.  Thanks!


> 
> > BTW, "cont" is documented as "Resume guest VCPU execution", which
> > is not true when using preconfig.  Maybe it's better to add a
> > separate QMP command for "create machine and devices" instead of
> > overloading the semantics of "cont"?
> My bad, I've missed it, I can fixup 'cont' description to match
> its behavior with --preconfig taken in account.
> 
> I'm not so sure about adding a new command is better though, I recall
> Markus being against adding new commands unless we have to,
> but I don't have strong inclination both ways so it's up to you.
> 
> I'm more inclined towards reusing 'cont', it seems logical 
> (/me looking from the point if I were user).

'cont' seemed logical to me at first, until I read its
documentation.  Then I think it makes things very confusing,
especially if we combine -preconfig with -S and/or -incoming.

A separate command would have less room for ambiguity.

-- 
Eduardo

On Mon, 23 Apr 2018 17:45:31 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Mon, Apr 23, 2018 at 06:55:14PM +0200, Igor Mammedov wrote:
> > On Mon, 23 Apr 2018 10:05:54 -0300
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> >   
> > > On Mon, Apr 23, 2018 at 11:50:16AM +0200, Igor Mammedov wrote:  
> > > > On Fri, 20 Apr 2018 08:31:18 +0200
> > > > Markus Armbruster <armbru@redhat.com> wrote:
> > > >     
> > > > > Eduardo Habkost <ehabkost@redhat.com> writes:
> > > > >     
> > > > > > 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?      
> > > > (1)
> > > > As David said -S pause point is practically breakpoint on some
> > > > instruction of built/existing machine and current monitor commands
> > > > expect it to be valid. Moving -S before machine_run_board_init()
> > > > will break semantics of current -S pause point (i.e. user expectation
> > > > on existing machine) as well as most of the commands that evolved
> > > > in environment where machine already existed.    
> > > 
> > > OK, so what's missing here is a clear description what the user
> > > can expect on -S.  
> > Currently it's fully configured machine with all CLI options taken
> > in account in paused state in initial state or with state it is getting
> > from migration stream if -incoming were used in combination with -S.
> >   
> > > > Hence a new -preconfig option and runstate to avoid breaking
> > > > exiting users and being able to cleanly handle configuration that
> > > > affects machine_run_board_init().
> > > >     
> > > > > > Exactly.  In other words, what exactly must be done before the
> > > > > > monitor is available when using -S,    
> > > > for MUST, it should be commands that affect machine_run_board_init()
> > > > like being added set-numa-node
> > > >     
> > > > > > and what exactly can be postponed after "cont" when using -S?    
> > > > hotplug configuration and various runtime query commands that
> > > > expect built machine. (today it's most of the commands)
> > > > 
> > > > wrt configuration commands we should split them into coldplug
> > > > and hotplug ones (some could be both).
> > > >        
> > > > > >> > > 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.      
> > > > > 
> > > > > This sounds like there are things we can do with -S but can't
> > > > > --preconfig now.  Is that correct?    
> > > > yes, we can't do at --preconfig time anything that requires built machine.    
> > > 
> > > "built machine" is a very broad description.  We need to specify
> > > more clearly what "built machine" means for an external user.
> > > Does it mean having the QOM tree available?  Does it mean having
> > > the VCPU threads created?  Without defining what -S really must
> > > provide, we won't be able to deprecate and replace it.  
> > (*2) how about s/built machine/machine ready to execute guest code/,
> > that's what it is now.  
> 
> This is a bit better, we still need to be clear about what
> "ready" means.  e.g.: can users expect the VCPU threads be
> already running?
> 
> Anyway, the details don't need to be sorted out immediately.  IMO
> the most important part is to describe the difference between
> -preconfig and -S.
> 
> > 
> >   
> > > > > > 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?    
> > > > we probably won't be able to deprecate -S in foreseeable future,
> > > > for that we would need to be able to do everything starting from
> > > > machine_run_board_init() to current pause point.
> > > > But we can gradually move configuration commands to -preconfig time
> > > > and gradually add CLI equivalents for that aren't possible at -S time
> > > > (like Paolo suggested picking to be used machine model at runtime)    
> > > 
> > > This could be a good plan, if we can explain why exactly -S is
> > > still needed.  
> > For a while -S would be need at least for compat reasons, if we ever
> > get to point where at -preconfig time machine could be build up to the
> > point -S provides[2] then we can talk about deprecating it, for now it's
> > way too premature to do something about it /I mean documenting intent
> > which is not there yet and might never materialize as there is no real
> > demand to deprecate it/.  
> 
> Yeah, compatibility is the main reason we can't simply deprecate
> or remove -S immediately.  We just need to find out what exactly
> is important on -S.
> 
> 
> >   
> > > [...]  
> > > > > >>                       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.      
> > > > > 
> > > > > Par for the course, don't feel bad about it.
> > > > >     
> > > > > >> 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.    
> > > > Where would you prefer it being documented?    
> > > 
> > > I suggest qemu-options.hx and/or qemu-doc.texi.  
> > Regarding qemu-options.hx patch
> >  "[PATCH for-2.13 v5 03/11] cli: add --preconfig option" 
> > adds doc text describing --preconfig option with explanation of how
> > 'cont' could be used (including in combination with -S).
> > 
> > I'll try to come up with a text for qemu-doc.texi, not about
> > deprecating -S but about when --preconfig should be used vs -S
> > and where to get list of commands that could be used at preconfig state.  
> 
> Sounds good to me.  Thanks!
how about something like this:

diff --git a/qemu-tech.texi b/qemu-tech.texi
index 52a56ae..6951258 100644
--- a/qemu-tech.texi
+++ b/qemu-tech.texi
@@ -5,6 +5,7 @@
 * CPU emulation::
 * Translator Internals::
 * QEMU compared to other emulators::
+* Managed start up options::
 * Bibliography::
 @end menu
 
@@ -314,6 +315,44 @@ VirtualBox [9], Xen [10] and KVM [11] are based on QEMU. QEMU-SystemC
 [12] uses QEMU to simulate a system where some hardware devices are
 developed in SystemC.
 
+@node Managed start up options
+@section Managed start up options
+
+In system mode emulation, it's possible to create VM in paused state using
+-S command line option. In this state the machine is completely initialized
+according to command line options and ready to execute VM code but VCPU threads
+are not executing any code. VM state in this paused state depends on way QEMU
+was started. It could be in:
+@table @asis
+@item initial state (after reset/power on state)
+@item with direct kernel loading initial state could be ammended to execute
+code loaded by QEMU in VM's RAM and with incomming migration
+@item with incomming migrartion, initial state will by ammended by the migrated
+machine state after migration completes.
+@end table
+
+This paused state is typically used by users to query machine state and/or
+additionally configure machine (hotplug devices) in runtime before allowing
+VM code to run.
+
+However at -S pause point it's impossible to configure options that affect
+initial VM creation (like: -smp/-m/-numa ...) or cold plug devices. That's
+when -preconfig command line option should be used. It allows to pause
+QEMU before initial VM creation in preconfig state, query being created
+VM at runtime and configure start up options depending on previous query
+results. In preconfig state QEMU allows to configure VM only via QMP monitor
+with a limited command set which doesn't depend on completely initialized
+machine, which includes but not limited to:
+@table @asis
+@item qmp_capabilities
+@item query-qmp-schema
+@item query-commands
+@item query-status
+@end table
+The full list of commands is in QMP schema which could be queried with
+query-qmp-schema, where commands supported at preconfig state have option
+'allowed-in-preconfig' set to true.
+
 @node Bibliography
 @section Bibliography

> >   
> > > BTW, "cont" is documented as "Resume guest VCPU execution", which
> > > is not true when using preconfig.  Maybe it's better to add a
> > > separate QMP command for "create machine and devices" instead of
> > > overloading the semantics of "cont"?  
> > My bad, I've missed it, I can fixup 'cont' description to match
> > its behavior with --preconfig taken in account.
> > 
> > I'm not so sure about adding a new command is better though, I recall
> > Markus being against adding new commands unless we have to,
> > but I don't have strong inclination both ways so it's up to you.
> > 
> > I'm more inclined towards reusing 'cont', it seems logical 
> > (/me looking from the point if I were user).  
> 
> 'cont' seemed logical to me at first, until I read its
> documentation.  Then I think it makes things very confusing,
> especially if we combine -preconfig with -S and/or -incoming.
> 
> A separate command would have less room for ambiguity.
I've added following instead of reusing 'cont':

##                                                                               
# @exit-preconfig:                                                               
#                                                                                
# Exit from "preconfig" state                                                    
#                                                                                
# Since 2.13                                                                     
#                                                                                
# Returns: nothing                                                               
#                                                                                
# Notes: Command makes QEMU exit from preconfig state and proceeds with          
# VM initialization using configuration data provided on command line            
# and via QMP monitor at preconfig state. Command is available only at           
# preconfig state (i.e. if --preconfig command line option).                     
#                                                                                
# Example:                                                                       
#                                                                                
# -> { "execute": "exit-preconfig" }                                             
# <- { "return": {} }                                                            
#                                                                                
##                                                                               
{ 'command': 'exit-preconfig', 'allowed-in-preconfig': true } 

On 04/26/2018 09:39 AM, Igor Mammedov wrote:

>>> I'll try to come up with a text for qemu-doc.texi, not about
>>> deprecating -S but about when --preconfig should be used vs -S
>>> and where to get list of commands that could be used at preconfig state.  
>>
>> Sounds good to me.  Thanks!
> how about something like this:
> 
> diff --git a/qemu-tech.texi b/qemu-tech.texi
> index 52a56ae..6951258 100644
> --- a/qemu-tech.texi
> +++ b/qemu-tech.texi
> @@ -5,6 +5,7 @@
>  * CPU emulation::
>  * Translator Internals::
>  * QEMU compared to other emulators::
> +* Managed start up options::
>  * Bibliography::
>  @end menu
>  
> @@ -314,6 +315,44 @@ VirtualBox [9], Xen [10] and KVM [11] are based on QEMU. QEMU-SystemC
>  [12] uses QEMU to simulate a system where some hardware devices are
>  developed in SystemC.
>  
> +@node Managed start up options
> +@section Managed start up options
> +
> +In system mode emulation, it's possible to create VM in paused state using
> +-S command line option. In this state the machine is completely initialized
> +according to command line options and ready to execute VM code but VCPU threads
> +are not executing any code. VM state in this paused state depends on way QEMU
> +was started. It could be in:
> +@table @asis
> +@item initial state (after reset/power on state)
> +@item with direct kernel loading initial state could be ammended to execute

s/ammended/amended/

> +code loaded by QEMU in VM's RAM and with incomming migration

s/incomming/incoming/

> +@item with incomming migrartion, initial state will by ammended by the migrated

and again, for both words
s/migrartion/migration/

> +machine state after migration completes.
> +@end table
> +
> +This paused state is typically used by users to query machine state and/or
> +additionally configure machine (hotplug devices) in runtime before allowing
> +VM code to run.
> +
> +However at -S pause point it's impossible to configure options that affect
> +initial VM creation (like: -smp/-m/-numa ...) or cold plug devices. That's
> +when -preconfig command line option should be used. It allows to pause

s/to pause/pausing/

> +QEMU before initial VM creation in preconfig state, query being created
> +VM at runtime and configure start up options depending on previous query

Reads awkwardly.  Maybe:

It allows pausing QEMU before the initial VM creation, in a new
preconfig state, where additional queries and configuration can be
performed via QMP before moving on to the resulting configuration startup.

> +results. In preconfig state QEMU allows to configure VM only via QMP monitor
> +with a limited command set which doesn't depend on completely initialized
> +machine, which includes but not limited to:

In the preconfig state, QEMU only allows a limited set of commands over
the QMP monitor, where the commands do not depend on an initialized
machine, including but not limited to:

> +@table @asis
> +@item qmp_capabilities
> +@item query-qmp-schema
> +@item query-commands
> +@item query-status
> +@end table
> +The full list of commands is in QMP schema which could be queried with
> +query-qmp-schema, where commands supported at preconfig state have option
> +'allowed-in-preconfig' set to true.
> +
>  @node Bibliography
>  @section Bibliography
> 

>> A separate command would have less room for ambiguity.
> I've added following instead of reusing 'cont':
> 
> ##                                                                               
> # @exit-preconfig:                                                               

This should definitely be mentioned in the docs section of commands
permitted during preconfig.

> #                                                                                
> # Exit from "preconfig" state                                                    
> #                                                                                
> # Since 2.13                                                                     
> #                                                                                
> # Returns: nothing                                                               
> #                                                                                
> # Notes: Command makes QEMU exit from preconfig state and proceeds with          
> # VM initialization using configuration data provided on command line            
> # and via QMP monitor at preconfig state. Command is available only at           
> # preconfig state (i.e. if --preconfig command line option).                     
> #                                                                                
> # Example:                                                                       
> #                                                                                
> # -> { "execute": "exit-preconfig" }                                             
> # <- { "return": {} }                                                            
> #                                                                                
> ##                                                                               
> { 'command': 'exit-preconfig', 'allowed-in-preconfig': true } 
> 
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

On Thu, 26 Apr 2018 09:55:40 -0500
Eric Blake <eblake@redhat.com> wrote:

> On 04/26/2018 09:39 AM, Igor Mammedov wrote:
> 
> >>> I'll try to come up with a text for qemu-doc.texi, not about
> >>> deprecating -S but about when --preconfig should be used vs -S
> >>> and where to get list of commands that could be used at preconfig state.    
> >>
> >> Sounds good to me.  Thanks!  
> > how about something like this:
> > 
> > diff --git a/qemu-tech.texi b/qemu-tech.texi
> > index 52a56ae..6951258 100644
> > --- a/qemu-tech.texi
> > +++ b/qemu-tech.texi
> > @@ -5,6 +5,7 @@
> >  * CPU emulation::
> >  * Translator Internals::
> >  * QEMU compared to other emulators::
> > +* Managed start up options::
> >  * Bibliography::
> >  @end menu
> >  
> > @@ -314,6 +315,44 @@ VirtualBox [9], Xen [10] and KVM [11] are based on QEMU. QEMU-SystemC
> >  [12] uses QEMU to simulate a system where some hardware devices are
> >  developed in SystemC.
> >  
> > +@node Managed start up options
> > +@section Managed start up options
> > +
> > +In system mode emulation, it's possible to create VM in paused state using
> > +-S command line option. In this state the machine is completely initialized
> > +according to command line options and ready to execute VM code but VCPU threads
> > +are not executing any code. VM state in this paused state depends on way QEMU
> > +was started. It could be in:
> > +@table @asis
> > +@item initial state (after reset/power on state)
> > +@item with direct kernel loading initial state could be ammended to execute  
> 
> s/ammended/amended/
> 
> > +code loaded by QEMU in VM's RAM and with incomming migration  
> 
> s/incomming/incoming/
> 
> > +@item with incomming migrartion, initial state will by ammended by the migrated  
> 
> and again, for both words
> s/migrartion/migration/
> 
> > +machine state after migration completes.
> > +@end table
> > +
> > +This paused state is typically used by users to query machine state and/or
> > +additionally configure machine (hotplug devices) in runtime before allowing
> > +VM code to run.
> > +
> > +However at -S pause point it's impossible to configure options that affect
> > +initial VM creation (like: -smp/-m/-numa ...) or cold plug devices. That's
> > +when -preconfig command line option should be used. It allows to pause  
> 
> s/to pause/pausing/
> 
> > +QEMU before initial VM creation in preconfig state, query being created
> > +VM at runtime and configure start up options depending on previous query  
> 
> Reads awkwardly.  Maybe:
> 
> It allows pausing QEMU before the initial VM creation, in a new
> preconfig state, where additional queries and configuration can be
> performed via QMP before moving on to the resulting configuration startup.
> 
> > +results. In preconfig state QEMU allows to configure VM only via QMP monitor
> > +with a limited command set which doesn't depend on completely initialized
> > +machine, which includes but not limited to:  
> 
> In the preconfig state, QEMU only allows a limited set of commands over
> the QMP monitor, where the commands do not depend on an initialized
> machine, including but not limited to:

thanks for review,
I amended patches as you suggested.
I plan to respin v6 today with these changes since exit-preconfig
affected several patches in series.

> 
> > +@table @asis
> > +@item qmp_capabilities
> > +@item query-qmp-schema
> > +@item query-commands
> > +@item query-status
> > +@end table
> > +The full list of commands is in QMP schema which could be queried with
> > +query-qmp-schema, where commands supported at preconfig state have option
> > +'allowed-in-preconfig' set to true.
> > +
> >  @node Bibliography
> >  @section Bibliography
> >   
> 
> >> A separate command would have less room for ambiguity.  
> > I've added following instead of reusing 'cont':
> > 
> > ##                                                                               
> > # @exit-preconfig:                                                                 
> 
> This should definitely be mentioned in the docs section of commands
> permitted during preconfig.
done

> 
> > #                                                                                
> > # Exit from "preconfig" state                                                    
> > #                                                                                
> > # Since 2.13                                                                     
> > #                                                                                
> > # Returns: nothing                                                               
> > #                                                                                
> > # Notes: Command makes QEMU exit from preconfig state and proceeds with          
> > # VM initialization using configuration data provided on command line            
> > # and via QMP monitor at preconfig state. Command is available only at           
> > # preconfig state (i.e. if --preconfig command line option).                     
> > #                                                                                
> > # Example:                                                                       
> > #                                                                                
> > # -> { "execute": "exit-preconfig" }                                             
> > # <- { "return": {} }                                                            
> > #                                                                                
> > ##                                                                               
> > { 'command': 'exit-preconfig', 'allowed-in-preconfig': true } 
> > 
> >   
> 

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

Makes sense to me.  As I recall, the chief problem I had here with
PAPR hotplug was that -S stopped *after* the machine_reset, which
meant the only mechanisms we really had to inform the guest of the
hardware were the hotplug mechanisms.  Sort of.  It got complicated
because of the feature negotiation system that PAPR guests have.

At present -S doesn't really operate like a "stop before hitting the
power", it instead acts more like a breakpoint on the first
instruction of the firmware.

If we were to move the -S stop to before the machine_reset, I believe
that might simplify several things in our hotplug handling code for
papr.

> 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"?
> 
> >
> > Example of configuration session:
> > $QEMU -smp 2 -preconfig ...
> >
> > QMP:
> > # get CPUs layout for current target/machine/CLI
> > -> {'execute': 'query-hotpluggable-cpus' }  
> > <- {'return': [
> >        {'props': {'core-id': 0, 'thread-id': 0, 'socket-id': 1}, ... },
> >        {'props': {'core-id': 0, 'thread-id': 0, 'socket-id': 0}, ... }
> >    ]}
> >
> > # configure 1st node
> > -> {'execute': 'set-numa-node', 'arguments': { 'type': 'node', 'nodeid': 0 } }  
> > <- {'return': {}}
> > -> {'execute': 'set-numa-node', 'arguments': { 'type': 'cpu',   
> >        'node-id': 0, 'core-id': 0, 'thread-id': 0, 'socket-id': 1, }
> >    }
> > <- {'return': {}}
> >
> > # configure 2nd node
> > -> {'execute': 'set-numa-node', 'arguments': { 'type': 'node', 'nodeid': 1 } }
> > -> {'execute': 'set-numa-node', 'arguments': { 'type': 'cpu',  
> >        'node-id': 1, 'core-id': 0, 'thread-id': 0, 'socket-id': 0 }
> >    }
> > <- {'return': {}}
> >
> > # [optional] verify configuration
> > -> {'execute': 'query-hotpluggable-cpus' }  
> > <- {'return': [
> >        {'props': {'core-id': 0, 'thread-id': 0, 'node-id': 0, 'socket-id': 1}, ... },
> >        {'props': {'core-id': 0, 'thread-id': 0, 'node-id': 1, 'socket-id': 0}, ... }
> >    ]}
> >
> >
> > Git tree:
> >     https://github.com/imammedo/qemu.git qmp_preconfig_v3
> >
> > Ref to v1:
> >     https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg03583.html
> >     Message-Id: <1508170976-96869-1-git-send-email-imammedo@redhat.com>
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson