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@xxxxxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list