6:28 a.m.
This small series converts the two places where we autostart
libvirtd/virtlockd over to use virFileFindResource so that
./tools/virsh -c qemu:///session will use the daemons from
the source tree instead of /usr/sbin
Daniel P. Berrange (3):
Use virFileFindResource to locate libvirtd daemon
Use virFileFindResource to locate virtlockd daemon
Make autostart of virtlockd actually work
src/locking/lock_daemon.c | 20 +++++++++++++++++++
src/locking/lock_driver_lockd.c | 25 +++++++----------------
src/remote/remote_driver.c | 44 ++++++-----------------------------------
3 files changed, 33 insertions(+), 56 deletions(-)
--
1.9.0
6:28 a.m.
New subject: [libvirt] [PATCH 1/3] Use virFileFindResource to locate libvirtd daemon
Make the remote driver use virFileFindResource to find the
libvirt daemon path, so that it executes the in-builddir
daemon if run from source tree.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/remote/remote_driver.c | 44 ++++++--------------------------------------
1 file changed, 6 insertions(+), 38 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index ed7dde6..d9be756 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -174,37 +174,6 @@ remoteStateInitialize(bool privileged ATTRIBUTE_UNUSED,
}
#endif
-#ifndef WIN32
-/**
- * remoteFindDaemonPath:
- *
- * Tries to find the path to the libvirtd binary.
- *
- * Returns path on success or NULL in case of error.
- */
-static const char *
-remoteFindDaemonPath(void)
-{
- static const char *serverPaths[] = {
- SBINDIR "/libvirtd",
- SBINDIR "/libvirtd_dbg",
- NULL
- };
- size_t i;
- const char *customDaemon = virGetEnvBlockSUID("LIBVIRTD_PATH");
-
- if (customDaemon)
- return customDaemon;
-
- for (i = 0; serverPaths[i]; i++) {
- if (virFileIsExecutable(serverPaths[i])) {
- return serverPaths[i];
- }
- }
- return NULL;
-}
-#endif
-
static void
remoteDomainBuildEventLifecycle(virNetClientProgramPtr prog ATTRIBUTE_UNUSED,
@@ -887,14 +856,13 @@ doRemoteOpen(virConnectPtr conn,
}
if ((flags & VIR_DRV_OPEN_REMOTE_AUTOSTART) &&
- !(daemonPath = remoteFindDaemonPath())) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Unable to locate libvirtd daemon in %s "
- "(to override, set $LIBVIRTD_PATH to the "
- "name of the libvirtd binary)"),
- SBINDIR);
+ !(daemonPath = virFileFindResourceFull("libvirtd",
+ NULL, NULL,
+ "daemon",
+ SBINDIR,
+ "LIBVIRTD_PATH")))
goto failed;
- }
+
if (!(priv->client = virNetClientNewUNIX(sockname,
flags &
VIR_DRV_OPEN_REMOTE_AUTOSTART,
daemonPath)))
--
1.9.0
9:01 a.m.
New subject: [libvirt] [PATCH 1/3] Use virFileFindResource to locate libvirtd daemon
On 04/25/2014 05:28 AM, Daniel P. Berrange wrote:
Make the remote driver use virFileFindResource to find the
libvirt daemon path, so that it executes the in-builddir
daemon if run from source tree.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/remote/remote_driver.c | 44 ++++++--------------------------------------
1 file changed, 6 insertions(+), 38 deletions(-)
Nice diffstat - proof that we made a good factorization.
-static const char *
-remoteFindDaemonPath(void)
-{
- static const char *serverPaths[] = {
- SBINDIR "/libvirtd",
- SBINDIR "/libvirtd_dbg",
- NULL
- };
- size_t i;
- const char *customDaemon = virGetEnvBlockSUID("LIBVIRTD_PATH");
-
- if (customDaemon)
- return customDaemon;
The old code assumes LIBVIRTD_PATH provides "/path/to/libvirtd"
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Unable to locate libvirtd daemon in %s "
- "(to override, set $LIBVIRTD_PATH to the "
- "name of the libvirtd binary)"),
- SBINDIR);
+ !(daemonPath = virFileFindResourceFull("libvirtd",
+ NULL, NULL,
+ "daemon",
+ SBINDIR,
+ "LIBVIRTD_PATH")))
the new code assumes LIBVIRTD_PATH provides "/path/to" which contains
libvirtd.
Don't know how many users this will impact, but the only former
documentation of LIBVIRTD_PATH was in the error message. I can live
with the change in semantics, since it is not formally documented on the
web page as something users would normally fiddle with.
ACK.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
9:25 a.m.
New subject: [libvirt] [PATCH 1/3] Use virFileFindResource to locate libvirtd daemon
On Fri, Apr 25, 2014 at 08:01:58AM -0600, Eric Blake wrote:
On 04/25/2014 05:28 AM, Daniel P. Berrange wrote:
> Make the remote driver use virFileFindResource to find the
> libvirt daemon path, so that it executes the in-builddir
> daemon if run from source tree.
>
> Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
> ---
> src/remote/remote_driver.c | 44 ++++++--------------------------------------
> 1 file changed, 6 insertions(+), 38 deletions(-)
Nice diffstat - proof that we made a good factorization.
> -static const char *
> -remoteFindDaemonPath(void)
> -{
> - static const char *serverPaths[] = {
> - SBINDIR "/libvirtd",
> - SBINDIR "/libvirtd_dbg",
> - NULL
> - };
> - size_t i;
> - const char *customDaemon = virGetEnvBlockSUID("LIBVIRTD_PATH");
> -
> - if (customDaemon)
> - return customDaemon;
The old code assumes LIBVIRTD_PATH provides "/path/to/libvirtd"
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("Unable to locate libvirtd daemon in %s "
> - "(to override, set $LIBVIRTD_PATH to the "
> - "name of the libvirtd binary)"),
> - SBINDIR);
> + !(daemonPath = virFileFindResourceFull("libvirtd",
> + NULL, NULL,
> + "daemon",
> + SBINDIR,
> + "LIBVIRTD_PATH")))
the new code assumes LIBVIRTD_PATH provides "/path/to" which contains
libvirtd.
Don't know how many users this will impact, but the only former
documentation of LIBVIRTD_PATH was in the error message. I can live
with the change in semantics, since it is not formally documented on the
web page as something users would normally fiddle with.
The 'run' script uses this, but we can just delete those lines from
the 'run' script now, since we'll "do the right thing"
automatically
Regards,
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 :|
11:52 a.m.
New subject: [libvirt] [PATCH 1/3] Use virFileFindResource to locate libvirtd daemon
On 04/25/2014 08:25 AM, Daniel P. Berrange wrote:
On Fri, Apr 25, 2014 at 08:01:58AM -0600, Eric Blake wrote:
> On 04/25/2014 05:28 AM, Daniel P. Berrange wrote:
>> Make the remote driver use virFileFindResource to find the
>> libvirt daemon path, so that it executes the in-builddir
>> daemon if run from source tree.
>>
>
> the new code assumes LIBVIRTD_PATH provides "/path/to" which contains
> libvirtd.
>
> Don't know how many users this will impact, but the only former
> documentation of LIBVIRTD_PATH was in the error message. I can live
> with the change in semantics, since it is not formally documented on the
> web page as something users would normally fiddle with.
The 'run' script uses this, but we can just delete those lines from
the 'run' script now, since we'll "do the right thing"
automatically
In fact, you HAVE to fix the run script, or using it will break :) (I
just tested it now, on a virgin VM with no libvirt installed and just an
in-tree build: 'tools/virsh list' works now where it previously didn't,
but without a tweak to run.in, './run tools/virsh list' started failing.
So amendment to my earlier posting: my ACK is conditional on this being
squashed in (and similarly for 2/3):
diff --git i/run.in w/run.in
index 2211f24..c8f12a5 100644
--- i/run.in
+++ w/run.in
@@ -1,6 +1,6 @@
#!/bin/sh
# libvirt 'run' programs locally script
-# Copyright (C) 2012-2013 Red Hat, Inc.
+# Copyright (C) 2012-2014 Red Hat, Inc.
#
# This library is free software; you can redistribute it and/or
# modify it under the terms of the GNU Lesser General Public
@@ -56,7 +56,6 @@ export LD_LIBRARY_PATH
export LIBVIRT_DRIVER_DIR="$b/src/.libs"
export LIBVIRT_LOCK_MANAGER_PLUGIN_DIR="$b/src/.libs"
export VIRTLOCKD_PATH="$b/src/virtlockd"
-export LIBVIRTD_PATH="$b/daemon/libvirtd"
# This is a cheap way to find some use-after-free and uninitialized
# read problems when using glibc.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
11:54 a.m.
New subject: [libvirt] [PATCH 1/3] Use virFileFindResource to locate libvirtd daemon
On Fri, Apr 25, 2014 at 10:52:23AM -0600, Eric Blake wrote:
On 04/25/2014 08:25 AM, Daniel P. Berrange wrote:
> On Fri, Apr 25, 2014 at 08:01:58AM -0600, Eric Blake wrote:
>> On 04/25/2014 05:28 AM, Daniel P. Berrange wrote:
>>> Make the remote driver use virFileFindResource to find the
>>> libvirt daemon path, so that it executes the in-builddir
>>> daemon if run from source tree.
>>>
>>
>> the new code assumes LIBVIRTD_PATH provides "/path/to" which contains
>> libvirtd.
>>
>> Don't know how many users this will impact, but the only former
>> documentation of LIBVIRTD_PATH was in the error message. I can live
>> with the change in semantics, since it is not formally documented on the
>> web page as something users would normally fiddle with.
>
> The 'run' script uses this, but we can just delete those lines from
> the 'run' script now, since we'll "do the right thing"
automatically
In fact, you HAVE to fix the run script, or using it will break :) (I
just tested it now, on a virgin VM with no libvirt installed and just an
in-tree build: 'tools/virsh list' works now where it previously didn't,
but without a tweak to run.in, './run tools/virsh list' started failing.
So amendment to my earlier posting: my ACK is conditional on this being
squashed in (and similarly for 2/3):
diff --git i/run.in w/run.in
index 2211f24..c8f12a5 100644
--- i/run.in
+++ w/run.in
@@ -1,6 +1,6 @@
#!/bin/sh
# libvirt 'run' programs locally script
-# Copyright (C) 2012-2013 Red Hat, Inc.
+# Copyright (C) 2012-2014 Red Hat, Inc.
#
# This library is free software; you can redistribute it and/or
# modify it under the terms of the GNU Lesser General Public
@@ -56,7 +56,6 @@ export LD_LIBRARY_PATH
export LIBVIRT_DRIVER_DIR="$b/src/.libs"
export LIBVIRT_LOCK_MANAGER_PLUGIN_DIR="$b/src/.libs"
export VIRTLOCKD_PATH="$b/src/virtlockd"
-export LIBVIRTD_PATH="$b/daemon/libvirtd"
We actually don't want to delete it because this script serves
dual purposes. It is supposed to be usable for running apps
outside libvirt. ie
./run virt-manager
Such apps won't trigger the automatic path change, so we must just
just alter the env var, not delete it
Regards,
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 :|
11:57 a.m.
New subject: [libvirt] [PATCH 1/3] Use virFileFindResource to locate libvirtd daemon
On 04/25/2014 10:54 AM, Daniel P. Berrange wrote:
> # This library is free software; you can redistribute it and/or
> # modify it under the terms of the GNU Lesser General Public
> @@ -56,7 +56,6 @@ export LD_LIBRARY_PATH
> export LIBVIRT_DRIVER_DIR="$b/src/.libs"
> export LIBVIRT_LOCK_MANAGER_PLUGIN_DIR="$b/src/.libs"
> export VIRTLOCKD_PATH="$b/src/virtlockd"
> -export LIBVIRTD_PATH="$b/daemon/libvirtd"
We actually don't want to delete it because this script serves
dual purposes. It is supposed to be usable for running apps
outside libvirt. ie
./run virt-manager
Such apps won't trigger the automatic path change, so we must just
just alter the env var, not delete it
Fair enough. The alteration is thus:
export LIBVIRTD_PATH="$b/daemon"
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6:28 a.m.
New subject: [libvirt] [PATCH 2/3] Use virFileFindResource to locate virtlockd daemon
Make the lock plugin use virFileFindResource to find the
virtlockd daemon path, so that it executes the in-builddir
daemon if run from source tree.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/locking/lock_driver_lockd.c | 25 +++++++------------------
1 file changed, 7 insertions(+), 18 deletions(-)
diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
index afa3bac..c67bda6 100644
--- a/src/locking/lock_driver_lockd.c
+++ b/src/locking/lock_driver_lockd.c
@@ -81,22 +81,6 @@ struct _virLockManagerLockDaemonDriver {
static virLockManagerLockDaemonDriverPtr driver = NULL;
-#define VIRTLOCKD_PATH SBINDIR "/virtlockd"
-
-static const char *
-virLockManagerLockDaemonFindDaemon(void)
-{
- const char *customDaemon = virGetEnvBlockSUID("VIRTLOCKD_PATH");
-
- if (customDaemon)
- return customDaemon;
-
- if (virFileIsExecutable(VIRTLOCKD_PATH))
- return VIRTLOCKD_PATH;
-
- return NULL;
-}
-
static int virLockManagerLockDaemonLoadConfig(const char *configFile)
{
virConfPtr conf;
@@ -266,8 +250,13 @@ static virNetClientPtr virLockManagerLockDaemonConnectionNew(bool
privileged,
if (!(lockdpath = virLockManagerLockDaemonPath(privileged)))
goto error;
- if (!privileged)
- daemonPath = virLockManagerLockDaemonFindDaemon();
+ if (!privileged &&
+ !(daemonPath = virFileFindResourceFull("virtlockd",
+ NULL, NULL,
+ "src",
+ SBINDIR,
+ "VIRTLOCKD_PATH")))
+ goto error;
if (!(client = virNetClientNewUNIX(lockdpath,
daemonPath != NULL,
--
1.9.0
9:04 a.m.
New subject: [libvirt] [PATCH 2/3] Use virFileFindResource to locate virtlockd daemon
On 04/25/2014 05:28 AM, Daniel P. Berrange wrote:
Make the lock plugin use virFileFindResource to find the
virtlockd daemon path, so that it executes the in-builddir
daemon if run from source tree.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/locking/lock_driver_lockd.c | 25 +++++++------------------
1 file changed, 7 insertions(+), 18 deletions(-)
-static const char *
-virLockManagerLockDaemonFindDaemon(void)
-{
- const char *customDaemon = virGetEnvBlockSUID("VIRTLOCKD_PATH");
-
- if (customDaemon)
- return customDaemon;
-
As in 1/3, this is a change in semantics, where VIRTLOCKD_PATH used to
be "/path/to/virtlockd" and is now "/path/to"; but was otherwise
undocumented.
ACK.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6:28 a.m.
New subject: [libvirt] [PATCH 3/3] Make autostart of virtlockd actually work
The virnetsocket.c API is hardcoded to pass --timeout=30 to
any daemon it auto-starts. For inexplicable reasons the virtlockd
daemon did not implement the --timeout option, so it would
immediately exit on autostart with an error.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/locking/lock_daemon.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 969f901..3379f29 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -1143,6 +1143,7 @@ virLockDaemonUsage(const char *argv0, bool privileged)
" -h | --help Display program help:\n"
" -v | --verbose Verbose messages.\n"
" -d | --daemon Run as a daemon & write PID
file.\n"
+ " -t | --timeout <secs> Exit after timeout period.\n"
" -f | --config <file> Configuration file.\n"
" -V | --version Display version information.\n"
" -p | --pid-file <file> Change name of PID file.\n"
@@ -1195,6 +1196,7 @@ int main(int argc, char **argv) {
char *pid_file = NULL;
int pid_file_fd = -1;
char *sock_file = NULL;
+ int timeout = -1; /* -t: Shutdown timeout */
char *state_file = NULL;
bool implicit_conf = false;
mode_t old_umask;
@@ -1206,6 +1208,7 @@ int main(int argc, char **argv) {
{ "verbose", no_argument, &verbose, 'v'},
{ "daemon", no_argument, &godaemon, 'd'},
{ "config", required_argument, NULL, 'f'},
+ { "timeout", required_argument, NULL, 't'},
{ "pid-file", required_argument, NULL, 'p'},
{ "version", no_argument, NULL, 'V' },
{ "help", no_argument, NULL, 'h' },
@@ -1226,6 +1229,7 @@ int main(int argc, char **argv) {
while (1) {
int optidx = 0;
int c;
+ char *tmp;
c = getopt_long(argc, argv, "ldf:p:t:vVh", opts, &optidx);
@@ -1244,6 +1248,16 @@ int main(int argc, char **argv) {
godaemon = 1;
break;
+ case 't':
+ if (virStrToLong_i(optarg, &tmp, 10, &timeout) != 0
+ || timeout <= 0
+ /* Ensure that we can multiply by 1000 without overflowing. */
+ || timeout > INT_MAX / 1000) {
+ VIR_ERROR(_("Invalid value for timeout"));
+ exit(EXIT_FAILURE);
+ }
+ break;
+
case 'p':
VIR_FREE(pid_file);
if (VIR_STRDUP_QUIET(pid_file, optarg) < 0)
@@ -1407,6 +1421,12 @@ int main(int argc, char **argv) {
}
}
+ if (timeout != -1) {
+ VIR_DEBUG("Registering shutdown timeout %d", timeout);
+ virNetServerAutoShutdown(lockDaemon->srv,
+ timeout);
+ }
+
if ((virLockDaemonSetupSignals(lockDaemon->srv)) < 0) {
ret = VIR_LOCK_DAEMON_ERR_SIGNAL;
goto cleanup;
--
1.9.0
9:11 a.m.
New subject: [libvirt] [PATCH 3/3] Make autostart of virtlockd actually work
On 04/25/2014 05:28 AM, Daniel P. Berrange wrote:
The virnetsocket.c API is hardcoded to pass --timeout=30 to
any daemon it auto-starts. For inexplicable reasons the virtlockd
daemon did not implement the --timeout option, so it would
immediately exit on autostart with an error.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/locking/lock_daemon.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
@@ -1226,6 +1229,7 @@ int main(int argc, char **argv) {
while (1) {
int optidx = 0;
int c;
+ char *tmp;
c = getopt_long(argc, argv, "ldf:p:t:vVh", opts, &optidx);
Oh my - we were previously accepting -t (but not --timeout) then doing
nothing with it.
@@ -1244,6 +1248,16 @@ int main(int argc, char **argv) {
godaemon = 1;
break;
+ case 't':
ACK
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3864
days inactive
3864
days old
10 comments
2 participants
participants (2)
-
Daniel P. Berrange -
Eric Blake