Re: [libvirt] [PATCH] Firewall : let libvirtd proceed after verifying locking args
On Fri, Dec 26, 2014 at 03:54:12PM +0530, Prerna Saxena wrote: > I recently encountered a situation where an unclean ebtables shutdown > caused /var/lib/ebtables/lock to be left behind. When libvirtd was > started on such a system, it caused libvirtd to "hang". Reason: So you are saying that the lockfile exists, but no ebtables process is still running ? > While probing to check if locking was supported, libvirt runs this > command synchronously : > # /usr/sbin/ebtables --concurrent -L > And this seemed to go on with msgs like : > Trying to obtain lock /var/lib/ebtables/lock > Trying to obtain lock /var/lib/ebtables/lock > Trying to obtain lock /var/lib/ebtables/lock > > Result: > Libvirtd never recovered from this scenario, and the system was > essentially unable to start any VMs. > > The following patch fixes this scenario: > --- > >From ec245eccc03e8a69dc2c2e6edbf30a7b34eb74d0 Mon Sep 17 00:00:00 2001 > From: Prerna Saxena > Date: Fri, 26 Dec 2014 15:24:45 -0500 > Subject: [PATCH] Firewall : let libvirtd proceed after verifying valid locking > args. > > Commit dc33e6e4a5a5d42 introduces locking args to be run with [eb/ip]tables to > determine whether locking is supported. However, this command needs to be > run asynchronously ( as against its present synchronous run), and needs to be > gracefully terminated once the job is done. Else it can potentially stall > libvirtd > with messages like : > "Trying to acquire lock ..." This doesn't really fully fix the problem - the stale lockfile still exists, so as soon as you try to start a VM with filters, we're still going to get stuck. You're just delaying the point at which anyone will see the problem, which I'm not convinced is good as the admin is far more likely to see the problem during libvirtd startup, than when launching a VM. Really, the fault lies with ebtables for just using the existance of the lock file as its mutex. It should be fixed to use fcntl locks on the file as its mutex, which guarantees you can never get any stale lock files even when exiting uncleanly. 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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Firewall : let libvirtd proceed after verifying locking args
Ping !
On Friday 26 December 2014 03:54 PM, Prerna Saxena wrote:
> I recently encountered a situation where an unclean ebtables shutdown
> caused /var/lib/ebtables/lock to be left behind. When libvirtd was
> started on such a system, it caused libvirtd to "hang". Reason:
> While probing to check if locking was supported, libvirt runs this
> command synchronously :
> # /usr/sbin/ebtables --concurrent -L
> And this seemed to go on with msgs like :
> Trying to obtain lock /var/lib/ebtables/lock
> Trying to obtain lock /var/lib/ebtables/lock
> Trying to obtain lock /var/lib/ebtables/lock
>
> Result:
> Libvirtd never recovered from this scenario, and the system was
> essentially unable to start any VMs.
>
> The following patch fixes this scenario:
> ---
> >From ec245eccc03e8a69dc2c2e6edbf30a7b34eb74d0 Mon Sep 17 00:00:00 2001
> From: Prerna Saxena
> Date: Fri, 26 Dec 2014 15:24:45 -0500
> Subject: [PATCH] Firewall : let libvirtd proceed after verifying valid locking
> args.
>
> Commit dc33e6e4a5a5d42 introduces locking args to be run with [eb/ip]tables to
> determine whether locking is supported. However, this command needs to be
> run asynchronously ( as against its present synchronous run), and needs to be
> gracefully terminated once the job is done. Else it can potentially stall
> libvirtd
> with messages like :
> "Trying to acquire lock ..."
>
> Signed-off-by: Prerna Saxena
> ---
> src/util/virfirewall.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
> index b536912..c120717 100644
> --- a/src/util/virfirewall.c
> +++ b/src/util/virfirewall.c
> @@ -121,12 +121,16 @@ virFirewallCheckUpdateLock(bool *lockflag,
> {
> int status; /* Ignore failed commands without logging them */
> virCommandPtr cmd = virCommandNewArgs(args);
> -if (virCommandRun(cmd, &status) < 0 || status) {
> +status = virCommandRunAsync(cmd, NULL);
> +if (status < 0) {
> VIR_INFO("locking not supported by %s", args[0]);
> +goto cleanup;
> } else {
> VIR_INFO("using locking for %s", args[0]);
> *lockflag = true;
> }
> +cleanup:
> +virCommandAbort(cmd);
> virCommandFree(cmd);
> }
>
--
Prerna Saxena
Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Firewall : let libvirtd proceed after verifying locking args
I recently encountered a situation where an unclean ebtables shutdown
caused /var/lib/ebtables/lock to be left behind. When libvirtd was
started on such a system, it caused libvirtd to "hang". Reason:
While probing to check if locking was supported, libvirt runs this
command synchronously :
# /usr/sbin/ebtables --concurrent -L
And this seemed to go on with msgs like :
Trying to obtain lock /var/lib/ebtables/lock
Trying to obtain lock /var/lib/ebtables/lock
Trying to obtain lock /var/lib/ebtables/lock
Result:
Libvirtd never recovered from this scenario, and the system was
essentially unable to start any VMs.
The following patch fixes this scenario:
---
>From ec245eccc03e8a69dc2c2e6edbf30a7b34eb74d0 Mon Sep 17 00:00:00 2001
From: Prerna Saxena
Date: Fri, 26 Dec 2014 15:24:45 -0500
Subject: [PATCH] Firewall : let libvirtd proceed after verifying valid locking
args.
Commit dc33e6e4a5a5d42 introduces locking args to be run with [eb/ip]tables to
determine whether locking is supported. However, this command needs to be
run asynchronously ( as against its present synchronous run), and needs to be
gracefully terminated once the job is done. Else it can potentially stall
libvirtd
with messages like :
"Trying to acquire lock ..."
Signed-off-by: Prerna Saxena
---
src/util/virfirewall.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index b536912..c120717 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -121,12 +121,16 @@ virFirewallCheckUpdateLock(bool *lockflag,
{
int status; /* Ignore failed commands without logging them */
virCommandPtr cmd = virCommandNewArgs(args);
-if (virCommandRun(cmd, &status) < 0 || status) {
+status = virCommandRunAsync(cmd, NULL);
+if (status < 0) {
VIR_INFO("locking not supported by %s", args[0]);
+goto cleanup;
} else {
VIR_INFO("using locking for %s", args[0]);
*lockflag = true;
}
+cleanup:
+virCommandAbort(cmd);
virCommandFree(cmd);
}
--
1.8.4.2
Regards,
--
Prerna Saxena,
IBM Systems & technology Labs,
Bangalore
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
