cli: Print more info about console commands

Googling for 'Graphics requested but DISPLAY is not set' shows there's
some confusion about virt-install's behavior in this area. This gives
more output in several related cases about what commands we are
running and the state of the VM

Signed-off-by: Cole Robinson <crobinso@redhat.com>

tests/magicuri

@@ -4,32 +4,34 @@ import argparse
 import os
 import sys
 
+sys.path.insert(0, os.path.dirname(os.path.dirname(__file__)))
+from virtinst import URI
+
 
 def parse_options():
-    hvs = ["qemu", "xen", "lxc", "test", "vz"]
     description = ("Generate a fake URI for use with virt-manager/virtinst "
         "that wraps a standard test:/// URI but pretends to be a different "
         "hypervisor. See virtinst/uri.py MagicURI for format details. "
-        "Example: magicuri qemu --fake-remote")
+        "Example: magicuri.py qemu+tcp://fakeuri.example.com/system")
     parser = argparse.ArgumentParser(description=description)
 
-    parser.add_argument("hv",
+    parser.add_argument("fakeuri",
-            help="URI hypervisor to mock", choices=hvs)
+            help="The libvirt URI we should fake")
     parser.add_argument("--capsfile",
             help="Path to file to use for capabilities XML")
     parser.add_argument("--domcapsfile",
             help="Path to file to use for domain capabilities XML")
     parser.add_argument("--driverxml",
             help="Path to driver xml (defaults to testdriver.xml)")
-    parser.add_argument("--fake-remote", action="store_true",
-            help="Fake a remote connection")
 
     options = parser.parse_args()
 
     testdir = os.path.abspath(os.path.dirname(__file__))
     capsdir = os.path.join(testdir, "data/capabilities/")
 
-    hv = options.hv
+    uriobj = URI(options.fakeuri)
+    hv = uriobj.scheme
+
     capsfile = None
     domcapsfile = None
     if hv == "qemu":
@@ -41,9 +43,6 @@ def parse_options():
         capsfile = capsdir + "lxc.xml"
     elif hv == "vz":
         capsfile = capsdir + "vz.xml"
-    elif hv != "test":
-        parser.print_help()
-        sys.exit(1)
 
     if options.capsfile:
         capsfile = os.path.abspath(options.capsfile)
@@ -54,21 +53,18 @@ def parse_options():
     if options.driverxml:
         driverxml = os.path.abspath(options.driverxml)
 
-    return hv, capsfile, domcapsfile, options.fake_remote, driverxml
+    return options.fakeuri, capsfile, domcapsfile, driverxml
 
 
 def main():
-    hv, capsfile, domcapsfile, fake_remote, driverxml = parse_options()
+    fakeuri, capsfile, domcapsfile, driverxml = parse_options()
     uri = "__virtinst_test__test://%s" % driverxml
+    uri += ",fakeuri=%s" % fakeuri
 
-    if hv != "test":
-        uri += ",%s" % hv
     if capsfile:
         uri += ",caps=%s" % capsfile
     if domcapsfile:
         uri += ",domcaps=%s" % domcapsfile
-    if fake_remote:
-        uri += ",remote"
 
     if driverxml and not os.path.exists(driverxml):
         print("%s does not exist" % capsfile)

tests/test_cli.py

@@ -1112,18 +1112,18 @@ c.add_valid("--file %(EXISTIMG1)s --file %(EXISTIMG1)s")  # Multiple existing fi
 c.add_valid("--file %(NEWIMG1)s --file-size .00001 --nonsparse")  # Nonexistent file
 
 c = vinst.add_category("console-tests", "--nodisks")
-c.add_valid("--pxe", grep="testsuite console command: ['virt-viewer'")  # mock default graphics+virt-viewer usage
+c.add_valid("--pxe", grep="graphical console command: virt-viewer")  # mock default graphics+virt-viewer usage
 c.add_valid("--pxe --graphics spice,gl=on", grep="--attach")  # using virt-viewer --attach option for gl
 c.add_valid("--pxe --destroy-on-exit", grep="Restarting guest.\n")  # destroy-on-exit
 c.add_valid("--pxe --transient --destroy-on-exit", grep="Domain creation completed.")  # destroy-on-exit + transient
