[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 2/5] virsh: Fix semantics of --config for "update-device" command



On 03/21/2013 05:42 PM, 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.
> ---
>  tools/virsh-domain.c | 41 ++++++++++++++++++++++++++++++-----------
>  tools/virsh.pod      | 21 ++++++++++++++-------
>  2 files changed, 44 insertions(+), 18 deletions(-)
> 
[...]
> @@ -9267,7 +9275,22 @@ cmdUpdateDevice(vshControl *ctl, const vshCmd *cmd)
>      const char *from = NULL;
>      char *buffer = NULL;
>      bool ret = false;
> -    unsigned int flags;
> +    unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
> +    bool current = vshCommandOptBool(cmd, "current");
> +    bool config = vshCommandOptBool(cmd, "config");
> +    bool live = vshCommandOptBool(cmd, "live");
> +    bool persistent = vshCommandOptBool(cmd, "persistent");
> +
> +    VSH_EXCLUSIVE_OPTIONS_VAR(persistent, live);

Previously, --persistent --live was working and made sense as well, but
you are disallowing that now.  With that in mind, why do you allow
--persistent --config then?

>From my POV, I'd leave --persistent --live --config allowed.  The change
in what --persistent does (affects also running domain, but it didn't
before), is OK with me.

ACK after 1.0.4 with that one line removed.

Martin


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]