From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:44229)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1UIEQZ-00059h-Da
	for qemu-devel@nongnu.org; Wed, 20 Mar 2013 04:31:21 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1UIEQY-0003jf-6c
	for qemu-devel@nongnu.org; Wed, 20 Mar 2013 04:31:19 -0400
Received: from mail-wg0-f45.google.com ([74.125.82.45]:61315)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1UIEQY-0003jE-1O
	for qemu-devel@nongnu.org; Wed, 20 Mar 2013 04:31:18 -0400
Received: by mail-wg0-f45.google.com with SMTP id dq12so1110832wgb.24
	for <qemu-devel@nongnu.org>; Wed, 20 Mar 2013 01:31:17 -0700 (PDT)
Date: Wed, 20 Mar 2013 09:31:14 +0100
From: Stefan Hajnoczi <stefanha@gmail.com>
Message-ID: <20130320083114.GE5953@stefanha-thinkpad.muc.redhat.com>
References: <1363722113-9276-1-git-send-email-keith.busch@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1363722113-9276-1-git-send-email-keith.busch@intel.com>
Subject: Re: [Qemu-devel] [PATCH v3] NVMe: Initial commit
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Keith Busch <keith.busch@intel.com>
Cc: qemu-devel@nongnu.org, Keith Busch <keith.busch@gmail.com>

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