Libguestfs - Nov 2009 - [PATCH] (Alternate?) locking patch

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/
-------------- next part --------------
>From d6bd9f635307f09bc7ce247ccbebeafc519f2bfb Mon Sep 17 00:00:00 2001
From: Richard Jones <rjones at redhat.com>
Date: Thu, 19 Nov 2009 13:48:59 +0000
Subject: [PATCH] generator: Acquire lock to prevent two parallel runs of the
generator.

This commit acquires a lock on a file to prevent two parallel runs of
the generator from stomping on each other.  The second run will wait
for the first to complete before starting.

The lock is acquired on the "HACKING" file because it's convenient
--
we are already checking this file exists to make sure that we don't
start off in the wrong directory.

Tested by adding some artificial sleeps in the code to observe
locking behaviour between two parallel runs.
---
 src/generator.ml |   34 +++++++++++++++++++++++++++++-----
 1 files changed, 29 insertions(+), 5 deletions(-)
 mode change 100644 => 100755 src/generator.ml

diff --git a/src/generator.ml b/src/generator.ml
old mode 100644
new mode 100755
index c261ea2..30491e0
--- a/src/generator.ml
+++ b/src/generator.ml
@@ -10176,18 +10176,39 @@ let output_to filename    in
   close
 
+let perror msg = function
+  | Unix.Unix_error (err, _, _) ->
+      eprintf "%s: %s\n" msg (Unix.error_message err)
+  | exn ->
+      eprintf "%s: %s\n" msg (Printexc.to_string exn)
+
 (* Main program. *)
 let ()    check_functions ();
 
-  if not (Sys.file_exists "HACKING") then (
-    eprintf "\
+  let lock_fd +    try Unix.openfile "HACKING" [Unix.O_RDWR] 0
+    with
+    | Unix.Unix_error (Unix.ENOENT, _, _) ->
+	eprintf "\
 You are probably running this from the wrong directory.
 Run it from the top source directory using the command
   src/generator.ml
 ";
-    exit 1
-  );
+	exit 1
+    | exn ->
+	perror "open: HACKING" exn;
+	exit 1 in
+
+  (* Acquire a lock so parallel builds won't try to run the generator
+   * twice at the same time.  Subsequent builds will wait for the
+   * first one to finish.
+   *)
+  (try Unix.lockf lock_fd Unix.F_LOCK 1
+   with
+   | exn ->
+       perror "lock: HACKING" exn;
+       exit 1);
 
   let close = output_to "src/guestfs_protocol.x" in
   generate_xdr ();
@@ -10339,4 +10360,7 @@ Run it from the top source directory using the command
    *)
   let chan = open_out "src/stamp-generator" in
   fprintf chan "1\n";
-  close_out chan
+  close_out chan;
+
+  (* Release lock. *)
+  Unix.close lock_fd
-- 
1.6.5.2

On 19/11/09 13:54, Richard W.M. Jones wrote:
>> From d6bd9f635307f09bc7ce247ccbebeafc519f2bfb Mon Sep 17 00:00:00 2001
> From: Richard Jones<rjones at redhat.com>
> Date: Thu, 19 Nov 2009 13:48:59 +0000
> Subject: [PATCH] generator: Acquire lock to prevent two parallel runs of
the generator.
>
> This commit acquires a lock on a file to prevent two parallel runs of
> the generator from stomping on each other.  The second run will wait
> for the first to complete before starting.
>
> The lock is acquired on the "HACKING" file because it's
convenient --
> we are already checking this file exists to make sure that we don't
> start off in the wrong directory.
>
> Tested by adding some artificial sleeps in the code to observe
> locking behaviour between two parallel runs.
> ---
>   src/generator.ml |   34 +++++++++++++++++++++++++++++-----
>   1 files changed, 29 insertions(+), 5 deletions(-)
>   mode change 100644 =>  100755 src/generator.ml
>
> diff --git a/src/generator.ml b/src/generator.ml
> old mode 100644
> new mode 100755
> index c261ea2..30491e0
> --- a/src/generator.ml
> +++ b/src/generator.ml
> @@ -10176,18 +10176,39 @@ let output_to filename >     in
>     close
>
> +let perror msg = function
> +  | Unix.Unix_error (err, _, _) ->
> +      eprintf "%s: %s\n" msg (Unix.error_message err)
> +  | exn ->
> +      eprintf "%s: %s\n" msg (Printexc.to_string exn)
> +
>   (* Main program. *)
>   let () >     check_functions ();
>
> -  if not (Sys.file_exists "HACKING") then (
> -    eprintf "\
> +  let lock_fd > +    try Unix.openfile "HACKING"
[Unix.O_RDWR] 0
> +    with
> +    | Unix.Unix_error (Unix.ENOENT, _, _) ->
> +	eprintf "\
>   You are probably running this from the wrong directory.
>   Run it from the top source directory using the command
>     src/generator.ml
>   ";
> -    exit 1
> -  );
> +	exit 1
> +    | exn ->
> +	perror "open: HACKING" exn;
> +	exit 1 in
> +
> +  (* Acquire a lock so parallel builds won't try to run the generator
> +   * twice at the same time.  Subsequent builds will wait for the
> +   * first one to finish.
> +   *)
> +  (try Unix.lockf lock_fd Unix.F_LOCK 1
> +   with
> +   | exn ->
> +       perror "lock: HACKING" exn;
> +       exit 1);
>
>     let close = output_to "src/guestfs_protocol.x" in
>     generate_xdr ();
> @@ -10339,4 +10360,7 @@ Run it from the top source directory using the
command
>      *)
>     let chan = open_out "src/stamp-generator" in
>     fprintf chan "1\n";
> -  close_out chan
> +  close_out chan;
> +
> +  (* Release lock. *)
> +  Unix.close lock_fd
> -- 1.6.5.2
I wouldn't bother releasing the lock as it'll be released anyway. 
Otherwise, I prefer this to wrapped locking.

ACK.

Matt
-- 
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team

M:       +44 (0)7977 267231
GPG ID:  D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490