Re: [PATCH v4 3/3] cxlflash: Virtual LUN support

[Manoj Kumar <manoj@linux.vnet.ibm.com>]

Mikey:

Comments inline below.

- Manoj Kumar

On 8/11/2015 10:24 PM, Michael Neuling wrote:

>> The ioctls have a standard header structure, with version etc. that are
>> sanity checked before we get here. The other fields are sanity checked
>> where they are used, i.e. in get_context().
>
> That was my point.  Let's do it upfront so it's clear they've been
> checked.

Thanks for clarifying. We are going to sanity check the common header
portions up front. The others that are specific to the ioctl, will be
checked prior to use in each ioctl handler.


>>> Virt has not been sanity checked.  So now resize can contain bad data.
>>>
>>>
>>>> +	resize.rsrc_handle = rsrc_handle;
>>
>> Same as above. As mentioned earlier, the size is immaterial. The rest of
>> the parameters are set here (rsrc_handle).
>
> Humm.
>

Same as above.

>>>> +#define LXT_LUNIDX_SHIFT  8	/* LXT entry, shift for LUN index */
>>>> +#define LXT_PERM_SHIFT    4	/* LXT entry, shift for permission bits */
>>>
>>> What is LXT?
>>
>> LXT = lun translation table. There is one LXT entry per set of
>> contiguous blocks for a virtual LUN (known both to the host and to the
>> AFU). Will clarify this with inline comments.
>
> Please document this acronym before using it.

Good point. Will add a comment clarifying what an LXT is.