Re: [PATCH] kvm tools: Allow remapping guest TTY into host PTS

[Asias He <asias.hejun@gmail.com>]

On 09/15/2011 04:53 PM, Sasha Levin wrote:
> This patch adds the '-tty' option to 'kvm run' which allows the user to
> remap a guest TTY into a PTS on the host.
> 
> Usage:
> 	'kvm run --tty [id] [other options]'
> 
> The tty will be mapped to a pts and will be printed on the screen:
> 	'  Info: Assigned terminal 1 to pty /dev/pts/X'
> 
> At this point, it is possible to communicate with the guest using that pty.
> 
> This is useful for debugging guest kernel using KGDB:
> 
> 1. Run the guest:
> 	'kvm run -k [vmlinuz] -p "kdbgoc=ttyS1 kdbgwait" --tty 1'
> 
> And see which PTY got assigned to ttyS1.
> 
> 2. Run GDB on the host:
> 	'gdb [vmlinuz]'
> 
> 3. Connect to the guest (from within GDB):
> 	'target remote /dev/pty/X'
> 
> 4. Start debugging! (enter 'continue' to continue boot).
> 
> Cc: David Evensky <evensky@dancer.ca.sandia.gov>
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> ---
>  tools/kvm/Makefile           |    1 +
>  tools/kvm/builtin-run.c      |   12 ++++++++
>  tools/kvm/hw/serial.c        |   46 ++++++++++++++++++--------------
>  tools/kvm/include/kvm/term.h |   11 ++++---
>  tools/kvm/term.c             |   60 +++++++++++++++++++++++++++++++++--------
>  tools/kvm/virtio/console.c   |    6 ++--
>  6 files changed, 96 insertions(+), 40 deletions(-)
> 
> diff --git a/tools/kvm/Makefile b/tools/kvm/Makefile
> index efa032d..fef624d 100644
> --- a/tools/kvm/Makefile
> +++ b/tools/kvm/Makefile
> @@ -115,6 +115,7 @@ OBJS	+= bios/bios-rom.o
>  
>  LIBS	+= -lrt
>  LIBS	+= -lpthread
> +LIBS	+= -lutil
>  
>  # Additional ARCH settings for x86
>  ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/i386/ -e s/sun4u/sparc64/ \
> diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
> index 5dafb15..b5c63ca 100644
> --- a/tools/kvm/builtin-run.c
> +++ b/tools/kvm/builtin-run.c
> @@ -172,6 +172,15 @@ static int virtio_9p_rootdir_parser(const struct option *opt, const char *arg, i
>  	return 0;
>  }
>  
> +static int tty_parser(const struct option *opt, const char *arg, int unset)
> +{
> +	int tty = atoi(arg);
> +
> +	term_set_tty(tty);
> +
> +	return 0;
> +}
> +
>  static int shmem_parser(const struct option *opt, const char *arg, int unset)
>  {
>  	const u64 default_size = SHMEM_DEFAULT_SIZE;
> @@ -316,6 +325,9 @@ static const struct option options[] = {
>  	OPT_STRING('\0', "console", &console, "serial or virtio",
>  			"Console to use"),
>  	OPT_STRING('\0', "dev", &dev, "device_file", "KVM device file"),
> +	OPT_CALLBACK('\0', "tty", NULL, "tty id",
> +		     "Remap guest TTY into a pty on the host",
> +		     tty_parser),
>  
>  	OPT_GROUP("Kernel options:"),
>  	OPT_STRING('k', "kernel", &kernel_filename, "kernel",
> diff --git a/tools/kvm/hw/serial.c b/tools/kvm/hw/serial.c
> index b3b233f..11fa5d4 100644
> --- a/tools/kvm/hw/serial.c
> +++ b/tools/kvm/hw/serial.c
> @@ -14,6 +14,7 @@
>  
>  struct serial8250_device {
>  	pthread_mutex_t		mutex;
> +	u8			id;
>  
>  	u16			iobase;
>  	u8			irq;
> @@ -42,6 +43,7 @@ static struct serial8250_device devices[] = {
>  	[0]	= {
>  		.mutex			= PTHREAD_MUTEX_INITIALIZER,
>  
> +		.id			= 0,
>  		.iobase			= 0x3f8,
>  		.irq			= 4,
>  
> @@ -51,6 +53,7 @@ static struct serial8250_device devices[] = {
>  	[1]	= {
>  		.mutex			= PTHREAD_MUTEX_INITIALIZER,
>  
> +		.id			= 1,
>  		.iobase			= 0x2f8,
>  		.irq			= 3,
>  
> @@ -60,6 +63,7 @@ static struct serial8250_device devices[] = {
>  	[2]	= {
>  		.mutex			= PTHREAD_MUTEX_INITIALIZER,
>  
> +		.id			= 2,
>  		.iobase			= 0x3e8,
>  		.irq			= 4,
>  
> @@ -69,6 +73,7 @@ static struct serial8250_device devices[] = {
>  	[3]	= {
>  		.mutex			= PTHREAD_MUTEX_INITIALIZER,
>  
> +		.id			= 3,
>  		.iobase			= 0x2e8,
>  		.irq			= 3,
>  
> @@ -111,10 +116,10 @@ static void serial8250__receive(struct kvm *kvm, struct serial8250_device *dev)
>  		return;
>  	}
>  
> -	if (!term_readable(CONSOLE_8250))
> +	if (!term_readable(CONSOLE_8250, dev->id))
>  		return;
>  
> -	c		= term_getc(CONSOLE_8250);
> +	c = term_getc(CONSOLE_8250, dev->id);
>  
>  	if (c < 0)
>  		return;
> @@ -123,30 +128,31 @@ static void serial8250__receive(struct kvm *kvm, struct serial8250_device *dev)
>  	dev->lsr	|= UART_LSR_DR;
>  }
>  
> -/*
> - * Interrupts are injected for ttyS0 only.
> - */
>  void serial8250__inject_interrupt(struct kvm *kvm)
>  {
> -	struct serial8250_device *dev = &devices[0];
> +	int i;
>  
> -	mutex_lock(&dev->mutex);
> +	for (i = 0; i < 4; i++) {
> +		struct serial8250_device *dev = &devices[i];
>  
> -	serial8250__receive(kvm, dev);
> +		mutex_lock(&dev->mutex);
>  
> -	if (dev->ier & UART_IER_RDI && dev->lsr & UART_LSR_DR)
> -		dev->iir		= UART_IIR_RDI;
> -	else if (dev->ier & UART_IER_THRI)
> -		dev->iir		= UART_IIR_THRI;
> -	else
> -		dev->iir		= UART_IIR_NO_INT;
> +		serial8250__receive(kvm, dev);
>  
> -	if (dev->iir != UART_IIR_NO_INT) {
> -		kvm__irq_line(kvm, dev->irq, 0);
> -		kvm__irq_line(kvm, dev->irq, 1);
> -	}
> +		if (dev->ier & UART_IER_RDI && dev->lsr & UART_LSR_DR)
> +			dev->iir		= UART_IIR_RDI;
> +		else if (dev->ier & UART_IER_THRI)
> +			dev->iir		= UART_IIR_THRI;
> +		else
> +			dev->iir		= UART_IIR_NO_INT;
>  
> -	mutex_unlock(&dev->mutex);
> +		if (dev->iir != UART_IIR_NO_INT) {
> +			kvm__irq_line(kvm, dev->irq, 0);
> +			kvm__irq_line(kvm, dev->irq, 1);
> +		}
> +
> +		mutex_unlock(&dev->mutex);
> +	}
>  }
>  
>  void serial8250__inject_sysrq(struct kvm *kvm)
> @@ -217,7 +223,7 @@ static bool serial8250_out(struct ioport *ioport, struct kvm *kvm, u16 port, voi
>  			char *addr = data;
>  
>  			if (!(dev->mcr & UART_MCR_LOOP))
> -				term_putc(CONSOLE_8250, addr, size);
> +				term_putc(CONSOLE_8250, addr, size, dev->id);
>  
>  			dev->iir		= UART_IIR_NO_INT;
>  			break;
> diff --git a/tools/kvm/include/kvm/term.h b/tools/kvm/include/kvm/term.h
> index 4d580e1..37ec731 100644
> --- a/tools/kvm/include/kvm/term.h
> +++ b/tools/kvm/include/kvm/term.h
> @@ -6,12 +6,13 @@
>  #define CONSOLE_8250	1
>  #define CONSOLE_VIRTIO	2
>  
> -int term_putc_iov(int who, struct iovec *iov, int iovcnt);
> -int term_getc_iov(int who, struct iovec *iov, int iovcnt);
> -int term_putc(int who, char *addr, int cnt);
> -int term_getc(int who);
> +int term_putc_iov(int who, struct iovec *iov, int iovcnt, int term);
> +int term_getc_iov(int who, struct iovec *iov, int iovcnt, int term);
> +int term_putc(int who, char *addr, int cnt, int term);
> +int term_getc(int who, int term);
>  
> -bool term_readable(int who);
> +bool term_readable(int who, int term);
> +void term_set_tty(int term);
>  void term_init(void);
>  
>  #endif /* KVM__TERM_H */
> diff --git a/tools/kvm/term.c b/tools/kvm/term.c
> index fa4382d..45e8bab 100644
> --- a/tools/kvm/term.c
> +++ b/tools/kvm/term.c
> @@ -5,6 +5,8 @@
>  #include <unistd.h>
>  #include <sys/uio.h>
>  #include <signal.h>
> +#include <pty.h>
> +#include <utmp.h>
>  
>  #include "kvm/read-write.h"
>  #include "kvm/term.h"
> @@ -20,14 +22,16 @@ bool term_got_escape	= false;
>  
>  int active_console;
>  
> -int term_getc(int who)
> +int term_fds[4][2];
> +
> +int term_getc(int who, int term)
>  {
>  	int c;
>  
>  	if (who != active_console)
>  		return -1;
>  
> -	if (read_in_full(STDIN_FILENO, &c, 1) < 0)
> +	if (read_in_full(term_fds[term][0], &c, 1) < 0)

We need a Macro like:

#define TERM_FD_IN	0
#define TERM_FD_OUT	1

>  		return -1;
>  
>  	c &= 0xff;
> @@ -48,26 +52,27 @@ int term_getc(int who)
>  	return c;
>  }
>  
> -int term_putc(int who, char *addr, int cnt)
> +int term_putc(int who, char *addr, int cnt, int term)
>  {
> +	int ret;
> +
>  	if (who != active_console)
>  		return -1;
>  
>  	while (cnt--)
> -		fprintf(stdout, "%c", *addr++);
> +		ret = write(term_fds[term][1], addr++, 1);

Here and some other places too.

And I am seeing:

term.c: In function ‘term_putc’:
term.c:57:6: error: variable ‘ret’ set but not used
[-Werror=unused-but-set-variable]
cc1: all warnings being treated as errors


> -	fflush(stdout);
>  	return cnt;
>  }
>  
> -int term_getc_iov(int who, struct iovec *iov, int iovcnt)
> +int term_getc_iov(int who, struct iovec *iov, int iovcnt, int term)
>  {
>  	int c;
>  
>  	if (who != active_console)
>  		return 0;
>  
> -	c = term_getc(who);
> +	c = term_getc(who, term);
>  
>  	if (c < 0)
>  		return 0;
> @@ -77,18 +82,18 @@ int term_getc_iov(int who, struct iovec *iov, int iovcnt)
>  	return sizeof(char);
>  }
>  
> -int term_putc_iov(int who, struct iovec *iov, int iovcnt)
> +int term_putc_iov(int who, struct iovec *iov, int iovcnt, int term)
>  {
>  	if (who != active_console)
>  		return 0;
>  
> -	return writev(STDOUT_FILENO, iov, iovcnt);
> +	return writev(term_fds[term][1], iov, iovcnt);
>  }
>  
> -bool term_readable(int who)
> +bool term_readable(int who, int term)
>  {
>  	struct pollfd pollfd = (struct pollfd) {
> -		.fd	= STDIN_FILENO,
> +		.fd	= term_fds[term][0],
>  		.events	= POLLIN,
>  		.revents = 0,
>  	};
> @@ -101,7 +106,10 @@ bool term_readable(int who)
>  
>  static void term_cleanup(void)
>  {
> -	tcsetattr(STDIN_FILENO, TCSANOW, &orig_term);
> +	int i;
> +
> +	for (i = 0; i < 4; i++)
> +		tcsetattr(term_fds[i][0], TCSANOW, &orig_term);
>  }
>  
>  static void term_sig_cleanup(int sig)
> @@ -111,9 +119,31 @@ static void term_sig_cleanup(int sig)
>  	raise(sig);
>  }
>  
> +void term_set_tty(int term)
> +{
> +	struct termios orig_term;
> +	int master, slave;
> +	char new_pty[PATH_MAX];
> +
> +	if (tcgetattr(STDIN_FILENO, &orig_term) < 0)
> +		die("unable to save initial standard input settings");
> +
> +	orig_term.c_lflag &= ~(ICANON | ECHO | ISIG);
> +
> +	if (openpty(&master, &slave, new_pty, &orig_term, NULL) < 0)
> +		return;
> +
> +	close(slave);
> +
> +	pr_info("Assigned terminal %d to pty %s\n", term, new_pty);
> +
> +	term_fds[term][0] = term_fds[term][1] = master;
> +}
> +
>  void term_init(void)
>  {
>  	struct termios term;
> +	int i;
>  
>  	if (tcgetattr(STDIN_FILENO, &orig_term) < 0)
>  		die("unable to save initial standard input settings");
> @@ -122,6 +152,12 @@ void term_init(void)
>  	term.c_lflag &= ~(ICANON | ECHO | ISIG);
>  	tcsetattr(STDIN_FILENO, TCSANOW, &term);
>  
> +	for (i = 0; i < 4; i++)
> +		if (term_fds[i][0] == 0) {
> +			term_fds[i][0] = STDIN_FILENO;
> +			term_fds[i][1] = STDOUT_FILENO;
> +		}
> +
>  	signal(SIGTERM, term_sig_cleanup);
>  	atexit(term_cleanup);
>  }
> diff --git a/tools/kvm/virtio/console.c b/tools/kvm/virtio/console.c
> index c0ccd6c..b880162 100644
> --- a/tools/kvm/virtio/console.c
> +++ b/tools/kvm/virtio/console.c
> @@ -67,9 +67,9 @@ static void virtio_console__inject_interrupt_callback(struct kvm *kvm, void *par
>  
>  	vq = param;
>  
> -	if (term_readable(CONSOLE_VIRTIO) && virt_queue__available(vq)) {
> +	if (term_readable(CONSOLE_VIRTIO, 0) && virt_queue__available(vq)) {
>  		head = virt_queue__get_iov(vq, iov, &out, &in, kvm);
> -		len = term_getc_iov(CONSOLE_VIRTIO, iov, in);
> +		len = term_getc_iov(CONSOLE_VIRTIO, iov, in, 0);
>  		virt_queue__set_used_elem(vq, head, len);
>  		virtio_pci__signal_vq(kvm, &cdev.vpci, vq - cdev.vqs);
>  	}
> @@ -100,7 +100,7 @@ static void virtio_console_handle_callback(struct kvm *kvm, void *param)
>  
>  	while (virt_queue__available(vq)) {
>  		head = virt_queue__get_iov(vq, iov, &out, &in, kvm);
> -		len = term_putc_iov(CONSOLE_VIRTIO, iov, out);
> +		len = term_putc_iov(CONSOLE_VIRTIO, iov, out, 0);
>  		virt_queue__set_used_elem(vq, head, len);
>  	}
>  

-- 
Asias He