On Fri, Jun 09, 2017 at 10:27:33AM +0200, Igor Mammedov wrote: > On Thu, 08 Jun 2017 15:00:53 -0500 > Michael Roth <mdr...@linux.vnet.ibm.com> wrote: > > > Quoting David Gibson (2017-05-30 23:35:57) > > > On Tue, May 30, 2017 at 06:04:45PM +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 <lviv...@redhat.com> > > > > > > So, I think this is a reasonable concept, at least in terms of > > > cleanliness and not doing unnecessary work. However, if it's fixing > > > bugs, I suspect that means we still have problems elsewhere. > > > > I was hoping a lot of these issues would go away once we default > > the initial/reset DRC states to "coldplugged". I think your pending > > patch: > > > > "spapr: Make DRC reset force DRC into known state" > > > > But I didn't consider the fact that libvirt will be issuing these > > hotplugs *after* reset, so those states would indeed need to > > be fixed up again to reflect boot-time,attached as opposed to > > boot-time,unattached before starting the target. > > > > So I do think this patch addresses a specific bug that isn't > > obviously fixable elsewhere. > > > > To me it seems like the only way to avoid doing something like > > what this patch does is to migrate all attached DRCs from the > > source in all cases. > > > > This would break backward-migration though, unless we switch from > > using subregions for DRCs to explicitly disabling DRC migration > > based on machine type. > we could leave old machines broken and fix only new machine types, > then it would be easy ot migrate 'additional' DRC state as subsection > only on new for new machines. > > > > > That approach seems to similar to what x86 does, e.g. > > hw/acpi/ich9.c and hw/acpi/piix.c migrate vmstate_memhp_state > > (corresponding to all DIMMs' slot status) in all cases where > > memory hotplug is enabled. If they were to do this using > > subregions for DIMMs in a transitional state I think similar > > issues would pop up in that code as well. > > > > Even if we take this route, we still need to explicitly suppress > > hotplug events during INMIGRATE to avoid extra events going on > > the queue. *Unless* we similarly rely purely on the ones sent by > > the source. > pc/q35 might also lose events if device is hotplugged during migration, > in addition migration would fail anyway since dst qemu > should be launched with all devices that are present on src. > > ex: consider if one hotplugs DIMM during migration, it creates > RAM region mapped into guest and that region might be transferred > as part of VMState (not sure if it even works) > and considering dst qemu has no idea about hotplugged memory mapping, > the migration would fail on receiving unknown VMState. > > Hotplug generally doesn't work during migration, so it should be disabled > in a generic way on migration start and re-enabled on target > on migration completion. How about blocking device_add when > INMIGRATE state and unblocking it when switching to runnig on dst?
Yeah, that sounds like a good idea. We need to cover the case of
migration during an incomplete hot (un)plug as well as hotplug during
a migration.
--
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
signature.asc
Description: PGP signature
