[libvirt] [PATCH] virNetSocketNewConnectUNIX: Use flocks when spawning a daemon
Ján Tomko
jtomko at redhat.com
Tue Apr 14 16:52:51 UTC 2015
On Thu, Apr 02, 2015 at 06:21:01PM +0200, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1200149
>
> Even though we have a mutex mechanism so that two clients don't spawn
> two daemons, it's not strong enough. It can happen that while one
> client is spawning the daemon, the other one fails to connect.
> Basically two possible errors can happen:
>
> error: Failed to connect socket to '/home/mprivozn/.cache/libvirt/libvirt-sock': Connection refused
>
> or:
>
> error: Failed to connect socket to '/home/mprivozn/.cache/libvirt/libvirt-sock': No such file or directory
>
> The problem in both cases is, the daemon is only starting up, while we
> are trying to connect (and fail). We should postpone the connecting
> phase until the daemon is started (by the other thread that is
> spawning it). In order to do that, create a file lock 'libvirt-lock'
> in the directory where session daemon would create its socket. So even
> when called from multiple processes, spawning a daemon will serialize
> on the file lock. So only the first to come will spawn the daemon.
>
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>
> diff to v1:
> -hopefully this one is race free
>
> src/rpc/virnetsocket.c | 148 ++++++++++++-------------------------------------
> 1 file changed, 36 insertions(+), 112 deletions(-)
ACK, with a few changes to the lock file path
>
> diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
> index 6b019cc..2b891ae 100644
> --- a/src/rpc/virnetsocket.c
> +++ b/src/rpc/virnetsocket.c
> @@ -543,10 +539,10 @@ int virNetSocketNewConnectUNIX(const char *path,
> const char *binary,
> virNetSocketPtr *retsock)
> {
> - char *binname = NULL;
> - char *pidpath = NULL;
> - int fd, passfd = -1;
> - int pidfd = -1;
> + char *lockpath = NULL;
> + int lockfd = -1;
> + int fd = -1;
> + int retries = 100;
> virSocketAddr localAddr;
> virSocketAddr remoteAddr;
>
> @@ -561,6 +557,22 @@ int virNetSocketNewConnectUNIX(const char *path,
> return -1;
> }
>
> + if (spawnDaemon) {
> + if (virPidFileConstructPath(false, NULL, "libvirt-lock", &lockpath) < 0)
> + goto error;
Here the lock gets named: .cache/libvirt/libvirt-lock.pid, even though it does not
contain a pid and we could be using this function to spawn virlockd too, not just libvirtd.
Can you move the last_component call here and name it "%s.lock",
binname?
Jan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150414/5a3c8271/attachment-0001.sig>
More information about the libvir-list
mailing list