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

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

 



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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]