-c.add_valid("--pxe --graphics vnc --noreboot", grep="testsuite console command: ['virt-viewer'")  # mock virt-viewer waiting, with noreboot magic
+c.add_valid("--pxe --graphics vnc --noreboot", grep="graphical console command: virt-viewer")  # mock virt-viewer waiting, with noreboot magic
 c.add_valid("--nographics --cdrom %(EXISTIMG1)s")  # console warning about cdrom + nographics
 c.add_valid("--nographics --console none --location %(TREEDIR)s", grep="Directory tree installs typically")  # warning about directory trees not working well
-c.add_valid("--pxe --nographics --transient", grep="testsuite console command: ['virsh'")  # --transient handling
+c.add_valid("--pxe --nographics --transient", grep="text console command: virsh")  # --transient handling
-c.add_valid("--pxe --nographics --autoconsole graphical", grep="testsuite console command: ['virt-viewer'")  # force --autoconsole graphical
+c.add_valid("--pxe --nographics --autoconsole graphical", grep="graphical console command: virt-viewer")  # force --autoconsole graphical
-c.add_valid("--pxe --autoconsole text", grep="testsuite console command: ['virsh'")  # force --autoconsole text
+c.add_valid("--pxe --autoconsole text", grep="text console command: virsh")  # force --autoconsole text
 c.add_valid("--connect %(URI-KVM)s --install fedora28 --cloud-init", grep="Password for first root login")  # make sure we print the root login password
-c.add_valid("--connect %(URI-KVM)s --install fedora28 --cloud-init", grep="testsuite console command: ['virsh'")  # make sure we notify about text console
+c.add_valid("--connect %(URI-KVM)s --install fedora28 --cloud-init", grep="text console command: virsh")  # make sure we notify about text console
 c.add_invalid("--pxe --autoconsole badval")  # bad --autoconsole value
 
 

virtinst/cli.py

@@ -361,11 +361,15 @@ def validate_disk(dev, warn_overwrite=False):
     check_path_search(dev.conn, dev.path)
 
 
-def _run_console(domain, args):
+def _run_console(domain, console_type, args):
     ignore = domain
     log.debug("Running: %s", " ".join(args))
+    argstr = " ".join([shlex.quote(a) for a in args])
+    print_stdout(
+        _("Running %(console_type)s console command: %(command)s") %
+        {"command": argstr, "console_type": console_type})
+
     if in_testsuite():
-        print_stdout("testsuite console command: %s" % args)
         args = ["/bin/test"]
 
     child = os.fork()
@@ -391,7 +395,7 @@ def _gfx_console(guest, domain):
     if guest.has_gl() or guest.has_listen_none():
         args.append("--attach")
 
-    return _run_console(domain, args)
+    return _run_console(domain, "graphical", args)
 
 
 def _txt_console(guest, domain):
@@ -399,7 +403,7 @@ def _txt_console(guest, domain):
             "--connect", guest.conn.uri,
             "console", guest.name]
 
-    return _run_console(domain, args)
+    return _run_console(domain, "text", args)
 
 
 def connect_console(guest, domain, consolecb, wait, destroy_on_exit):
@@ -416,10 +420,13 @@ def connect_console(guest, domain, consolecb, wait, destroy_on_exit):
 
     # If we connected the console, wait for it to finish
     try:
-        os.waitpid(child, 0)
+        errcode = os.waitpid(child, 0)[1]
     except OSError as e:  # pragma: no cover
         log.debug("waitpid error: %s", e)
 
+    if errcode:
+        log.warning(_("Console command returned failure."))
+
     if destroy_on_exit and domain.isActive():
         log.debug("console exited and destroy_on_exit passed, destroying")
         domain.destroy()

virtinst/virtinstall.py

@@ -761,6 +761,9 @@ def check_domain(installer, domain, conscb, transient, waithandler):
             return  # pragma: no cover
 
     # If we reach here, the VM still appears to be running.
+    msg = "\n"
+    msg += _("Domain is still running. Installation may be in progress.")
+
     if not waithandler.wait_is_requested:
         # User either:
         #   used --noautoconsole
@@ -768,12 +771,13 @@ def check_domain(installer, domain, conscb, transient, waithandler):
         if not installer.has_install_phase():
             return
 
-        print_stdout(
+        msg += "\n"
-            _("Domain installation still in progress. You can reconnect"
+        msg += _("You can reconnect to the console to complete the "
-              " to \nthe console to complete the installation process."))
+                 "installation process.")
+        print_stdout(msg)
         sys.exit(0)
 
-    print_stdout(_("Domain installation still in progress."))
+    print_stdout(msg)
     print_stdout(waithandler.get_time_string())
 
     # Wait loop