[PATCH] libxl: remove LIBXL_MAXMEM_CONSTANT

[PATCH] libxl: remove LIBXL_MAXMEM_CONSTANT

[Stefano Stabellini]

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
---
 tools/libxl/libxl.c          |    9 ++++-----
 tools/libxl/libxl_dom.c      |    3 +--
 tools/libxl/libxl_internal.h |    1 -
 3 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index b9a1941..9556a92 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -4572,11 +4572,11 @@ int libxl_domain_setmaxmem(libxl_ctx *ctx, uint32_t domid, uint32_t max_memkb)
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "memory_static_max must be greater than or or equal to memory_dynamic_max\n");
         goto out;
     }
-    rc = xc_domain_setmaxmem(ctx->xch, domid, max_memkb + LIBXL_MAXMEM_CONSTANT);
+    rc = xc_domain_setmaxmem(ctx->xch, domid, max_memkb);
     if (rc != 0) {
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
                 "xc_domain_setmaxmem domid=%d memkb=%d failed "
-                "rc=%d\n", domid, max_memkb + LIBXL_MAXMEM_CONSTANT, rc);
+                "rc=%d\n", domid, max_memkb, rc);
         goto out;
     }
 
@@ -4796,12 +4796,11 @@ retry_transaction:
 
     if (enforce) {
         memorykb = new_target_memkb + videoram;
-        rc = xc_domain_setmaxmem(ctx->xch, domid, memorykb +
-                LIBXL_MAXMEM_CONSTANT);
+        rc = xc_domain_setmaxmem(ctx->xch, domid, memorykb);
         if (rc != 0) {
             LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
                     "xc_domain_setmaxmem domid=%d memkb=%d failed "
-                    "rc=%d\n", domid, memorykb + LIBXL_MAXMEM_CONSTANT, rc);
+                    "rc=%d\n", domid, memorykb, rc);
             abort_transaction = 1;
             goto out;
         }
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index a16d4a1..923ba5c 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -408,8 +408,7 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
         }
     }
 
-    if (xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb +
-        LIBXL_MAXMEM_CONSTANT) < 0) {
+    if (xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb) < 0) {
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn't set max memory");
         return ERROR_FAIL;
     }
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 934465a..d5c5b68 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -89,7 +89,6 @@
 #define LIBXL_QEMU_BODGE_TIMEOUT 2
 #define LIBXL_XENCONSOLE_LIMIT 1048576
 #define LIBXL_XENCONSOLE_PROTOCOL "vt100"
-#define LIBXL_MAXMEM_CONSTANT 1024
 #define LIBXL_PV_EXTRA_MEMORY 1024
 #define LIBXL_HVM_EXTRA_MEMORY 2048
 #define LIBXL_MIN_DOM0_MEM (128*1024)
-- 
1.7.10.4

Re: [PATCH] libxl: remove LIBXL_MAXMEM_CONSTANT

[Don Slutz]

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.

   -Don Slutz


>  tools/libxl/libxl.c          |    9 ++++-----
>  tools/libxl/libxl_dom.c      |    3 +--
>  tools/libxl/libxl_internal.h |    1 -
>  3 files changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index b9a1941..9556a92 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -4572,11 +4572,11 @@ int libxl_domain_setmaxmem(libxl_ctx *ctx, uint32_t domid, uint32_t max_memkb)
>          LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "memory_static_max must be greater than or or equal to memory_dynamic_max\n");
>          goto out;
>      }
> -    rc = xc_domain_setmaxmem(ctx->xch, domid, max_memkb + LIBXL_MAXMEM_CONSTANT);
> +    rc = xc_domain_setmaxmem(ctx->xch, domid, max_memkb);
>      if (rc != 0) {
>          LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
>                  "xc_domain_setmaxmem domid=%d memkb=%d failed "
> -                "rc=%d\n", domid, max_memkb + LIBXL_MAXMEM_CONSTANT, rc);
> +                "rc=%d\n", domid, max_memkb, rc);
>          goto out;
>      }
>  
> @@ -4796,12 +4796,11 @@ retry_transaction:
>  
>      if (enforce) {
>          memorykb = new_target_memkb + videoram;
> -        rc = xc_domain_setmaxmem(ctx->xch, domid, memorykb +
> -                LIBXL_MAXMEM_CONSTANT);
> +        rc = xc_domain_setmaxmem(ctx->xch, domid, memorykb);
>          if (rc != 0) {
>              LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
>                      "xc_domain_setmaxmem domid=%d memkb=%d failed "
> -                    "rc=%d\n", domid, memorykb + LIBXL_MAXMEM_CONSTANT, rc);
> +                    "rc=%d\n", domid, memorykb, rc);
>              abort_transaction = 1;
>              goto out;
>          }
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index a16d4a1..923ba5c 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -408,8 +408,7 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
>          }
>      }
>  
> -    if (xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb +
> -        LIBXL_MAXMEM_CONSTANT) < 0) {
> +    if (xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb) < 0) {
>          LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn't set max memory");
>          return ERROR_FAIL;
>      }
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 934465a..d5c5b68 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -89,7 +89,6 @@
>  #define LIBXL_QEMU_BODGE_TIMEOUT 2
>  #define LIBXL_XENCONSOLE_LIMIT 1048576
>  #define LIBXL_XENCONSOLE_PROTOCOL "vt100"
> -#define LIBXL_MAXMEM_CONSTANT 1024
>  #define LIBXL_PV_EXTRA_MEMORY 1024
>  #define LIBXL_HVM_EXTRA_MEMORY 2048
>  #define LIBXL_MIN_DOM0_MEM (128*1024)
> 

