On Tue, Jul 01, 2008 at 04:21:38PM +0100, John Levon wrote:
See implementation here:
http://cr.opensolaris.org/~johnlev/virt-console/
(inside libvirt.hg/patches/libvirt/virt-console)
This splits virsh console into a separate binary to allow it to be
setuid-root on Solaris (where we check permissions then drop privilege).
It also fixes a number of RFEs
--- libvirt-0.4.0/src/Makefile.in 2008-06-27 06:17:20.865621099
-0700
+++ libvirt-1/src/Makefile.in 2008-06-27 06:19:55.983265305 -0700
You can exclude Makefile.in from patches, since its auto-generated.
--- libvirt-0.4.0/src/virsh.c 2008-06-27 13:30:32.395824295 -0700
+++ libvirt-new/src/virsh.c 2008-06-27 13:29:26.021985284 -0700
@@ -48,7 +48,7 @@
#endif
#include "internal.h"
-#include "console.h"
+#include "util.h"
static char *progname;
static int sigpipe;
@@ -446,7 +446,7 @@ cmdConnect(vshControl * ctl, vshCmd * cm
* "console" command
*/
static vshCmdInfo info_console[] = {
- {"syntax", "console <domain>"},
+ {"syntax", "console [--verbose] <domain>"},
{"help", gettext_noop("connect to the guest console")},
{"desc",
gettext_noop("Connect the virtual serial console for the guest")},
@@ -454,6 +454,7 @@ static vshCmdInfo info_console[] = {
};
static vshCmdOptDef opts_console[] = {
+ {"verbose", VSH_OT_BOOL, 0, gettext_noop("verbose console")},
{"domain", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("domain name, id
or uuid")},
{NULL, 0, 0, NULL}
};
@@ -463,50 +464,37 @@ static vshCmdOptDef opts_console[] = {
static int
cmdConsole(vshControl * ctl, vshCmd * cmd)
{
- xmlDocPtr xml = NULL;
- xmlXPathObjectPtr obj = NULL;
- xmlXPathContextPtr ctxt = NULL;
- virDomainPtr dom;
+ int verbose = vshCommandOptBool(cmd, "verbose");
+ char *argv[] = { "/usr/bin/virt-console", NULL, NULL, NULL, NULL };
This should probably be
char *argv[] = { BINDIR "/virt-console", NULL, NULL, NULL, NULL };
so that it auto-adjusts when people pass --prefix to configure
- obj = xmlXPathEval(BAD_CAST
"string(/domain/devices/console/@tty)", ctxt);
- if ((obj != NULL) && ((obj->type == XPATH_STRING) &&
- (obj->stringval != NULL) && (obj->stringval[0]
!= 0))) {
- if (vshRunConsole((const char *)obj->stringval) == 0)
- ret = TRUE;
- } else {
- vshPrintExtra(ctl, _("No console available for domain\n"));
+ ret = virExecNoPipe(ctl->conn, argv, &pid);
+ if (ret == -1) {
+ vshError(ctl, FALSE, _("Couldn't execute
/usr/bin/virt-console."));
+ return FALSE;
}
- xmlXPathFreeObject(obj);
- cleanup:
- if (ctxt)
- xmlXPathFreeContext(ctxt);
- if (xml)
- xmlFreeDoc(xml);
- virDomainFree(dom);
- return ret;
+ waitpid(pid, NULL, 0);
Ought to do this in a while loop to handle EINTR.
--- /dev/null 2008-06-30 17:03:12.000000000 -0700
+++ libvirt-new/src/virt-console.c 2008-06-30 16:58:36.079071463 -0700
I'd prefer to keep the source in the 'console.c' file instead of renaming
it, just to make historical diff tracking a little easier.
+#include "internal.h"
+
+/* ie Ctrl-] as per telnet */
+#define CTRL_CLOSE_BRACKET '\35'
+
+static int got_signal;
+static int verbose;
+static const char *dom_name;
+static const char *conn_name;
+
+static void
+do_signal(int sig ATTRIBUTE_UNUSED)
+{
+ got_signal = 1;
+}
+
+#ifndef HAVE_CFMAKERAW
+static void
+cfmakeraw(struct termios *attr)
+{
+ attr->c_iflag &= ~(IGNBRK | BRKINT | PARMRK | ISTRIP
+ | INLCR | IGNCR | ICRNL | IXON);
+ attr->c_oflag &= ~OPOST;
+ attr->c_lflag &= ~(ECHO | ECHONL | ICANON | ISIG | IEXTEN);
+ attr->c_cflag &= ~(CSIZE | PARENB);
+ attr->c_cflag |= CS8;
+
+#ifdef __sun
+ attr->c_cc[VMIN] = 0;
+ attr->c_cc[VTIME] = 0;
+#endif
+}
+#endif /* !HAVE_CFMAKERAW */
A question for Jim here ... does gnulib have a cfmakeraw() implementation
we can use ?
+static int
+get_domain(virConnectPtr *conn, virDomainPtr *dom,
+ virDomainInfo *info, int lookup_by_id)
+{
+ int ret = 0;
+ int id;
+
+ __priv_bracket(PRIV_ON);
+
+ *conn = virConnectOpenAuth(conn_name, virConnectAuthPtrDefault, 0);
We ought to pass VIR_CONNECT_RO as the 3rd arg here, since the console
doesn't require write access.
+ if (*conn == NULL) {
+ fprintf(stderr, _("Failed to connect to the hypervisor"));
+ exit(EXIT_FAILURE);
+ }
+
+ *dom = virDomainLookupByName(*conn, dom_name);
+
+ if (*dom == NULL)
+ *dom = virDomainLookupByUUIDString(*conn, dom_name);
+ if (*dom == NULL && lookup_by_id &&
+ xstrtol_i(dom_name, NULL, 10, &id) == 0 && id >= 0)
Can use virStrToLong_i from util.h here.
+ obj = xmlXPathEval(BAD_CAST
"string(/domain/devices/console/@tty)", ctxt);
The virXPathString() method from xml.h will simplify the following handling
+ if (obj == NULL)
+ goto cleanup;
+ if (obj->type != XPATH_STRING)
+ goto cleanup;
+ if (!obj->stringval || obj->stringval[0] == '\0')
+ goto cleanup;
+
+ tty = strdup((const char *)obj->stringval);
+
+cleanup:
+ if (obj != NULL)
+ xmlXPathFreeObject(obj);
+ free(doc);
+ if (ctxt != NULL)
+ xmlXPathFreeContext(ctxt);
+ if (xml != NULL)
+ xmlFreeDoc(xml);
+ return tty;
+}
+static int
+check_for_reboot(void)
+{
+ virConnectPtr conn = NULL;
+ virDomainPtr dom = NULL;
+ virDomainInfo info;
+ int tries = 0;
+ int ret = 0;
+
+retry:
+ if (dom != NULL)
+ put_domain(conn, dom);
+
+ /*
+ * Domain ID will vary across reboot, so don't lookup by a given ID.
+ */
+ if (!get_domain(&conn, &dom, &info, 0))
+ return 0;
+
+ switch (info.state) {
+ case VIR_DOMAIN_RUNNING:
+ case VIR_DOMAIN_BLOCKED:
+ case VIR_DOMAIN_PAUSED:
+ ret = 1;
+ goto out;
+ break;
+
+ case VIR_DOMAIN_CRASHED:
+ if (verbose)
+ fprintf(stderr, _("Domain \"%s\" has crashed."),
dom_name);
+ goto out;
+ break;
+
+ case VIR_DOMAIN_NOSTATE:
+ default:
+ break;
+
+ case VIR_DOMAIN_SHUTDOWN:
+ if (verbose)
+ fprintf(stderr, _("Domain \"%s\" is shutting down.\n"),
dom_name);
+ tries = 0;
+ break;
+
+ case VIR_DOMAIN_SHUTOFF:
+ if (verbose)
+ fprintf(stderr, _("Domain \"%s\" has shut down."),
dom_name);
+ goto out;
+ break;
+ }
+
+ tries++;
+ if (tries == 1) {
+ goto retry;
+ } else if (tries == 2) {
+ sleep(1);
+ goto retry;
+ }
I think we probably need to wait a little longer than this - 5 times with a
1 second sleep perhaps. Under heavy load it can take a while for reboot to
complete
+
+out:
+ put_domain(conn, dom);
+ return ret;
+}
+
+static int
+open_tty(void)
+{
+ char *tty;
+ int ttyfd;
+
+ if ((tty = get_domain_tty()) == NULL) {
+ if (domain_is_running() != 1) {
+ fprintf(stderr, _("Domain \"%s\" is not running.\n"),
dom_name);
+ exit(EXIT_FAILURE);
+ }
+
+ fprintf(stderr,
+ _("Couldn't get console for domain \"%s\"\n"),
dom_name);
+ exit(EXIT_FAILURE);
+ }
+
+ __priv_bracket(PRIV_ON);
+ if ((ttyfd = open(tty, O_NOCTTY | O_RDWR)) < 0) {
+ fprintf(stderr, _("Unable to open tty %s: %s\n"),
+ tty, strerror(errno));
+ exit(EXIT_FAILURE);
+ }
+
+ if (lockf(ttyfd, F_TLOCK, 0) == -1) {
Ahh good idea to lock the tty.
+ if (errno == EACCES || errno == EAGAIN) {
+ fprintf(stderr,
+ _("Console for domain \"%s\" is in use.\n"),
dom_name);
+ } else {
+ fprintf(stderr, _("Unable to lock tty %s: %s\n"),
+ tty, strerror(errno));
+ }
+ exit(EXIT_FAILURE);
+ }
+ __priv_bracket(PRIV_OFF);
+
+ free(tty);
When forward porting you can use VIR_FREE, from memory.h
+retry:
+
+ ttyfd = open_tty();
+
+ if (!retrying && verbose) {
+ printf("Connected to domain %s\n", dom_name);
+ printf("Escape character is '^]'\n");
+ }
+
+ if (tcgetattr(STDIN_FILENO, &ttyattr) < 0) {
+ fprintf(stderr, _("Unable to get tty attributes: %s\n"),
+ strerror(errno));
+ exit(EXIT_FAILURE);
+ }
+
+ rawattr = ttyattr;
+ cfmakeraw(&rawattr);
+
+ if (tcsetattr(STDIN_FILENO, TCSAFLUSH, &rawattr) < 0) {
+ fprintf(stderr, _("Unable to set tty attributes: %s\n"),
+ strerror(errno));
+ exit(EXIT_FAILURE);
+ }
+
+ /* Now lets process STDIN & tty forever.... */
+ for (; !got_signal ;) {
+ unsigned int i;
+ struct pollfd fds[] = {
+ { STDIN_FILENO, POLLIN, 0 },
+ { ttyfd, POLLIN, 0 },
+ };
+
+ /* Wait for data to be available for reading on
+ STDIN or the tty */
+ if (poll(fds, (sizeof(fds)/sizeof(struct pollfd)), -1) < 0) {
+ if (got_signal)
+ goto cleanup;
+
+ if (errno == EINTR || errno == EAGAIN)
+ continue;
+
+ fprintf(stderr, _("Failure waiting for I/O: %s\n"),
+ strerror(errno));
+ goto cleanup;
+ }
+
+ for (i = 0 ; i < (sizeof(fds)/sizeof(struct pollfd)) ; i++) {
+ if (!fds[i].revents)
+ continue;
+
+ /* Process incoming data available for read */
+ if (fds[i].revents & POLLIN) {
+ char buf[4096];
+ int got, sent = 0, destfd;
+
+ if ((got = read(fds[i].fd, buf, sizeof(buf))) < 0) {
+ fprintf(stderr, _("Failure reading input: %s\n"),
+ strerror(errno));
+ goto cleanup;
+ }
+
+ if (!got) {
+ tcsetattr(STDIN_FILENO, TCSAFLUSH, &ttyattr);
+ if (!check_for_reboot())
+ goto done;
+ close_tty(ttyfd);
+ retrying = 1;
+ goto retry;
+ }
+
+ if (got == 1 && buf[0] == CTRL_CLOSE_BRACKET)
+ goto done;
+
+ /* Data from stdin goes to the TTY,
+ data from the TTY goes to STDOUT */
+ if (fds[i].fd == STDIN_FILENO)
+ destfd = ttyfd;
+ else
+ destfd = STDOUT_FILENO;
+
+ while (sent < got) {
+ int done;
+ if ((done = write(destfd, buf + sent, got - sent)) <= 0) {
+ fprintf(stderr, _("Failure writing output: %s\n"),
+ strerror(errno));
+ goto cleanup;
+ }
+ sent += done;
+ }
+ } else if (fds[i].revents & POLLHUP) {
+ tcsetattr(STDIN_FILENO, TCSAFLUSH, &ttyattr);
+ if (!check_for_reboot())
+ goto done;
I like the support for re-connecting after reboot. At the same time I
wonder if we need to make the an optional command line flag. Some apps
using virsh console, may rely on the fact that it exits when a VM
shuts down.
Regards,
Daniel
--
|: Red Hat, Engineering, London -o-
http://people.redhat.com/berrange/ :|
|:
http://libvirt.org -o-
http://virt-manager.org -o-
http://ovirt.org :|
|:
http://autobuild.org -o-
http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|