From 6d99469c696ea691a908ad8a65314475e43b7bd0 Mon Sep 17 00:00:00 2001
From: Laszlo Ersek <lersek@redhat.com>
Date: Wed, 23 Mar 2022 11:43:30 +0100
Subject: [PATCH] nbdkit, qemuNBD: run_unix: formally require externally
provided socket
At this point, virt-v2v never relies on the Unix domain sockets created
inside the "run_unix" implementations. Simplify the code by removing this
option.
Consequently, the internally created temporary directory only holds the
NBD server's PID file, and never its UNIX domain socket. Therefore:
(1) we no longer need the libguestfs socket dir to be our temp dir,
(2) we need not change the file mode bits on the temp dir,
(3) we can rename "tmpdir" to the more specific "piddir".
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2066773
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20220323104330.9667-1-lersek@redhat.com>
Acked-by: Richard W.M. Jones <rjones@redhat.com>
(cherry picked from commit 9788b06765af335b054aba03f41d1b829ed13092)
---
input/input_disk.ml | 4 ++--
input/input_libvirt.ml | 8 ++++----
input/input_ova.ml | 2 +-
input/input_vddk.ml | 2 +-
input/input_vmx.ml | 4 ++--
input/input_xen_ssh.ml | 2 +-
input/vCenter.ml | 2 +-
lib/nbdkit.ml | 24 +++++-------------------
lib/nbdkit.mli | 6 +-----
lib/qemuNBD.ml | 25 +++++--------------------
lib/qemuNBD.mli | 6 +-----
output/output.ml | 4 ++--
output/output_null.ml | 2 +-
output/output_rhv_upload.ml | 2 +-
14 files changed, 28 insertions(+), 65 deletions(-)
diff --git a/input/input_disk.ml b/input/input_disk.ml
index dc3bed6f..c08548ee 100644
--- a/input/input_disk.ml
+++ b/input/input_disk.ml
@@ -109,7 +109,7 @@ module Disk = struct
Nbdkit.add_arg cmd "file" disk;
if Nbdkit.version nbdkit_config >= (1, 22, 0) then
Nbdkit.add_arg cmd "cache" "none";
- let _, pid = Nbdkit.run_unix ~socket cmd in
+ let _, pid = Nbdkit.run_unix socket cmd in
(* --exit-with-parent should ensure nbdkit is cleaned
* up when we exit, but it's not supported everywhere.
@@ -120,7 +120,7 @@ module Disk = struct
let cmd = QemuNBD.create disk in
QemuNBD.set_snapshot cmd true; (* protective overlay *)
QemuNBD.set_format cmd (Some format);
- let _, pid = QemuNBD.run_unix ~socket cmd in
+ let _, pid = QemuNBD.run_unix socket cmd in
On_exit.kill pid
) args;
diff --git a/input/input_libvirt.ml b/input/input_libvirt.ml
index ee836aa0..ad7e20e8 100644
--- a/input/input_libvirt.ml
+++ b/input/input_libvirt.ml
@@ -87,7 +87,7 @@ and setup_servers dir disks =
Nbdkit.add_arg cmd "hostname" hostname;
Nbdkit.add_arg cmd "port" (string_of_int port);
Nbdkit.add_arg cmd "shared" "true";
- let _, pid = Nbdkit.run_unix ~socket cmd in
+ let _, pid = Nbdkit.run_unix socket cmd in
(* --exit-with-parent should ensure nbdkit is cleaned
* up when we exit, but it's not supported everywhere.
@@ -98,7 +98,7 @@ and setup_servers dir disks =
| HTTP url ->
let cor = dir // "convert" in
let cmd = Nbdkit_curl.create_curl ~cor url in
- let _, pid = Nbdkit.run_unix ~socket cmd in
+ let _, pid = Nbdkit.run_unix socket cmd in
(* --exit-with-parent should ensure nbdkit is cleaned
* up when we exit, but it's not supported everywhere.
@@ -113,7 +113,7 @@ and setup_servers dir disks =
Nbdkit.add_arg cmd "file" filename;
if Nbdkit.version nbdkit_config >= (1, 22, 0) then
Nbdkit.add_arg cmd "cache" "none";
- let _, pid = Nbdkit.run_unix ~socket cmd in
+ let _, pid = Nbdkit.run_unix socket cmd in
(* --exit-with-parent should ensure nbdkit is cleaned
* up when we exit, but it's not supported everywhere.
@@ -125,7 +125,7 @@ and setup_servers dir disks =
let cmd = QemuNBD.create filename in
QemuNBD.set_snapshot cmd true; (* protective overlay *)
QemuNBD.set_format cmd format;
- let _, pid = QemuNBD.run_unix ~socket cmd in
+ let _, pid = QemuNBD.run_unix socket cmd in
On_exit.kill pid
) disks
diff --git a/input/input_ova.ml b/input/input_ova.ml
index c94ddc79..796cc3bc 100644
--- a/input/input_ova.ml
+++ b/input/input_ova.ml
@@ -192,7 +192,7 @@ module OVA = struct
let cmd = QemuNBD.create qemu_uri in
QemuNBD.set_snapshot cmd true; (* protective overlay *)
QemuNBD.set_format cmd None; (* auto-detect format *)
- let _, pid = QemuNBD.run_unix ~socket cmd in
+ let _, pid = QemuNBD.run_unix socket cmd in
On_exit.kill pid
) qemu_uris;
diff --git a/input/input_vddk.ml b/input/input_vddk.ml
index 29764095..f8bf3d28 100644
--- a/input/input_vddk.ml
+++ b/input/input_vddk.ml
@@ -196,7 +196,7 @@ information on these settings.
?nfchostport ?password_file:options.input_password ?port
~server ?snapshot ~thumbprint ?transports ?user
path in
- let _, pid = Nbdkit.run_unix ~socket nbdkit in
+ let _, pid = Nbdkit.run_unix socket nbdkit in
On_exit.kill pid
) disks;
diff --git a/input/input_vmx.ml b/input/input_vmx.ml
index 3aa49fa6..34ae99a3 100644
--- a/input/input_vmx.ml
+++ b/input/input_vmx.ml
@@ -66,7 +66,7 @@ module VMX = struct
(absolute_path_from_other_file vmx_filename filename) in
QemuNBD.set_snapshot cmd true; (* protective overlay *)
QemuNBD.set_format cmd (Some "vmdk");
- let _, pid = QemuNBD.run_unix ~socket cmd in
+ let _, pid = QemuNBD.run_unix socket cmd in
On_exit.kill pid
) filenames
@@ -108,7 +108,7 @@ module VMX = struct
let bandwidth = options.bandwidth in
let nbdkit = Nbdkit_ssh.create_ssh ?bandwidth ~cor ~password
~server ?port ?user abs_path in
- let _, pid = Nbdkit.run_unix ~socket nbdkit in
+ let _, pid = Nbdkit.run_unix socket nbdkit in
On_exit.kill pid
) filenames
);
diff --git a/input/input_xen_ssh.ml b/input/input_xen_ssh.ml
index 85e24bce..989a0cc7 100644
--- a/input/input_xen_ssh.ml
+++ b/input/input_xen_ssh.ml
@@ -118,7 +118,7 @@ module XenSSH = struct
let bandwidth = options.bandwidth in
let nbdkit = Nbdkit_ssh.create_ssh ?bandwidth ~cor ~password
?port ~server ?user path in
- let _, pid = Nbdkit.run_unix ~socket nbdkit in
+ let _, pid = Nbdkit.run_unix socket nbdkit in
On_exit.kill pid
) disks;
diff --git a/input/vCenter.ml b/input/vCenter.ml
index 40d594f0..8a1a5655 100644
--- a/input/vCenter.ml
+++ b/input/vCenter.ml
@@ -117,7 +117,7 @@ let rec start_nbdkit_for_path ?bandwidth ?cor ?password_file
Nbdkit_curl.create_curl ?bandwidth ?cor
~cookie_script ~cookie_script_renew
~sslverify https_url in
- let _, pid = Nbdkit.run_unix ~socket nbdkit in
+ let _, pid = Nbdkit.run_unix socket nbdkit in
pid
and get_https_url dcPath uri server path =
diff --git a/lib/nbdkit.ml b/lib/nbdkit.ml
index 9ee6f39c..07896684 100644
--- a/lib/nbdkit.ml
+++ b/lib/nbdkit.ml
@@ -102,27 +102,13 @@ let add_env cmd name value = cmd.env <- (name, value) :: cmd.env
let add_filter_if_available cmd filter =
if probe_filter filter then add_filter cmd filter
-let run_unix ?socket cmd =
- (* Create a temporary directory where we place the socket and PID file.
- * Use the libguestfs socket directory, so it is more likely the full path
- * of the UNIX sockets will fit in the (limited) socket pathname.
- *)
- let tmpdir =
- let base_dir = (open_guestfs ())#get_sockdir () in
- let t = Mkdtemp.temp_dir ~base_dir "v2vnbdkit." in
- (* tmpdir must be readable (but not writable) by "other" so that
- * qemu can open the sockets.
- *)
- chmod t 0o755;
- On_exit.rmdir t;
- t in
+let run_unix socket cmd =
+ (* Create a temporary directory where we place the PID file. *)
+ let piddir = Mkdtemp.temp_dir "v2vnbdkit." in
+ On_exit.rmdir piddir;
let id = unique () in
- let pidfile = tmpdir // sprintf "nbdkit%d.pid" id in
- let socket =
- match socket with
- | None -> tmpdir // sprintf "nbdkit%d.sock" id
- | Some socket -> socket in
+ let pidfile = piddir // sprintf "nbdkit%d.pid" id in
(* Construct the final command line. *)
let add_arg, add_args_reversed, get_args =
diff --git a/lib/nbdkit.mli b/lib/nbdkit.mli
index dc2fd04b..5ba83ab0 100644
--- a/lib/nbdkit.mli
+++ b/lib/nbdkit.mli
@@ -92,14 +92,10 @@ val add_args : cmd -> (string * string) list -> unit
val add_env : cmd -> string -> string -> unit
(** Add name=value environment variable. *)
-val run_unix : ?socket:string -> cmd -> string * int
+val run_unix : string -> cmd -> string * int
(** Start nbdkit command listening on a Unix domain socket, waiting
for the process to start up.
- If optional [?socket] parameter is omitted, then a temporary
- Unix domain socket name is created. If [?socket] is present
- then this overrides the temporary name.
-
Returns the Unix domain socket name and the nbdkit process ID.
The --exit-with-parent, --foreground, --pidfile, --newstyle and
diff --git a/lib/qemuNBD.ml b/lib/qemuNBD.ml
index 2c999b9f..ae21b17c 100644
--- a/lib/qemuNBD.ml
+++ b/lib/qemuNBD.ml
@@ -62,30 +62,15 @@ let create disk = { disk; snapshot = false; format = None }
let set_snapshot cmd snap = cmd.snapshot <- snap
let set_format cmd format = cmd.format <- format
-let run_unix ?socket { disk; snapshot; format } =
+let run_unix socket { disk; snapshot; format } =
assert (disk <> "");
- (* Create a temporary directory where we place the socket and PID file.
- * Use the libguestfs socket directory, so it is more likely the full path
- * of the UNIX sockets will fit in the (limited) socket pathname.
- *)
- let tmpdir =
- let base_dir = (open_guestfs ())#get_sockdir () in
- let t = Mkdtemp.temp_dir ~base_dir "v2vqemunbd." in
- (* tmpdir must be readable (but not writable) by "other" so that
- * qemu can open the sockets.
- *)
- chmod t 0o755;
- On_exit.rmdir t;
- t in
+ (* Create a temporary directory where we place the PID file. *)
+ let piddir = Mkdtemp.temp_dir "v2vqemunbd." in
+ On_exit.rmdir piddir;
let id = unique () in
- let pidfile = tmpdir // sprintf "qemunbd%d.pid" id in
-
- let socket =
- match socket with
- | Some socket -> socket
- | None -> tmpdir // sprintf "qemunbd%d.sock" id in
+ let pidfile = piddir // sprintf "qemunbd%d.pid" id in
(* Construct the qemu-nbd command line. *)
let args = ref [] in
diff --git a/lib/qemuNBD.mli b/lib/qemuNBD.mli
index 83871c5b..e10d3106 100644
--- a/lib/qemuNBD.mli
+++ b/lib/qemuNBD.mli
@@ -43,12 +43,8 @@ val set_snapshot : cmd -> bool -> unit
val set_format : cmd -> string option -> unit
(** Set the format [--format] parameter. *)
-val run_unix : ?socket:string -> cmd -> string * int
+val run_unix : string -> cmd -> string * int
(** Start qemu-nbd command listening on a Unix domain socket,
waiting for the process to start up.
- If optional [?socket] parameter is omitted, then a temporary
- Unix domain socket name is created. If [?socket] is present
- then this overrides the temporary name.
-
Returns the Unix domain socket name and the qemu-nbd process ID. *)
diff --git a/output/output.ml b/output/output.ml
index 7256b547..10e685c4 100644
--- a/output/output.ml
+++ b/output/output.ml
@@ -90,7 +90,7 @@ let output_to_local_file ?(changeuid = fun f -> f ())
let cmd = Nbdkit.add_arg cmd "cache" "none" in
cmd
);
- let _, pid = Nbdkit.run_unix ~socket cmd in
+ let _, pid = Nbdkit.run_unix socket cmd in
(* --exit-with-parent should ensure nbdkit is cleaned
* up when we exit, but it's not supported everywhere.
@@ -101,7 +101,7 @@ let output_to_local_file ?(changeuid = fun f -> f ())
let cmd = QemuNBD.create filename in
QemuNBD.set_snapshot cmd false;
QemuNBD.set_format cmd (Some "qcow2");
- let _, pid = QemuNBD.run_unix ~socket cmd in
+ let _, pid = QemuNBD.run_unix socket cmd in
On_exit.kill pid
| _ ->
diff --git a/output/output_null.ml b/output/output_null.ml
index 86d81eaa..c8e27c0b 100644
--- a/output/output_null.ml
+++ b/output/output_null.ml
@@ -70,7 +70,7 @@ module Null = struct
let () =
let cmd = Nbdkit.create ~quiet:true "null" in
Nbdkit.add_arg cmd "size" "7E";
- let _, pid = Nbdkit.run_unix ~socket cmd in
+ let _, pid = Nbdkit.run_unix socket cmd in
(* --exit-with-parent should ensure nbdkit is cleaned
* up when we exit, but it's not supported everywhere.
diff --git a/output/output_rhv_upload.ml b/output/output_rhv_upload.ml
index 72463e57..828996b3 100644
--- a/output/output_rhv_upload.ml
+++ b/output/output_rhv_upload.ml
@@ -398,7 +398,7 @@ e command line has to match the number of guest disk images (for this guest: %d)
Nbdkit.add_arg cmd "insecure" "true";
if is_ovirt_host then
Nbdkit.add_arg cmd "is_ovirt_host" "true";
- let _, pid = Nbdkit.run_unix ~socket cmd in
+ let _, pid = Nbdkit.run_unix socket cmd in
List.push_front pid nbdkit_pids
) (List.combine disks disk_uuids);
--
2.31.1