Re: [PATCH 4/4] qemu: Generate memory device aliases according to slot number

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

 




On 11/03/2016 02:12 AM, Peter Krempa wrote:
> The memory device alias needs to be treated as machine ABI as qemu is
> using it in the migration stream for section labels. To simplify this
> generate the alias from the slot number unless an existing broken
> configuration is detected.
> 
> With this patch the aliases are predictable and even certain
> configurations which would not be migratable previously are fixed.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1359135
> ---
>  src/qemu/qemu_alias.c                              | 27 ++++++++++++++++++----
>  src/qemu/qemu_alias.h                              |  3 ++-
>  src/qemu/qemu_hotplug.c                            |  4 +++-
>  .../qemuxml2argv-memory-hotplug-dimm-addr.args     |  4 ++--
>  4 files changed, 29 insertions(+), 9 deletions(-)
> 

Revisiting after my face-palm for patch2.

> diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
> index 9737158..8521a44 100644
> --- a/src/qemu/qemu_alias.c
> +++ b/src/qemu/qemu_alias.c
> @@ -332,17 +332,34 @@ qemuAssignDeviceRNGAlias(virDomainDefPtr def,
>  }
> 
> 
> +/**
> + * qemuAssignDeviceMemoryAlias:
> + * @def: domain definition. Necessary only if @oldAlias is true.

s/old/use  (multiple places)

> + * @mem: memory device definition
> + * @oldAlias: Generate the alias according to the order of the device in @def
> + *            rather than according to the slot number for legacy reasons.
> + *
> + * Generates alias for a memory device according to slot number if @oldAlias is
> + * false or according to order in @def->mems otherwise.
> + *
> + * Returns 0 on success, -1 on error.
> + */
>  int
>  qemuAssignDeviceMemoryAlias(virDomainDefPtr def,
> -                            virDomainMemoryDefPtr mem)
> +                            virDomainMemoryDefPtr mem,
> +                            bool oldAlias)
>  {
>      size_t i;
>      int maxidx = 0;
>      int idx;
> 
> -    for (i = 0; i < def->nmems; i++) {
> -        if ((idx = qemuDomainDeviceAliasIndex(&def->mems[i]->info, "dimm")) >= maxidx)
> -            maxidx = idx + 1;
> +    if (oldAlias) {
> +        for (i = 0; i < def->nmems; i++) {
> +            if ((idx = qemuDomainDeviceAliasIndex(&def->mems[i]->info, "dimm")) >= maxidx)
> +                maxidx = idx + 1;
> +        }
> +    } else {
> +        maxidx = mem->info.addr.dimm.slot;
>      }
> 
>      if (virAsprintf(&mem->info.alias, "dimm%d", maxidx) < 0)
> @@ -475,7 +492,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps)
>              return -1;
>      }
>      for (i = 0; i < def->nmems; i++) {
> -        if (virAsprintf(&def->mems[i]->info.alias, "dimm%zu", i) < 0)
> +        if (qemuAssignDeviceMemoryAlias(NULL, def->mems[i], false) < 0)

Since this is primarily for the startup path, then false is OK...
Although I can only wonder what would happen in the QemuAttach path...

>              return -1;
>      }
> 
> diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h
> index d298a4d..1d93912 100644
> --- a/src/qemu/qemu_alias.h
> +++ b/src/qemu/qemu_alias.h
> @@ -58,7 +58,8 @@ int qemuAssignDeviceRNGAlias(virDomainDefPtr def,
>                               virDomainRNGDefPtr rng);
> 
>  int qemuAssignDeviceMemoryAlias(virDomainDefPtr def,
> -                                virDomainMemoryDefPtr mems);
> +                                virDomainMemoryDefPtr mems,
> +                                bool ble);

s/ble/useAlias

> 
>  int qemuAssignDeviceShmemAlias(virDomainDefPtr def,
>                                 virDomainShmemDefPtr shmem,
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 75477cd..77176fb 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -2130,7 +2130,9 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
>      if (qemuDomainAssignMemoryDeviceSlot(vm->def, mem) < 0)
>          goto cleanup;
> 
> -    if (qemuAssignDeviceMemoryAlias(vm->def, mem) < 0)
> +    /* in cases where we are using a VM with aliases generated according to the
> +     * index of the memory device we need to keep continue using that scheme */
> +    if (qemuAssignDeviceMemoryAlias(vm->def, mem, priv->memHotplugAliasMismatch) < 0)

This ends up being a long line > 80 cols...

ACK w/ variable name cleanup

John

>          goto cleanup;
> 
>      if (virAsprintf(&objalias, "mem%s", mem->info.alias) < 0)
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm-addr.args b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm-addr.args
> index 23403df..fdbb4c3 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm-addr.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm-addr.args
> @@ -15,8 +15,8 @@ QEMU_AUDIO_DRV=none \
>  mem-path=/dev/hugepages2M/libvirt/qemu,size=536870912,host-nodes=1-3,\
>  policy=bind \
>  -device pc-dimm,node=0,memdev=memdimm0,id=dimm0,slot=0,addr=4294967296 \
> --object memory-backend-ram,id=memdimm1,size=536870912 \
> --device pc-dimm,node=0,memdev=memdimm1,id=dimm1,slot=2 \
> +-object memory-backend-ram,id=memdimm2,size=536870912 \
> +-device pc-dimm,node=0,memdev=memdimm2,id=dimm2,slot=2 \
>  -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
>  -nographic \
>  -nodefaults \
> 

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]