Re: [PATCH v3 1/2] dm: prevent table type changes after initial table load

[Kiyoshi Ueda <k-ueda@ct.jp.nec.com>]

Hi Mike,

Some small comments below.

For others,
Acked-by: Kiyoshi Ueda <k-ueda@ct.jp.nec.com>

On 05/25/2010 11:43 PM +0900, Mike Snitzer wrote:
> @@ -1176,6 +1176,21 @@ static int table_load(struct dm_ioctl *p
>  		goto out;
>  	}
>  
> +	/* Protect md->type against concurrent table loads. */
> +	dm_lock_md_type(md);
> +	if (dm_get_md_type(md) == DM_TYPE_NONE) {
> +		/* initial table load, set md's type based on table's type */
> +		dm_set_md_type(md, t);

                dm_set_md_type(md, dm_table_get_type(t));

Although I don't impose this interface, I feel it is straightforward
for this function.  Then, dm_set_md_type() just set the type in md->type;

	void dm_set_md_type(struct mapped_device *md, unsigned type)
	{
		md->type = type;
	}

If you take the current interface, I feel it's not normal coding style;

> +void dm_set_md_type(struct mapped_device *md, struct dm_table* t)

Should be "dm_table *t" instead of "dm_table* t".


> @@ -66,6 +66,11 @@ int dm_table_alloc_md_mempools(struct dm
>  void dm_table_free_md_mempools(struct dm_table *t);
>  struct dm_md_mempools *dm_table_get_md_mempools(struct dm_table *t);
>  
> +void dm_lock_md_type(struct mapped_device *md);
> +void dm_unlock_md_type(struct mapped_device *md);
> +void dm_set_md_type(struct mapped_device *md, struct dm_table* t);

The same coding style here.

Thanks,
Kiyoshi Ueda