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

[Cyrill Gorcunov]

On 06/10/2016 06:05 PM, Vladimir Davydov wrote:
...
>>
>> 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?

vtty_idr relies on tty_mutex, if we add one more locks here
I fear it will be more complex than it should. Currently
this console stuff serves one purpose only -- wait for
container to up if console requested early.

> 
>> +
>> +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.

Not really, find is a bit faster than alloc, but sure, will drop.

> 
>> +
>> +	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.

Hmm. You know currently we keep slaves in the map as reference point
(because master peer may be never opened during whole container lifetime).
Letme think if we can do that.

Thanks for comments!

> 
>>  	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);


-- 
    Cyrill