[libvirt] [PATCH 2/4] qemu: process: detect if dimm aliases are broken on reconnect

[Peter Krempa]

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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20161110/e815322a/attachment-0001.sig>