Re: [PATCH 13/42] migration-test: Check for shared memory like for everything else

[Peter Xu]

On Wed, Jun 21, 2023 at 12:07:20PM +0200, Juan Quintela wrote:
> Peter Xu <peterx@redhat.com> wrote:
> > On Fri, Jun 09, 2023 at 12:49:14AM +0200, Juan Quintela wrote:
> >> Makes things easier and cleaner.
> >> 
> >> Signed-off-by: Juan Quintela <quintela@redhat.com>
> >> ---
> >>  tests/qtest/migration-test.c | 20 ++++++++++++--------
> >>  1 file changed, 12 insertions(+), 8 deletions(-)
> >> 
> >> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> >> index daaf5cd71a..5837060138 100644
> >> --- a/tests/qtest/migration-test.c
> >> +++ b/tests/qtest/migration-test.c
> >> @@ -645,13 +645,6 @@ static int test_migrate_start(QTestState **from, 
> >> QTestState **to,
> >>      const char *arch = qtest_get_arch();
> >>      const char *memory_size;
> >>  
> >> -    if (args->use_shmem) {
> >> -        if (!g_file_test("/dev/shm", G_FILE_TEST_IS_DIR)) {
> >> -            g_test_skip("/dev/shm is not supported");
> >> -            return -1;
> >> -        }
> >> -    }
> >
> > Maybe assert on: "!args->use_shmem || shm_supported()" here?
> 
> Nope.
> 
> We are being extra defensive in some tests.

This will protect a new test passing in use_shmem=true without checking
shm_supported().  It'll then fail at starting the VM I think otherwise.

> 
> It is tested here
> 
> >> -    qtest_add_func("/migration/ignore_shared", test_ignore_shared);
> >> +    if (shm_supported()) {
> >> +        qtest_add_func("/migration/ignore_shared", test_ignore_shared);
> >> +    }
> 
> Checking (in the same code path) once in the same file looks like enough
> to me.
> 
> Thanks, Juan.
> 

-- 
Peter Xu