[Qemu-devel] [PATCH v2] spapr: manage hotplugged devices while the VM is not started

[Qemu-devel] [PATCH v2] spapr: manage hotplugged devices while the VM is not started

[Laurent Vivier]

For QEMU, a hotlugged device is a device added using the HMP/QMP
interface.
For SPAPR, a hotplugged device is a device added while the
machine is running. In this case QEMU doesn't update internal
state but relies on the OS for this part

In the case of migration, when we (libvirt) hotplug a device
on the source guest, we (libvirt) generally hotplug the same
device on the destination guest. But in this case, the machine
is stopped (RUN_STATE_INMIGRATE) and QEMU must not expect
the OS will manage it as an hotplugged device as it will
be "imported" by the migration.

This patch changes the meaning of "hotplugged" in spapr.c
to manage a QEMU hotplugged device like a "coldplugged" one
when the machine is awaiting an incoming migration.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
v2:
- don't replace dev->hotplugged to test if CPU hotplug
  is supported.

 hw/ppc/spapr.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index ab3aab1..f0c543c 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2521,6 +2521,12 @@ static void spapr_nmi(NMIState *n, int cpu_index, Error **errp)
     }
 }
 
+static bool spapr_coldplugged(DeviceState *dev)
+{
+    return runstate_check(RUN_STATE_INMIGRATE) ||
+           !dev->hotplugged;
+}
+
 static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size,
                            uint32_t node, bool dedicated_hp_event_source,
                            Error **errp)
@@ -2531,6 +2537,7 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size,
     int i, fdt_offset, fdt_size;
     void *fdt;
     uint64_t addr = addr_start;
+    bool coldplugged = spapr_coldplugged(dev);
 
     for (i = 0; i < nr_lmbs; i++) {
         drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_LMB,
@@ -2542,9 +2549,9 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size,
                                                 SPAPR_MEMORY_BLOCK_SIZE);
 
         drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
-        drck->attach(drc, dev, fdt, fdt_offset, !dev->hotplugged, errp);
+        drck->attach(drc, dev, fdt, fdt_offset, coldplugged, errp);
         addr += SPAPR_MEMORY_BLOCK_SIZE;
