Re: [PATCH v2 14/18] hw/block/nvme: support identify namespace descriptor list

[Klaus Jensen]

On Jul  3 10:27, Philippe Mathieu-Daudé wrote:
> On 7/3/20 8:34 AM, Klaus Jensen wrote:
> > From: Klaus Jensen <k.jensen@samsung.com>
> > 
> > Since we are not providing the NGUID or EUI64 fields, we must support
> > the Namespace UUID. We do not have any way of storing a persistent
> > unique identifier, so conjure up a UUID that is just the namespace id.
> > 
> > Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> > Reviewed-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
> > ---
> >  hw/block/nvme.c       | 41 +++++++++++++++++++++++++++++++++++++++++
> >  hw/block/trace-events |  1 +
> >  2 files changed, 42 insertions(+)
> > 
> > diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> > index 8230e0e3826b..65c2fa3ac1f4 100644
> > --- a/hw/block/nvme.c
> > +++ b/hw/block/nvme.c
> > @@ -971,6 +971,45 @@ static uint16_t nvme_identify_nslist(NvmeCtrl *n, 
> > NvmeIdentify *c)
> >      return ret;
> >  }
> >  
> > +static uint16_t nvme_identify_ns_descr_list(NvmeCtrl *n, NvmeIdentify *c)
> > +{
> > +    uint32_t nsid = le32_to_cpu(c->nsid);
> > +    uint64_t prp1 = le64_to_cpu(c->prp1);
> > +    uint64_t prp2 = le64_to_cpu(c->prp2);
> > +
> > +    uint8_t list[NVME_IDENTIFY_DATA_SIZE];
> > +
> > +    struct data {
> > +        struct {
> > +            NvmeIdNsDescr hdr;
> > +            uint8_t v[16];
> 
> You might consider to use QemuUUID from "qemu/uuid.h". The benefits
> are you can use qemu_uuid_parse() qemu_uuid_unparse*() for tracing,
> and DEFINE_PROP_UUID() in case you want to set a particular UUID
> from command line, it case it is important to the guest.
> 

Yes, definitely. Niklas also does this in his patch for namespace types
support. And I think that it's very important that it can be made
persistent, which would require a device property.

Thus, if it is OK with the rest of you, I would like to defer this to
when we merge the multiple namespaces patch and add "uuid" as a nvme-ns
device property there. Then, we do not have to add the uuid property on
the nvme device now and then have to keep it around when the namespace
related properties moves to the nvme-ns device.