Libguestfs - May 2018 - Re: [PATCH] tests: Increase appliance memory when testing 256+ disks.

Currently the tests fail on x86 with recent kernels:

  FAIL: test-255-disks.sh

This confused me for a while because our other test program
(utils/max-disks/max-disks.pl) reports that it should be possible to
add 255 disks.

Well it turns out that the default amount of appliance memory is
sufficient if you're just adding disks, but if you try to add _and_
partition those disks there's insufficient memory and the daemon falls
over with an out of memory error.

I considered increasing the default appliance memory, and this is
certainly one way to fix the problem.  However this penalises every
user for what is a fairly niche use case.  This takes an alternative
approach of increasing the appliance memory for the affected tests.

Related links:

  https://bugzilla.redhat.com/show_bug.cgi?id=1478201
 
https://rwmj.wordpress.com/2017/04/28/how-many-disks-can-you-add-to-a-virtual-linux-machine-contd/

Rich.

---
 tests/disks/test-add-disks.c | 9 +++++++++
 utils/max-disks/max-disks.pl | 1 +
 2 files changed, 10 insertions(+)

diff --git a/tests/disks/test-add-disks.c b/tests/disks/test-add-disks.c
index a7365d1d1..f3eb87bb4 100644
--- a/tests/disks/test-add-disks.c
+++ b/tests/disks/test-add-disks.c
@@ -98,6 +98,7 @@ main (int argc, char *argv[])
   guestfs_h *g;
   char *tmpdir;
   ssize_t n = -1; /* -1: not set  0: max  > 0: specific value */
+  int m;
 
   g = guestfs_create ();
   if (g == NULL)
@@ -158,6 +159,14 @@ main (int argc, char *argv[])
   }
   ndisks = n;
 
+  /* Increase memory available to the appliance.  On x86 the default
+   * is not enough to both detect and partition 256 disks.
+   */
+  m = guestfs_get_memsize (g);
+  if (m == -1 ||
+      guestfs_set_memsize (g, m * 20 / 5) == -1)
+    error (EXIT_FAILURE, 0, "get or set memsize failed");
+
   tmpdir = guestfs_get_cachedir (g);
   if (tmpdir == NULL)
     exit (EXIT_FAILURE);
diff --git a/utils/max-disks/max-disks.pl b/utils/max-disks/max-disks.pl
index f7fe5bca2..714357fbb 100755
--- a/utils/max-disks/max-disks.pl
+++ b/utils/max-disks/max-disks.pl
@@ -41,6 +41,7 @@ my $mid = 1024;
 
 # Get the kernel under test.
 my $g = Sys::Guestfs->new ();
+$g->set_memsize ($g->get_memsize () * 20 / 5);
 $g->launch ();
 my %kernel = $g->utsname;
 $g->close ();
-- 
2.15.1

On Wed, May 16, 2018 at 12:51:19PM +0100, Richard W.M. Jones
wrote:
> Currently the tests fail on x86 with recent kernels:
> 
>   FAIL: test-255-disks.sh
> 
> This confused me for a while because our other test program
> (utils/max-disks/max-disks.pl) reports that it should be possible to
> add 255 disks.
> 
> Well it turns out that the default amount of appliance memory is
> sufficient if you're just adding disks, but if you try to add _and_
> partition those disks there's insufficient memory and the daemon falls
> over with an out of memory error.
> 
> I considered increasing the default appliance memory, and this is
> certainly one way to fix the problem.  However this penalises every
> user for what is a fairly niche use case.  This takes an alternative
> approach of increasing the appliance memory for the affected tests.
I wonder how much it would really penalize users in practice ? KVM doesn't
fault in memory pages unless the guest OS actually touches them. So increasing
the memory size would mean the guest kernel has to allocate a bit more memory
for page tables, but aside from that the extra RAM would be unused unless the
user did some guestfs operation that needed it. Perhaps most likely cause of
using the extra "free" RAM might be page cache in the guest but not
sure how
much that comes into play with guestfs operations ?

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

On Wednesday, 16 May 2018 13:51:20 CEST Richard W.M. Jones
wrote:
> ---
>  tests/disks/test-add-disks.c | 9 +++++++++
>  utils/max-disks/max-disks.pl | 1 +
>  2 files changed, 10 insertions(+)
> 
> diff --git a/tests/disks/test-add-disks.c b/tests/disks/test-add-disks.c
> index a7365d1d1..f3eb87bb4 100644
> --- a/tests/disks/test-add-disks.c
> +++ b/tests/disks/test-add-disks.c
> @@ -98,6 +98,7 @@ main (int argc, char *argv[])
>    guestfs_h *g;
>    char *tmpdir;
>    ssize_t n = -1; /* -1: not set  0: max  > 0: specific value */
> +  int m;
>  
>    g = guestfs_create ();
>    if (g == NULL)
> @@ -158,6 +159,14 @@ main (int argc, char *argv[])
>    }
>    ndisks = n;
>  
> +  /* Increase memory available to the appliance.  On x86 the default
> +   * is not enough to both detect and partition 256 disks.
> +   */
> +  m = guestfs_get_memsize (g);
> +  if (m == -1 ||
> +      guestfs_set_memsize (g, m * 20 / 5) == -1)
> +    error (EXIT_FAILURE, 0, "get or set memsize failed");
> +
>    tmpdir = guestfs_get_cachedir (g);
>    if (tmpdir == NULL)
>      exit (EXIT_FAILURE);
This is OK.
> diff --git a/utils/max-disks/max-disks.pl b/utils/max-disks/max-disks.pl
> index f7fe5bca2..714357fbb 100755
> --- a/utils/max-disks/max-disks.pl
> +++ b/utils/max-disks/max-disks.pl
> @@ -41,6 +41,7 @@ my $mid = 1024;
>  
>  # Get the kernel under test.
>  my $g = Sys::Guestfs->new ();
> +$g->set_memsize ($g->get_memsize () * 20 / 5);
>  $g->launch ();
>  my %kernel = $g->utsname;
>  $g->close ();
I don't see how this changes the situation, given that the handle
here is used only to get the information of the kernel running in the
appliance.  I guess should rather be done to the handle created in the
test_mid subroutine?

-- 
Pino Toscano