On 03/28/2011 06:02 PM, Eric Blake wrote:
On 03/27/2011 02:59 AM, Laine Stump wrote:
On 03/26/2011 08:12 AM, Eric Blake wrote:
Diego reported a bug where virsh tries to initialize a readline
history directory during 'make check' run as root, but fails
because /root was read-only.
It turns out that I could reproduce this as non-root, by using:
mv ~/.virsh{,.bak}
chmod a-w ~
make check -C tests TESTS=int-overflow
chmod u+w ~
mv ~/.virsh{.bak,}
* tests/int-overflow: Don't trigger interactive mode.
Reported by Diego Elio PettenÃ.
---
echo "error: failed to get domain '4294967298'"> exp || fail=1
-echo domname 4294967298 | $abs_top_builddir/tools/virsh --quiet \
+$abs_top_builddir/tools/virsh --quiet \
--connect test://$abs_top_srcdir/examples/xml/test/testnode.xml \
-> /dev/null 2> err || fail=1
+ 'domname 4294967298; quit'> /dev/null 2> err || fail=1
diff -u err exp || fail=1
Why the "; quit" in the command? a quit is implicit anyway.
A quit may be implicit, but the problem at hand is what $? gets set to
in the shell after virsh is done. In batch mode, virsh exits with the
status of the last command executed, and we expect the domname command
to fail (invalid argument), whereas quit (trivially) succeeds. That is,
since the shell script expects $? to be zero, the quit command is
necessary to avoid polluting the exit status with the failure from
domname while still proving that virsh didn't abort command processing
merely because one command failed. We didn't need the quit in
interactive mode, because EOF is treated the same as an implicit quit
command.
ACK, with or without the quit - it works either way.
Pushed with the quit.
What about, on top of this, something like the attached (untested,
uncompiled :)) patch? It enables readline only when stdin is a TTY, and
MinGW already has isatty.
Paolo
diff --git a/tools/virsh.c b/tools/virsh.c
index 50ca50f..a4371c2 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -246,6 +246,9 @@ static void vshOpenLogFile(vshControl *ctl);
static void vshOutputLogFile(vshControl *ctl, int log_level, const char *format, va_list ap);
static void vshCloseLogFile(vshControl *ctl);
+static char *vshReadlineNoReadline (vshControl *, const char *);
+static char *(*vshReadline)(vshControl *, const char *) = vshReadlineNoReadline;
+
static int vshParseArgv(vshControl *ctl, int argc, char **argv);
static const char *vshCmddefGetInfo(const vshCmdDef *cmd, const char *info);
@@ -12105,6 +12108,12 @@ vshReadlineCompletion(const char *text, int start,
return matches;
}
+static char *
+vshReadlineReadline (vshControl *ctl ATTRIBUTE_UNUSED, const char *prompt)
+{
+ return readline (prompt);
+}
+
static int
vshReadlineInit(vshControl *ctl)
@@ -12141,6 +12150,7 @@ vshReadlineInit(vshControl *ctl)
VIR_FREE(userdir);
read_history(ctl->historyfile);
+ vshReadline = vshReadlineReadline;
return 0;
}
@@ -12161,12 +12171,6 @@ vshReadlineDeinit (vshControl *ctl)
VIR_FREE(ctl->historyfile);
}
-static char *
-vshReadline (vshControl *ctl ATTRIBUTE_UNUSED, const char *prompt)
-{
- return readline (prompt);
-}
-
#else /* !USE_READLINE */
static int
@@ -12182,8 +12186,10 @@ vshReadlineDeinit (vshControl *ctl ATTRIBUTE_UNUSED)
/* empty */
}
+#endif /* !USE_READLINE */
+
static char *
-vshReadline (vshControl *ctl, const char *prompt)
+vshReadlineNoReadline (vshControl *ctl, const char *prompt)
{
char line[1024];
char *r;
@@ -12201,8 +12207,6 @@ vshReadline (vshControl *ctl, const char *prompt)
return vshStrdup (ctl, r);
}
-#endif /* !USE_READLINE */
-
/*
* Deinitialize virsh
*/
@@ -12531,9 +12535,8 @@ main(int argc, char **argv)
" 'quit' to quit\n\n"));
}
- if (vshReadlineInit(ctl) < 0) {
- vshDeinit(ctl);
- exit(EXIT_FAILURE);
+ if (isatty(STDIN_FILENO)) {
+ vshReadlineInit(ctl);
}
do {
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list