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

[Cyrill Gorcunov]

On Thu, Jun 09, 2016 at 01:34:31PM +0300, Vladimir Davydov wrote:
> On Mon, Jun 06, 2016 at 07:26:57PM +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
> > 
> > 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>
> > ---
> >  include/linux/ve.h  |    2 ++
> >  kernel/ve/ve.c      |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  kernel/ve/vecalls.c |   23 +++++++++++++++++++++--
> >  3 files changed, 71 insertions(+), 2 deletions(-)
> > 
> > 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);
> >  
> > +int ve_console_wait(envid_t veid);
> > +
> >  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
> > @@ -260,6 +260,49 @@ struct user_namespace *ve_init_user_ns(v
> >  }
> >  EXPORT_SYMBOL(ve_init_user_ns);
> >  
> > +static DEFINE_IDR(ve_idr_console);
> > +static DECLARE_RWSEM(ve_console_sem);
> > +
> > +int ve_console_wait(envid_t veid)
> > +{
> > +	DECLARE_COMPLETION_ONSTACK(console_work);
> > +	int ret;
> > +
> > +	down_write(&ve_console_sem);
> > +	if (idr_find(&ve_idr_console, veid)) {
> > +		up_write(&ve_console_sem);
> > +		return -EEXIST;
> > +	}
> > +
> > +	ret = idr_alloc(&ve_idr_console, &console_work, veid, veid + 1, GFP_KERNEL);
> > +	if (ret < 0) {
> > +		if (ret == -ENOSPC)
> > +			ret = -EEXIST;
> > +	} else
> > +		ret = 0;
> > +	downgrade_write(&ve_console_sem);
> > +
> > +	if (!ret) {
> > +		ret = wait_for_completion_interruptible(&console_work);
> > +		idr_remove(&ve_idr_console, veid);
> > +	}
> > +
> > +	up_read(&ve_console_sem);
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL(ve_console_wait);
> > +
> > +static void ve_console_notify(struct ve_struct *ve)
> > +{
> > +	struct completion *console_work;
> > +
> > +	down_read(&ve_console_sem);
> > +	console_work = idr_find(&ve_idr_console, ve->veid);
> > +	if (console_work)
> > +		complete(console_work);
> > +	up_read(&ve_console_sem);
> > +}
> > +
> >  int nr_threads_ve(struct ve_struct *ve)
> >  {
> >  	return cgroup_task_count(ve->css.cgroup);
> > @@ -494,6 +537,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.
> > +	 */
> > +	ve_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
> > @@ -991,8 +991,27 @@ static int ve_configure(envid_t veid, un
> >  	int err = -ENOKEY;
> >  
> >  	ve = get_ve_by_id(veid);
> > -	if (!ve)
> > -		return -EINVAL;
> > +	if (!ve) {
> > +
> > +		if (key != VE_CONFIGURE_OPEN_TTY)
> > +			return -EINVAL;
> > +		/*
> > +		 * Offline console management:
> > +		 * wait until ve is up and proceed.
> > +		 */
> 
> What if a VE is created right here, before we call ve_console_wait()?
> Looks like the caller will hang forever...

Yeah, could be a race. Thanks!

> > +		err = ve_console_wait(veid);
> > +		if (err)
> > +			return err;
> > +
> > +		/*
> > +		 * A container should not exit immediately once
> > +		 * started but if it does, for any reason, simply
> > +		 * exit out gracefully.
> > +		 */
> > +		ve = get_ve_by_id(veid);
> > +		if (!ve)
> > +			return -ENOENT;
> > +	}
> 
> Can't we fold this into vtty_open_master()? The latter doesn't need ve
> object, it only needs veid, which is known here.

Will do.