Re: [PATCH] libxl: remove LIBXL_MAXMEM_CONSTANT

[Stefano Stabellini]

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,
or we just have to keep this around for now and maybe just add a comment
on why it is needed.


> >  tools/libxl/libxl.c          |    9 ++++-----
> >  tools/libxl/libxl_dom.c      |    3 +--
> >  tools/libxl/libxl_internal.h |    1 -
> >  3 files changed, 5 insertions(+), 8 deletions(-)
> > 
> > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> > index b9a1941..9556a92 100644
> > --- a/tools/libxl/libxl.c
> > +++ b/tools/libxl/libxl.c
> > @@ -4572,11 +4572,11 @@ int libxl_domain_setmaxmem(libxl_ctx *ctx, uint32_t domid, uint32_t max_memkb)
> >          LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "memory_static_max must be greater than or or equal to memory_dynamic_max\n");
> >          goto out;
> >      }
> > -    rc = xc_domain_setmaxmem(ctx->xch, domid, max_memkb + LIBXL_MAXMEM_CONSTANT);
> > +    rc = xc_domain_setmaxmem(ctx->xch, domid, max_memkb);
> >      if (rc != 0) {
> >          LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
> >                  "xc_domain_setmaxmem domid=%d memkb=%d failed "
> > -                "rc=%d\n", domid, max_memkb + LIBXL_MAXMEM_CONSTANT, rc);
> > +                "rc=%d\n", domid, max_memkb, rc);
> >          goto out;
> >      }
> >  
> > @@ -4796,12 +4796,11 @@ retry_transaction:
> >  
> >      if (enforce) {
> >          memorykb = new_target_memkb + videoram;
> > -        rc = xc_domain_setmaxmem(ctx->xch, domid, memorykb +
> > -                LIBXL_MAXMEM_CONSTANT);
> > +        rc = xc_domain_setmaxmem(ctx->xch, domid, memorykb);
> >          if (rc != 0) {
> >              LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
> >                      "xc_domain_setmaxmem domid=%d memkb=%d failed "
> > -                    "rc=%d\n", domid, memorykb + LIBXL_MAXMEM_CONSTANT, rc);
> > +                    "rc=%d\n", domid, memorykb, rc);
> >              abort_transaction = 1;
> >              goto out;
> >          }
> > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> > index a16d4a1..923ba5c 100644
> > --- a/tools/libxl/libxl_dom.c
> > +++ b/tools/libxl/libxl_dom.c
> > @@ -408,8 +408,7 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
> >          }
> >      }
> >  
> > -    if (xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb +
> > -        LIBXL_MAXMEM_CONSTANT) < 0) {
> > +    if (xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb) < 0) {
> >          LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn't set max memory");
> >          return ERROR_FAIL;
> >      }
> > diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> > index 934465a..d5c5b68 100644
> > --- a/tools/libxl/libxl_internal.h
> > +++ b/tools/libxl/libxl_internal.h
> > @@ -89,7 +89,6 @@
> >  #define LIBXL_QEMU_BODGE_TIMEOUT 2
> >  #define LIBXL_XENCONSOLE_LIMIT 1048576
> >  #define LIBXL_XENCONSOLE_PROTOCOL "vt100"
> > -#define LIBXL_MAXMEM_CONSTANT 1024
> >  #define LIBXL_PV_EXTRA_MEMORY 1024
> >  #define LIBXL_HVM_EXTRA_MEMORY 2048
> >  #define LIBXL_MIN_DOM0_MEM (128*1024)
> > 
> 

Re: [PATCH] libxl: remove LIBXL_MAXMEM_CONSTANT

[Ian Campbell]

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?

Ian.

Re: [PATCH] libxl: remove LIBXL_MAXMEM_CONSTANT

[Ian Campbell]

On Wed, 2015-02-25 at 15:07 +0000, 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.

When you resend please could you include a reference (either here of
below the ---) to http://bugs.xenproject.org/xen/bug/23 so one of us
remembers to close it.

Re: [PATCH] libxl: remove LIBXL_MAXMEM_CONSTANT

[Stefano Stabellini]

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.

---

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

Re: [PATCH] libxl: remove LIBXL_MAXMEM_CONSTANT

[Ian Campbell]

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

Re: [PATCH] libxl: remove LIBXL_MAXMEM_CONSTANT

[Stefano Stabellini]

On Thu, 12 Mar 2015, Ian Campbell wrote:
> 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?

I would leave it like this for this release, then remove the constant
entirely at the beginning of the next one, when the required QEMU patch
is going to be more widespread.  We will have to introduce the concept
of minimum QEMU version supported by Xen, but at that point requiring
QEMU >= v2.2 won't be so bad. 


> > ---
> > 
> > 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
> 
>