[libvirt] RFC: virt-console
Daniel P. Berrange
berrange at redhat.com
Tue Jul 1 18:35:56 UTC 2008
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 :|
More information about the libvir-list
mailing list