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