On Sep 28 02:33, Dmitry Fomichev wrote: > > -----Original Message----- > > From: Klaus Jensen <i...@irrelevant.dk> > > > > If it really needs to be memory mapped, then I think a hostmem-based > > approach similar to what Andrzej did for PMR is needed (I think that > > will get rid of the CONFIG_POSIX ifdef at least, but still leave it > > slightly tricky to get it to work on all platforms AFAIK). > > Ok, it looks that using the HostMemoryBackendFile backend will be > more appropriate. This will remove the need for conditional compile. > > The mmap() portability is pretty decent across software platforms. > Any poor Windows user who is forced to emulate ZNS on mingw will be > able to do so, just without having zone state persistency. Considering > how specialized this stuff is in first place, I estimate the number of users > affected by this "limitation" to be exactly zero. >
QEMU is a cross platform project - we should strive for portability. Alienating developers that use a Windows platform and calling them out as "poor" is not exactly good for the zoned ecosystem. > > But really, > > since we do not require memory semantics for this, then I think the > > abstraction is fundamentally wrong. > > > > Seriously, what is wrong with using mmap :) ? It is used successfully for > similar applications, for example - > https://github.com/open-iscsi/tcmu-runner/blob/master/file_zbc.c > There is nothing fundamentally wrong with mmap. I just think it is the wrong abstraction here (and it limits portability for no good reason). For PMR there is a good reason - it requires memory semantics. > > I am, of course, blowing my own horn, since my implementation uses a > > portable blockdev for this. > > > > You are making it sound like the entire WDC series relies on this approach. > Actually, the persistency is introduced in the second to last patch in the > series and it only adds a couple of lines of code in the i/o path to mark > zones dirty. This is possible because of using mmap() and I find the way > it is done to be quite elegant, not ugly :) > No, I understand that your implementation works fine without persistance, but persistance is key. That is why my series adds it in the first patch. Without persistence it is just a toy. And the QEMU device is not just an "NVMe-version" of null_blk. And I don't think I ever called the use of mmap ugly. I called out the physical memory API shenanigans as a hack. > > Another issue is the complete lack of endian conversions. Does it > > matter? It depends. Will anyone ever use this on a big endian host and > > move the meta data backing file to a little endian host? Probably not. > > So does it really matter? Probably not, but it is cutting corners. > > After I had replied this, I considered a follow-up, because there are probably QEMU developers that would call me out on this. This definitely DOES matter to QEMU. > > Great point on endianness! Naturally, all file backed values are stored in > their native endianness. This way, there is no extra overhead on big endian > hardware architectures. Portability concerns can be easily addressed by > storing metadata endianness as a byte flag in its header. Then, during > initialization, the metadata validation code can detect the possible > discrepancy in endianness and automatically convert the metadata to the > endianness of the host. This part is out of scope of this series, but I would > be able to contribute such a solution as an enhancement in the future. > It is not out of scope. I don't see why we should merge something that is arguably buggy. Bottomline is that I just don't see why we should accept an implementation that a) excludes some platforms (Windows) from using persistence; and b) contains endianness conversion issues when there is a portable implementation posted that at least tries to convert endianness as needed.
signature.asc
Description: PGP signature
