[libvirt] [PATCH RFC] virsh: Fix semantics of --config for "update-device" command

[Eric Blake]

On 03/15/2013 10:37 AM, Peter Krempa wrote:
> The man page states that with --config the next boot is affected. This
> can be understood as if _only_ the next bood was affected. This isn't

s/bood/boot/

> true if the machine is running.
> 
> This patch adds the full --live, --config, --current infrastructure and
> tweaks stuff to correctly support the obsolete --persistent flag.
> ---
> 
> Notes:
>     - This patch will be greatly simplified with macros from:
>       http://www.redhat.com/archives/libvir-list/2013-March/msg00268.html
>     
>     - There are multiple places like this in virsh that will need update too.
>       (detach-device for example)

I take it you plan on doing those after seeing how this RFC goes?

> 
>     - https://bugzilla.redhat.com/show_bug.cgi?id=921398
> 
>  tools/virsh-domain.c | 53 +++++++++++++++++++++++++++++++++++++++++-----------
>  tools/virsh.pod      | 22 +++++++++++++++-------
>  2 files changed, 57 insertions(+), 18 deletions(-)
> 

> +++ b/tools/virsh-domain.c
> @@ -9304,13 +9304,21 @@ static const vshCmdOptDef opts_update_device[] = {
>       .help = N_("XML file")
>      },
>      {.name = "persistent",
> -     .type = VSH_OT_ALIAS,
> -     .help = "config"
> +     .type = VSH_OT_BOOL,
> +     .help = N_("make live change persistent")

So you are documenting this option again, now that it doesn't match up
to any of the other options.

> +++ b/tools/virsh.pod

> 
> +If I<--live> is specified, affect a running domain.
> +If I<--config> is specified, affect the next startup of a persistent domain.
> +If I<--current> is specified, affect the current domain state.
> +Both I<--live> and I<--config> flags may be given, but I<--current> is
> +exclusive. Not specifying any flag is the same as specifying I<--current>.
> +
> +For compatibility purposes, I<--persistent> is alias of I<--config> and
> +I<--live> if the domain is running. This flag isn't compatible with the
> +other domain status flags.

Rather:

For compatibility purposes, I<--persistent> behaves like I<--config> for
an offline domain, and like I<--live> I<--config> for a running domain.

Why must we force mutual exclusion?  That is, --persistent --current
doesn't make sense, but --persistent --config is just redundant, and
--persistent --live is redundant for a running domain (it would fail for
an offline domain, though).  In fact, the semantics of --persistent seem
a bit nicer; should we be adding this option to all the commands that
support both --config and --live, since it is friendlier to type a
single option that behaves correctly according to domain state than it
is to figure out whether it is safe to use --live?

At any rate, I think this patch is moving in the right direction.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130320/d8705719/attachment-0001.sig>