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