If the OS is not started, QEMU sends an event to the OS
that is lost and cannot be recovered. An unplug is not
able to restore QEMU in a coherent state.
So, while the OS is not started, disable CPU and memory hotplug.
We guess the OS is started if the CAS has been negotiated.

This series also revert previous fix which was not really fixing
the hotplug problem when the OS is not running.

v2:
- fix indent
- remove useless local_err
- allow hotplug if the VM is not started (pre-launch or incoming state)
- remove vector 6, instead just mark the end of CAS negotiation

Laurent Vivier (2):
  spapr: disable hotplugging without OS
  Revert "spapr: fix memory hot-unplugging"

 hw/ppc/spapr.c             | 51 +++++++++++++++++++++++++++++++++++++++++++---
 hw/ppc/spapr_drc.c         | 20 +++---------------
 hw/ppc/spapr_hcall.c       |  1 +
 include/hw/ppc/spapr.h     |  2 ++
 include/hw/ppc/spapr_drc.h |  1 -
 5 files changed, 54 insertions(+), 21 deletions(-)

-- 
2.9.4

On 06/08/2017 02:27 PM, Laurent Vivier wrote:
> If the OS is not started, QEMU sends an event to the OS
> that is lost and cannot be recovered. An unplug is not
> able to restore QEMU in a coherent state.
> So, while the OS is not started, disable CPU and memory hotplug.
> We guess the OS is started if the CAS has been negotiated.
>
> This series also revert previous fix which was not really fixing
> the hotplug problem when the OS is not running.
>
> v2:
> - fix indent
> - remove useless local_err
> - allow hotplug if the VM is not started (pre-launch or incoming state)
> - remove vector 6, instead just mark the end of CAS negotiation
>
> Laurent Vivier (2):
>    spapr: disable hotplugging without OS
>    Revert "spapr: fix memory hot-unplugging"
>
>   hw/ppc/spapr.c             | 51 +++++++++++++++++++++++++++++++++++++++++++---
>   hw/ppc/spapr_drc.c         | 20 +++---------------
>   hw/ppc/spapr_hcall.c       |  1 +
>   include/hw/ppc/spapr.h     |  2 ++
>   include/hw/ppc/spapr_drc.h |  1 -
>   5 files changed, 54 insertions(+), 21 deletions(-)
>
Tested-by: Daniel Barboza <danielhb@linux.vnet.ibm.com>

This is curious. I was having a little look into hotplug/unplug and DRC 
states yesterday
while taking a look at the latest David's cleanup. I was trying to find 
out a way to tune
spapr_drc_needed() in a way that we don't accidentally migrate more DRCs 
than necessary
and at the same time handle that scenario of libvirt migration after 
hotplug (libvirt hotplugs
the device on target instead of adding it in the command line).

I would like to migrate the DRCs if and only if:

1 - a device was hotplugged
2 - a device is undergoing hotplug/unplug

(2) is easy and is already taken care of. But (1) is tricky because, 
before this patch, if a migration
occurs *before* CAS we don't need to migrate the DRC - the object will 
go through the state
changes after the migration. With this series this scenario is not going 
to happen, then
we can happily add a

if (dev->hotplugged) return true;

in spapr_drc_needed and fix the libvirt migration scenario again.



Daniel

On Thu, Jun 08, 2017 at 03:35:58PM -0300, Daniel Henrique Barboza wrote:
> 
> 
> On 06/08/2017 02:27 PM, Laurent Vivier wrote:
> > If the OS is not started, QEMU sends an event to the OS
> > that is lost and cannot be recovered. An unplug is not
> > able to restore QEMU in a coherent state.
> > So, while the OS is not started, disable CPU and memory hotplug.
> > We guess the OS is started if the CAS has been negotiated.
> > 
> > This series also revert previous fix which was not really fixing
> > the hotplug problem when the OS is not running.
> > 
> > v2:
> > - fix indent
> > - remove useless local_err
> > - allow hotplug if the VM is not started (pre-launch or incoming state)
> > - remove vector 6, instead just mark the end of CAS negotiation
> > 
> > Laurent Vivier (2):
> >    spapr: disable hotplugging without OS
> >    Revert "spapr: fix memory hot-unplugging"
> > 
> >   hw/ppc/spapr.c             | 51 +++++++++++++++++++++++++++++++++++++++++++---
> >   hw/ppc/spapr_drc.c         | 20 +++---------------
> >   hw/ppc/spapr_hcall.c       |  1 +
> >   include/hw/ppc/spapr.h     |  2 ++
> >   include/hw/ppc/spapr_drc.h |  1 -
> >   5 files changed, 54 insertions(+), 21 deletions(-)
> > 
> Tested-by: Daniel Barboza <danielhb@linux.vnet.ibm.com>
> 
> This is curious. I was having a little look into hotplug/unplug and DRC
> states yesterday
> while taking a look at the latest David's cleanup. I was trying to find out
> a way to tune
> spapr_drc_needed() in a way that we don't accidentally migrate more DRCs
> than necessary
> and at the same time handle that scenario of libvirt migration after hotplug
> (libvirt hotplugs
> the device on target instead of adding it in the command line).
> 
> I would like to migrate the DRCs if and only if:
> 
> 1 - a device was hotplugged
> 2 - a device is undergoing hotplug/unplug
> 
> (2) is easy and is already taken care of. But (1) is tricky because, before
> this patch, if a migration
> occurs *before* CAS we don't need to migrate the DRC - the object will go
> through the state
> changes after the migration. With this series this scenario is not going to
> happen, then
> we can happily add a
> 
> if (dev->hotplugged) return true;
> 
> in spapr_drc_needed and fix the libvirt migration scenario again.

As mentioned in another thread, I'm looking at explicitly resetting
all DRC states during the CAS process.  I think that will simplify
these cases.

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