[Devel] [PATCH rh7 1/2] ve: Introduce ve::devmnt list

Maxim Patlasov mpatlasov at openvz.org
Wed May 6 15:05:59 PDT 2015


Kirill, see please a couple of inline comments below...

On 04/29/2015 01:29 AM, Kirill Tkhai wrote:
> 1)Porting patch "ve: mount option list" by Maxim Patlasov:
>
> The patch adds new fields to ve_struct: devmnt_list and devmnt_mutex.
>
> devmnt_list is the head of list of ve_devmnt structs. Each host block device
> visible from CT can have no more than one struct ve_devmnt linked in
> ve->devmnt_list. If ve_devmnt is present, it can be found by 'dev' field.
> Each ve_devmnt struct may bear two strings: hidden and allowed options.
>
> hidden_options will be automatically added to CT-user-supplied mount options
> after checking allowed_options. Only options listed in allowed_options are
> allowed.
>
> devmnt_mutex is to protect operations on the list of ve_devmnt structs.
>
> 2)Porting patch "vecalls: VE_CONFIGURE_MOUNT_OPTIONS" by Maxim Patlasov.
>
> Reworking the interface using cgroups. Each CT now has a file:
>
> [ve_cgroup_mnt_pnt]/[CTID]/ve.mount_opts
>
> for configuring permittions for a block device. Below is permittions line
> example:
>
> "0 xxx;1 balloon_ino=12,pfcache_csum,pfcache=/vz/pfcache;2 barrier=1"
>
> Here, xxx is st_rdev of device, '1' starts comma-separated list of
> hidden options, and '2' is allowed ones.
>
> Signed-off-by: Kirill Tkhai <ktkhai at odin.com>
> ---
>   include/linux/ve.h |   11 ++++
>   kernel/ve/ve.c     |  148 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 159 insertions(+)
>
> diff --git a/include/linux/ve.h b/include/linux/ve.h
> index 61f31fd..98dd244 100644
> --- a/include/linux/ve.h
> +++ b/include/linux/ve.h
> @@ -123,6 +123,9 @@ struct ve_struct {
>   	struct net		*ve_netns;
>   	struct mutex		sync_mutex;
>   
> +	struct list_head	devmnt_list;
> +	struct mutex		devmnt_mutex;
> +
>   	struct kmapset_key	ve_sysfs_perms;
>   	struct list_head	ve_cgroup_head;
>   #if IS_ENABLED(CONFIG_DEVTMPFS)
> @@ -130,6 +133,14 @@ struct ve_struct {
>   #endif
>   };
>   
> +struct ve_devmnt {
> +	struct list_head	link;
> +
> +	dev_t                   dev;
> +	char			*allowed_options;
> +	char			*hidden_options; /* balloon_ino, etc. */
> +};
> +
>   #define VE_MEMINFO_DEFAULT      1       /* default behaviour */
>   #define VE_MEMINFO_SYSTEM       0       /* disable meminfo virtualization */
>   
> diff --git a/kernel/ve/ve.c b/kernel/ve/ve.c
> index ee2435c..e963b56 100644
> --- a/kernel/ve/ve.c
> +++ b/kernel/ve/ve.c
> @@ -749,6 +749,8 @@ static struct cgroup_subsys_state *ve_create(struct cgroup *cg)
>   	INIT_LIST_HEAD(&ve->devices);
>   	INIT_LIST_HEAD(&ve->ve_cgroup_head);
>   	INIT_LIST_HEAD(&ve->ve_list);
> +	INIT_LIST_HEAD(&ve->devmnt_list);
> +	mutex_init(&ve->devmnt_mutex);
>   	kmapset_init_key(&ve->ve_sysfs_perms);
>   
>   	return &ve->css;
> @@ -782,11 +784,33 @@ static void ve_offline(struct cgroup *cg)
>   	}
>   }
>   
> +static void ve_devmnt_free(struct ve_devmnt *devmnt)
> +{
> +	if (!devmnt)
> +		return;
> +
> +	kfree(devmnt->allowed_options);
> +	kfree(devmnt->hidden_options);
> +	kfree(devmnt);
> +}
> +
> +static void free_ve_devmnts(struct ve_struct *ve)
> +{
> +	while (!list_empty(&ve->devmnt_list)) {
> +		struct ve_devmnt *devmnt;
> +
> +		devmnt = list_first_entry(&ve->devmnt_list, struct ve_devmnt, link);
> +		list_del(&devmnt->link);
> +		ve_devmnt_free(devmnt);
> +	}
> +}
> +
>   static void ve_destroy(struct cgroup *cg)
>   {
>   	struct ve_struct *ve = cgroup_ve(cg);
>   
>   	kmapset_unlink(&ve->ve_sysfs_perms, &ve_sysfs_perms);
> +	free_ve_devmnts(ve);
>   
>   	ve_log_destroy(ve);
>   	kfree(ve->binfmt_misc);
> @@ -934,6 +958,125 @@ static int ve_legacy_veid_read(struct cgroup *cg, struct cftype *cft,
>   	return seq_printf(m, "%u\n", cgroup_ve(cg)->veid);
>   }
>   
> +/*
> + * 'data' for VE_CONFIGURE_MOUNT_OPTIONS is a zero-terminated string
> + * consisting of substrings separated by MNTOPT_DELIM.
> + */
> +#define MNTOPT_DELIM ';'
> +
> +/*
> + * Each substring has the form of "<type> <comma-separated-list-of-options>"
> + * where types are:
> + */
> +enum {
> +	MNTOPT_DEVICE = 0,
> +	MNTOPT_HIDDEN = 1,
> +	MNTOPT_ALLOWED = 2,
> +};
> +
> +/*
> + * 'ptr' points to the first character of buffer to parse
> + * 'endp' points to the last character of buffer to parse
> + */
> +static int ve_parse_mount_options(const char *ptr, const char *endp,
> +				  struct ve_devmnt *devmnt)
> +{
> +	while (*ptr) {
> +		const char *delim = strchr(ptr, MNTOPT_DELIM) ? : endp;
> +		char *space = strchr(ptr, ' ');
> +		int type;
> +		char *options, c;
> +		int options_size = delim - space;
> +		char **opts_pp = NULL; /* where to store 'options' */
> +
> +		if (delim == ptr || !space || options_size <= 1)
> +			return -EINVAL;
> +
> +		if (sscanf(ptr, "%d%c", &type, &c) != 2 || c != ' ')
> +			return -EINVAL;
> +
> +		if (type == MNTOPT_DEVICE) {
> +			unsigned int dev;
> +			if (devmnt->dev)
> +				return -EINVAL; /* Already set */
> +			if (sscanf(space + 1, "%u%c", &dev, &c) != 2 ||
> +			    c != ';')
> +				return -EINVAL;
> +			devmnt->dev = new_decode_dev(dev);

I think major:minor would be better (human readable) string 
representation of devmnt->dev than unsigned integer st_rdev.

> +			goto next;
> +		}
> +
> +	        options = kmalloc(options_size, GFP_KERNEL);
> +		if (!options)
> +			return -ENOMEM;
> +
> +		strncpy(options, space + 1, options_size - 1);
> +		options[options_size - 1] = 0;
> +
> +		switch (type) {
> +		case MNTOPT_ALLOWED:
> +			opts_pp = &devmnt->allowed_options;
> +			break;
> +		case MNTOPT_HIDDEN:
> +			opts_pp = &devmnt->hidden_options;
> +			break;
> +		};
> +
> +		/* wrong type or already set */
> +		if (!opts_pp || *opts_pp) {
> +			kfree(options);
> +			return -EINVAL;
> +		}
> +
> +		*opts_pp = options;
> +next:
> +		if (!*delim)
> +			break;
> +
> +		ptr = delim + 1;
> +	}
> +
> +	if (!devmnt->dev)
> +		return -EINVAL;
> +	return 0;
> +}
> +
> +static int ve_mount_opts_write(struct cgroup *cg, struct cftype *cft,
> +			       const char *buffer)
> +{
> +	struct ve_struct *ve = cgroup_ve(cg);
> +	struct ve_devmnt *devmnt, *old;
> +	int size, err;
> +
> +	size = strlen(buffer);
> +	if (size <= 1)
> +		return -EINVAL;
> +
> +	devmnt = kzalloc(sizeof(*devmnt), GFP_KERNEL);
> +	if (!devmnt)
> +		return -ENOMEM;
> +
> +	err = ve_parse_mount_options(buffer, buffer + size, devmnt);
> +	if (err) {
> +		ve_devmnt_free(devmnt);
> +		return err;
> +	}
> +
> +	mutex_lock(&ve->devmnt_mutex);
> +	list_for_each_entry(old, &ve->devmnt_list, link) {
> +		/* Delete old devmnt */
> +		if (old->dev == devmnt->dev) {
> +			list_del(&old->link);
> +			ve_devmnt_free(old);
> +			break;
> +		}
> +	}
> +	list_add(&devmnt->link, &ve->devmnt_list);
> +	mutex_unlock(&ve->devmnt_mutex);
> +
> +	return 0;
> +}
> +
>   static struct cftype ve_cftypes[] = {
>   	{
>   		.name = "state",
> @@ -946,6 +1089,11 @@ static struct cftype ve_cftypes[] = {
>   		.flags = CFTYPE_NOT_ON_ROOT,
>   		.read_seq_string = ve_legacy_veid_read,
>   	},
> +	{
> +		.name = "mount_opts",
> +		.flags = CFTYPE_NOT_ON_ROOT,
> +		.write_string = ve_mount_opts_write,
> +	},

Why no read method?

Thanks,
Maxim



More information about the Devel mailing list