[libvirt] [PATCH v2 04/10] qemu: Introduce qemuBuildMemCommandLine

[Peter Krempa]

On Wed, Feb 17, 2016 at 21:25:37 -0500, John Ferlan wrote:
> Add new function to manage adding the '-m' memory options to the command
> line removing that task from the mainline qemuBuildCommandLine
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/qemu/qemu_command.c | 89 +++++++++++++++++++++++++++++--------------------
>  1 file changed, 53 insertions(+), 36 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index bff5b09..2c22a08 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -5225,7 +5225,7 @@ qemuBuildSmpArgStr(const virDomainDef *def,
>  
>  static int
>  qemuBuildMemPathStr(virQEMUDriverConfigPtr cfg,
> -                    virDomainDefPtr def,
> +                    const virDomainDef *def,
>                      virQEMUCapsPtr qemuCaps,
>                      virCommandPtr cmd)
>  {
> @@ -5288,6 +5288,56 @@ qemuBuildMemPathStr(virQEMUDriverConfigPtr cfg,
>      return 0;
>  }
>  
> +
> +static int
> +qemuBuildMemCommandLine(virCommandPtr cmd,
> +                        virQEMUDriverConfigPtr cfg,
> +                        const virDomainDef *def,
> +                        virQEMUCapsPtr qemuCaps)
> +{
> +    if (qemuDomainDefValidateMemoryHotplug(def, qemuCaps, NULL) < 0)
> +        goto error;

[1] (below .. )

> +
> +    virCommandAddArg(cmd, "-m");
> +
> +    if (virDomainDefHasMemoryHotplug(def)) {
> +        /* Use the 'k' suffix to let qemu handle the units */
> +        virCommandAddArgFormat(cmd, "size=%lluk,slots=%u,maxmem=%lluk",
> +                               virDomainDefGetMemoryInitial(def),
> +                               def->mem.memory_slots,
> +                               def->mem.max_memory);
> +
> +    } else {
> +       virCommandAddArgFormat(cmd, "%llu",
> +                              virDomainDefGetMemoryInitial(def) / 1024);
> +    }
> +
> +    /*
> +     * Add '-mem-path' (and '-mem-prealloc') parameter here only if
> +     * there is no numa node specified.
> +     */
> +    if (!virDomainNumaGetNodeCount(def->numa) &&
> +        qemuBuildMemPathStr(cfg, def, qemuCaps, cmd) < 0)
> +        goto error;
> +
> +    if (def->mem.locked && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_MLOCK)) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                       _("memory locking not supported by QEMU binary"));
> +        goto error;
> +    }
> +    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MLOCK)) {
> +        virCommandAddArg(cmd, "-realtime");
> +        virCommandAddArgFormat(cmd, "mlock=%s",
> +                               def->mem.locked ? "on" : "off");
> +    }
> +
> +    return 0;
> +
> + error:

This has nothing to clean up, so the error label can be omitted and -1
returned directly.

> +    return -1;
> +}
> +
> +
>  static int
>  qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
>                      virDomainDefPtr def,
> @@ -6827,45 +6877,12 @@ qemuBuildCommandLine(virConnectPtr conn,
>      if (qemuBuildDomainLoaderCommandLine(cmd, def, qemuCaps) < 0)
>          goto error;
>  
> -    if (!migrateURI && !snapshot &&
> -        qemuDomainAlignMemorySizes(def) < 0)
> +    if (!migrateURI && !snapshot && qemuDomainAlignMemorySizes(def) < 0)
>          goto error;

Later on we could add a function that will collate all the operations
that tweak the domain config prior to start and extract it from the
command line builder, which would be called prior to the validator, ...

>  
> -    if (qemuDomainDefValidateMemoryHotplug(def, qemuCaps, NULL) < 0)

[1] ... and then this call could be moved to the validator from patch 1.

But that's not for this patch.

> +    if (qemuBuildMemCommandLine(cmd, cfg, def, qemuCaps) < 0)
>          goto error;

ACK if you remove the 'error' label from the newly added function.

Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160218/9481be9b/attachment-0001.sig>