Re: [Qemu-devel] [PATCH v2 4/5] test: Postcopy

[Marcel Apfelbaum]

On 05/03/2016 01:34 PM, Dr. David Alan Gilbert wrote:
> * Marcel Apfelbaum (address@hidden) wrote:
> > On 05/03/2016 12:22 PM, Dr. David Alan Gilbert wrote:
>
> > > > > +        QDECREF(rsp);
> > > > > +        usleep(1000 * 100);
> > > > > +    } while (!completed);
> > > > > +}
> > > >
> > > > It is possible that the migration will not finish (from some reason) ?
> > > > Do you expect the test runner to stop the test?
> > >
> > > The migration should always complete in postcopy; failure to complete
> > > is failure of the test;   although I've not explicitly added any timeouts.
> >
> > OK, so on a failure I run make-check and it never ends ? :)
> > For build-bots there is no problem since they kill tests on timeout,
> > but we are running it manually.
> > However, we can add the test as is and if it 'behaves' is OK.
>
> Most of the failure cases will cause an assert/crash - a hang should be
> unlikely.

Agreed

  I'm assuming a bunch of tests can in principal hang if they go wrong;

Not sure about that.

> if there's some qtest structure for timeouts I can add it.

Not that I know of.

But again, let's see how it behaves, maybe there is no problem at all.

> > > <snip>
> > >
> > > > > +int main(int argc, char **argv)
> > > > > +{
> > > > > +    char template[] = "/tmp/postcopy-test-XXXXXX";
> > > >
> > > > I would not explicitly use /tmp/
> > >
> > > The ivshmem-test, vhost-user-test and test-qga seem to do it this way;
> > > what's your preferred fix?
> >
> > You could use the P_tmpdir macro instead of '/tmp', but again,
> > if all the tests assume /tmp existence maybe is not an issue.
>
> I don't see any use of P_tmpdir in qemu at all; we do have one use of
> g_get_tmp_dir (in util/memfd.c).

Even better! I like the way it look for it:
    On UNIX, this is taken from the TMPDIR environment variable.
    If the variable is not set, P_tmpdir is used, as defined by the system C 
library.
    Failing that, a hard-coded default of "/tmp" is returned.

> > > > > +    int ret;
> > > > > +
> > > > > +    g_test_init(&argc, &argv, NULL);
> > > > > +
> > > > > +    if (!ufd_version_check()) {
> > > > > +        return 0;
> > > > > +    }
> > > > > +
> > > > > +    tmpfs = mkdtemp(template);
> > > > > +    if (!tmpfs) {
> > > > > +        g_test_message("mkdtemp on path (%s): %s\n", template, 
> > > > > strerror(errno));
> > > > > +    }
> > > > > +    g_assert(tmpfs);
> > > > > +
> > > > > +    module_call_init(MODULE_INIT_QOM);
> > > > > +
> > > > > +    qtest_add_func("/postcopy", test_migrate);
> > > > > +
> > > >
> > > > How much time does this test takes? If is too long, maybe we should not run it
> > > > automatically as part of make check, but using an environment variable.
> > >
> > > 4 seconds on my laptop; it seems reasonable.
> >
> > make-check takes about 1 and a half minute for x86_64 configuration, 4 seconds
> > more seems reasonable indeed.
>
> Dave
>
> > Thanks,
> > Marcel
> >
> >
> > > Dave
> > > --
> > > Dr. David Alan Gilbert / address@hidden / Manchester, UK
> --
> Dr. David Alan Gilbert / address@hidden / Manchester, UK