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

Re: [libvirt] [PATCH v3] memtune: Let virsh know the unlimited value for memory tunables



On 01/12/2011 12:56 AM, Nikunj A. Dadhania wrote:
> Here is the patch, now the set calls are also ull. 
> 
> Still virCgroupGetMemoryUsage is not changed, this will require changes
> in virDomainInfoPtr (info->memory). I am not sure if I should have them
> in this patch.

It can be a separate patch, if desired, but it is probably still needed.

virDomainGetInfo is inherently limited - we can't change that API due to
backwards compatibility issues, so if we ever encounter a system with
more than 4T memory (1k * 4G limit of unsigned long), then the maxMem
and memory fields of virDomainInfoPtr have to be clamped at 4G on any
system with 32-bit long, rather than wrapping around.  We probably also
ought to document this limitation, and point to getMemoryParameter as
the way to find a more accurate memory limits if virDomainGetInfo
returned clamped values.

> From: Nikunj A. Dadhania <nikunj linux vnet ibm com>
> 
> Display unlimited when the memory cgroup settings says so. Unlimited is
> represented by INT64_MAX in memory cgroup.
> 
> v3: Make virCgroupSet memory call ull

> +++ b/src/util/cgroup.c
> @@ -858,8 +858,11 @@ int virCgroupForDomain(virCgroupPtr driver ATTRIBUTE_UNUSED,
>   *
>   * Returns: 0 on success
>   */
> -int virCgroupSetMemory(virCgroupPtr group, unsigned long kb)
> +int virCgroupSetMemory(virCgroupPtr group, unsigned long long kb)
>  {
> +    if (kb > (VIR_DOMAIN_MEMORY_PARAM_UNLIMITED >> 10))
> +        return -EINVAL;
> +
>      return virCgroupSetValueU64(group,
>                                  VIR_CGROUP_CONTROLLER_MEMORY,
>                                  "memory.limit_in_bytes",

Not quite right.  What if I want to change from a finite limit back to
unlimited?  Then virCgroupSetMemory(group, UINT64_MAX) must recognize
that kb == VIR_DOMAIN_MEMORY_PARAM_UNLIMITED, and call
virCgroupSetValueU64(INT64_MAX) rathe than virCgroupSetValueU64(kb<<10).

> @@ -883,6 +886,7 @@ int virCgroupGetMemoryUsage(virCgroupPtr group, unsigned long *kb)
>                                 "memory.usage_in_bytes", &usage_in_bytes);
>      if (ret == 0)
>          *kb = (unsigned long) usage_in_bytes >> 10;
> +
>      return ret;

Why the whitespace change?

> @@ -927,8 +936,11 @@ int virCgroupGetMemoryHardLimit(virCgroupPtr group, unsigned long *kb)
>   *
>   * Returns: 0 on success
>   */
> -int virCgroupSetMemorySoftLimit(virCgroupPtr group, unsigned long kb)
> +int virCgroupSetMemorySoftLimit(virCgroupPtr group, unsigned long long kb)
>  {
> +    if (kb > (VIR_DOMAIN_MEMORY_PARAM_UNLIMITED >> 10))
> +        return -EINVAL;
> +

Likewise on needing to allow the user to set the value back to unlimited.

> +++ b/tools/virsh.c
> @@ -2987,9 +2987,14 @@ cmdMemtune(vshControl * ctl, const vshCmd * cmd)
>                               params[i].value.l);
>                      break;
>                  case VIR_DOMAIN_MEMORY_PARAM_ULLONG:
> -                    vshPrint(ctl, "%-15s: %llu\n", params[i].field,
> -                             params[i].value.ul);
> +                {
> +                    if (params[i].value.ul == VIR_DOMAIN_MEMORY_PARAM_UNLIMITED)
> +                        vshPrint(ctl, "%-15s: unlimited\n", params[i].field);
> +                    else
> +                        vshPrint(ctl, "%-15s: %llu\n", params[i].field,
> +                                 params[i].value.ul);

Do we want any back-compat considerations?  That is, if a newer virsh is
talking to an older server, which still answered INT64_MAX>>10 instead
of the new VIR_DOMAIN_MEMORY_PARAM_UNLIMITED, should we recognize that
situation as another reason to print "unlimited"?

-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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