On Thu, Nov 10, 2016 at 08:56:48 -0500, John Ferlan wrote:
On 11/10/2016 04:11 AM, Peter Krempa wrote:
On Wed, Nov 09, 2016 at 18:40:28 -0500, John Ferlan wrote:
On 11/03/2016 02:12 AM, Peter Krempa wrote:
Detect on reconnect to a running qemu VM whether the alias of a hotpluggable memory device (dimm) does not match the dimm slot number where it's connected to. This is necessary as qemu is actually considering the alias as machine ABI used to connect the backend object to the dimm device.
This will require us to keep them consistent so that we can reliably restore them on migration. In some situations it was currently possible to create a mismatched configuration and qemu would refuse to restore the migration stream.
To avoid breaking existing VMs we'll need to keep the old algorithm though. --- src/qemu/qemu_domain.h | 3 +++ src/qemu/qemu_process.c | 25 +++++++++++++++++++++++++ 2 files changed, 28 insertions(+)
[...]
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 1b67aee..15b8ec8 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3205,6 +3205,29 @@ qemuDomainPerfRestart(virDomainObjPtr vm) return 0; }
+ +static void +qemuProcessReconnectCheckMemoryHotplugMismatch(virDomainObjPtr vm) +{ + size_t i; + int aliasidx; + virDomainDefPtr def = vm->def; + qemuDomainObjPrivatePtr priv = vm->privateData; + + if (!virDomainDefHasMemoryHotplug(def) || def->nmems == 0) + return; + + for (i = 0; i < def->nmems; i++) { + aliasidx = qemuDomainDeviceAliasIndex(&def->mems[i]->info, "dimm");
When/how does the info.alias get filled in during restart processing?
The live definition along with aliases is saved to the status XML and then reloaded from the disk.
Otherwise if we'd not remember the aliases the whole hotunplug code would not work.
face-palm - I knew I was missing something obvious, but the brain just wasn't able to recognize it, <sigh>.
Still consider the changed XML example from patch1 (without any hotplug), we have:
<address type='dimm' slot='0' ...> => alias == "dimm0" <address type='dimm' slot='2' ...> => alias == "dimm1"
so the bool could be "memAliasOrderMismatch" or "memAliasUnordered".
Okay, I'll go with the former, since that describes exactly the details.
Since it's not necessarily "Hotplug" related, I think changing the variable and function name should be done, but it's not a requirement for the ACK since it's understandable why Hotplug is used.
John
FWIW: One other oddball path that "might" disrupt things is the ignore_value(qemuDomainUpdateMemoryDeviceInfo(...) hotplug code where we're not "guaranteed" that the dimm.slot is filled in...
It should not ever happen, but I thought about this option originally. The code tolerates this for local usage since it's not really fatal, but rejects migration if the slot or base address is missing.