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

[Philippe Mathieu-Daudé]

On 7/3/20 12:00 PM, Klaus Jensen wrote:
> 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.

No objection, it was a simple suggestion to consider for later ;)