From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34268) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bzeV2-0007l2-2w for qemu-devel@nongnu.org; Thu, 27 Oct 2016 02:49:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bzeUz-000696-2d for qemu-devel@nongnu.org; Thu, 27 Oct 2016 02:49:16 -0400 Received: from mx1.redhat.com ([209.132.183.28]:34952) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1bzeUy-00068r-R5 for qemu-devel@nongnu.org; Thu, 27 Oct 2016 02:49:12 -0400 From: Markus Armbruster References: <1477432927-5451-1-git-send-email-ashish.mittal@veritas.com> <20161026044128.GA2677@localhost.localdomain> <20161027010959.GJ2677@localhost.localdomain> Date: Thu, 27 Oct 2016 08:49:09 +0200 In-Reply-To: <20161027010959.GJ2677@localhost.localdomain> (Jeff Cody's message of "Wed, 26 Oct 2016 21:09:59 -0400") Message-ID: <87vawei8wq.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v1] block/vxhs: Add Veritas HyperScale VxHS block device support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jeff Cody Cc: Buddhi Madhav , "kwolf@redhat.com" , Ashish Mittal , "stefanha@gmail.com" , "qemu-devel@nongnu.org" , Rakesh Ranjan , Ketan Nilangekar , Abhijit Dey , "famz@redhat.com" , "pbonzini@redhat.com" , Ashish Mittal Jeff Cody writes: > On Wed, Oct 26, 2016 at 09:33:30PM +0000, Buddhi Madhav wrote: >>=20 >>=20 >> On 10/25/16, 9:41 PM, "Jeff Cody" wrote: >>=20 >> >On Tue, Oct 25, 2016 at 03:02:07PM -0700, Ashish Mittal wrote: >> >> This patch adds support for a new block device type called "vxhs". >> >> Source code for the library that this code loads can be downloaded fr= om: >> >> https://github.com/MittalAshish/libqnio.git >> >> >> > >> >I grabbed the latest of libqnio, compiled it (had to disable -Werror), = and >> >tried it out. I was able to do a qemu-img info on a raw file, but it >> >would >> >just hang when trying a format such as qcow2. I am assuming >> >this is a limitation of test_server, and not libqnio. >>=20 >> On my build I did not get any build errors. >>=20 > > Likely a difference in gcc versions; I am compiling with gcc 6.2.1. Here > are the warnings I get (this is just for your benefit, not really part of > this driver review): > > lib/qnio/cJSON.c: In function =E2=80=98cJSON_strcasecmp=E2=80=99: > lib/qnio/cJSON.c:41:5: warning: this =E2=80=98if=E2=80=99 clause does not= guard... [-Wmisleading-indentation] > if (!s1) return (s1=3D=3Ds2)?0:1;if (!s2) return 1; > ^~ > lib/qnio/cJSON.c:41:34: note: ...this statement, but the latter is mislea= dingly indented as if it is guarded by the =E2=80=98if=E2=80=99 > if (!s1) return (s1=3D=3Ds2)?0:1;if (!s2) return 1; > ^~ > lib/qnio/cJSON.c: In function =E2=80=98print_object=E2=80=99: > lib/qnio/cJSON.c:440:9: warning: this =E2=80=98if=E2=80=99 clause does no= t guard... [-Wmisleading-indentation] > if (fmt) *ptr++=3D'\n';*ptr=3D0; > ^~ > lib/qnio/cJSON.c:440:30: note: ...this statement, but the latter is misle= adingly indented as if it is guarded by the =E2=80=98if=E2=80=99 > if (fmt) *ptr++=3D'\n';*ptr=3D0; > ^ > lib/qnio/cJSON.c: In function =E2=80=98cJSON_DetachItemFromArray=E2=80=99: > lib/qnio/cJSON.c:467:5: warning: this =E2=80=98if=E2=80=99 clause does no= t guard... [-Wmisleading-indentation] > if (c->prev) c->prev->next=3Dc->next;if (c->next) c->next->prev=3Dc-= >prev;if (c=3D=3Darray->child) array->child=3Dc->next;c->prev=3Dc->next=3D0= ;return c;} > ^~ > lib/qnio/cJSON.c:467:40: note: ...this statement, but the latter is misle= adingly indented as if it is guarded by the =E2=80=98if=E2=80=99 > if (c->prev) c->prev->next=3Dc->next;if (c->next) c->next->prev=3Dc-= >prev;if (c=3D=3Darray->child) array->child=3Dc->next;c->prev=3Dc->next=3D0= ;return c;} Looks like this code hasn't been competently reviewed, not even fed to a state-of-the-art static analyzer. As long as that remains the case, linking with it outside experimental settings feels unadvisable. [...]