[libvirt] [PATCH] vz: implement memory setting functions in driver

[Dmitry Guryanov]

On Thu, 2015-11-05 at 13:13 +0300, Nikolay Shirokovskiy wrote:
> Implement functions for vz containers only. vz VMs memory
> managment thru libvirt yet to be designed.
> 
> vz memory management for containers is rather different from other
> drivers.
> 
> 1. Whole bunch of memory settings functions change both cur_balloon and
> maximum
>  memory to the same value. The reason is that active container memory
> settings is not limited by memory set at container startup as in virtual
> machine case. Thus there is no real need in 2 parameters and this patch takes
> an effort to map this situation to an existent interface.
> 
> 2. virDomainSetMemory semantics is extended.
> Current documentation states that this function changes memory only for
> a current domain. At the same time virsh documentation states that behaviour
> of
> 'setmem' without flags (that eventually calls virDomainSetMemory) is
> hypervisor
> specific. I think we can extend sematics of the function to the defined in
> virsh. First this is already done for virDomainSetMaxMemory, second it would
> be
> convinient for vz driver. With current semantics virDomainSetMemory can't be
> implemented as vz always set persistent state if live state is set. With
> extended semantics function just holds the vz driver behaviour and
> we can use 'setmem' without flags to change domain memory.
> 
> 3. VIR_DOMAIN_MEM_CURRENT is not supported.
> There are 2 reasons. First is to be consistent with vzDomainAttachDeviceFlags
> and vzDomainDetachDeviceFlags. Second is that this flag has no good
> correllation with vz behaviour. I'm not sure what use case of this flag is,
> I suppose it is to call the function without inquiring the current state of
> domain so you will not fail because of incorrect flags combination. But with
> vz
> this behaviour is impossible. The function will fail if domain is in active
> state and you specify this flag as you can't change running domain config
> only.
> 
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>
> ---
>  src/libvirt-domain.c |    5 ++-
>  src/vz/vz_driver.c   |   81
> ++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/vz/vz_sdk.c      |   23 ++++++++++++++
>  src/vz/vz_sdk.h      |    1 +
>  4 files changed, 108 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index de7eb04..56ab907 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -1882,8 +1882,9 @@ virDomainSetMaxMemory(virDomainPtr domain, unsigned long
> memory)
>   * to Domain0 i.e. the domain where the application runs.
>   * This function may require privileged access to the hypervisor.
>   *
> - * This command only changes the runtime configuration of the domain,
> - * so can only be called on an active domain.
> + * This command is hypervisor-specific for whether active, persistent,
> + * or both configurations are changed; for more control, use
> + * virDomainSetMemoryFlags().
>   *
>   * Returns 0 in case of success and -1 in case of failure.
>   */
> diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
> index 6f1cbfb..cab47ec 100644
> --- a/src/vz/vz_driver.c
> +++ b/src/vz/vz_driver.c
> @@ -1438,6 +1438,84 @@ vzNodeGetFreeMemory(virConnectPtr conn
> ATTRIBUTE_UNUSED)
>      return freeMem;
>  }
>  
> +static int vzDomainSetMemoryDefault(virDomainObjPtr dom, unsigned long
> memory)
> +{
> +    if (!IS_CT(dom->def)) {
> +        virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> +                       _("Can't change memory size for vz VM"));
> +        return -1;
> +    }
> +
> +    if (prlsdkSetMemsize(dom, memory >> 10) < 0)
> +        return -1;
> +
> +    virDomainDefSetMemoryInitial(dom->def, memory);
> +    dom->def->mem.cur_balloon = memory;
> +
> +    return 0;
> +}
> +
> +static int vzDomainSetMemoryFlags(virDomainPtr domain, unsigned long memory,
> +                                  unsigned int flags)
> +{
> +    virDomainObjPtr dom = NULL;
> +    int ret = -1;
> +
> +    virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
> +                  VIR_DOMAIN_AFFECT_CONFIG |
> +                  VIR_DOMAIN_MEM_MAXIMUM, -1);
> +
> +    if (!(dom = vzDomObjFromDomain(domain)))
> +        return -1;
> +
> +    if (!(flags & VIR_DOMAIN_AFFECT_CONFIG)) {
> +        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                       _("device attach needs VIR_DOMAIN_AFFECT_CONFIG "
> +                         "flag to be set"));

