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@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list