Re: [Qemu-devel] [PATCH v3] NVMe: Initial commit

[Markus Armbruster]

Stefan Hajnoczi <address@hidden> writes:

> On Tue, Mar 19, 2013 at 01:41:53PM -0600, Keith Busch wrote:
>> +static int nvme_init(PCIDevice *pci_dev)
>> +{
>> +    NvmeCtrl *n = NVME(pci_dev);
>> +    NvmeIdCtrl *id = &n->id_ctrl;
>> +
>> +    int i;
>> +    int64_t bs_size;
>> +    char serial[sizeof(id->sn)];
>> +    uint8_t *pci_conf;
>> +
>> +    if (!(n->conf.bs)) {
>> +        return -1;
>> +    }
>> +
>> +    bs_size =  bdrv_getlength(n->conf.bs);
>> +    if (bs_size <= 0) {
>> +        return -1;
>> +    }
>> +
>> +    n->instance = instance++;
>> +    blkconf_serial(&n->conf, &n->serial);
>> +    if (!n->serial) {
>> + snprintf((char *)serial, sizeof(serial), "NVMeQx10%02x",
>> n->instance);
>> +    }
>
> A patch that adds serial autogeneration to virtio-blk was recently
> posted.  Here were my concerns:
>
> Autogenerated IDs have been proposed (for other devices?) before and I
> think we should avoid them.

ACK

> The autogenerated serial is not persistent.  It can change between QEMU
> invocations - due to internal changes in QEMU or because the management
> tool reordered -device options.
>
> Users will be confused and their guests may stop working if they depend
> on an ID like this.

Only IDE devices auto-generate their serial number.  Can't fix that
mistake, because it's ABI[*].  All other devices don't.  Let's keep it
that way.

> The solution is to do persistent naming either by really passing -drive
> serial= or with udev inside the guest using the bus address (PCI devfn)
> like the new persistent network interface naming for Linux.

ACK


[*] I could have avoided the mistake for ide-hd and ide-cd, but I didn't
see the problem then.  Perhaps we could've killed it for AHCI.