On Tue, Jul 02, 2019 at 10:39:36AM -0700, Matt Fitzpatrick wrote: > Adding namespace management support to the nvme device. Namespace creation > requires contiguous block space for a simple method of allocation. > > I wrote this a few years ago based on Keith's fork and nvmeqemu fork and > have recently re-synced with the latest trunk. Some data structures in > nvme.h are a bit more filled out that strictly necessary as this is also the > base for sr-iov and IOD patched to be submitted later. >
Hi Matt, Nice! I'm always happy when new features for the nvme device is posted! I'll be happy to review it, but I won't start going through it in details because I believe the approach to supporting multiple namespaces is flawed. We had a recent discussion on this and I also got some unrelated patches rejected due to implementing it similarly by carving up the image. I have posted a long series that includes a patch for multiple namespaces. It is implemented by introducing a fresh `nvme-ns` device model that represents a namespace and attaches to a bus created by the parent `nvme` controller device. The core issue is that a qemu image /should/ be attachable to other devices (say ide) and not strictly tied to the one device model. Thus, we cannot just shove a bunch of namespaces into a single image. But, in light of your patch, I'm not convinced that my implementation is the correct solution. Maybe the abstraction should not be an `nvme-ns` device, but a `nvme-nvm` device that when attached changes TNVMCAP and UNVMCAP? Maybe you have some input for this? Or we could have both and dynamically create the nvme-ns devices on top of nvme-nvm devices. I think it would still require a 1-to-1 mapping, but it could be a way to support the namespace management capability. Cheers, Klaus
