Re: [PATCH v3] hw/block/nvme: fix lbaf formats initialization

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 4/16/21 1:59 PM, Gollu Appalanaidu wrote:
> Currently LBAF formats are being intialized based on metadata
> size if and only if nvme-ns "ms" parameter is non-zero value.
> Since FormatNVM command being supported device parameter "ms"
> may not be the criteria to initialize the supported LBAFs.
> 
> Signed-off-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
> ---
> -v3: Remove "mset" constraint  check if ms < 8, "mset" can be
>  set even when ms < 8 and non-zero.
> 
> -v2: Addressing review comments (Klaus)
>  Change the current "pi" and "ms" constraint check such that it
>  will throw the error if ms < 8 and if namespace protection info,
>  location and metadata settings are set.
>  Splitting this from compare fix patch series.
> 
>  hw/block/nvme-ns.c | 58 ++++++++++++++++++++--------------------------
>  1 file changed, 25 insertions(+), 33 deletions(-)

> +    NvmeLBAF lbaf[16] = {

Unrelated to your change, but better to use a read-only array:

       static const NvmeLBAF lbaf[16] = {

> +        [0] = { .ds =  9           },
> +        [1] = { .ds =  9, .ms =  8 },
> +        [2] = { .ds =  9, .ms = 16 },
> +        [3] = { .ds =  9, .ms = 64 },
> +        [4] = { .ds = 12           },
> +        [5] = { .ds = 12, .ms =  8 },
> +        [6] = { .ds = 12, .ms = 16 },
> +        [7] = { .ds = 12, .ms = 64 },
> +    };
> +
> +    memcpy(&id_ns->lbaf, &lbaf, sizeof(lbaf));
> +    id_ns->nlbaf = 7;