Re: [Qemu-devel] [PATCH v2 RFC] block/vxhs: Initial commit to add Veritas HyperScale VxHS block device support

[Kevin Wolf]

Am 08.08.2016 um 04:01 hat Ashish Mittal geschrieben:
> This patch adds support for a new block device type called "vxhs".
> Source code for the library that this code loads can be downloaded from:
> https://github.com/MittalAshish/libqnio.git
> 
> Version 2 patch submission fixes the following issues:
> (1) Removed code to dlopen library. We now check if libqnio is installed 
> during
>     configure, and directly link with it.
> (2) Changed file headers to mention GPLv2-or-later license.
> (3) Removed unnecessary type casts and inlines.
> (4) Removed custom tokenize function and modified code to use g_strsplit.
> (5) Replaced malloc/free with g_new/g_free and removed code that checks for
>     memory allocation failure conditions.
> (6) Removed some block ops implementations that were place-holders only.
> (7) Removed all custom debug messages. Added new messages in 
> block/trace-events
> (8) Other miscellaneous corrections.
> 
> TODO: QAPI changes and (fixes to) review comments from Stefan.
> Signed-off-by: Ashish Mittal <address@hidden>

I just had a very high level look at the driver, and maybe it's better
to discuss those general design points first before going to the details
of the actual code.


So perhaps the most important thing is that I'm wondering whether we
really got the right split between qemu and libqnio yet. Generally a
library should be agnostic regarding its users, but libqnio has quite a
few functions that contain 'qemu' in their name. To me, this is a sign
that they might fit better in the qemu block driver.

The effect of keeping these function in libqnio is that, for example,
you're reimplementing qemu's IO vector functions which you could
directly use from the block driver, or that you do manual request
splitting that the qemu block layer could be doing for you if you told
it what the maximum size is.


Another point is that these days it's preferrable to implement
.bdrv_co_preadv/pwritev rather than .bdrv_aio_readv/writev. Your native
interfaces seem to be already byte based, so this should be a good
match.

Kevin