From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49594) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aIvw0-0005GF-Ba for qemu-devel@nongnu.org; Tue, 12 Jan 2016 05:12:17 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aIvvw-0006eb-S4 for qemu-devel@nongnu.org; Tue, 12 Jan 2016 05:12:16 -0500 Received: from mx1.redhat.com ([209.132.183.28]:55622) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aIvvw-0006eV-KT for qemu-devel@nongnu.org; Tue, 12 Jan 2016 05:12:12 -0500 From: Markus Armbruster References: <20151227070342.25181.45645.malonedeb@soybean.canonical.com> <87pox8t6op.fsf@blackfin.pond.sub.org> <20160111195920.GA8743@localhost.localdomain> Date: Tue, 12 Jan 2016 11:12:09 +0100 In-Reply-To: <20160111195920.GA8743@localhost.localdomain> (Keith Busch's message of "Mon, 11 Jan 2016 19:59:21 +0000") Message-ID: <87egdnnm2u.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: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Keith Busch Cc: Bug 1529449 <1529449@bugs.launchpad.net>, qemu-devel@nongnu.org, Tom Yan Keith Busch writes: > On Mon, Jan 11, 2016 at 05:35:50PM +0100, Markus Armbruster wrote: >> Tom Yan 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? > > Requiring a serial was a concious choice to push that responsibility > on the user, but I don't see a problem having the code provide default > serial string if the user does not over ride it. > > If you've multiple nvme devices in your guest, creating the same serial > could cause problems with multipathing if they're basing end device > uniqueness on the serial (some do). If we have the code provide the > serial, perhaps it would be best to make each unique. That's easy enough > to append an incrementing number to the end of the serial. I don't have a strong opinion on whether serial can remain mandatory or should become optional. If we make up serial numbers, they better be unique, of course. I do have a strong opinion on something else: the error reporting. Please convert the device to realize(), and add the necessary error_setg().