Re: [Xen-devel] [PATCH] libxl: fix dom0 autoballooning with Xen 4.8
On 02/02/2017 04:42 AM, Wei Liu wrote: I saw this mail but didn't realise it required my input, sorry. I suppose it didn't and I was shamelessly fishing for a review - so you have my apologies :-). But the patch does fix an annoying, easily encountered bug which I'm eager to see resolved in the next libvirt release. On Wed, Feb 01, 2017 at 04:00:54PM -0700, Jim Fehlig wrote: Jim Fehlig wrote: xen.git commit 57f8b13c changed several of the libxl memory get/set functions to take 64 bit parameters. The libvirt libxl driver still uses uint32_t variables for these various parameters, which is particularly problematic for the libxl_set_memory_target() function. When dom0 autoballooning is enabled, libvirt (like xl) determines the memory needed to start a domain and the memory available. If memory available is less than memory needed, dom0 is ballooned down by passing a negative value to libxl_set_memory_target() 'target_memkb' parameter. Prior to xen.git commit 57f8b13c, 'target_memkb' was an int32_t. Subtracting a larger uint32 from a smaller uint32 and assigning it to int32 resulted in a negative number. After commit 57f8b13c, the same subtraction is widened to a int64, resulting in a large positive number. The simple fix taken by this patch is to assign the difference of the uint32 values to a temporary int32 variable, which is then passed to 'target_memkb' parameter of libxl_set_memory_target(). Note that it is undesirable to change libvirt to use 64 bit variables since it requires setting LIBXL_API_VERSION to 0x040800. Currently libvirt supports LIBXL_API_VERSION >= 0x040400, essentially Xen >= 4.4. Any comments on this patch? xen-devel is already in cc, but I'll add Wei (one of the Xen tools maintainers) and Juergen (author of commit 57f8b13c) for some additional exposure :-). Currently, "out-of-the-box" Xen >= 4.8 + libvirt setup (dom0 autoballooning enabled) is broken without this fix. Regards, Jim Signed-off-by: Jim Fehlig--- src/libxl/libxl_domain.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index a5314b0..ed73cd2 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -909,6 +909,7 @@ libxlDomainFreeMem(libxl_ctx *ctx, libxl_domain_config *d_config) { uint32_t needed_mem; uint32_t free_mem; +int32_t target_mem; int tries = 3; int wait_secs = 10; @@ -922,7 +923,8 @@ libxlDomainFreeMem(libxl_ctx *ctx, libxl_domain_config *d_config) if (free_mem >= needed_mem) return 0; -if (libxl_set_memory_target(ctx, 0, free_mem - needed_mem, +target_mem = free_mem - needed_mem; +if (libxl_set_memory_target(ctx, 0, target_mem, This should do the trick. Thanks. There are other ways to skin this cat and if folks prefer one of those I can push a followup. But for now, so that we have a fix, I'll push this variant. Regards, Jim ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: fix dom0 autoballooning with Xen 4.8
I saw this mail but didn't realise it required my input, sorry. On Wed, Feb 01, 2017 at 04:00:54PM -0700, Jim Fehlig wrote: > Jim Fehlig wrote: > > xen.git commit 57f8b13c changed several of the libxl memory > > get/set functions to take 64 bit parameters. The libvirt > > libxl driver still uses uint32_t variables for these various > > parameters, which is particularly problematic for the > > libxl_set_memory_target() function. > > > > When dom0 autoballooning is enabled, libvirt (like xl) determines > > the memory needed to start a domain and the memory available. If > > memory available is less than memory needed, dom0 is ballooned > > down by passing a negative value to libxl_set_memory_target() > > 'target_memkb' parameter. Prior to xen.git commit 57f8b13c, > > 'target_memkb' was an int32_t. Subtracting a larger uint32 from > > a smaller uint32 and assigning it to int32 resulted in a negative > > number. After commit 57f8b13c, the same subtraction is widened > > to a int64, resulting in a large positive number. The simple > > fix taken by this patch is to assign the difference of the > > uint32 values to a temporary int32 variable, which is then > > passed to 'target_memkb' parameter of libxl_set_memory_target(). > > > > Note that it is undesirable to change libvirt to use 64 bit > > variables since it requires setting LIBXL_API_VERSION to 0x040800. > > Currently libvirt supports LIBXL_API_VERSION >= 0x040400, > > essentially Xen >= 4.4. > > Any comments on this patch? xen-devel is already in cc, but I'll add Wei (one > of > the Xen tools maintainers) and Juergen (author of commit 57f8b13c) for some > additional exposure :-). > > Currently, "out-of-the-box" Xen >= 4.8 + libvirt setup (dom0 autoballooning > enabled) is broken without this fix. > > Regards, > Jim > > > > > Signed-off-by: Jim Fehlig> > --- > > src/libxl/libxl_domain.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c > > index a5314b0..ed73cd2 100644 > > --- a/src/libxl/libxl_domain.c > > +++ b/src/libxl/libxl_domain.c > > @@ -909,6 +909,7 @@ libxlDomainFreeMem(libxl_ctx *ctx, libxl_domain_config > > *d_config) > > { > > uint32_t needed_mem; > > uint32_t free_mem; > > +int32_t target_mem; > > int tries = 3; > > int wait_secs = 10; > > > > @@ -922,7 +923,8 @@ libxlDomainFreeMem(libxl_ctx *ctx, libxl_domain_config > > *d_config) > > if (free_mem >= needed_mem) > > return 0; > > > > -if (libxl_set_memory_target(ctx, 0, free_mem - needed_mem, > > +target_mem = free_mem - needed_mem; > > +if (libxl_set_memory_target(ctx, 0, target_mem, This should do the trick. Wei. > > /* relative */ 1, 0) < 0) > > goto error; > > > ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: fix dom0 autoballooning with Xen 4.8
Jim Fehlig wrote: > xen.git commit 57f8b13c changed several of the libxl memory > get/set functions to take 64 bit parameters. The libvirt > libxl driver still uses uint32_t variables for these various > parameters, which is particularly problematic for the > libxl_set_memory_target() function. > > When dom0 autoballooning is enabled, libvirt (like xl) determines > the memory needed to start a domain and the memory available. If > memory available is less than memory needed, dom0 is ballooned > down by passing a negative value to libxl_set_memory_target() > 'target_memkb' parameter. Prior to xen.git commit 57f8b13c, > 'target_memkb' was an int32_t. Subtracting a larger uint32 from > a smaller uint32 and assigning it to int32 resulted in a negative > number. After commit 57f8b13c, the same subtraction is widened > to a int64, resulting in a large positive number. The simple > fix taken by this patch is to assign the difference of the > uint32 values to a temporary int32 variable, which is then > passed to 'target_memkb' parameter of libxl_set_memory_target(). > > Note that it is undesirable to change libvirt to use 64 bit > variables since it requires setting LIBXL_API_VERSION to 0x040800. > Currently libvirt supports LIBXL_API_VERSION >= 0x040400, > essentially Xen >= 4.4. Any comments on this patch? xen-devel is already in cc, but I'll add Wei (one of the Xen tools maintainers) and Juergen (author of commit 57f8b13c) for some additional exposure :-). Currently, "out-of-the-box" Xen >= 4.8 + libvirt setup (dom0 autoballooning enabled) is broken without this fix. Regards, Jim > > Signed-off-by: Jim Fehlig> --- > src/libxl/libxl_domain.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c > index a5314b0..ed73cd2 100644 > --- a/src/libxl/libxl_domain.c > +++ b/src/libxl/libxl_domain.c > @@ -909,6 +909,7 @@ libxlDomainFreeMem(libxl_ctx *ctx, libxl_domain_config > *d_config) > { > uint32_t needed_mem; > uint32_t free_mem; > +int32_t target_mem; > int tries = 3; > int wait_secs = 10; > > @@ -922,7 +923,8 @@ libxlDomainFreeMem(libxl_ctx *ctx, libxl_domain_config > *d_config) > if (free_mem >= needed_mem) > return 0; > > -if (libxl_set_memory_target(ctx, 0, free_mem - needed_mem, > +target_mem = free_mem - needed_mem; > +if (libxl_set_memory_target(ctx, 0, target_mem, > /* relative */ 1, 0) < 0) > goto error; > ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel