[libvirt] [PATCH] To provide more accurate help messages of iface/net/pool-define in virsh help and man virsh

--- tools/virsh-interface.c | 4 ++-- tools/virsh-network.c | 4 ++-- tools/virsh-pool.c | 4 ++-- tools/virsh.pod | 8 +++++--- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c index d4ec854..6b4fd5f 100644 --- a/tools/virsh-interface.c +++ b/tools/virsh-interface.c @@ -507,10 +507,10 @@ cmdInterfaceDumpXML(vshControl *ctl, const vshCmd *cmd) */ static const vshCmdInfo info_interface_define[] = { {.name = "help", - .data = N_("define (but don't start) a physical host interface from an XML file") + .data = N_("define (but don't start) or update a physical host interface from an XML file") }, {.name = "desc", - .data = N_("Define a physical host interface.") + .data = N_("Define a physical host interface or update an existing one.") }, {.name = NULL} }; diff --git a/tools/virsh-network.c b/tools/virsh-network.c index fc08b09..0db333c 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -189,10 +189,10 @@ cmdNetworkCreate(vshControl *ctl, const vshCmd *cmd) */ static const vshCmdInfo info_network_define[] = { {.name = "help", - .data = N_("define (but don't start) a network from an XML file") + .data = N_("define (but don't start) or update a network from an XML file") }, {.name = "desc", - .data = N_("Define a network.") + .data = N_("Define a network or update an existing one.") }, {.name = NULL} }; diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index 7c40b5b..b0acd89 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -342,10 +342,10 @@ cmdPoolCreateAs(vshControl *ctl, const vshCmd *cmd) */ static const vshCmdInfo info_pool_define[] = { {.name = "help", - .data = N_("define (but don't start) a pool from an XML file") + .data = N_("define (but don't start) or update a pool from an XML file") }, {.name = "desc", - .data = N_("Define a pool.") + .data = N_("Define a pool or update an existing one.") }, {.name = NULL} }; diff --git a/tools/virsh.pod b/tools/virsh.pod index 849ae31..8086885 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2472,7 +2472,8 @@ to get a description of the XML network format used by libvirt. =item B<net-define> I<file> Define a persistent virtual network from an XML I<file>, the network is just -defined but not instantiated (started). +defined but not instantiated (started). If a persistent virtual network with +the same name and UUID already exists, it will be replaced with the new XML. =item B<net-destroy> I<network> @@ -2631,7 +2632,7 @@ See also B<iface-unbridge> for undoing this operation. =item B<iface-define> I<file> Define a host interface from an XML I<file>, the interface is just defined but -not started. +not started. If a host interface with the same name already exists, it will be replaced with the new XML. =item B<iface-destroy> I<interface> @@ -2778,7 +2779,8 @@ I<type>. =item B<pool-define> I<file> -Create, but do not start, a pool object from the XML I<file>. +Create, but do not start, a pool object from the XML I<file>. If a pool object +with the same name and UUID already exists, it will be replaced with the new XML. =item B<pool-define-as> I<name> I<--print-xml> I<type> [I<source-host>] [I<source-path>] [I<source-dev>] [I<source-name>] [<target>] -- 1.8.1.4

On Mon, Jul 28, 2014 at 03:20:25PM +0800, Jianwei Hu wrote:
--- tools/virsh-interface.c | 4 ++-- tools/virsh-network.c | 4 ++-- tools/virsh-pool.c | 4 ++-- tools/virsh.pod | 8 +++++--- 4 files changed, 11 insertions(+), 9 deletions(-)
Thisis true for the normal "define" for domains as well and you haven't changed it there. I would, however, suggest leaving the messages like this and specifying somewhere (in manual, for example), that *any* define action may be used to override existing definition; that should be common to know between libvirt users and we don't have to specify it with every new "define". Martin
diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c index d4ec854..6b4fd5f 100644 --- a/tools/virsh-interface.c +++ b/tools/virsh-interface.c @@ -507,10 +507,10 @@ cmdInterfaceDumpXML(vshControl *ctl, const vshCmd *cmd) */ static const vshCmdInfo info_interface_define[] = { {.name = "help", - .data = N_("define (but don't start) a physical host interface from an XML file") + .data = N_("define (but don't start) or update a physical host interface from an XML file") }, {.name = "desc", - .data = N_("Define a physical host interface.") + .data = N_("Define a physical host interface or update an existing one.") }, {.name = NULL} }; diff --git a/tools/virsh-network.c b/tools/virsh-network.c index fc08b09..0db333c 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -189,10 +189,10 @@ cmdNetworkCreate(vshControl *ctl, const vshCmd *cmd) */ static const vshCmdInfo info_network_define[] = { {.name = "help", - .data = N_("define (but don't start) a network from an XML file") + .data = N_("define (but don't start) or update a network from an XML file") }, {.name = "desc", - .data = N_("Define a network.") + .data = N_("Define a network or update an existing one.") }, {.name = NULL} }; diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index 7c40b5b..b0acd89 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -342,10 +342,10 @@ cmdPoolCreateAs(vshControl *ctl, const vshCmd *cmd) */ static const vshCmdInfo info_pool_define[] = { {.name = "help", - .data = N_("define (but don't start) a pool from an XML file") + .data = N_("define (but don't start) or update a pool from an XML file") }, {.name = "desc", - .data = N_("Define a pool.") + .data = N_("Define a pool or update an existing one.") }, {.name = NULL} }; diff --git a/tools/virsh.pod b/tools/virsh.pod index 849ae31..8086885 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2472,7 +2472,8 @@ to get a description of the XML network format used by libvirt. =item B<net-define> I<file>
Define a persistent virtual network from an XML I<file>, the network is just -defined but not instantiated (started). +defined but not instantiated (started). If a persistent virtual network with +the same name and UUID already exists, it will be replaced with the new XML.
=item B<net-destroy> I<network>
@@ -2631,7 +2632,7 @@ See also B<iface-unbridge> for undoing this operation. =item B<iface-define> I<file>
Define a host interface from an XML I<file>, the interface is just defined but -not started. +not started. If a host interface with the same name already exists, it will be replaced with the new XML.
=item B<iface-destroy> I<interface>
@@ -2778,7 +2779,8 @@ I<type>.
=item B<pool-define> I<file>
-Create, but do not start, a pool object from the XML I<file>. +Create, but do not start, a pool object from the XML I<file>. If a pool object +with the same name and UUID already exists, it will be replaced with the new XML.
=item B<pool-define-as> I<name> I<--print-xml> I<type> [I<source-host>] [I<source-path>] [I<source-dev>] [I<source-name>] [<target>] -- 1.8.1.4
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On 13/08/14 18:30, Martin Kletzander wrote:
On Mon, Jul 28, 2014 at 03:20:25PM +0800, Jianwei Hu wrote:
--- tools/virsh-interface.c | 4 ++-- tools/virsh-network.c | 4 ++-- tools/virsh-pool.c | 4 ++-- tools/virsh.pod | 8 +++++--- 4 files changed, 11 insertions(+), 9 deletions(-)
Thisis true for the normal "define" for domains as well and you haven't changed it there. I would, however, suggest leaving the messages like this and specifying somewhere (in manual, for example), that *any* define action may be used to override existing definition; that should be common to know between libvirt users and we don't have to specify it with every new "define".
Yes, I agree with you. Thanks for your reviewing.
Martin
diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c index d4ec854..6b4fd5f 100644 --- a/tools/virsh-interface.c +++ b/tools/virsh-interface.c @@ -507,10 +507,10 @@ cmdInterfaceDumpXML(vshControl *ctl, const vshCmd *cmd) */ static const vshCmdInfo info_interface_define[] = { {.name = "help", - .data = N_("define (but don't start) a physical host interface from an XML file") + .data = N_("define (but don't start) or update a physical host interface from an XML file") }, {.name = "desc", - .data = N_("Define a physical host interface.") + .data = N_("Define a physical host interface or update an existing one.") }, {.name = NULL} }; diff --git a/tools/virsh-network.c b/tools/virsh-network.c index fc08b09..0db333c 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -189,10 +189,10 @@ cmdNetworkCreate(vshControl *ctl, const vshCmd *cmd) */ static const vshCmdInfo info_network_define[] = { {.name = "help", - .data = N_("define (but don't start) a network from an XML file") + .data = N_("define (but don't start) or update a network from an XML file") }, {.name = "desc", - .data = N_("Define a network.") + .data = N_("Define a network or update an existing one.") }, {.name = NULL} }; diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index 7c40b5b..b0acd89 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -342,10 +342,10 @@ cmdPoolCreateAs(vshControl *ctl, const vshCmd *cmd) */ static const vshCmdInfo info_pool_define[] = { {.name = "help", - .data = N_("define (but don't start) a pool from an XML file") + .data = N_("define (but don't start) or update a pool from an XML file") }, {.name = "desc", - .data = N_("Define a pool.") + .data = N_("Define a pool or update an existing one.") }, {.name = NULL} }; diff --git a/tools/virsh.pod b/tools/virsh.pod index 849ae31..8086885 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2472,7 +2472,8 @@ to get a description of the XML network format used by libvirt. =item B<net-define> I<file>
Define a persistent virtual network from an XML I<file>, the network is just -defined but not instantiated (started). +defined but not instantiated (started). If a persistent virtual network with +the same name and UUID already exists, it will be replaced with the new XML.
=item B<net-destroy> I<network>
@@ -2631,7 +2632,7 @@ See also B<iface-unbridge> for undoing this operation. =item B<iface-define> I<file>
Define a host interface from an XML I<file>, the interface is just defined but -not started. +not started. If a host interface with the same name already exists, it will be replaced with the new XML.
=item B<iface-destroy> I<interface>
@@ -2778,7 +2779,8 @@ I<type>.
=item B<pool-define> I<file>
-Create, but do not start, a pool object from the XML I<file>. +Create, but do not start, a pool object from the XML I<file>. If a pool object +with the same name and UUID already exists, it will be replaced with the new XML.
=item B<pool-define-as> I<name> I<--print-xml> I<type> [I<source-host>] [I<source-path>] [I<source-dev>] [I<source-name>] [<target>] -- 1.8.1.4
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On 07/28/2014 01:20 AM, Jianwei Hu wrote:
---
Long subject line. Please try to keep patch subjects around 60 characters or less (look at 'git shortlog -30' to see examples), and limited to just the "what". Then use the body of the commit to give the "why". I'd suggest: virsh: improve help text for define commands The various virsh define commands (iface-define, net-define, pool-define) can also be used to update an existing definition. Mention this in the help text. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 14/08/14 00:25, Eric Blake wrote:
--- Long subject line. Please try to keep patch subjects around 60 characters or less (look at 'git shortlog -30' to see examples), and
On 07/28/2014 01:20 AM, Jianwei Hu wrote: limited to just the "what". Then use the body of the commit to give the "why". I'd suggest:
virsh: improve help text for define commands
The various virsh define commands (iface-define, net-define, pool-define) can also be used to update an existing definition. Mention this in the help text.
Got it, thanks for your suggestion. I accepted Martin's advice, the patch would be withdrew, thanks.
participants (4)
-
Eric Blake
-
hujianwei
-
Jianwei Hu
-
Martin Kletzander