The message seems to be copy-pasted from device functions


> +        goto cleanup;
> +    }
> +
> +    if (!virDomainObjIsActive(dom) && (flags & VIR_DOMAIN_AFFECT_LIVE)) {
> +        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                       _("cannot do live update a device on "
> +                         "inactive domain"));
The same


> +        goto cleanup;
> +    }
> +
> +    if (virDomainObjIsActive(dom) && !(flags & VIR_DOMAIN_AFFECT_LIVE)) {
> +        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                       _("Updates on a running domain need "
> +                         "VIR_DOMAIN_AFFECT_LIVE flag"));
> +        goto cleanup;
> +    }
> +

Can we move these checks to a separate function, because already 3 functions
will do the same quite complex checks?



> +    ret = vzDomainSetMemoryDefault(dom, memory);
> +
> + cleanup:
> +
> +    virObjectUnlock(dom);
> +    return ret;
> +}
> +
> +static int vzDomainSetMemory(virDomainPtr domain, unsigned long memory)
> +{
> +    virDomainObjPtr dom = NULL;
> +    int ret = -1;
> +
> +    if (!(dom = vzDomObjFromDomain(domain)))
> +        return -1;
> +
> +    ret = vzDomainSetMemoryDefault(dom, memory);
> +
> +    virObjectUnlock(dom);
> +    return ret;
> +}
> +
> +static int vzDomainSetMaxMemory(virDomainPtr domain, unsigned long memory)
> +{
> +    return vzDomainSetMemory(domain, memory);
> +}
> +
>  static virHypervisorDriver vzDriver = {
>      .name = "vz",
>      .connectOpen = vzConnectOpen,            /* 0.10.0 */
> @@ -1499,6 +1577,9 @@ static virHypervisorDriver vzDriver = {
>      .domainBlockStatsFlags = vzDomainBlockStatsFlags, /* 1.2.17 */
>      .domainInterfaceStats = vzDomainInterfaceStats, /* 1.2.17 */
>      .domainMemoryStats = vzDomainMemoryStats, /* 1.2.17 */
> +    .domainSetMemoryFlags = vzDomainSetMemoryFlags, /* 1.2.21 */
> +    .domainSetMemory = vzDomainSetMemory, /* 1.2.21 */
> +    .domainSetMaxMemory = vzDomainSetMaxMemory, /* 1.2.21 */
>  };
>  
>  static virConnectDriver vzConnectDriver = {
> diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
> index 89c9e89..2d0f688 100644
> --- a/src/vz/vz_sdk.c
> +++ b/src/vz/vz_sdk.c
> @@ -4036,3 +4036,26 @@ prlsdkGetMemoryStats(virDomainObjPtr dom,
>  
>      return ret;
>  }
> +
> +int prlsdkSetMemsize(virDomainObjPtr dom, unsigned int memsize)
> +{
> +    int ret = -1;
> +    vzDomObjPtr privdom = dom->privateData;
> +    PRL_HANDLE job = PRL_INVALID_HANDLE;
> +    PRL_RESULT pret;
> +
> +    job = PrlVm_BeginEdit(privdom->sdkdom);
> +    if (PRL_FAILED(waitJob(job)))
> +        goto out;
> +
> +    pret = PrlVmCfg_SetRamSize(privdom->sdkdom, memsize);
> +    prlsdkCheckRetGoto(pret, out);
> +
> +    job = PrlVm_CommitEx(privdom->sdkdom, PVCF_DETACH_HDD_BUNDLE);
> +    if (PRL_FAILED(waitJob(job)))
> +        goto out;
> +
> +    ret = 0;
> + out:
> +    return ret;
> +}
> diff --git a/src/vz/vz_sdk.h b/src/vz/vz_sdk.h
> index ebe4591..88fef4a 100644
> --- a/src/vz/vz_sdk.h
> +++ b/src/vz/vz_sdk.h
> @@ -76,3 +76,4 @@ int
>  prlsdkGetVcpuStats(virDomainObjPtr dom, int idx, unsigned long long *time);
>  int
>  prlsdkGetMemoryStats(virDomainObjPtr dom, virDomainMemoryStatPtr stats,
> unsigned int nr_stats);
> +int prlsdkSetMemsize(virDomainObjPtr dom, unsigned int memsize);