[libvirt] [PATCH] virsh: Allow users to reedit rejected XML

[Dave Allan]

On Wed, May 09, 2012 at 03:36:26PM +0200, Michal Privoznik wrote:
> If users {net-,pool-,}edit but make a mistake in XML all changes
> are permanently lost. However, if virsh is running in interactive
> mode we can as user if he wants to re-edit the file and correct
> the mistakes.

ACK to the design, what a great usability enhancement.

Dave

> ---
>  tools/console.c |   40 +++++++++++++++++++++++++---------------
>  tools/console.h |    1 +
>  tools/virsh.c   |   41 +++++++++++++++++++++++++++++++++++------
>  3 files changed, 61 insertions(+), 21 deletions(-)
> 
> diff --git a/tools/console.c b/tools/console.c
> index 34fde05..90e54e3 100644
> --- a/tools/console.c
> +++ b/tools/console.c
> @@ -298,13 +298,36 @@ vshGetEscapeChar(const char *s)
>      return *s;
>  }
>  
> +int vshMakeStdinRaw(struct termios *ttyattr, bool report_errors) {
> +    struct termios rawattr;
> +
> +    if (tcgetattr(STDIN_FILENO, ttyattr) < 0) {
> +        if (report_errors)
> +            VIR_ERROR(_("unable to get tty attributes: %s"),
> +                      strerror(errno));
> +        return -1;
> +    }
> +
> +    rawattr = *ttyattr;
> +    cfmakeraw(&rawattr);
> +
> +    if (tcsetattr(STDIN_FILENO, TCSAFLUSH, &rawattr) < 0) {
> +        if (report_errors)
> +            VIR_ERROR(_("unable to set tty attributes: %s"),
> +                      strerror(errno));
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
>  int vshRunConsole(virDomainPtr dom,
>                    const char *dev_name,
>                    const char *escape_seq,
>                    unsigned int flags)
>  {
>      int ret = -1;
> -    struct termios ttyattr, rawattr;
> +    struct termios ttyattr;
>      void (*old_sigquit)(int);
>      void (*old_sigterm)(int);
>      void (*old_sigint)(int);
> @@ -317,21 +340,8 @@ int vshRunConsole(virDomainPtr dom,
>         result in it being echoed back already), and
>         also ensure Ctrl-C, etc is blocked, and misc
>         other bits */
> -    if (tcgetattr(STDIN_FILENO, &ttyattr) < 0) {
> -        VIR_ERROR(_("unable to get tty attributes: %s"),
> -                  strerror(errno));
> -        return -1;
> -    }
> -
> -    rawattr = ttyattr;
> -    cfmakeraw(&rawattr);
> -
> -    if (tcsetattr(STDIN_FILENO, TCSAFLUSH, &rawattr) < 0) {
> -        VIR_ERROR(_("unable to set tty attributes: %s"),
> -                  strerror(errno));
> +    if (vshMakeStdinRaw(&ttyattr, true) < 0)
>          goto resettty;
> -    }
> -
>  
>      /* Trap all common signals so that we can safely restore
>         the original terminal settings on STDIN before the
> diff --git a/tools/console.h b/tools/console.h
> index 2b5440c..97c97cd 100644
> --- a/tools/console.h
> +++ b/tools/console.h
> @@ -30,6 +30,7 @@ int vshRunConsole(virDomainPtr dom,
>                    const char *escape_seq,
>                    unsigned int flags);
>  
> +int vshMakeStdinRaw(struct termios *ttyattr, bool report_errors);
>  # endif /* !WIN32 */
>  
>  #endif /* __VIR_CONSOLE_H__ */
> diff --git a/tools/virsh.c b/tools/virsh.c
> index dd9292a..3537e2e 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -33,6 +33,7 @@
>  #include <signal.h>
>  #include <poll.h>
>  #include <strings.h>
> +#include <termios.h>
>  
>  #include <libxml/parser.h>
>  #include <libxml/tree.h>
> @@ -15806,11 +15807,14 @@ cmdEdit (vshControl *ctl, const vshCmd *cmd)
>  {
>      bool ret = false;
>      virDomainPtr dom = NULL;
> +    virDomainPtr dom_edited = NULL;
>      char *tmp = NULL;
>      char *doc = NULL;
>      char *doc_edited = NULL;
>      char *doc_reread = NULL;
>      unsigned int flags = VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_INACTIVE;
> +    int c = 0;
> +    struct termios ttyattr;
>  
>      if (!vshConnectionUsability(ctl, ctl->conn))
>          goto cleanup;
> @@ -15828,6 +15832,7 @@ cmdEdit (vshControl *ctl, const vshCmd *cmd)
>      tmp = editWriteToTempFile (ctl, doc);
>      if (!tmp) goto cleanup;
>  
> +reedit:
>      /* Start the editor. */
>      if (editFile (ctl, tmp) == -1) goto cleanup;
>  
> @@ -15858,19 +15863,43 @@ cmdEdit (vshControl *ctl, const vshCmd *cmd)
>      }
>  
>      /* Everything checks out, so redefine the domain. */
> -    virDomainFree (dom);
> -    dom = virDomainDefineXML(ctl->conn, doc_edited);
> -    if (!dom)
> -        goto cleanup;
> +    dom_edited = virDomainDefineXML(ctl->conn, doc_edited);
> +    if (!dom_edited) {
> +        /* Redefine failed. If we are in interactive mode ask user
> +         * if he wants to re-edit the XML. */
> +        if (!ctl->imode ||
> +            vshMakeStdinRaw(&ttyattr, false) < 0)
> +            goto cleanup;
> +
> +        virshReportError(ctl);
> +
> +        while (true) {
> +            vshPrint(ctl, "\rFailed. Try again? (y/Y/n/N) [Y]:");
> +            c = getchar();
> +            c = c_toupper(c);
> +            if (c == '\n' || c == '\r' || c == 'Y' || c == 'N')
> +                break;
> +        }
> +
> +        tcsetattr(STDIN_FILENO, TCSAFLUSH, &ttyattr);
> +
> +        if (c == 'N')
> +            goto cleanup;
> +
> +        vshPrint(ctl, "\r\n");
> +        goto reedit;
> +    }
>  
>      vshPrint (ctl, _("Domain %s XML configuration edited.\n"),
>                virDomainGetName(dom));
>  
>      ret = true;
>  
> - cleanup:
> +cleanup:
>      if (dom)
> -        virDomainFree (dom);
> +        virDomainFree(dom);
> +    if (dom_edited)
> +        virDomainFree(dom_edited);
>  
>      VIR_FREE(doc);
>      VIR_FREE(doc_edited);
> -- 
> 1.7.8.5
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list