Re: [PATCH] libxl: remove LIBXL_MAXMEM_CONSTANT

[Ian Campbell <ian.campbell@citrix.com>]

On Thu, 2015-03-12 at 11:02 +0000, Stefano Stabellini wrote:
> On Thu, 26 Feb 2015, Ian Campbell wrote:
> > On Thu, 2015-02-26 at 12:19 +0000, Stefano Stabellini wrote:
> > > On Wed, 25 Feb 2015, Don Slutz wrote:
> > > > On 02/25/15 10:07, Stefano Stabellini wrote:
> > > > > LIBXL_MAXMEM_CONSTANT is used to increase the maxmem setting for a
> > > > > domain by a constant amount. As it is not clear the reason why we should
> > > > > be doing this, remove the constant.
> > > > > 
> > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > > > CC: mlatimer@suse.com
> > > > > CC: ian.campbell@citrix.com
> > > > > ---
> > > > 
> > > > I think that some sort of link to commit 901230f in QEMU:
> > > > 
> > > > ----
> > > > commit 901230fd8ce053cc21312a2eca2f3ba9f1d103f2
> > > > Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > > Date:   Wed Dec 3 08:15:19 2014 -0500
> > > > 
> > > >     xen-hvm: increase maxmem before calling xc_domain_populate_physmap
> > > > 
> > > >     Increase maxmem before calling xc_domain_populate_physmap_exact to
> > > >     avoid the risk of running out of guest memory. This way we can also
> > > >     avoid complex memory calculations in libxl at domain construction
> > > >     time.
> > > > 
> > > >     This patch fixes an abort() when assigning more than 4 NICs to a VM.
> > > > 
> > > >     upstream-commit-id: c1d322e6048796296555dd36fdd102d7fa2f50bf
> > > > 
> > > >     Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > >     Signed-off-by: Don Slutz <dslutz@verizon.com>
> > > > ----
> > > > 
> > > > Because after this patch and without a "correct" QEMU, the number of
> > > > e1000 NICs a guest can use is less then 4.
> > > 
> > > That is true, in fact is not even a single emulated NIC in my tests.
> > > I can either ask for a backport of
> > > c1d322e6048796296555dd36fdd102d7fa2f50bf "xen-hvm: increase maxmem
> > > before calling xc_domain_populate_physmap" to all QEMU stable branches,
> > 
> > It can't hurt to ask, I think?
> > 
> > > or we just have to keep this around for now and maybe just add a comment
> > > on why it is needed.
> > 
> > (assuming they say no to the backports)
> > 
> > Could we at least make it x86/HVM specific?
>  
> The backport was made but only to 2.2 that is far too recent still. I
> think we should keep the constant for the Xen 4.6 release. But we should
> defenitely write a comment on what's for.

Can we also make it x86/HVM only?

What about scaling it with the number of NICs? Don't we already do
something like that?

> 
> ---
> 
> Document what is the purpose of LIBXL_MAXMEM_CONSTANT
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> 
> ---
> 
> http://bugs.xenproject.org/xen/bug/23
> 
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 934465a..f9e04d8 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -89,6 +89,14 @@
>  #define LIBXL_QEMU_BODGE_TIMEOUT 2
>  #define LIBXL_XENCONSOLE_LIMIT 1048576
>  #define LIBXL_XENCONSOLE_PROTOCOL "vt100"
> +/* LIBXL_MAXMEM_CONSTANT is used to set maxmem higher than the actual amount of
> + * memory of the domain by a constant amount at creation time.
> + * The extra memory is allocated by upstream QEMU based device models up
> + * to v2.1 for the ROM files of emulated network cards.
> + *
> + * v2.2 and later QEMUs are able to increase maxmem themselves whenever needed.
> + * qemu-xen-traditional doesn't allocate memory for ROM files.
> + */
>  #define LIBXL_MAXMEM_CONSTANT 1024
>  #define LIBXL_PV_EXTRA_MEMORY 1024
>  #define LIBXL_HVM_EXTRA_MEMORY 2048