2011/1/13 Nikunj A. Dadhania <[email protected]>: > On Wed, 12 Jan 2011 10:21:04 -0700, Eric Blake <[email protected]> wrote: >> 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. >> > virCgroupGetMemoryUsage is called only from lxcDomainGetInfo, that is > the reason I thought that this change may not be needed. > >> > +++ 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"? >> > As Mattias suggested in the other mail, this adds more complications. My > take is to have VIR_DOMAIN_MEMORY_PARAM_UNLIMITED as the max value. > > Here is the patch which adds setting as well as displaying the > "unlimited" values. Now in virsh to specify "unlimited" the user would > need to pass "-1" > > for example: > virsh # memtune lxcbb1 --hard-limit -1 > > From: Nikunj A. Dadhania <[email protected]> > > Display and set unlimited when the memory cgroup settings. Unlimited is > represented by INT64_MAX in memory cgroup. > > v4: Fix handling of setting unlimited values > v3: Make virCgroupSet memory call ull > > Signed-off-by: Nikunj A. Dadhania <[email protected]> > Reported-by: Justin Clift <[email protected]> > --- > include/libvirt/libvirt.h.in | 1 > src/lxc/lxc_driver.c | 2 - > src/qemu/qemu_driver.c | 2 - > src/util/cgroup.c | 93 > +++++++++++++++++++++++++++++++----------- > src/util/cgroup.h | 14 +++--- > tools/virsh.c | 13 +++++- > 6 files changed, 90 insertions(+), 35 deletions(-) > > diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in > index 3c6a54a..3ee47b9 100644 > --- a/include/libvirt/libvirt.h.in > +++ b/include/libvirt/libvirt.h.in > @@ -696,6 +696,7 @@ typedef enum { > */ > > #define VIR_DOMAIN_MEMORY_FIELD_LENGTH 80 > +#define VIR_DOMAIN_MEMORY_PARAM_UNLIMITED INT64_MAX
If I understand it correctly we currently use INT64_MAX>>10 to indicate "unlimited". Therefore, we should define VIR_DOMAIN_MEMORY_PARAM_UNLIMITED to INT64_MAX>>10 as this would preserve the existing behavior. I think this patch started to fix the problem that virsh memtune reports 9007199254740991 (== INT64_MAX>>10) when the value actually means unlimited. We want virsh to report "unlimited" in this case. As we cannot fix the problem in a better way (like using -1 for unlimited) anymore, we try to workaround it: First add a define VIR_DOMAIN_MEMORY_PARAM_UNLIMITED for the magic value INT64_MAX>>10, but stick to this value, because changing it could break existing applications. Second make virsh memtune detect VIR_DOMAIN_MEMORY_PARAM_UNLIMITED and print "unlimited" in that case instead of the actual numeric value. Third make virsh memtune accept -1 as unlimited and translate it to VIR_DOMAIN_MEMORY_PARAM_UNLIMITED. You already addressed the second and third point in your patch so we're close to a proper workaround. Matthias -- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
