Re: QEMU's tests/unit/test-iov times out on NetBSD and OpenBSD

["Daniel P. Berrangé" <berrange@redhat.com>]

On Fri, Jan 19, 2024 at 05:13:25PM +0100, Thomas Huth wrote:
> On 19/01/2024 17.07, Daniel P. Berrangé wrote:
> > On Fri, Jan 19, 2024 at 03:55:49PM +0000, Thomas Huth wrote:
> > > 
> > >   Hi,
> > > 
> > > since we recently introduced test timouts in QEMU's meson set up, I noticed
> > > that the tests/unit/test-iov times out when doing "make vm-build-netbsd
> > > BUILD_TARGET=check-unit" (or vm-build-openbsd).
> > > 
> > > And indeed, when increasing the timeout, you can see that the test-iov runs
> > > for multiple minutes on these BSDs while it finishes within few seconds on
> > > Linux.
> > > 
> > > I had a closer look at the test, and the problem seems to be the
> > > 
> > >   usleep(g_test_rand_int_range(0, 30));
> > > 
> > > in the test_io() function. If I get that right, the usleep() seems to be
> > > more or less precise on (modern) Linux, but it seems like it sleeps for
> > > multiple milliseconds (not microseconds) on the BSDs. Since it is used in a
> > > nested loop, these milliseconds add up to a long time in total during the
> > > test.
> > > 
> > > Does anybody have an idea how to fix that? Is there a more precise (but stil
> > > portable) way to sleep less long here? Or could we maybe remove the usleep()
> > > here completely (it does not seem to have a real benefit for testing as far
> > > as I can see)?
> > 
> > 'g_usleep' has the same API contract, but is implemented in terms
> > of 'nanosleep' on *NIX. So as a quick test, try switching usleep
> > to g_usleep and see if we get lucky.
> 
> No, that seems to behave the same way, unfortunately.
> 
> Do you see a reason why we'd really need the usleep() here at all?
> Otherwise, I think I'll send a patch to simply remove it...

We're looping on iov_send() on a non-blocking socket.

In the EAGAIN scenario we select() to wait for writability which is
good.

In the scenario where we wrote at least 1 byte, however, we have
the usleep(). Presumably the idea is that we should not immediately
try iov_send again as it might not be ready to send more data. This
should only be needed, however, if there is still more data waiting
to be sent and we should select() instead anyway. So I think this:

               do {
                   s = g_test_rand_int_range(0, j - k + 1);
                   r = iov_send(sv[1], iov, niov, k, s);
                   g_assert(memcmp(iov, siov, sizeof(*iov)*niov) == 0);
                   if (r >= 0) {
                       k += r;
                       usleep(g_test_rand_int_range(0, 30));
                   } else if (errno == EAGAIN) {
                       select(sv[1]+1, NULL, &fds, NULL, NULL);
                       continue;
                   } else {
                       perror("send");
                       exit(1);
                   }
               } while(k < j);

should change to:


               do {
                   s = g_test_rand_int_range(0, j - k + 1);
                   r = iov_send(sv[1], iov, niov, k, s);
                   g_assert(memcmp(iov, siov, sizeof(*iov)*niov) == 0);
		   if (r == -1 && errno == EAGAIN) {
		       r = 0;
		   }
                   if (r >= 0)
                       k += r;
		       if (k < j) {
                         select(sv[1]+1, NULL, &fds, NULL, NULL);
                         continue;
		       }
                   } else {
                       perror("send");
                       exit(1);
                   }
               } while(k < j);


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