[libvirt] [jenkins@libvirt.sigxcpu.org: Build failed in Jenkins: libvirt-tck-debian-wheezy-qemu-session #227]

Hi, this commit ae2163f852448fafce3bfcb2814e823d18e394ea Only let VM drivers block libvirtd timed shutdown broke the libvirt-tck integration build with qemu:///session: http://honk.sigxcpu.org:8001/job/libvirt-tck-debian-wheezy-qemu-session/227/... I won't have a chance to look into this in detail the near future but I thought I'd at least send out a notice. Cheers, -- Guido

As of 1a50ba2cb07d8bb2aa724062889deb9efd7ad9e9 qemu capabilities probing takes longer since we timeout waiting for the monitor socket. When probing qemu for different architectures this can add up so the daemon auto shutdown timeout is reached and the client doesn't have a chance to connect. To avoid that inhibit daemon shutdown during capabilities probing. This fixes http://honk.sigxcpu.org:8001/job/libvirt-tck-debian-wheezy-qemu-session/227/ --- src/qemu/qemu_driver.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e099c5c..7e26fcd 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -640,6 +640,8 @@ qemuStartup(bool privileged, /* Don't have a dom0 so start from 1 */ qemu_driver->nextvmid = 1; + qemu_driver->inhibitCallback(true, qemu_driver->inhibitOpaque); + if (virDomainObjListInit(&qemu_driver->domains) < 0) goto out_of_memory; @@ -917,6 +919,7 @@ qemuStartup(bool privileged, if (!qemu_driver->workerPool) goto error; + qemu_driver->inhibitCallback(false, qemu_driver->inhibitOpaque); qemuDriverUnlock(qemu_driver); qemuAutostartDomains(qemu_driver); @@ -930,8 +933,10 @@ qemuStartup(bool privileged, out_of_memory: virReportOOMError(); error: - if (qemu_driver) + if (qemu_driver) { + qemu_driver->inhibitCallback(false, qemu_driver->inhibitOpaque); qemuDriverUnlock(qemu_driver); + } if (conn) virConnectClose(conn); VIR_FREE(base); -- 1.7.10.4

On 12/05/2012 01:17 PM, Guido Günther wrote:
As of 1a50ba2cb07d8bb2aa724062889deb9efd7ad9e9 qemu capabilities probing takes longer since we timeout waiting for the monitor socket. When probing qemu for different architectures this can add up so the daemon auto shutdown timeout is reached and the client doesn't have a chance to connect. To avoid that inhibit daemon shutdown during capabilities probing.
This fixes
http://honk.sigxcpu.org:8001/job/libvirt-tck-debian-wheezy-qemu-session/227/ --- src/qemu/qemu_driver.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Wed, Dec 05, 2012 at 03:38:04PM -0700, Eric Blake wrote:
On 12/05/2012 01:17 PM, Guido Günther wrote:
As of 1a50ba2cb07d8bb2aa724062889deb9efd7ad9e9 qemu capabilities probing takes longer since we timeout waiting for the monitor socket. When probing qemu for different architectures this can add up so the daemon auto shutdown timeout is reached and the client doesn't have a chance to connect. To avoid that inhibit daemon shutdown during capabilities probing.
This fixes
http://honk.sigxcpu.org:8001/job/libvirt-tck-debian-wheezy-qemu-session/227/ --- src/qemu/qemu_driver.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
ACK.
Hmm, not sure I agree with this approach. I think that libvirtd code itself should inhibit shutdown, while 'virStateInitialize' is running. ie, put the inhibit calls in daemonRunStateInit() in libvirtd.c Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

As of 1a50ba2cb07d8bb2aa724062889deb9efd7ad9e9 qemu capabilities probing takes longer since we timeout waiting for the monitor socket. When probing qemu for different architectures this can add up so the daemon auto shutdown timeout is reached and the client doesn't have a chance to connect. To avoid that inhibit daemon shutdown during driver initialization (which includes capabilities probing). This fixes http://honk.sigxcpu.org:8001/job/libvirt-tck-debian-wheezy-qemu-session/227/ --- daemon/libvirtd.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 91b3c11..7acdbdf 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -846,6 +846,10 @@ static void daemonRunStateInit(void *opaque) { virNetServerPtr srv = opaque; + /* Since driver initialization can take time inhibit daemon shutdown until + we're done so clients get a chance to connect */ + daemonInhibitCallback(true, srv); + /* Start the stateful HV drivers * This is deliberately done after telling the parent process * we're ready, since it can take a long time and this will @@ -856,8 +860,7 @@ static void daemonRunStateInit(void *opaque) VIR_ERROR(_("Driver state initialization failed")); /* Ensure the main event loop quits */ kill(getpid(), SIGTERM); - virObjectUnref(srv); - return; + goto out; } #ifdef HAVE_DBUS @@ -879,9 +882,10 @@ static void daemonRunStateInit(void *opaque) } } #endif - /* Only now accept clients from network */ virNetServerUpdateServices(srv, true); +out: + daemonInhibitCallback(false, srv); virObjectUnref(srv); } -- 1.7.10.4

On Thu, Dec 06, 2012 at 04:47:18PM +0100, Guido Günther wrote:
As of 1a50ba2cb07d8bb2aa724062889deb9efd7ad9e9 qemu capabilities probing takes longer since we timeout waiting for the monitor socket. When probing qemu for different architectures this can add up so the daemon auto shutdown timeout is reached and the client doesn't have a chance to connect. To avoid that inhibit daemon shutdown during driver initialization (which includes capabilities probing).
This fixes
http://honk.sigxcpu.org:8001/job/libvirt-tck-debian-wheezy-qemu-session/227/ --- daemon/libvirtd.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 91b3c11..7acdbdf 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -846,6 +846,10 @@ static void daemonRunStateInit(void *opaque) { virNetServerPtr srv = opaque;
+ /* Since driver initialization can take time inhibit daemon shutdown until + we're done so clients get a chance to connect */ + daemonInhibitCallback(true, srv); + /* Start the stateful HV drivers * This is deliberately done after telling the parent process * we're ready, since it can take a long time and this will @@ -856,8 +860,7 @@ static void daemonRunStateInit(void *opaque) VIR_ERROR(_("Driver state initialization failed")); /* Ensure the main event loop quits */ kill(getpid(), SIGTERM); - virObjectUnref(srv); - return; + goto out; }
#ifdef HAVE_DBUS @@ -879,9 +882,10 @@ static void daemonRunStateInit(void *opaque) } } #endif - /* Only now accept clients from network */ virNetServerUpdateServices(srv, true); +out: + daemonInhibitCallback(false, srv); virObjectUnref(srv); }
ACK, if you rename the goto label 'out' to 'cleanup' to follow more common naming convention Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Thu, Dec 06, 2012 at 03:51:43PM +0000, Daniel P. Berrange wrote:
On Thu, Dec 06, 2012 at 04:47:18PM +0100, Guido Günther wrote:
As of 1a50ba2cb07d8bb2aa724062889deb9efd7ad9e9 qemu capabilities probing takes longer since we timeout waiting for the monitor socket. When probing qemu for different architectures this can add up so the daemon auto shutdown timeout is reached and the client doesn't have a chance to connect. To avoid that inhibit daemon shutdown during driver initialization (which includes capabilities probing).
This fixes
http://honk.sigxcpu.org:8001/job/libvirt-tck-debian-wheezy-qemu-session/227/ --- daemon/libvirtd.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 91b3c11..7acdbdf 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -846,6 +846,10 @@ static void daemonRunStateInit(void *opaque) { virNetServerPtr srv = opaque;
+ /* Since driver initialization can take time inhibit daemon shutdown until + we're done so clients get a chance to connect */ + daemonInhibitCallback(true, srv); + /* Start the stateful HV drivers * This is deliberately done after telling the parent process * we're ready, since it can take a long time and this will @@ -856,8 +860,7 @@ static void daemonRunStateInit(void *opaque) VIR_ERROR(_("Driver state initialization failed")); /* Ensure the main event loop quits */ kill(getpid(), SIGTERM); - virObjectUnref(srv); - return; + goto out; }
#ifdef HAVE_DBUS @@ -879,9 +882,10 @@ static void daemonRunStateInit(void *opaque) } } #endif - /* Only now accept clients from network */ virNetServerUpdateServices(srv, true); +out: + daemonInhibitCallback(false, srv); virObjectUnref(srv); }
ACK, if you rename the goto label 'out' to 'cleanup' to follow more common naming convention
Agreed. cleanup; beats out: by a factor of ten. Pushed with that change. Thanks, -- Guido
Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (3)
-
Daniel P. Berrange
-
Eric Blake
-
Guido Günther