-        if (!dev->hotplugged) {
+        if (coldplugged) {
             /* guests expect coldplugged LMBs to be pre-allocated */
             drck->set_allocation_state(drc, SPAPR_DR_ALLOCATION_STATE_USABLE);
             drck->set_isolation_state(drc, SPAPR_DR_ISOLATION_STATE_UNISOLATED);
@@ -2553,7 +2560,7 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size,
     /* send hotplug notification to the
      * guest only in case of hotplugged memory
      */
-    if (dev->hotplugged) {
+    if (!coldplugged) {
         if (dedicated_hp_event_source) {
             drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_LMB,
                     addr_start / SPAPR_MEMORY_BLOCK_SIZE);
@@ -2867,6 +2874,7 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
     int smt = kvmppc_smt_threads();
     CPUArchId *core_slot;
     int index;
+    bool coldplugged = spapr_coldplugged(dev);
 
     core_slot = spapr_find_cpu_slot(MACHINE(hotplug_dev), cc->core_id, &index);
     if (!core_slot) {
@@ -2888,7 +2896,7 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
 
     if (drc) {
         sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
-        drck->attach(drc, dev, fdt, fdt_offset, !dev->hotplugged, &local_err);
+        drck->attach(drc, dev, fdt, fdt_offset, coldplugged, &local_err);
         if (local_err) {
             g_free(fdt);
             error_propagate(errp, local_err);
@@ -2896,7 +2904,7 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
         }
     }
 
-    if (dev->hotplugged) {
+    if (!coldplugged) {
         /*
          * Send hotplug notification interrupt to the guest only in case
          * of hotplugged CPUs.
-- 
2.9.4

Re: [Qemu-devel] [Qemu-ppc] [PATCH v2] spapr: manage hotplugged devices while the VM is not started

[Greg Kurz]

[-- Attachment #1: Type: text/plain, Size: 4483 bytes --]

On Wed, 31 May 2017 11:25:16 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> For QEMU, a hotlugged device is a device added using the HMP/QMP
> interface.
> For SPAPR, a hotplugged device is a device added while the
> machine is running. In this case QEMU doesn't update internal
> state but relies on the OS for this part
> 
> In the case of migration, when we (libvirt) hotplug a device
> on the source guest, we (libvirt) generally hotplug the same
> device on the destination guest. But in this case, the machine
> is stopped (RUN_STATE_INMIGRATE) and QEMU must not expect
> the OS will manage it as an hotplugged device as it will
> be "imported" by the migration.
> 
> This patch changes the meaning of "hotplugged" in spapr.c
> to manage a QEMU hotplugged device like a "coldplugged" one
> when the machine is awaiting an incoming migration.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

> v2:
> - don't replace dev->hotplugged to test if CPU hotplug
>   is supported.
> 
>  hw/ppc/spapr.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index ab3aab1..f0c543c 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2521,6 +2521,12 @@ static void spapr_nmi(NMIState *n, int cpu_index, Error **errp)
>      }
>  }
>  
> +static bool spapr_coldplugged(DeviceState *dev)
> +{
> +    return runstate_check(RUN_STATE_INMIGRATE) ||
> +           !dev->hotplugged;
> +}
> +
>  static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size,
>                             uint32_t node, bool dedicated_hp_event_source,
>                             Error **errp)
> @@ -2531,6 +2537,7 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size,
>      int i, fdt_offset, fdt_size;
>      void *fdt;
>      uint64_t addr = addr_start;
> +    bool coldplugged = spapr_coldplugged(dev);
>  
>      for (i = 0; i < nr_lmbs; i++) {
>          drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_LMB,
> @@ -2542,9 +2549,9 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size,
>                                                  SPAPR_MEMORY_BLOCK_SIZE);
>  
>          drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> -        drck->attach(drc, dev, fdt, fdt_offset, !dev->hotplugged, errp);
> +        drck->attach(drc, dev, fdt, fdt_offset, coldplugged, errp);
>          addr += SPAPR_MEMORY_BLOCK_SIZE;
> -        if (!dev->hotplugged) {
> +        if (coldplugged) {
>              /* guests expect coldplugged LMBs to be pre-allocated */
>              drck->set_allocation_state(drc, SPAPR_DR_ALLOCATION_STATE_USABLE);
>              drck->set_isolation_state(drc, SPAPR_DR_ISOLATION_STATE_UNISOLATED);
> @@ -2553,7 +2560,7 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size,
>      /* send hotplug notification to the
>       * guest only in case of hotplugged memory
>       */
> -    if (dev->hotplugged) {
> +    if (!coldplugged) {
>          if (dedicated_hp_event_source) {
>              drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_LMB,
>                      addr_start / SPAPR_MEMORY_BLOCK_SIZE);
> @@ -2867,6 +2874,7 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
>      int smt = kvmppc_smt_threads();
>      CPUArchId *core_slot;
>      int index;
> +    bool coldplugged = spapr_coldplugged(dev);
>  
>      core_slot = spapr_find_cpu_slot(MACHINE(hotplug_dev), cc->core_id, &index);
>      if (!core_slot) {
> @@ -2888,7 +2896,7 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
>  
>      if (drc) {
>          sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> -        drck->attach(drc, dev, fdt, fdt_offset, !dev->hotplugged, &local_err);
> +        drck->attach(drc, dev, fdt, fdt_offset, coldplugged, &local_err);
>          if (local_err) {
>              g_free(fdt);
>              error_propagate(errp, local_err);
> @@ -2896,7 +2904,7 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
>          }
>      }
>  
> -    if (dev->hotplugged) {
> +    if (!coldplugged) {
>          /*
>           * Send hotplug notification interrupt to the guest only in case
>           * of hotplugged CPUs.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [Qemu-devel] [PATCH v2] spapr: manage hotplugged devices while the VM is not started

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH v2] spapr: manage hotplugged devices while the VM is not started
Type: series
Message-id: 20170531092516.5112-1-lvivier@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
fatal: unable to access 'https://github.com/patchew-project/qemu/': Failed to connect to github.com port 443: Connection timed out
error: Could not fetch 3c8cf5a9c21ff8782164d1def7f44bd888713384
Traceback (most recent call last):
  File "/usr/bin/patchew", line 409, in test_one
    git_clone_repo(clone, r["repo"], r["head"], logf)
  File "/usr/bin/patchew", line 48, in git_clone_repo
    stdout=logf, stderr=logf)
  File "/usr/lib64/python3.5/subprocess.py", line 581, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['git', 'remote', 'add', '-f', '--mirror=fetch', '3c8cf5a9c21ff8782164d1def7f44bd888713384', 'https://github.com/patchew-project/qemu']' returned non-zero exit status 1



---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

Re: [Qemu-devel] [PATCH v2] spapr: manage hotplugged devices while the VM is not started

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 5671 bytes --]

On Wed, May 31, 2017 at 11:25:16AM +0200, Laurent Vivier wrote:
> For QEMU, a hotlugged device is a device added using the HMP/QMP
> interface.
> For SPAPR, a hotplugged device is a device added while the
> machine is running. In this case QEMU doesn't update internal
> state but relies on the OS for this part
> 
> In the case of migration, when we (libvirt) hotplug a device
> on the source guest, we (libvirt) generally hotplug the same
> device on the destination guest. But in this case, the machine
> is stopped (RUN_STATE_INMIGRATE) and QEMU must not expect
> the OS will manage it as an hotplugged device as it will
> be "imported" by the migration.
> 
> This patch changes the meaning of "hotplugged" in spapr.c
> to manage a QEMU hotplugged device like a "coldplugged" one
> when the machine is awaiting an incoming migration.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> v2:
> - don't replace dev->hotplugged to test if CPU hotplug
>   is supported.

So, this addresses the specific points mentioned on the last version,
but not the wider query: what exactly in the previous behaviour was
causing problems.

On a related note, having better understood the DRC code while writing
my cleanups, I'm reconsidering your earlier suggestion of simply
disabling hotplugs until after CAS.

I rejected that approach before because I was assuming the problem was
due to hotplug events being lost, and I was confident that CAS marked
a transition from losing to not-losing events (other than by
accident).  However, later discussions have shown that the problem is
more to do with device tree updates being essentially duplicated
between the DT given to the guest during CAS and further updates from
hotplug events.

On that new understanding CAS does seem like a logical watershed,
since it's the point at which the guest gets the final version of the
"cold plugged" device tree.  I still have some concerns about the
details, but the basic approach seems sound.

Sorry for the misdirection.

> 
>  hw/ppc/spapr.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index ab3aab1..f0c543c 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2521,6 +2521,12 @@ static void spapr_nmi(NMIState *n, int cpu_index, Error **errp)
>      }
>  }
>  
> +static bool spapr_coldplugged(DeviceState *dev)
> +{
> +    return runstate_check(RUN_STATE_INMIGRATE) ||
> +           !dev->hotplugged;
> +}
> +
>  static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size,
>                             uint32_t node, bool dedicated_hp_event_source,
>                             Error **errp)
> @@ -2531,6 +2537,7 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size,
>      int i, fdt_offset, fdt_size;
>      void *fdt;
>      uint64_t addr = addr_start;
> +    bool coldplugged = spapr_coldplugged(dev);
>  
>      for (i = 0; i < nr_lmbs; i++) {
>          drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_LMB,
> @@ -2542,9 +2549,9 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size,
>                                                  SPAPR_MEMORY_BLOCK_SIZE);
>  
>          drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> -        drck->attach(drc, dev, fdt, fdt_offset, !dev->hotplugged, errp);
> +        drck->attach(drc, dev, fdt, fdt_offset, coldplugged, errp);
>          addr += SPAPR_MEMORY_BLOCK_SIZE;
> -        if (!dev->hotplugged) {
> +        if (coldplugged) {
>              /* guests expect coldplugged LMBs to be pre-allocated */
>              drck->set_allocation_state(drc, SPAPR_DR_ALLOCATION_STATE_USABLE);
>              drck->set_isolation_state(drc, SPAPR_DR_ISOLATION_STATE_UNISOLATED);
> @@ -2553,7 +2560,7 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size,
>      /* send hotplug notification to the
>       * guest only in case of hotplugged memory
>       */
> -    if (dev->hotplugged) {
> +    if (!coldplugged) {
>          if (dedicated_hp_event_source) {
>              drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_LMB,
>                      addr_start / SPAPR_MEMORY_BLOCK_SIZE);
> @@ -2867,6 +2874,7 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
>      int smt = kvmppc_smt_threads();
>      CPUArchId *core_slot;
>      int index;
> +    bool coldplugged = spapr_coldplugged(dev);
>  
>      core_slot = spapr_find_cpu_slot(MACHINE(hotplug_dev), cc->core_id, &index);
>      if (!core_slot) {
> @@ -2888,7 +2896,7 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
>  
>      if (drc) {
>          sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> -        drck->attach(drc, dev, fdt, fdt_offset, !dev->hotplugged, &local_err);
> +        drck->attach(drc, dev, fdt, fdt_offset, coldplugged, &local_err);
>          if (local_err) {
>              g_free(fdt);
>              error_propagate(errp, local_err);
> @@ -2896,7 +2904,7 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
>          }
>      }
>  
> -    if (dev->hotplugged) {
> +    if (!coldplugged) {
>          /*
>           * Send hotplug notification interrupt to the guest only in case
>           * of hotplugged CPUs.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH v2] spapr: manage hotplugged devices while the VM is not started

[Daniel Henrique Barboza]

On 06/09/2017 08:08 AM, Laurent Vivier wrote:
> For QEMU, a hotlugged device is a device added using the HMP/QMP
> interface.
> For SPAPR, a hotplugged device is a device added while the
> machine is running. In this case QEMU doesn't update internal
> state but relies on the OS for this part
>
> In the case of migration, when we (libvirt) hotplug a device
> on the source guest, we (libvirt) generally hotplug the same
> device on the destination guest. But in this case, the machine
> is stopped (RUN_STATE_INMIGRATE) and QEMU must not expect
> the OS will manage it as an hotplugged device as it will
> be "imported" by the migration.
>
> This patch changes the meaning of "hotplugged" in spapr.c
> to manage a QEMU hotplugged device like a "coldplugged" one
> when the machine is awaiting an incoming migration.
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> Reviewed-by: Greg Kurz <groug@kaod.org>
> ---
> v2: fix the minor nit reported by dgibson
>
>      I repost the patch as I think it is good to have
>      devices in a good state before migration (it is
>      generally a pre-requisite for migration),
>      so a hotplugged CPU before incoming migration
>      appears like a coldplugged one, and moreover
>      it fixes the use case in which CPU are hotplugged
>      on both side (src and dest), and we try to unplug
>      the CPU after the migration.

Reviewed-by: Daniel Barboza <danielhb@linux.vnet.ibm.com>
Tested-by: Daniel Barboza <danielhb@linux.vnet.ibm.com>


>
>   hw/ppc/spapr.c | 18 +++++++++++++-----
>   1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 91b4057..d5a3587 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2518,6 +2518,12 @@ static void spapr_nmi(NMIState *n, int cpu_index, Error **errp)
>       }
>   }
>
> +static bool spapr_coldplugged(DeviceState *dev)
> +{
> +    return runstate_check(RUN_STATE_INMIGRATE) ||
> +           !dev->hotplugged;
> +}
> +
>   static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size,
>                              uint32_t node, bool dedicated_hp_event_source,
>                              Error **errp)
> @@ -2528,6 +2534,7 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size,
>       int i, fdt_offset, fdt_size;
>       void *fdt;
>       uint64_t addr = addr_start;
> +    bool coldplugged = spapr_coldplugged(dev);
>
>       for (i = 0; i < nr_lmbs; i++) {
>           drc = spapr_drc_by_id(TYPE_SPAPR_DRC_LMB,
> @@ -2539,9 +2546,9 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size,
>                                                   SPAPR_MEMORY_BLOCK_SIZE);
>
>           drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> -        drck->attach(drc, dev, fdt, fdt_offset, !dev->hotplugged, errp);
> +        drck->attach(drc, dev, fdt, fdt_offset, coldplugged, errp);
>           addr += SPAPR_MEMORY_BLOCK_SIZE;
> -        if (!dev->hotplugged) {
> +        if (coldplugged) {
>               /* guests expect coldplugged LMBs to be pre-allocated */
>               drck->set_allocation_state(drc, SPAPR_DR_ALLOCATION_STATE_USABLE);
>               drck->set_isolation_state(drc, SPAPR_DR_ISOLATION_STATE_UNISOLATED);
> @@ -2550,7 +2557,7 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size,
>       /* send hotplug notification to the
>        * guest only in case of hotplugged memory
>        */
> -    if (dev->hotplugged) {
> +    if (!coldplugged) {
>           if (dedicated_hp_event_source) {
>               drc = spapr_drc_by_id(TYPE_SPAPR_DRC_LMB,
>                                     addr_start / SPAPR_MEMORY_BLOCK_SIZE);
> @@ -2863,6 +2870,7 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
>       int smt = kvmppc_smt_threads();
>       CPUArchId *core_slot;
>       int index;
> +    bool coldplugged = spapr_coldplugged(dev);
>
>       core_slot = spapr_find_cpu_slot(MACHINE(hotplug_dev), cc->core_id, &index);
>       if (!core_slot) {
> @@ -2884,7 +2892,7 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
>
>       if (drc) {
>           sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> -        drck->attach(drc, dev, fdt, fdt_offset, !dev->hotplugged, &local_err);
> +        drck->attach(drc, dev, fdt, fdt_offset, coldplugged, &local_err);
>           if (local_err) {
>               g_free(fdt);
>               error_propagate(errp, local_err);
> @@ -2892,7 +2900,7 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
>           }
>       }
>
> -    if (dev->hotplugged) {
> +    if (!coldplugged) {
>           /*
>            * Send hotplug notification interrupt to the guest only in case
>            * of hotplugged CPUs.

[Qemu-devel] [PATCH v2] spapr: manage hotplugged devices while the VM is not started

[Laurent Vivier]

For QEMU, a hotlugged device is a device added using the HMP/QMP
interface.
For SPAPR, a hotplugged device is a device added while the
machine is running. In this case QEMU doesn't update internal
state but relies on the OS for this part

In the case of migration, when we (libvirt) hotplug a device
on the source guest, we (libvirt) generally hotplug the same
device on the destination guest. But in this case, the machine
is stopped (RUN_STATE_INMIGRATE) and QEMU must not expect
the OS will manage it as an hotplugged device as it will
be "imported" by the migration.

This patch changes the meaning of "hotplugged" in spapr.c
to manage a QEMU hotplugged device like a "coldplugged" one
when the machine is awaiting an incoming migration.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
v2: fix the minor nit reported by dgibson

    I repost the patch as I think it is good to have
    devices in a good state before migration (it is
    generally a pre-requisite for migration), 
    so a hotplugged CPU before incoming migration
    appears like a coldplugged one, and moreover
    it fixes the use case in which CPU are hotplugged
    on both side (src and dest), and we try to unplug
    the CPU after the migration.

 hw/ppc/spapr.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 91b4057..d5a3587 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2518,6 +2518,12 @@ static void spapr_nmi(NMIState *n, int cpu_index, Error **errp)
     }
 }
 
+static bool spapr_coldplugged(DeviceState *dev)
+{
+    return runstate_check(RUN_STATE_INMIGRATE) ||
+           !dev->hotplugged;
+}
+
 static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size,
                            uint32_t node, bool dedicated_hp_event_source,
                            Error **errp)
@@ -2528,6 +2534,7 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size,
     int i, fdt_offset, fdt_size;
     void *fdt;
     uint64_t addr = addr_start;
+    bool coldplugged = spapr_coldplugged(dev);
 
     for (i = 0; i < nr_lmbs; i++) {
         drc = spapr_drc_by_id(TYPE_SPAPR_DRC_LMB,
@@ -2539,9 +2546,9 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size,
                                                 SPAPR_MEMORY_BLOCK_SIZE);
 
         drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
-        drck->attach(drc, dev, fdt, fdt_offset, !dev->hotplugged, errp);
+        drck->attach(drc, dev, fdt, fdt_offset, coldplugged, errp);
         addr += SPAPR_MEMORY_BLOCK_SIZE;
-        if (!dev->hotplugged) {
+        if (coldplugged) {
             /* guests expect coldplugged LMBs to be pre-allocated */
             drck->set_allocation_state(drc, SPAPR_DR_ALLOCATION_STATE_USABLE);
             drck->set_isolation_state(drc, SPAPR_DR_ISOLATION_STATE_UNISOLATED);
@@ -2550,7 +2557,7 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size,
     /* send hotplug notification to the
      * guest only in case of hotplugged memory
      */
-    if (dev->hotplugged) {
+    if (!coldplugged) {
         if (dedicated_hp_event_source) {
             drc = spapr_drc_by_id(TYPE_SPAPR_DRC_LMB,
                                   addr_start / SPAPR_MEMORY_BLOCK_SIZE);
@@ -2863,6 +2870,7 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
     int smt = kvmppc_smt_threads();
     CPUArchId *core_slot;
     int index;
+    bool coldplugged = spapr_coldplugged(dev);
 
     core_slot = spapr_find_cpu_slot(MACHINE(hotplug_dev), cc->core_id, &index);
     if (!core_slot) {
@@ -2884,7 +2892,7 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
 
     if (drc) {
         sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
-        drck->attach(drc, dev, fdt, fdt_offset, !dev->hotplugged, &local_err);
+        drck->attach(drc, dev, fdt, fdt_offset, coldplugged, &local_err);
         if (local_err) {
             g_free(fdt);
             error_propagate(errp, local_err);
@@ -2892,7 +2900,7 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
         }
     }
 
-    if (dev->hotplugged) {
+    if (!coldplugged) {
         /*
          * Send hotplug notification interrupt to the guest only in case
          * of hotplugged CPUs.
-- 
2.9.4