Currently "chown_for_libvirt_rhbz_1045069" is best effort; if it fails, we
suppress the exception (we log it in verbose mode only, even).
That's not proved helpful: it almost certainly leads to later errors, but
those errors are less clear than the original (suppressed) exception.
Namely, the user sees something like
> Failed to connect to '/tmp/v2v.sKlulY/in0': Permission denied
rather than
> Failed to connect socket to '/var/run/libvirt/virtqemud-sock-ro':
> Connection refused
So just allow the exception to propagate outwards.
And then, now that "chown_for_libvirt_rhbz_1045069" will be able to fail,
hoist the call to "On_exit.rm_rf" before the call to
"chown_for_libvirt_rhbz_1045069", after creating the v2v temporary
directory. In the current order, if "chown_for_libvirt_rhbz_1045069" threw
an exception, then we'd leak the temp dir in the filesystem.
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2182024
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
Notes:
v2:
- new patch
lib/utils.mli | 5 +---
lib/utils.ml | 26 ++++++++------------
2 files changed, 11 insertions(+), 20 deletions(-)
diff --git a/lib/utils.mli b/lib/utils.mli
index cf88a467fd54..391a2a351ec7 100644
--- a/lib/utils.mli
+++ b/lib/utils.mli
@@ -67,10 +67,7 @@ val chown_for_libvirt_rhbz_1045069 : string -> unit
to root-owned files and directories. To fix this, provide
a function to chown things we might need to qemu:root so
qemu can access them. Note that root normally ignores
- permissions so can still access the resource.
-
- This is best-effort. If something fails then we carry
- on and hope for the best. *)
+ permissions so can still access the resource. *)
val error_if_no_ssh_agent : unit -> unit
diff --git a/lib/utils.ml b/lib/utils.ml
index 174c01b1e92f..7d69c9e0d177 100644
--- a/lib/utils.ml
+++ b/lib/utils.ml
@@ -149,21 +149,15 @@ let backend_is_libvirt () =
let rec chown_for_libvirt_rhbz_1045069 file =
let running_as_root = Unix.geteuid () = 0 in
- if running_as_root && backend_is_libvirt () then (
- try
- let user = Option.value ~default:"qemu" (libvirt_qemu_user ()) in
- let uid =
- if String.is_prefix user "+" then
- int_of_string (String.sub user 1 (String.length user - 1))
- else
- (Unix.getpwnam user).pw_uid in
- debug "setting owner of %s to %d:root" file uid;
- Unix.chown file uid 0
- with
- | exn -> (* Print exception, but continue. *)
- debug "could not set owner of %s: %s"
- file (Printexc.to_string exn)
- )
+ if running_as_root && backend_is_libvirt () then
+ let user = Option.value ~default:"qemu" (libvirt_qemu_user ()) in
+ let uid =
+ if String.is_prefix user "+" then
+ int_of_string (String.sub user 1 (String.length user - 1))
+ else
+ (Unix.getpwnam user).pw_uid in
+ debug "setting owner of %s to %d:root" file uid;
+ Unix.chown file uid 0
(* Get the local user that libvirt uses to run qemu when we are
* running as root. This is returned as an optional string
@@ -205,8 +199,8 @@ let error_if_no_ssh_agent () =
(* Create the directory containing inX and outX sockets. *)
let create_v2v_directory () =
let d = Mkdtemp.temp_dir "v2v." in
- chown_for_libvirt_rhbz_1045069 d;
On_exit.rm_rf d;
+ chown_for_libvirt_rhbz_1045069 d;
d
(* Wait for a file to appear until a timeout. *)