2011/1/13 Nikunj A. Dadhania <nikunj@xxxxxxxxxxxxxxxxxx>:
> On Wed, 12 Jan 2011 10:21:04 -0700, Eric Blake <eblake@xxxxxxxxxx> 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 <nikunj@xxxxxxxxxxxxxxxxxx>
>
> 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 <nikunj@xxxxxxxxxxxxxxxxxx>
> Reported-by: Justin Clift <jclift@xxxxxxxxxx>
> ---
> Â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
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list