Keith Busch <keith.bu...@intel.com> writes: > On Mon, Jan 11, 2016 at 05:35:50PM +0100, Markus Armbruster wrote: >> Tom Yan <tom.t...@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? > > 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().