Re: [Qemu-devel] [Qemu-block] [PATCH 16/16] nvme: support multiple namespaces

From: "Daniel P. Berrangé" <berrange@redhat.com>
To: qemu-block@nongnu.org, kwolf@redhat.com, qemu-devel@nongnu.org,
	armbru@redhat.com, keith.busch@intel.com, mreitz@redhat.com,
	lersek@redhat.com
Subject: Re: [Qemu-devel] [Qemu-block] [PATCH 16/16] nvme: support multiple namespaces
Date: Fri, 5 Jul 2019 14:49:29 +0100	[thread overview]
Message-ID: <20190705134929.GM32473@redhat.com> (raw)
In-Reply-To: <20190705133616.GA4707@bogfinke>

On Fri, Jul 05, 2019 at 03:36:17PM +0200, Klaus Birkelund wrote:
> On Fri, Jul 05, 2019 at 09:23:33AM +0200, Klaus Birkelund Jensen wrote:
> > This adds support for multiple namespaces by introducing a new 'nvme-ns'
> > device model. The nvme device creates a bus named from the device name
> > ('id'). The nvme-ns devices then connect to this and registers
> > themselves with the nvme device.
> > 
> > This changes how an nvme device is created. Example with two namespaces:
> > 
> >   -drive file=nvme0n1.img,if=none,id=disk1
> >   -drive file=nvme0n2.img,if=none,id=disk2
> >   -device nvme,serial=deadbeef,id=nvme0
> >   -device nvme-ns,drive=disk1,bus=nvme0,nsid=1
> >   -device nvme-ns,drive=disk2,bus=nvme0,nsid=2
> > 
> > A maximum of 256 namespaces can be configured.
> > 
>  
> Well that was embarrasing.
> 
> This patch breaks nvme-test.c. Which I obviously did not run.
> 
> In my defense, the test doesn't do much currently, but I'll of course
> fix the test for v2.

That highlights a more serious problem.  This series changes the syntx
for configuring the nvme device in a way that is not backwards compatible.
So anyone who is using QEMU with NVME will be broken when they upgrade
to the next QEMU release.

I understand why you wanted to restructure things to have a separate
nvme-ns device, but there needs to be some backcompat support in there
for the existing syntax to avoid breaking current users IMHO.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|