Re: [PATCH 03/13] qemu: Make memory alignment helper more universal

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

On 21.09.2015 19:21, Peter Krempa wrote:
> Extract the size determination into a separate function and reuse it
> across the memory device alignment functions. Since later we will need
> to decide the alignment size according to architecture let's pass def to
> the functions.
> ---
>  src/qemu/qemu_domain.c  | 26 ++++++++++++++++++--------
>  src/qemu/qemu_domain.h  |  3 ++-
>  src/qemu/qemu_hotplug.c |  4 ++--
>  3 files changed, 22 insertions(+), 11 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index f840b0d..a5e9b75 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -3367,30 +3367,39 @@ qemuDomainAgentAvailable(virDomainObjPtr vm,
>  }
> 
> 
> +static unsigned long long
> +qemuDomainGetMemorySizeAlignment(virDomainDefPtr def ATTRIBUTE_UNUSED)
> +{
> +    /* Align memory size. QEMU requires rounding to next 4KiB block.
> +     * We'll take the "traditional" path and round it to 1MiB*/
> +
> +    return 1024;
> +}
> +
> +
>  int
>  qemuDomainAlignMemorySizes(virDomainDefPtr def)
>  {
>      unsigned long long mem;
> +    unsigned long long align = qemuDomainGetMemorySizeAlignment(def);

How about:

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index a5e9b75..807112c 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3381,7 +3381,7 @@ int
 qemuDomainAlignMemorySizes(virDomainDefPtr def)
 {
     unsigned long long mem;
-    unsigned long long align = qemuDomainGetMemorySizeAlignment(def);
+    unsigned long long const align = qemuDomainGetMemorySizeAlignment(def);
     size_t ncells = virDomainNumaGetNodeCount(def->numa);
     size_t i;

>      size_t ncells = virDomainNumaGetNodeCount(def->numa);
>      size_t i;
> 
>      /* align NUMA cell sizes if relevant */
>      for (i = 0; i < ncells; i++) {
>          mem = virDomainNumaGetNodeMemorySize(def->numa, i);
> -        virDomainNumaSetNodeMemorySize(def->numa, i, VIR_ROUND_UP(mem, 1024));
> +        virDomainNumaSetNodeMemorySize(def->numa, i, VIR_ROUND_UP(mem, align));
>      }
> 
>      /* align initial memory size */
>      mem = virDomainDefGetMemoryInitial(def);
> -    virDomainDefSetMemoryInitial(def, VIR_ROUND_UP(mem, 1024));
> +    virDomainDefSetMemoryInitial(def, VIR_ROUND_UP(mem, align));
> 
> -    /* Align maximum memory size. QEMU requires rounding to next 4KiB block.
> -     * We'll take the "traditional" path and round it to 1MiB*/
> -    def->mem.max_memory = VIR_ROUND_UP(def->mem.max_memory, 1024);
> +    def->mem.max_memory = VIR_ROUND_UP(def->mem.max_memory, align);
> 
>      /* Align memory module sizes */
>      for (i = 0; i < def->nmems; i++)
> -        qemuDomainMemoryDeviceAlignSize(def->mems[i]);
> +        def->mems[i]->size = VIR_ROUND_UP(def->mems[i]->size, align);
> 
>      return 0;
>  }
> @@ -3405,9 +3414,10 @@ qemuDomainAlignMemorySizes(virDomainDefPtr def)
>   * size so this should be safe).
>   */
>  void
> -qemuDomainMemoryDeviceAlignSize(virDomainMemoryDefPtr mem)
> +qemuDomainMemoryDeviceAlignSize(virDomainDefPtr def,
> +                                virDomainMemoryDefPtr mem)
>  {
> -    mem->size = VIR_ROUND_UP(mem->size, 1024);
> +    mem->size = VIR_ROUND_UP(mem->size, qemuDomainGetMemorySizeAlignment(def));
>  }
> 
> 
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 8cf535f..64cd7e1 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -469,7 +469,8 @@ bool qemuDomainHasBlockjob(virDomainObjPtr vm, bool copy_only)
>      ATTRIBUTE_NONNULL(1);
> 
>  int qemuDomainAlignMemorySizes(virDomainDefPtr def);
> -void qemuDomainMemoryDeviceAlignSize(virDomainMemoryDefPtr mem);
> +void qemuDomainMemoryDeviceAlignSize(virDomainDefPtr def,
> +                                     virDomainMemoryDefPtr mem);
> 
>  virDomainChrSourceDefPtr qemuFindAgentConfig(virDomainDefPtr def);
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 4797836..afc5408 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1802,7 +1802,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
>      if (!(devstr = qemuBuildMemoryDeviceStr(mem, vm->def, priv->qemuCaps)))
>          goto cleanup;
> 
> -    qemuDomainMemoryDeviceAlignSize(mem);
> +    qemuDomainMemoryDeviceAlignSize(vm->def, mem);
> 
>      if (qemuBuildMemoryBackendStr(mem->size, mem->pagesize,
>                                    mem->targetNode, mem->sourceNodes, NULL,
> @@ -4273,7 +4273,7 @@ qemuDomainDetachMemoryDevice(virQEMUDriverPtr driver,
>          return -1;
>      }
> 
> -    qemuDomainMemoryDeviceAlignSize(memdef);
> +    qemuDomainMemoryDeviceAlignSize(vm->def, memdef);
> 
>      if ((idx = virDomainMemoryFindByDef(vm->def, memdef)) < 0) {
>          virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> 

ACK

Michal

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