[libvirt] [PATCH v2] virsh: Use option alias for outmoded "--persistent"

Osier Yang jyang at redhat.com
Fri Mar 9 05:16:41 UTC 2012


On 2012年03月09日 10:55, Daniel Veillard wrote:
> On Thu, Mar 08, 2012 at 07:38:57PM +0800, Osier Yang wrote:
>> Since VIR_DOMAIN_AFFECT_{LIVE,CONFIG,CURRENT} was created,
>> all new virsh commands use "--config" to represents the
>> persistent changing. This patch add "--config" option
>> for the old commands which still use "--persistent",
>> and "--persistent" is now alias of "--config".
>>
>> tools/virsh.c: (use "--config", and "--persistent" is
>>      alias of "--config" now).
>>      cmdDomIfSetLink, cmdDomIfGetLink, cmdAttachDevice,
>>      cmdDetachDevice, cmdUpdateDevice, cmdAttachInterface,
>>      cmdDetachInterface, cmdAttachDisk, cmdDetachDisk
>>
>> toos/virsh.pod: Update docs of the changed commands, and
>>      add some missed docs for "--config" (detach-interface,
>>      detach-disk, and detach-device).
>> ---
>>   tools/virsh.c   |   51 ++++++++++++++++++++++++++++-------------------
>>   tools/virsh.pod |   59 ++++++++++++++++++++++++++++++++++++------------------
>>   2 files changed, 69 insertions(+), 41 deletions(-)
>>
>> diff --git a/tools/virsh.c b/tools/virsh.c
>> index ee8ff31..9a16ef8 100644
>> --- a/tools/virsh.c
>> +++ b/tools/virsh.c
>> @@ -1643,7 +1643,8 @@ static const vshCmdOptDef opts_domif_setlink[] = {
>>       {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
>>       {"interface", VSH_OT_DATA, VSH_OFLAG_REQ, N_("interface device (MAC Address)")},
>>       {"state", VSH_OT_DATA, VSH_OFLAG_REQ, N_("new state of the device")},
>> -    {"persistent", VSH_OT_BOOL, 0, N_("persist interface state")},
>> +    {"persistent", VSH_OT_ALIAS, 0, "config"},
>> +    {"config", VSH_OT_BOOL, 0, N_("affect next boot")},
>>       {NULL, 0, 0, NULL}
>>   };
>>
>> @@ -1658,7 +1659,7 @@ cmdDomIfSetLink (vshControl *ctl, const vshCmd *cmd)
>>       unsigned char macaddr[VIR_MAC_BUFLEN];
>>       const char *element;
>>       const char *attr;
>> -    bool persistent;
>> +    bool config;
>>       bool ret = false;
>>       unsigned int flags = 0;
>>       int i;
>> @@ -1680,7 +1681,7 @@ cmdDomIfSetLink (vshControl *ctl, const vshCmd *cmd)
>>       if (vshCommandOptString(cmd, "state",&state)<= 0)
>>           goto cleanup;
>>
>> -    persistent = vshCommandOptBool(cmd, "persistent");
>> +    config = vshCommandOptBool(cmd, "config");
>>
>>       if (STRNEQ(state, "up")&&  STRNEQ(state, "down")) {
>>           vshError(ctl, _("invalid link state '%s'"), state);
>> @@ -1688,13 +1689,13 @@ cmdDomIfSetLink (vshControl *ctl, const vshCmd *cmd)
>>       }
>>
>>       /* get persistent or live description of network device */
>> -    desc = virDomainGetXMLDesc(dom, persistent?VIR_DOMAIN_XML_INACTIVE:0);
>> +    desc = virDomainGetXMLDesc(dom, config ? VIR_DOMAIN_XML_INACTIVE : 0);
>>       if (desc == NULL) {
>>           vshError(ctl, _("Failed to get domain description xml"));
>>           goto cleanup;
>>       }
>>
>> -    if (persistent)
>> +    if (config)
>>           flags = VIR_DOMAIN_AFFECT_CONFIG;
>>       else
>>           flags = VIR_DOMAIN_AFFECT_LIVE;
>> @@ -1818,7 +1819,8 @@ static const vshCmdInfo info_domif_getlink[] = {
>>   static const vshCmdOptDef opts_domif_getlink[] = {
>>       {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
>>       {"interface", VSH_OT_DATA, VSH_OFLAG_REQ, N_("interface device (MAC Address)")},
>> -    {"persistent", VSH_OT_BOOL, 0, N_("Get persistent interface state")},
>> +    {"persistent", VSH_OT_ALIAS, 0, "config"},
>> +    {"config", VSH_OT_BOOL, 0, N_("Get persistent interface state")},
>>       {NULL, 0, 0, NULL}
>>   };
>>
>> @@ -1852,7 +1854,7 @@ cmdDomIfGetLink (vshControl *ctl, const vshCmd *cmd)
>>           return false;
>>       }
>>
>> -    if (vshCommandOptBool(cmd, "persistent"))
>> +    if (vshCommandOptBool(cmd, "config"))
>>           flags = VIR_DOMAIN_XML_INACTIVE;
>>
>>       desc = virDomainGetXMLDesc(dom, flags);
>> @@ -13460,7 +13462,8 @@ static const vshCmdInfo info_attach_device[] = {
>>   static const vshCmdOptDef opts_attach_device[] = {
>>       {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
>>       {"file",   VSH_OT_DATA, VSH_OFLAG_REQ, N_("XML file")},
>> -    {"persistent", VSH_OT_BOOL, 0, N_("persist device attachment")},
>> +    {"persistent", VSH_OT_ALIAS, 0, "config"},
>> +    {"config", VSH_OT_BOOL, 0, N_("affect next boot")},
>>       {NULL, 0, 0, NULL}
>>   };
>>
>> @@ -13490,7 +13493,7 @@ cmdAttachDevice(vshControl *ctl, const vshCmd *cmd)
>>           return false;
>>       }
>>
>> -    if (vshCommandOptBool(cmd, "persistent")) {
>> +    if (vshCommandOptBool(cmd, "config")) {
>>           flags = VIR_DOMAIN_AFFECT_CONFIG;
>>           if (virDomainIsActive(dom) == 1)
>>              flags |= VIR_DOMAIN_AFFECT_LIVE;
>> @@ -13771,7 +13774,8 @@ static const vshCmdInfo info_detach_device[] = {
>>   static const vshCmdOptDef opts_detach_device[] = {
>>       {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
>>       {"file",   VSH_OT_DATA, VSH_OFLAG_REQ, N_("XML file")},
>> -    {"persistent", VSH_OT_BOOL, 0, N_("persist device detachment")},
>> +    {"persistent", VSH_OT_ALIAS, 0, "config"},
>> +    {"config", VSH_OT_BOOL, 0, N_("affect next boot")},
>>       {NULL, 0, 0, NULL}
>>   };
>>
>> @@ -13799,7 +13803,7 @@ cmdDetachDevice(vshControl *ctl, const vshCmd *cmd)
>>           goto cleanup;
>>       }
>>
>> -    if (vshCommandOptBool(cmd, "persistent")) {
>> +    if (vshCommandOptBool(cmd, "config")) {
>>           flags = VIR_DOMAIN_AFFECT_CONFIG;
>>           if (virDomainIsActive(dom) == 1)
>>              flags |= VIR_DOMAIN_AFFECT_LIVE;
>> @@ -13835,7 +13839,8 @@ static const vshCmdInfo info_update_device[] = {
>>   static const vshCmdOptDef opts_update_device[] = {
>>       {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
>>       {"file",   VSH_OT_DATA, VSH_OFLAG_REQ, N_("XML file")},
>> -    {"persistent", VSH_OT_BOOL, 0, N_("persist device update")},
>> +    {"persistent", VSH_OT_ALIAS, 0, "config"},
>> +    {"config", VSH_OT_BOOL, 0, N_("affect next boot")},
>>       {"force",  VSH_OT_BOOL, 0, N_("force device update")},
>>       {NULL, 0, 0, NULL}
>>   };
>> @@ -13866,7 +13871,7 @@ cmdUpdateDevice(vshControl *ctl, const vshCmd *cmd)
>>           return false;
>>       }
>>
>> -    if (vshCommandOptBool(cmd, "persistent")) {
>> +    if (vshCommandOptBool(cmd, "config")) {
>>           flags = VIR_DOMAIN_AFFECT_CONFIG;
>>           if (virDomainIsActive(dom) == 1)
>>              flags |= VIR_DOMAIN_AFFECT_LIVE;
>> @@ -13910,7 +13915,8 @@ static const vshCmdOptDef opts_attach_interface[] = {
>>       {"mac",    VSH_OT_DATA, 0, N_("MAC address")},
>>       {"script", VSH_OT_DATA, 0, N_("script used to bridge network interface")},
>>       {"model", VSH_OT_DATA, 0, N_("model type")},
>> -    {"persistent", VSH_OT_BOOL, 0, N_("persist interface attachment")},
>> +    {"persistent", VSH_OT_ALIAS, 0, "config"},
>> +    {"config", VSH_OT_BOOL, 0, N_("affect next boot")},
>>       {"inbound", VSH_OT_DATA, VSH_OFLAG_NONE, N_("control domain's incoming traffics")},
>>       {"outbound", VSH_OT_DATA, VSH_OFLAG_NONE, N_("control domain's outgoing traffics")},
>>       {NULL, 0, 0, NULL}
>> @@ -14067,7 +14073,7 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
>>
>>       xml = virBufferContentAndReset(&buf);
>>
>> -    if (vshCommandOptBool(cmd, "persistent")) {
>> +    if (vshCommandOptBool(cmd, "config")) {
>>           flags = VIR_DOMAIN_AFFECT_CONFIG;
>>           if (virDomainIsActive(dom) == 1)
>>               flags |= VIR_DOMAIN_AFFECT_LIVE;
>> @@ -14105,7 +14111,8 @@ static const vshCmdOptDef opts_detach_interface[] = {
>>       {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
>>       {"type",   VSH_OT_DATA, VSH_OFLAG_REQ, N_("network interface type")},
>>       {"mac",    VSH_OT_STRING, 0, N_("MAC address")},
>> -    {"persistent", VSH_OT_BOOL, 0, N_("persist interface detachment")},
>> +    {"persistent", VSH_OT_ALIAS, 0, "config"},
>> +    {"config", VSH_OT_BOOL, 0, N_("affect next boot")},
>>       {NULL, 0, 0, NULL}
>>   };
>>
>> @@ -14199,7 +14206,7 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
>>           goto cleanup;
>>       }
>>
>> -    if (vshCommandOptBool(cmd, "persistent")) {
>> +    if (vshCommandOptBool(cmd, "config")) {
>>           flags = VIR_DOMAIN_AFFECT_CONFIG;
>>           if (virDomainIsActive(dom) == 1)
>>               flags |= VIR_DOMAIN_AFFECT_LIVE;
>> @@ -14246,7 +14253,8 @@ static const vshCmdOptDef opts_attach_disk[] = {
>>       {"cache",     VSH_OT_STRING, 0, N_("cache mode of disk device")},
>>       {"type",    VSH_OT_STRING, 0, N_("target device type")},
>>       {"mode",    VSH_OT_STRING, 0, N_("mode of device reading and writing")},
>> -    {"persistent", VSH_OT_BOOL, 0, N_("persist disk attachment")},
>> +    {"persistent", VSH_OT_ALIAS, 0, "config"},
>> +    {"config", VSH_OT_BOOL, 0, N_("affect next boot")},
>>       {"sourcetype", VSH_OT_STRING, 0, N_("type of source (block|file)")},
>>       {"serial", VSH_OT_STRING, 0, N_("serial of disk device")},
>>       {"shareable", VSH_OT_BOOL, 0, N_("shareable between domains")},
>> @@ -14555,7 +14563,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
>>
>>       xml = virBufferContentAndReset(&buf);
>>
>> -    if (vshCommandOptBool(cmd, "persistent")) {
>> +    if (vshCommandOptBool(cmd, "config")) {
>>           flags = VIR_DOMAIN_AFFECT_CONFIG;
>>           if (virDomainIsActive(dom) == 1)
>>               flags |= VIR_DOMAIN_AFFECT_LIVE;
>> @@ -14805,7 +14813,8 @@ static const vshCmdInfo info_detach_disk[] = {
>>   static const vshCmdOptDef opts_detach_disk[] = {
>>       {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
>>       {"target", VSH_OT_DATA, VSH_OFLAG_REQ, N_("target of disk device")},
>> -    {"persistent", VSH_OT_BOOL, 0, N_("persist disk detachment")},
>> +    {"persistent", VSH_OT_ALIAS, 0, "config"},
>> +    {"config", VSH_OT_BOOL, 0, N_("affect next boot")},
>>       {NULL, 0, 0, NULL}
>>   };
>>
>> @@ -14841,7 +14850,7 @@ cmdDetachDisk(vshControl *ctl, const vshCmd *cmd)
>>                                          VSH_PREPARE_DISK_XML_NONE)))
>>           goto cleanup;
>>
>> -    if (vshCommandOptBool(cmd, "persistent")) {
>> +    if (vshCommandOptBool(cmd, "config")) {
>>           flags = VIR_DOMAIN_AFFECT_CONFIG;
>>           if (virDomainIsActive(dom) == 1)
>>               flags |= VIR_DOMAIN_AFFECT_LIVE;
>> diff --git a/tools/virsh.pod b/tools/virsh.pod
>> index b4b8e85..64b00ee 100644
>> --- a/tools/virsh.pod
>> +++ b/tools/virsh.pod
>> @@ -551,17 +551,20 @@ B<Explanation of fields>  (fields appear in the folowing order):
>>
>>   Get network interface stats for a running domain.
>>
>> -=item B<domif-setlink>  I<domain>  I<interface-device>  I<state>  I<--persistent>
>> +=item B<domif-setlink>  I<domain>  I<interface-device>  I<state>  [I<--config>]
>>
>>   Modify link state of the domain's virtual interface. Possible values for
>> -state are "up" and "down. If --persistent is specified, only the persistent
>> -configuration of the domain is modified.
>> +state are "up" and "down. If I<--config>  is specified, only the persistent
>> +configuration of the domain is modified, for compatibility purposes,
>> +I<--persistent>  is alias of I<--config>.
>>   I<interface-device>  can be the interface's target name or the MAC address.
>>
>> -=item B<domif-getlink>  I<domain>  I<interface-device>  I<--persistent>
>> +=item B<domif-getlink>  I<domain>  I<interface-device>  [I<--config>]
>> +
>> +Query link state of the domain's virtual interface. If I<--config>
>> +is specified, query the persistent configuration, for compatibility
>> +purposes, I<--persistent>  is alias of I<--config>.
>>
>> -Query link state of the domain's virtual interface. If --persistent
>> -is specified, query the persistent configuration.
>>   I<interface-device>  can be the interface's target name or the MAC address.
>>
>>   =item B<domiftune>  I<domain>  I<interface-device>
>> @@ -1475,10 +1478,13 @@ format of the device sections to get the most accurate set of accepted values.
>>
>>   =over 4
>>
>> -=item B<attach-device>  I<domain-id>  I<FILE>
>> +=item B<attach-device>  I<domain-id>  I<FILE>  [I<--config>]
>>
>>   Attach a device to the domain, using a device definition in an XML file.
>>   See the documentation to learn about libvirt XML format for a device.
>> +If I<--config>  is specified, alter persistent configuration, effect observed
>> +on next boot, for compatibility purposes, I<--persistent>  is alias of
>> +I<--config>.
>>   For cdrom and floppy devices, this command only replaces the media within
>>   the single existing device; consider using B<update-device>  for this usage.
>>   For passthrough host devices, see also B<nodedev-detach>, needed if
>> @@ -1486,7 +1492,7 @@ the device does not use managed mode.
>>
>>   =item B<attach-disk>  I<domain-id>  I<source>  I<target>
>>   [I<--driver driver>] [I<--subdriver subdriver>] [I<--cache cache>]
>> -[I<--type type>] [I<--mode mode>] [I<--persistent>] [I<--sourcetype soucetype>]
>> +[I<--type type>] [I<--mode mode>] [I<--config>] [I<--sourcetype soucetype>]
>>   [I<--serial serial>] [I<--shareable>] [I<--rawio>] [I<--address address>]
>>   [I<--multifunction>]
>>
>> @@ -1498,7 +1504,8 @@ I<type>  can indicate I<cdrom>  or I<floppy>  as alternative to the disk default,
>>   although this use only replaces the media within the existing virtual cdrom or
>>   floppy device; consider using B<update-device>  for this usage instead.
>>   I<mode>  can specify the two specific mode I<readonly>  or I<shareable>.
>> -I<persistent>  indicates the changes will affect the next boot of the domain.
>> +I<--config>  indicates the changes will affect the next boot of the domain,
>> +for compatibility purposes, I<--persistent>  is alias of I<--config>.
>>   I<sourcetype>  can indicate the type of source (block|file)
>>   I<cache>  can be one of "default", "none", "writethrough", "writeback",
>>   "directsync" or "unsafe".
>> @@ -1512,7 +1519,7 @@ address.
>>
>>   =item B<attach-interface>  I<domain-id>  I<type>  I<source>
>>   [I<--target target>] [I<--mac mac>] [I<--script script>] [I<--model model>]
>> -[I<--persistent>] [I<--inbound average,peak,burst>] [I<--outbound average,peak,burst>]
>> +[I<--config>] [I<--inbound average,peak,burst>] [I<--outbound average,peak,burst>]
>>
>>   Attach a new network interface to the domain.
>>   I<type>  can be either I<network>  to indicate a physical network device or
>> @@ -1523,7 +1530,8 @@ I<mac>  allows to specify the MAC address of the network interface.
>>   I<script>  allows to specify a path to a script handling a bridge instead of
>>   the default one.
>>   I<model>  allows to specify the model type.
>> -I<persistent>  indicates the changes will affect the next boot of the domain.
>> +I<--config>  indicates the changes will affect the next boot of the domain,
>> +for compatibility purposes, I<--persistent>  is alias of I<--config>.
>>   I<inbound>  and I<outbound>  control the bandwidth of the interface. I<peak>
>>   and I<burst>  are optional, so "average,peak", "average,,burst" and
>>   "average" are also legal.
>> @@ -1532,34 +1540,45 @@ B<Note>: the optional target value is the name of a device to be created
>>   as the back-end on the node. If not provided a device named "vnetN" or "vifN"
>>   will be created automatically.
>>
>> -=item B<detach-device>  I<domain-id>  I<FILE>
>> +=item B<detach-device>  I<domain-id>  I<FILE>  [I<--config>]
>>
>>   Detach a device from the domain, takes the same kind of XML descriptions
>>   as command B<attach-device>.
>> +If I<--config>  is specified, alter persistent configuration, effect observed
>> +on next boot, for compatibility purposes, I<--persistent>  is alias of
>> +I<--config>.
>>   For passthrough host devices, see also B<nodedev-reattach>, needed if
>>   the device does not use managed mode.
>>
>> -=item B<detach-disk>  I<domain-id>  I<target>
>> +=item B<detach-disk>  I<domain-id>  I<target>  [I<--config>]
>>
>>   Detach a disk device from a domain. The I<target>  is the device as seen
>>   from the domain.
>> +If I<--config>  is specified, alter persistent configuration, effect observed
>> +on next boot, for compatibility purposes, I<--persistent>  is alias of
>> +I<--config>.
>>
>> -=item B<detach-interface>  I<domain-id>  I<type>  [I<--mac mac>]
>> +=item B<detach-interface>  I<domain-id>  I<type>  [I<--mac mac>] [I<--config>]
>>
>>   Detach a network interface from a domain.
>>   I<type>  can be either I<network>  to indicate a physical network device or
>>   I<bridge>  to indicate a bridge to a device. It is recommended to use the
>>   I<mac>  option to distinguish between the interfaces if more than one are
>>   present on the domain.
>> +If I<--config>  is specified, alter persistent configuration, effect observed
>> +on next boot, for compatibility purposes, I<--persistent>  is alias of
>> +I<--config>.
>>
>> -=item B<update-device>  I<domain-id>  I<file>  [I<--persistent>] [I<--force>]
>> +=item B<update-device>  I<domain-id>  I<file>  [I<--config>] [I<--force>]
>>
>>   Update the characteristics of a device associated with I<domain-id>, based on
>> -the device definition in an XML I<file>.  If the I<--persistent>  option is
>> -used, the changes will affect the next boot of the domain. The I<--force>
>> -option can be used to force device update, e.g., to eject a CD-ROM even if it
>> -is locked/mounted in the domain. See the documentation to learn about libvirt
>> -XML format for a device.
>> +the device definition in an XML I<file>.
>> +If the I<--config>  option is used, the changes will affect the next boot of
>> +the domain, for compatibility purposes, I<--persistent>  is alias of
>> +I<--config>.
>> +The I<--force>  option can be used to force device update, e.g., to eject a
>> +CD-ROM even if it is locked/mounted in the domain. See the documentation to
>> +learn about libvirt XML format for a device.
>>
>>   =item B<change-media>  I<domain-id>  I<path>  [I<--eject>] [I<--insert>]
>>   [I<--update>] [I<source>] [I<--force>] [[I<--live>] [I<--config>] | [I<--current>]]
>
>    Looks fine to me, I'm just wondering a bit about
>      "affect next boot"
> since it's about the operation should we use a 3rd person as in
>      "affects next boot"
> instead ? If we never do this for all other descriptions, then keep
> as-is

Yes, it's used across virsh.c

>
>    but except for that grammar nitpicking looks fine, ACK
>
> Daniel
>

I pushed this as-is, thanks!

Osier




More information about the libvir-list mailing list