Bug 699847

Summary:	[RHEL 6.1] "man virsh" says that "URI" is optional in connect option.
Product:	Red Hat Enterprise Linux 6	Reporter:	asilva <asilva>
Component:	libvirt	Assignee:	Eric Blake <eblake>
Status:	CLOSED ERRATA	QA Contact:	Virtualization Bugs <virt-bugs>
Severity:	medium	Docs Contact:
Priority:	medium
Version:	6.2	CC:	dallan, dyuan, eblake, jmunilla, myamazak, mzhan, pschiffe, rwu, veillard, yupzhang
Target Milestone:	rc	Keywords:	Documentation, ManPageChange
Target Release:	---
Hardware:	All
OS:	Linux
Whiteboard:
Fixed In Version:	libvirt-0.9.4-8.el6	Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2011-12-06 11:06:21 UTC	Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:
Bug Depends On:
Bug Blocks:	658636

Description asilva 2011-04-26 18:44:27 UTC

> Description of problem:

virsh's man-page says about "connect" as follows.
--------------------------------------------------
# man virsh
<snip>
connect URI optional --readonly
--------------------------------------------------

However, URI is not needed if URI is given by LIBVIRT_DEFAULT_URI. 

> How reproducible:
Always

> Steps to Reproduce:
[root@host01 ~]#  virsh connect qemu:///system; echo $?

0

[root@host01 ~]# LIBVIRT_DEFAULT_URI=qemu:///system virsh connect; echo $?

0

If I change the LIBVIRT_DEFAULT_URI value to a unknown value, I will get error.

[root@host01 ~]# LIBVIRT_DEFAULT_URI=qemu:///test virsh connect; echo $?
error: internal error unexpected QEMU URI path '/test', try qemu:///system
error: failed to connect to the hypervisor
1
  
> Actual results:
virsh's man-page says that "URI" is optional.

> Expected results:
Add in man page explanation about LIBVIRT_DEFAULT_URI.

> Additional info:

Here where it tries to set the default hypervisor connection if no URI is passed, getting the LIBVIRT_DEFAULT_URI env variable. (source code src/libvirt.c)
--snip--
    /*
     *  If no URI is passed, then check for an environment string if not
     *  available probe the compiled in drivers to find a default hypervisor
     *  if detectable.
     */
    if (!name || name[0] == '\0') {
        char *defname = getenv("LIBVIRT_DEFAULT_URI");
        if (defname && *defname) {
            DEBUG("Using LIBVIRT_DEFAULT_URI %s", defname);
            name = defname;
        } else {
            name = NULL;
        }
    }
--snip--

Thanks.

Comment 3 Eric Blake 2011-07-07 21:04:41 UTC

The current convention in the virsh man page is that "optional" implies that all _subsequent_ arguments are option.  Thus:

# man virsh
<snip>
connect URI optional --readonly

is to be interpreted as "connect" (mandatory), "URI" (mandatory), "--readonly" (optional)

But I personally hate that convention.  I would much rather see us prune all mention of the word "optional" out of synopses, and instead use [] metasyntax:

connect URI [--readonly]

more in line with how 'virsh help connect' displays things.

It's a large, time-consuming, but mostly mechanical patch to do so.  Any volunteers?

Comment 4 Dave Allan 2011-07-08 02:39:24 UTC

(In reply to comment #3)
> But I personally hate that convention.  I would much rather see us prune all
> mention of the word "optional" out of synopses, and instead use [] metasyntax:

/me is not volunteering, but I agree.

Comment 6 Eric Blake 2011-08-29 12:57:54 UTC

This was fixed upstream with the following, and should already be part of libvirt-0.9.4-6.el6:

commit 08d3b0a2f8be57205973af320ece8ce36fbb5389
Author: Eric Blake <eblake>
Date:   Thu Jul 14 11:36:21 2011 -0600

    docs: improve virsh man page synopses
    
    "optional" is not a very good meta-syntactic construct in our man
    page.  I scrubbed this, and additionally improved some documentation
    on mutually exclusive options.  For example,
    
    [[--live] [--config] | [--current]]
    
    implies a set of optional flags, where within the set you can have
    either --current or a choice of 0, 1, or both --live and --config.
    
    * tools/virsh.pod: Use "[name]" rather than "optional name" for
    optional arguments.

Comment 10 dyuan 2011-08-31 07:12:49 UTC

Hi, Eric

2 tiny issues in man page and virsh help, will fix it in RHEL6.2 or not ?

1. The following arguments without [], maybe it's added after your patch..
send-key domain-id optional --codeset codeset optional --holdtime holdtime

2. The documentation of mutually exclusive options are different from "man virsh" and "virsh help"

# man virsh
--snip--
setmem domain-id kilobytes [[--config] [--live] | [--current]]

# virsh help setmem
--snip--
SYNOPSIS
    setmem <domain> <kilobytes> [--config] [--live] [--current]

Comment 11 Eric Blake 2011-08-31 14:01:25 UTC

(In reply to comment #10)
> Hi, Eric
> 
> 2 tiny issues in man page and virsh help, will fix it in RHEL6.2 or not ?
> 
> 1. The following arguments without [], maybe it's added after your patch..
> send-key domain-id optional --codeset codeset optional --holdtime holdtime

Aargh.  send-key was written before my patch to the man page, but merged after, so it snuck in with the old style.  We can still get that fixed for RHEL 6.2, so I'll move this back to assigned.

> 
> 2. The documentation of mutually exclusive options are different from "man
> virsh" and "virsh help"
> 
> # man virsh
> --snip--
> setmem domain-id kilobytes [[--config] [--live] | [--current]]
> 
> # virsh help setmem
> --snip--
> SYNOPSIS
>     setmem <domain> <kilobytes> [--config] [--live] [--current]

Intentional.  The man page is hand-written, and can afford to show extra meta-syntax for things such as mutually-exclusive options.  The 'virsh help' output is machine generated, and we do not have an infrastructure in place for any additional designations for mutual exclusion.  The 'virsh help' output is meant to be a quick refresher, while the 'man virsh' page should be authoritative, so there is nothing worth fixing here.  Besides, fixing the man page is still reasonable, but what you are asking for would be a rather invasive change to virsh.c which is too late for 6.2.

Comment 12 Eric Blake 2011-08-31 14:50:13 UTC

Awaiting upstream ACK:
https://www.redhat.com/archives/libvir-list/2011-August/msg01504.html

Comment 13 Eric Blake 2011-09-01 19:31:49 UTC

In POST:
http://post-office.corp.redhat.com/archives/rhvirt-patches/2011-September/msg00021.html

Comment 16 dyuan 2011-09-07 07:45:29 UTC

(In reply to comment #10)
> Hi, Eric
> 
> 2 tiny issues in man page and virsh help, will fix it in RHEL6.2 or not ?
> 
> 1. The following arguments without [], maybe it's added after your patch..
> send-key domain-id optional --codeset codeset optional --holdtime holdtime
> 

Verified this issue PASS with libvirt-0.9.4-9.el6.

# man virsh
--snip--
send-key domain-id [--codeset codeset] [--holdtime holdtime] keycode...

> 2. The documentation of mutually exclusive options are different from "man
> virsh" and "virsh help"
> 
> # man virsh
> --snip--
> setmem domain-id kilobytes [[--config] [--live] | [--current]]
> 
> # virsh help setmem
> --snip--
> SYNOPSIS
>     setmem <domain> <kilobytes> [--config] [--live] [--current]

Comment 17 errata-xmlrpc 2011-12-06 11:06:21 UTC

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHBA-2011-1513.html