From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:39557)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1aIfRm-0006eX-NJ
	for qemu-devel@nongnu.org; Mon, 11 Jan 2016 11:36:03 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1aIfRh-0006Yf-D4
	for qemu-devel@nongnu.org; Mon, 11 Jan 2016 11:35:58 -0500
Received: from mx1.redhat.com ([209.132.183.28]:46660)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1aIfRh-0006YW-7l
	for qemu-devel@nongnu.org; Mon, 11 Jan 2016 11:35:53 -0500
From: Markus Armbruster <armbru@redhat.com>
References: <20151227070342.25181.45645.malonedeb@soybean.canonical.com>
Date: Mon, 11 Jan 2016 17:35:50 +0100
In-Reply-To: <20151227070342.25181.45645.malonedeb@soybean.canonical.com> (Tom
	Yan's message of "Sun, 27 Dec 2015 07:03:42 -0000")
Message-ID: <87pox8t6op.fsf@blackfin.pond.sub.org>
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [Qemu-devel] [Bug 1529449] [NEW] serial is required for -device
	nvme
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: Tom Yan <tom.ty89@gmail.com>
Cc: Bug 1529449 <1529449@bugs.launchpad.net>, Keith Busch <keith.busch@intel.com>, qemu-devel@nongnu.org

Tom Yan <tom.ty89@gmail.com> writes:

> Public bug reported:
>
> I am not exactly sure if this is a bug, but I don't see why the option
> "serial" should be required for -device nvme like the option "drive".
> Truth is it seem to accept random string as its value anyway, if that's
> the case, couldn't qemu just generate one for it when it's not
> specified?

You should've included a reproducer.  Here are mine:

1. Bad error reporting on missing drive:

     $ upstream-qemu -nodefaults -device nvme
     upstream-qemu: -device nvme: Device initialization failed

   Expected: error reported like for other devices, e.g.

     $ upstream-qemu -nodefaults -device virtio-blk
     upstream-qemu: -device virtio-blk: drive property not set

2. Bad error reporting on empty drive:

     $ upstream-qemu -nodefaults -drive if=none,id=foo -device nvme,drive=foo
     upstream-qemu: -device nvme,drive=foo: Device initialization failed

   Expected: error is reported like for other devices, e.g.

     $ upstream-qemu -nodefaults -drive if=none,id=foo -device virtio-blk,drive=foo
     upstream-qemu: -device virtio-blk,drive=foo: Device needs media, but drive is empty

3. Bad handling of missing serial:

      $ upstream-qemu -nodefaults -drive if=none,id=foo,file=tmp.qcow2 -device nvme,drive=foo
      upstream-qemu: -device nvme,drive=foo: Device initialization failed

   Expected: either default the serial number, like some other devices
   do, or a decent error message.

I recommend to convert the device to realize(), and add the missing
error_setg().  Keith?