[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] Start of freeze for libvirt-0.9.7 and availability of rc1



On Wed, Nov 02, 2011 at 02:19:07PM -0600, Eric Blake wrote:
> On 11/02/2011 12:57 PM, Guido Günther wrote:
> >Built fine on most Debian architectures:
> >
> >	https://buildd.debian.org/status/package.php?p=libvirt&suite=experimental
> >
> >The built failure on amd64 is due to virnetsockettest failing with:
> >
> >  5) Socket UNIX Accept... libvir: RPC error : Path /build/buildd-libvirt_0.9.7~rc1-1-amd64-EGXZTE/libvirt-0.9.7~rc1/debian/build/tests/virnetsockettest-test.sock too long for unix socket: Cannot allocate memory
> >
> >since the socket path doesn't fit in UNIX_PATH_MAX. Since exceeding the
> >path shouldn't't be fatal I'm using the attached patch.
> 
> >
> >From: =?UTF-8?q?Guido=20G=C3=BCnther?=<agx sigxcpu org>
> >Date: Wed, 2 Nov 2011 19:02:42 +0100
> >Subject: Skip socket test if we exceed UNIX_PATH_MAX.
> >
> >As seen on the amd64 buildd with:
> 
> s/buildd/build/

This one was actually intentional. It's the build daemon, short buildd.

> >+++ b/tests/virnetsockettest.c
> >@@ -205,11 +205,13 @@ static int testSocketUNIXAccept(const void *data ATTRIBUTE_UNUSED)
> >      if (progname[0] == '/') {
> >          if (virAsprintf(&path, "%s-test.sock", progname)<  0) {
> >              virReportOOMError();
> >+            ret = EXIT_AM_SKIP;
> >              goto cleanup;
> >          }
> 
> Wrong place to be checking - virAsprintf() only fails on OOM (malloc
> failure), not on size fit, so your skip is unlikely to be hit.  I
> agree that it's okay to skip the test if run in a subdirectory so
> deep that a Unix socket cannot be created with a name that long, but
> that should be done strlen() check just before
> virNetSocketNewListenUNIX, not by looking for malloc failure.  And
> since we're most likely not out of memory, that may mean we also
> have a bug to fix in our error reporting quality within
> virNetSocketNewListenUNIX.

Yes - the analysis was correct, the patch was horribly broken though.
New versions attached. I wen't for a tempdir though to make sure the
test can succeed.
 -- Guido


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]