On Thu, Feb 19, 2015 at 04:43:44PM +0100, Michal Privoznik wrote:
On 19.02.2015 15:28, Martin Kletzander wrote:
> When editing a domain with 'virsh edit' and failing validation, the
> usual message pops up:
>
> Failed. Try again? [y,n,f,?]:
>
> The option 'f' (force) is pretty unusable. It tries to step into the
> same puddle again and again when it can at least try to force the
> definition.
The idea behind force was, that imagine two virshes running. In both you
start editing the same domain. In one you successfully save the changes
and domain XML is regenerated. However, the other one notices this and
ask you, that domain definition has changed meanwhile, if you want to
continue. And by choosing 'force' you can use the force, Luke.
I had no idea this could be a use case, the only thing I could think
of is that if it fails, I am running bad daemon, so I run the right
one and force it, but that never worked (the disconnection breaks the
re-edit), so I thought it's pretty pointless.
I can add a new letter for toggling validation if you'd rather see that.
>
> This patch adds support for relaxing definition in virsh-edit and makes
> 'virsh edit <domain>' more usable, mainly for testing.
>
> Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
> ---
> tools/virsh-domain.c | 5 +++++
> tools/virsh-edit.c | 5 ++++-
> tools/virsh.c | 4 ++--
> 3 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 2506b89..3dd676c 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -11265,6 +11265,11 @@ cmdEdit(vshControl *ctl, const vshCmd *cmd)
> } while (0)
> #define EDIT_DEFINE \
> (dom_edited = vshDomainDefine(ctl->conn, doc_edited, define_flags))
> +#define EDIT_RELAX \
> + do { \
> + define_flags &= ~VIR_DOMAIN_DEFINE_VALIDATE; \
> + } while (0);
> +
> #include "virsh-edit.c"
>
> vshPrint(ctl, _("Domain %s XML configuration edited.\n"),
> diff --git a/tools/virsh-edit.c b/tools/virsh-edit.c
> index 41a6d53..3186998 100644
> --- a/tools/virsh-edit.c
> +++ b/tools/virsh-edit.c
> @@ -1,7 +1,7 @@
> /*
> * virsh-edit.c: Implementation of generic virsh *-edit intelligence
> *
> - * Copyright (C) 2012 Red Hat, Inc.
> + * Copyright (C) 2012, 2015 Red Hat, Inc.
> *
> * This library is free software; you can redistribute it and/or
> * modify it under the terms of the GNU Lesser General Public
> @@ -119,6 +119,9 @@ do {
> break;
>
> case 'f':
> +#ifdef EDIT_RELAX
What could happen to the EDIT_RELAX to be undefined? I mean, it'll be
defined no matter what (since you're defining it here in this patch).
I define it only when editing a domain (although I forgot to undefine
it afterwards).
> + EDIT_RELAX;
> +#endif
> goto redefine;
> break;
>
> diff --git a/tools/virsh.c b/tools/virsh.c
> index aba34ce..5b154e1 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -1,7 +1,7 @@
> /*
> * virsh.c: a shell to exercise the libvirt API
> *
> - * Copyright (C) 2005, 2007-2014 Red Hat, Inc.
> + * Copyright (C) 2005, 2007-2015 Red Hat, Inc.
> *
> * This library is free software; you can redistribute it and/or
> * modify it under the terms of the GNU Lesser General Public
> @@ -541,7 +541,7 @@ vshAskReedit(vshControl *ctl, const char *msg)
> "",
> _("y - yes, start editor again"),
> _("n - no, throw away my changes"),
> - _("f - force, try to redefine again"),
> + _("f - force, try to redefine again (without
validation)"),
I'd s/(/(even /
unless new letter appears...
> _("? - print this help"),
> NULL);
> continue;
>
ACK though.
waiting for your opinion. I'm also thinking about revriting it so we
don't have to include a .c file.
Michal
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list