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.
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list