Libguestfs - Jun 2023 - [v2v PATCH v2 1/3] lib/utils: fix typo

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2182024

v1 was here:
<https://listman.redhat.com/archives/libguestfs/2023-June/031910.html>.

Make any "chown_for_libvirt_rhbz_1045069" failure a hard one; that way
the user gets to see more direct and more uniform error messages (namely
that we couldn't connect to libvirt).

Document those connection problems, and a simple way to start up all
libvirt daemons.

Laszlo Ersek (3):
  lib/utils: fix typo
  lib/utils: make "chown_for_libvirt_rhbz_1045069" fail hard
  docs/virt-v2v: document libvirt system instance startup

 docs/virt-v2v.pod | 23 ++++++++++++++++-
 lib/utils.ml      | 26 ++++++++------------
 lib/utils.mli     |  7 ++----
 3 files changed, 34 insertions(+), 22 deletions(-)


base-commit: 4f47d6431cab97c09bd42279e29c378e6e65fc03

Fix a small comment typo from commit 4e7f20684373 ("lib: Improve security
of in/out sockets when running virt-v2v as root", 2022-03-23).

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 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/utils.mli b/lib/utils.mli
index 5687bf75867e..cf88a467fd54 100644
--- a/lib/utils.mli
+++ b/lib/utils.mli
@@ -62,7 +62,7 @@ val backend_is_libvirt : unit -> bool
 (** Return true iff the current backend is libvirt. *)
 
 val chown_for_libvirt_rhbz_1045069 : string -> unit
-(** If running and root, and if the backend is libvirt, libvirt
+(** If running as root, and if the backend is libvirt, libvirt
     will run qemu as a non-root user.  This prevents access
     to root-owned files and directories.  To fix this, provide
     a function to chown things we might need to qemu:root so

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. *)

It has frequently tripped us up that on RHEL / Fedora, installing the
right set of libvirt RPMs (such as the one pulled in by
"libvirt-daemon-kvm") does not result in an immediately running
libvirt
system instance.  Document the need, and the simplest method, for starting
libvirt up manually.

Thanks: Daniel Berrang?
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2182024
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---

Notes:
    v2:
    
    - rewrite using the language suggested by Rich
    
    context:-U12

 docs/virt-v2v.pod | 23 +++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/docs/virt-v2v.pod b/docs/virt-v2v.pod
index 4d2f241ad723..ac15108ae332 100644
--- a/docs/virt-v2v.pod
+++ b/docs/virt-v2v.pod
@@ -250,24 +250,26 @@ metadata.  virt-v2v tries to guess the best default
metadata.  This is
 usually adequate but you can get finer control (eg. of memory and
 vCPUs) by using I<-i libvirtxml> instead.  Only guests that use a single
 disk can be imported this way.
 
 =item B<-i> B<libvirt>
 
 Set the input method to I<libvirt>.  This is the default.
 
 In this mode you have to specify a libvirt guest name or UUID on the
 command line.  You may also specify a libvirt connection URI (see
 I<-ic>).
 
+See L</Starting the libvirt system instance> below.
+
 =item B<-i> B<libvirtxml>
 
 Set the input method to I<libvirtxml>.
 
 In this mode you have to pass a libvirt XML file on the command line.
 This file is read in order to get metadata about the source guest
 (such as its name, amount of memory), and also to locate the input
 disks.  See L</Minimal XML for -i libvirtxml option> below.
 
 =item B<-i> B<local>
 
 This is the same as I<-i disk>.
@@ -461,25 +463,26 @@ and guest metadata is created in the associated YAML file:
 
  /dir/name.yaml
 
 where C<name> is the guest name.
 
 =item B<-o> B<libvirt>
 
 Set the output method to I<libvirt>.  This is the default.
 
 In this mode, the converted guest is created as a libvirt guest.  You
 may also specify a libvirt connection URI (see I<-oc>).
 
-See L<virt-v2v-output-local(1)>.
+See L</Starting the libvirt system instance> below, and
+L<virt-v2v-output-local(1)>.
 
 =item B<-o> B<local>
 
 Set the output method to I<local>.
 
 In this mode, the converted guest is written to a local directory
 specified by I<-os /dir> (the directory must exist).  The converted
 guest?s disks are written as:
 
  /dir/name-sda
  /dir/name-sdb
  [etc]
@@ -1373,24 +1376,26 @@ manually change ownership after virt-v2v has finished.
 =item Writing to libvirt
 
 When using I<-o libvirt>, you may need to run virt-v2v as root so that
 it can write to the libvirt system instance (ie. C<qemu:///system>)
 and to the default location for disk images (usually
 F</var/lib/libvirt/images>).
 
 You can avoid this by setting up libvirt connection authentication,
 see L<http://libvirt.org/auth.html>.  Alternatively, use
 I<-oc qemu:///session>, which will write to your per-user libvirt
 instance.
 
+See also L</Starting the libvirt system instance>.
+
 =item Writing to Openstack
 
 Because of how Cinder volumes are presented as F</dev> block devices,
 using I<-o openstack> normally requires that virt-v2v is run as root.
 
 =item Writing to Glance
 
 This does I<not> need root (in fact it probably won?t work), but may
 require either a special user and/or for you to source a script that
 sets authentication environment variables.  Consult the Glance
 documentation.
 
@@ -1521,24 +1526,40 @@ displayed to the user.
 The calling program should treat messages sent to stderr as error
 messages.  In addition, virt-v2v exits with a non-zero status
 code if there was a fatal error.
 
 =back
 
 Virt-v2v E<le> 0.9.1 did not support the I<--machine-readable>
 option at all.  The option was added when virt-v2v was rewritten in 2014.
 
 It is possible to specify a format string for controlling the output;
 see L<guestfs(3)/ADVANCED MACHINE READABLE OUTPUT>.
 
+=head2 Starting the libvirt system instance
+
+ Failed to connect socket to '/var/run/libvirt/virtqemud-sock': No such
file or directory
+ Failed to connect socket to '/var/run/libvirt/virtqemud-sock-ro':
Connection refused
+
+If you have just installed libvirt and virt-v2v, then you may see the
+errors above.  This is caused by libvirt daemons that provide various
+services not running straight after installation.  (This may depend on
+your distribution and vendor presets).
+
+To fix this on systemd-based distributions, do:
+
+ systemctl isolate multi-user.target
+
+See also L<https://bugzilla.redhat.com/2182024>.
+
 =head1 FILES
 
 =over 4
 
 =item F</usr/share/virtio-win>
 
 (Optional)
 
 If this directory is present, then virtio drivers for Windows guests
 will be found from this directory and installed in the guest during
 conversion.