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

[Stefan Hajnoczi]

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.

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.

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.

Stefan