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

John Ferlan jferlan at redhat.com
Thu Feb 19 19:26:59 UTC 2015



On 02/19/2015 11:24 AM, Martin Kletzander wrote:
> When editing a domain with 'virsh edit' and failing validation, the
> usual message pops up:
> 
>   Failed. Try again? [y,n,f,?]:
> 
> Turning of validation can be ussable, mainly for testing (but other

s/of/off
s/ussable/useful

> purposes too), so this patch adds support for relaxing definition in
> virsh-edit and makes 'virsh edit <domain>' more usable.
> 
> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
> ---
> v2:
>  - make a special case 'v' for turning off validation

Another choice 'i' "ignore validation" - only because when I see -v I
think "verbose" or "version" (just like -h is help)


Would it be worth while to add some "history" (eg, stuff from these
reviews) to virsh.pod under 'edit' in order to detail the various
options that you may be presented with if the edit fails, why you may
get them, and perhaps which to choose based on desired results.

The code below looks fine to me and I'm not going to object to which
letter you chose - I was just presenting options...

John
> 
>  tools/virsh-domain.c |  6 ++++++
>  tools/virsh-edit.c   | 21 +++++++++++++++++++--
>  tools/virsh.c        | 28 +++++++++++++++++++++-------
>  tools/virsh.h        |  4 ++--
>  4 files changed, 48 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 2506b89..b4761c6 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -11265,7 +11265,13 @@ 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"
> +#undefine EDIT_RELAX
> 
>      vshPrint(ctl, _("Domain %s XML configuration edited.\n"),
>               virDomainGetName(dom_edited));
> diff --git a/tools/virsh-edit.c b/tools/virsh-edit.c
> index 41a6d53..2a13a04 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
> @@ -62,6 +62,7 @@ do {
>      char *doc_reread = NULL;
>      const char *msg = NULL;
>      bool edit_success = false;
> +    bool relax_avail = false;
> 
>      /* Get the XML configuration of the object. */
>      doc = (EDIT_GET_XML);
> @@ -74,6 +75,11 @@ do {
>          goto edit_cleanup;
> 
>   reedit:
> +
> +#ifdef EDIT_RELAX
> +    relax_avail = true;
> +#endif
> +
>      /* Start the editor. */
>      if (vshEditFile(ctl, tmp) == -1)
>          goto edit_cleanup;
> @@ -112,7 +118,7 @@ do {
>          msg = _("Failed.");
> 
>      if (msg) {
> -        int c = vshAskReedit(ctl, msg);
> +        int c = vshAskReedit(ctl, msg, relax_avail);
>          switch (c) {
>          case 'y':
>              goto reedit;
> @@ -126,6 +132,17 @@ do {
>              goto edit_cleanup;
>              break;
> 
> +#ifdef EDIT_RELAX
> +        case 'v':
> +            if (relax_avail) {
> +                EDIT_RELAX;
> +                relax_avail = false;
> +                goto redefine;
> +                break;
> +            }
> +            /* fall-through */
> +#endif
> +
>          default:
>              vshError(ctl, "%s", msg);
>              break;
> diff --git a/tools/virsh.c b/tools/virsh.c
> index aba34ce..464af3d 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
> @@ -512,13 +512,14 @@ vshPrintRaw(vshControl *ctl, ...)
>   * edited file.
>   *
>   * Returns 'y' if he wants to
> - *         'f' if he forcibly wants to
>   *         'n' if he doesn't want to
> + *         'v' if he wants to try defining it again with validation turned off
> + *         'f' if he forcibly wants to
>   *         -1  on error
>   *          0  otherwise
>   */
>  int
> -vshAskReedit(vshControl *ctl, const char *msg)
> +vshAskReedit(vshControl *ctl, const char *msg, bool relax_avail)
>  {
>      int c = -1;
> 
> @@ -532,8 +533,9 @@ vshAskReedit(vshControl *ctl, const char *msg)
> 
>      while (true) {
>          /* TRANSLATORS: For now, we aren't using LC_MESSAGES, and the user
> -         * choices really are limited to just 'y', 'n', 'f' and '?'  */
> -        vshPrint(ctl, "\r%s %s", msg, _("Try again? [y,n,f,?]:"));
> +         * choices really are limited to just 'y', 'n', 'v', 'f' and '?'  */
> +        vshPrint(ctl, _("\r%s Try again? %s: "), msg,
> +                 relax_avail ? "[y,n,v,f,?]" : "[y,n,f,?]");
>          c = c_tolower(getchar());
> 
>          if (c == '?') {
> @@ -541,11 +543,21 @@ vshAskReedit(vshControl *ctl, const char *msg)
>                          "",
>                          _("y - yes, start editor again"),
>                          _("n - no, throw away my changes"),
> +                        NULL);
> +
> +            if (relax_avail) {
> +                vshPrintRaw(ctl,
> +                            _("v - turn off validation and try to redefine again"),
> +                            NULL);
> +            }
> +
> +            vshPrintRaw(ctl,
>                          _("f - force, try to redefine again"),
>                          _("? - print this help"),
>                          NULL);
>              continue;
> -        } else if (c == 'y' || c == 'n' || c == 'f') {
> +        } else if (c == 'y' || c == 'n' || c == 'f' ||
> +                   (relax_avail && c == 'v')) {
>              break;
>          }
>      }
> @@ -557,7 +569,9 @@ vshAskReedit(vshControl *ctl, const char *msg)
>  }
>  #else /* WIN32 */
>  int
> -vshAskReedit(vshControl *ctl, const char *msg ATTRIBUTE_UNUSED)
> +vshAskReedit(vshControl *ctl,
> +             const char *msg ATTRIBUTE_UNUSED,
> +             bool relax_avail ATTRIBUTE_UNUSED)
>  {
>      vshDebug(ctl, VSH_ERR_WARNING, "%s", _("This function is not "
>                                             "supported on WIN32 platform"));
> diff --git a/tools/virsh.h b/tools/virsh.h
> index 7d5d8a2..df2ea7f 100644
> --- a/tools/virsh.h
> +++ b/tools/virsh.h
> @@ -1,7 +1,7 @@
>  /*
>   * virsh.h: 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
> @@ -359,7 +359,7 @@ char *vshGetTypedParamValue(vshControl *ctl, virTypedParameterPtr item)
>  char *vshEditWriteToTempFile(vshControl *ctl, const char *doc);
>  int vshEditFile(vshControl *ctl, const char *filename);
>  char *vshEditReadBackFile(vshControl *ctl, const char *filename);
> -int vshAskReedit(vshControl *ctl, const char *msg);
> +int vshAskReedit(vshControl *ctl, const char *msg, bool relax_avail);
>  int vshStreamSink(virStreamPtr st, const char *bytes, size_t nbytes,
>                    void *opaque);
>  double vshPrettyCapacity(unsigned long long val, const char **unit);
> --
> 2.3.0
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
> 




More information about the libvir-list mailing list