[Devel] [PATCH rh7 v2] vtty: Allow to wait until container's console appear

[Vladimir Davydov]

On Fri, Jun 10, 2016 at 04:34:34PM +0300, Cyrill Gorcunov wrote:
> After tty code redesing we've been requiring container to start
> first before be able to connect into it via vzctl console command.
> Here we rather allow userspace tool to wait until container brought
> to life and proceed connecting into console.
> 
> https://jira.sw.ru/browse/PSBM-39463
> 
> Note: when someone tried to open several consoles for offline
> mode (say vzctl console 300 1 and vzctl console 300 2) simultaneously
> only one is allowed until VE is up, the second vzctl command will
> exit with -EBUSY.
> 
> v2:
>  - move everything into vtty code
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov at virtuozzo.com>
> CC: Vladimir Davydov <vdavydov at virtuozzo.com>
> CC: Konstantin Khorenko <khorenko at virtuozzo.com>
> CC: Igor Sukhih <igor at virtuozzo.com>
> CC: Pavel Emelyanov <xemul at virtuozzo.com>
> ---
>  drivers/tty/pty.c   |   67 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/ve.h  |    2 +
>  kernel/ve/ve.c      |    5 +++
>  kernel/ve/vecalls.c |    6 ++--
>  4 files changed, 77 insertions(+), 3 deletions(-)
> 
> Index: linux-pcs7.git/drivers/tty/pty.c
> ===================================================================
> --- linux-pcs7.git.orig/drivers/tty/pty.c
> +++ linux-pcs7.git/drivers/tty/pty.c
> @@ -1284,8 +1284,64 @@ static int __init vtty_init(void)
>  	return 0;
>  }
>  
> +static DECLARE_RWSEM(vtty_console_sem);
> +static DEFINE_IDR(vtty_idr_console);

We already have vtty_idr, may be reuse it?

> +
> +static struct ve_struct *vtty_get_ve_by_id(envid_t veid)
> +{
> +	DECLARE_COMPLETION_ONSTACK(console_work);
> +	struct ve_struct *ve;
> +	int ret;
> +
> +	down_write(&vtty_console_sem);
> +	ve = get_ve_by_id(veid);
> +	if (ve) {
> +		up_write(&vtty_console_sem);
> +		return ve;
> +	}
> +

> +	if (idr_find(&vtty_idr_console, veid)) {
> +		up_write(&vtty_console_sem);
> +		return ERR_PTR(-EBUSY);
> +	}

This block is useless - it's handled by ENOSPC check below.

> +
> +	ret = idr_alloc(&vtty_idr_console, &console_work, veid, veid + 1, GFP_KERNEL);
> +	if (ret < 0) {
> +		if (ret == -ENOSPC)
> +			ret = -EBUSY;
> +	} else
> +		ret = 0;
> +	up_write(&vtty_console_sem);
> +
> +	if (!ret)
> +		ret = wait_for_completion_interruptible(&console_work);
> +
> +	if (!ret)
> +		ve = get_ve_by_id(veid);
> +	else
> +		ve = ERR_PTR(ret);
> +
> +	down_write(&vtty_console_sem);
> +	if (!ret)
> +		idr_remove(&vtty_idr_console, veid);
> +	up_write(&vtty_console_sem);
> +	return ve;
> +}
> +
> +void vtty_console_notify(struct ve_struct *ve)
> +{
> +	struct completion *console_work;
> +
> +	down_read(&vtty_console_sem);
> +	console_work = idr_find(&vtty_idr_console, ve->veid);
> +	if (console_work)
> +		complete(console_work);
> +	up_read(&vtty_console_sem);
> +}
> +
>  int vtty_open_master(envid_t veid, int idx)
>  {
> +	struct ve_struct *ve = NULL;
>  	struct tty_struct *tty;
>  	struct file *file;
>  	char devname[64];
> @@ -1298,6 +1354,16 @@ int vtty_open_master(envid_t veid, int i
>  	if (fd < 0)
>  		return fd;
>  
> +	ve = vtty_get_ve_by_id(veid);
> +	if (IS_ERR_OR_NULL(ve)) {
> +		if (IS_ERR(ve))
> +			ret = PTR_ERR(ve);
> +		else
> +			ret = -ENOENT;
> +		ve = NULL;
> +		goto err_put_unused_fd;
> +	}
> +

Come to think of it, is this really necessary? Can't we just allocate
vtty_map in vtty_open_master and return master tty w/o open slave? Any
write/read will put the caller to sleep anyway.

>  	snprintf(devname, sizeof(devname), "v%utty%d", veid, idx);
>  	file = anon_inode_getfile(devname, &vtty_fops, NULL, O_RDWR);
>  	if (IS_ERR(file)) {
> @@ -1364,6 +1430,7 @@ int vtty_open_master(envid_t veid, int i
>  	mutex_unlock(&tty_mutex);
>  	ret = fd;
>  out:
> +	put_ve(ve);
>  	return ret;
>  
>  err_install:
> Index: linux-pcs7.git/include/linux/ve.h
> ===================================================================
> --- linux-pcs7.git.orig/include/linux/ve.h
> +++ linux-pcs7.git/include/linux/ve.h
> @@ -215,6 +215,8 @@ void ve_stop_ns(struct pid_namespace *ns
>  void ve_exit_ns(struct pid_namespace *ns);
>  int ve_start_container(struct ve_struct *ve);
>  
> +void vtty_console_notify(struct ve_struct *ve);
> +
>  extern bool current_user_ns_initial(void);
>  struct user_namespace *ve_init_user_ns(void);
>  
> Index: linux-pcs7.git/kernel/ve/ve.c
> ===================================================================
> --- linux-pcs7.git.orig/kernel/ve/ve.c
> +++ linux-pcs7.git/kernel/ve/ve.c
> @@ -494,6 +494,11 @@ int ve_start_container(struct ve_struct
>  
>  	get_ve(ve); /* for ve_exit_ns() */
>  
> +	/*
> +	 * Console waiter are to be notified at the very
> +	 * end when everything else is ready.
> +	 */
> +	vtty_console_notify(ve);
>  	return 0;
>  
>  err_iterate:
> Index: linux-pcs7.git/kernel/ve/vecalls.c
> ===================================================================
> --- linux-pcs7.git.orig/kernel/ve/vecalls.c
> +++ linux-pcs7.git/kernel/ve/vecalls.c
> @@ -990,6 +990,9 @@ static int ve_configure(envid_t veid, un
>  	struct ve_struct *ve;
>  	int err = -ENOKEY;
>  
> +	if (key == VE_CONFIGURE_OPEN_TTY)
> +		return vtty_open_master(veid, val);
> +
>  	ve = get_ve_by_id(veid);
>  	if (!ve)
>  		return -EINVAL;
> @@ -998,9 +1001,6 @@ static int ve_configure(envid_t veid, un
>  	case VE_CONFIGURE_OS_RELEASE:
>  		err = init_ve_osrelease(ve, data);
>  		break;
> -	case VE_CONFIGURE_OPEN_TTY:
> -		err = vtty_open_master(ve->veid, val);
> -		break;
>  	}
>  
>  	put_ve(ve);