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

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

 



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@xxxxxxxxxx>
> ---
>  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

Attachment: signature.asc
Description: Digital signature

--
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]