[libvirt] [PATCH] virsh-edit: Make force editing usable

[Martin Kletzander]

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 at 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 at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150219/5d355387/attachment-0001.sig>