Re: [PATCH v9 07/10] s390x/cpu_topology: CPU topology migration

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 9/28/22 10:34, Pierre Morel wrote:


On 9/8/22 20:04, Janis Schoetterl-Glausch wrote:
On Fri, 2022-09-02 at 09:55 +0200, Pierre Morel wrote:
The migration can only take place if both source and destination
of the migration both use or both do not use the CPU topology
facility.

We indicate a change in topology during migration postload for the
case the topology changed between source and destination.

You always set the report bit after migration, right?
In the last series you actually migrated the bit.
Why the change? With the code you have actually migrating the bit isn't
hard.

As for the moment the vCPU do not migrate from real CPU I thought based on the remark of Nico that there is no need to set the bit after a migration.


Signed-off-by: Pierre Morel <pmorel@xxxxxxxxxxxxx>
---
  hw/s390x/cpu-topology.c         | 79 +++++++++++++++++++++++++++++++++
  include/hw/s390x/cpu-topology.h |  1 +
  target/s390x/cpu-sysemu.c       |  8 ++++
  target/s390x/cpu.h              |  1 +
  4 files changed, 89 insertions(+)

diff --git a/hw/s390x/cpu-topology.c b/hw/s390x/cpu-topology.c
index 6098d6ea1f..b6bf839e40 100644
--- a/hw/s390x/cpu-topology.c
+++ b/hw/s390x/cpu-topology.c
@@ -19,6 +19,7 @@
  #include "target/s390x/cpu.h"
  #include "hw/s390x/s390-virtio-ccw.h"
  #include "hw/s390x/cpu-topology.h"
+#include "migration/vmstate.h"
  S390Topology *s390_get_topology(void)
  {
@@ -132,6 +133,83 @@ static void s390_topology_reset(DeviceState *dev)
      s390_cpu_topology_reset();
  }
+/**
+ * cpu_topology_postload
+ * @opaque: a pointer to the S390Topology
+ * @version_id: version identifier
+ *
+ * We check that the topology is used or is not used
+ * on both side identically.
+ *
+ * If the topology is in use we set the Modified Topology Change Report
+ * on the destination host.
+ */
+static int cpu_topology_postload(void *opaque, int version_id)
+{
+    S390Topology *topo = opaque;
+    int ret;
+
+    if (topo->topology_needed != s390_has_feat(S390_FEAT_CONFIGURATION_TOPOLOGY)) {

Does this function even run if topology_needed is false?
In that case there is no data saved, so no reason to load it either.
If so you can only check that both the source and the destination have
the feature enabled. You would need to always send the topology VMSD in
order to check that the feature is disabled.

Does qemu allow you to attempt to migrate to a host with another cpu
model?
If it disallowes that you wouldn't need to do any checks, right?

hum yes I must rework this

Thanks,
Pierre



I think my answer was wrong but it helped to correct a bug:
- Only machines > 7.x will be able to use topology and it is correct that migration to another machine is not possible.

- However we keep the possibility to not use the topology in 7.x and we are not able to migrate if one end use the topology and the other do not.

So I think cpu_topology_needed() must return true (no test needed) but the implementation of presave and postload are OK.

Regards,
Pierre



--
Pierre Morel
IBM Lab Boeblingen



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux