On Sun, Aug 07, 2016 at 07:01:01PM -0700, Ashish Mittal wrote: > This patch adds support for a new block device type called "vxhs".
I'd suggest this commit message should include an example of the syntax for using this on the CLI > 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. FYI, if you put notes about what changed in between postings below the '---' line, then with your patch is applied, git is intelligent enough to drop these notes. > > TODO: QAPI changes and (fixes to) review comments from Stefan. > Signed-off-by: Ashish Mittal <ashish.mit...@veritas.com> > --- eg, put your 'Changed in v2' stuff just here. > block/Makefile.objs | 2 + > block/trace-events | 40 ++ > block/vxhs.c | 1199 > +++++++++++++++++++++++++++++++++++++++++++++++++++ > block/vxhs.h | 294 +++++++++++++ > configure | 50 +++ > 5 files changed, 1585 insertions(+) > create mode 100644 block/vxhs.c > create mode 100644 block/vxhs.h > > diff --git a/block/vxhs.c b/block/vxhs.c > new file mode 100644 > index 0000000..9a82757 > --- /dev/null > +++ b/block/vxhs.c > @@ -0,0 +1,1199 @@ > +/* > + * QEMU Block driver for Veritas HyperScale (VxHS) > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + * > + */ > + > +#include "vxhs.h" > +#include <qnio/qnio_api.h> > +#include "trace.h" > + > +/* qnio client ioapi_ctx */ > +static void __attribute__((unused)) *global_qnio_ctx; > + > +/* insure init once */ > +static pthread_mutex_t __attribute__((unused)) of_global_ctx_lock = > + PTHREAD_MUTEX_INITIALIZER; Not sure why you're adding attribute__(unused) here, since both these vars are clearly used. BTW, since we're using glib, you should use G_GNUC_UNUSD as the annotation, but since these vars are used, just drop the annotation entirely. > + > +/* HyperScale Driver Version */ > +static const int __attribute__((unused)) vxhs_drv_version = 8895; This only seems to be used in a single trace() message, so can probably be just deleted entirely > diff --git a/configure b/configure > index f57fcc6..c4f6261 100755 > --- a/configure > +++ b/configure > @@ -320,6 +320,7 @@ vhdx="" > numa="" > tcmalloc="no" > jemalloc="no" > +vxhs="yes" > > # parse CC options first > for opt do > @@ -1150,6 +1151,11 @@ for opt do > ;; > --enable-jemalloc) jemalloc="yes" > ;; > + --disable-vxhs) vxhs="no" > + ;; > + --enable-vxhs) vxhs="yes" > + ;; > + > *) > echo "ERROR: unknown option $opt" > echo "Try '$0 --help' for more information" > @@ -1380,6 +1386,7 @@ disabled with --disable-FEATURE, default is enabled if > available: > numa libnuma support > tcmalloc tcmalloc support > jemalloc jemalloc support > + vxhs Veritas HyperScale vDisk backend support > > NOTE: The object files are built at the place where configure is launched > EOF > @@ -4543,6 +4550,43 @@ if do_cc -nostdlib -Wl,-r -Wl,--no-relax -o $TMPMO > $TMPO; then > fi > > ########################################## > +# Veritas HyperScale block driver VxHS > +# Check if libqnio is installed > +if test "$vxhs" != "no" ; then > + cat > $TMPC <<EOF > +#include <stdio.h> > +#include <qnio/qnio_api.h> > + > +void vxhs_inc_acb_segment_count(void *acb, int count); > +void vxhs_dec_acb_segment_count(void *acb, int count); > +void vxhs_set_acb_buffer(void *ptr, void *buffer); > + > +void vxhs_inc_acb_segment_count(void *ptr, int count) > +{ > +} > +void vxhs_dec_acb_segment_count(void *ptr, int count) > +{ > +} > +void vxhs_set_acb_buffer(void *ptr, void *buffer) > +{ > +} > +int main(void) { > + qemu_ck_initialize_lock(); > + return 0; > +} > +EOF > + vxhs_libs="-lqnioshim -lqnio" > + if compile_prog "" "$vxhs_libs" ; then > + vxhs=yes > + else > + if test "$vxhs" = "yes" ; then > + feature_not_found "vxhs block device" "Install libqnio. See github" > + fi > + vxhs=no > + fi > +fi There's nothing wrong with this code, but it occurs to me that since libqnio is a completely new library you've written yourself, it would be nice if you could add a pkg-config file to the libqio install. That would allow QEMU to detect libqnio using the standard pkg-config mechanism that most libraries uses these days. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
