The release-ram capability will run some extra code for postcopy to
release used ram right away, let's just turn that on for the postcopy
unix test always to torture that code path too to make sure release-ram
feature won't break again.  The recovery test needs to turn that off
since release-ram cannot coop with that.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 tests/migration-test.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/tests/migration-test.c b/tests/migration-test.c
index e079e0bdb6..deaec431fe 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -572,8 +572,9 @@ static void test_deprecated(void)
 }
 
 static int migrate_postcopy_prepare(QTestState **from_ptr,
-                                     QTestState **to_ptr,
-                                     bool hide_error)
+                                    QTestState **to_ptr,
+                                    bool hide_error,
+                                    bool release_ram)
 {
     char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
     QTestState *from, *to;
@@ -582,6 +583,10 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
         return -1;
     }
 
+    if (release_ram) {
+        migrate_set_capability(from, "release-ram", "true");
+    }
+
     migrate_set_capability(from, "postcopy-ram", "true");
     migrate_set_capability(to, "postcopy-ram", "true");
     migrate_set_capability(to, "postcopy-blocktime", "true");
@@ -625,7 +630,7 @@ static void test_postcopy(void)
 {
     QTestState *from, *to;
 
-    if (migrate_postcopy_prepare(&from, &to, false)) {
+    if (migrate_postcopy_prepare(&from, &to, false, true)) {
         return;
     }
     migrate_postcopy_start(from, to);
@@ -637,7 +642,8 @@ static void test_postcopy_recovery(void)
     QTestState *from, *to;
     char *uri;
 
-    if (migrate_postcopy_prepare(&from, &to, true)) {
+    /* The release-ram feature cannot work with postcopy recovery. */
+    if (migrate_postcopy_prepare(&from, &to, true, false)) {
         return;
     }
 
-- 
2.17.1

Peter Xu <peterx@redhat.com> wrote:
> The release-ram capability will run some extra code for postcopy to
> release used ram right away, let's just turn that on for the postcopy
> unix test always to torture that code path too to make sure release-ram
> feature won't break again.  The recovery test needs to turn that off
> since release-ram cannot coop with that.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

But I think that the proper thing to do here is to have two tests.  One
for postcopy and another for postcopy + release-ram.

On Tue, Jul 24, 2018 at 11:25:24AM +0200, Juan Quintela wrote:
> Peter Xu <peterx@redhat.com> wrote:
> > The release-ram capability will run some extra code for postcopy to
> > release used ram right away, let's just turn that on for the postcopy
> > unix test always to torture that code path too to make sure release-ram
> > feature won't break again.  The recovery test needs to turn that off
> > since release-ram cannot coop with that.
> >
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> 
> Reviewed-by: Juan Quintela <quintela@redhat.com>

Thanks.

> 
> But I think that the proper thing to do here is to have two tests.  One
> for postcopy and another for postcopy + release-ram.

Yeah I thought about it too, but I am not sure whether it'll worth it
to have a separate test for the release-ram feature (basically that's
some extra seconds for every unit test, even on relatively fast CPUs).
I did it this way since IMHO release-ram is mostly adding extra code
path to the postcopy logic, hence we should not miss much (or any) of
the old test path.  Ideally we should still cover all the postcopy
code path that we want to test.

Regards,

-- 
Peter Xu

* Peter Xu (peterx@redhat.com) wrote:
> On Tue, Jul 24, 2018 at 11:25:24AM +0200, Juan Quintela wrote:
> > Peter Xu <peterx@redhat.com> wrote:
> > > The release-ram capability will run some extra code for postcopy to
> > > release used ram right away, let's just turn that on for the postcopy
> > > unix test always to torture that code path too to make sure release-ram
> > > feature won't break again.  The recovery test needs to turn that off
> > > since release-ram cannot coop with that.
> > >
> > > Signed-off-by: Peter Xu <peterx@redhat.com>
> > 
> > Reviewed-by: Juan Quintela <quintela@redhat.com>
> 
> Thanks.
> 
> > 
> > But I think that the proper thing to do here is to have two tests.  One
> > for postcopy and another for postcopy + release-ram.
> 
> Yeah I thought about it too, but I am not sure whether it'll worth it
> to have a separate test for the release-ram feature (basically that's
> some extra seconds for every unit test, even on relatively fast CPUs).
> I did it this way since IMHO release-ram is mostly adding extra code
> path to the postcopy logic, hence we should not miss much (or any) of
> the old test path.  Ideally we should still cover all the postcopy
> code path that we want to test.

It's worth being a bit careful, since I'm not sure if release-ram has
ever been tested on hosts with larger page size; my suspicion is you
might get a spew of errors on Power.

Dave

> Regards,
> 
> -- 
> Peter Xu
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

On Tue, Jul 24, 2018 at 12:42:06PM +0100, Dr. David Alan Gilbert wrote:
> * Peter Xu (peterx@redhat.com) wrote:
> > On Tue, Jul 24, 2018 at 11:25:24AM +0200, Juan Quintela wrote:
> > > Peter Xu <peterx@redhat.com> wrote:
> > > > The release-ram capability will run some extra code for postcopy to
> > > > release used ram right away, let's just turn that on for the postcopy
> > > > unix test always to torture that code path too to make sure release-ram
> > > > feature won't break again.  The recovery test needs to turn that off
> > > > since release-ram cannot coop with that.
> > > >
> > > > Signed-off-by: Peter Xu <peterx@redhat.com>
> > > 
> > > Reviewed-by: Juan Quintela <quintela@redhat.com>
> > 
> > Thanks.
> > 
> > > 
> > > But I think that the proper thing to do here is to have two tests.  One
> > > for postcopy and another for postcopy + release-ram.
> > 
> > Yeah I thought about it too, but I am not sure whether it'll worth it
> > to have a separate test for the release-ram feature (basically that's
> > some extra seconds for every unit test, even on relatively fast CPUs).
> > I did it this way since IMHO release-ram is mostly adding extra code
> > path to the postcopy logic, hence we should not miss much (or any) of
> > the old test path.  Ideally we should still cover all the postcopy
> > code path that we want to test.
> 
> It's worth being a bit careful, since I'm not sure if release-ram has
> ever been tested on hosts with larger page size; my suspicion is you
> might get a spew of errors on Power.

Oh I hope not.  If it happens, please feel free to drop this last
patch for 3.0.  Or I can provide a x86-only version if preferred.

Regards,

-- 
Peter Xu