Re: [Qemu-devel] [PATCH] hw/block/nvme: add optional parameter num_namespaces for nvme device

[Fam Zheng <famz@redhat.com>]

On Thu, 07/19 12:33, Weiping Zhang wrote:
> Add an optional paramter num_namespaces for device, and set it
> to 1 by default.
> 
> Signed-off-by: Weiping Zhang <zhangweiping@didichuxing.com>
> ---
>  hw/block/nvme.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 156ecf3c41..b53be4b5c0 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -19,7 +19,7 @@
>   *      -drive file=<file>,if=none,id=<drive_id>
>   *      -device nvme,drive=<drive_id>,serial=<serial>,id=<id[optional]>, \
>   *              cmb_size_mb=<cmb_size_mb[optional]>, \
> - *              num_queues=<N[optional]>
> + *              num_queues=<N[optional]>,num_namespaces=<N[optional]>
>   *
>   * Note cmb_size_mb denotes size of CMB in MB. CMB is assumed to be at
>   * offset 0 in BAR2 and supports only WDS, RDS and SQS for now.
> @@ -1232,7 +1232,6 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
>      pci_config_set_class(pci_dev->config, PCI_CLASS_STORAGE_EXPRESS);
>      pcie_endpoint_cap_init(&n->parent_obj, 0x80);
>  
> -    n->num_namespaces = 1;
>      n->reg_size = pow2ceil(0x1004 + 2 * (n->num_queues + 1) * 4);
>      n->ns_size = bs_size / (uint64_t)n->num_namespaces;
>  
> @@ -1342,6 +1341,7 @@ static Property nvme_props[] = {
>      DEFINE_PROP_STRING("serial", NvmeCtrl, serial),
>      DEFINE_PROP_UINT32("cmb_size_mb", NvmeCtrl, cmb_size_mb, 0),
>      DEFINE_PROP_UINT32("num_queues", NvmeCtrl, num_queues, 64),
> +    DEFINE_PROP_UINT32("num_namespaces", NvmeCtrl, num_namespaces, 1),

You need to verify the user provided value. At least 0xFFFFFFFF is the broadcast
value and shouldn't be used, and 0 doesn't make much sense too, I guess.

>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> -- 
> 2.14.1
> 
> 

I think the data_offset calculations in nvme_rw need some changes too?

Fam