From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48542) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bpQvX-0003kr-Tv for qemu-devel@nongnu.org; Wed, 28 Sep 2016 22:18:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bpQvT-0007cR-M3 for qemu-devel@nongnu.org; Wed, 28 Sep 2016 22:18:22 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37192) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bpQvT-0007c7-FR for qemu-devel@nongnu.org; Wed, 28 Sep 2016 22:18:19 -0400 Date: Wed, 28 Sep 2016 22:18:16 -0400 From: Jeff Cody Message-ID: <20160929021816.GG2695@localhost.localdomain> References: <1475035789-685-1-git-send-email-ashish.mittal@veritas.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1475035789-685-1-git-send-email-ashish.mittal@veritas.com> Subject: Re: [Qemu-devel] [PATCH v7 RFC] block/vxhs: Initial commit to add Veritas HyperScale VxHS block device support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ashish Mittal Cc: qemu-devel@nongnu.org, pbonzini@redhat.com, kwolf@redhat.com, armbru@redhat.com, berrange@redhat.com, famz@redhat.com, ashish.mittal@veritas.com, stefanha@gmail.com, Ketan.Nilangekar@veritas.com, Abhijit.Dey@veritas.com On Tue, Sep 27, 2016 at 09:09:49PM -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 from: > https://github.com/MittalAshish/libqnio.git > > Sample command line using JSON syntax: > ./qemu-system-x86_64 -name instance-00000008 -S -vnc 0.0.0.0:0 -k en-us -vga cirrus -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5 -msg timestamp=on 'json:{"driver":"vxhs","vdisk_id":"{c3e9095a-a5ee-4dce-afeb-2a59fb387410}","server":[{"host":"172.172.17.4","port":"9999"},{"host":"172.172.17.2","port":"9999"}]}' > > Sample command line using URI syntax: > qemu-img convert -f raw -O raw -n /var/lib/nova/instances/_base/0c5eacd5ebea5ed914b6a3e7b18f1ce734c386ad vxhs://192.168.0.1:9999/%7Bc6718f6b-0401-441d-a8c3-1f0064d75ee0%7D > > Signed-off-by: Ashish Mittal Hi Ashish, You've received a lot of feedback to digest on your patch -- creating a whole new block driver can be difficult! If I may make a suggestion: it is usually more productive to address the feedback via email _before_ you code up and send out the next patch version, unless the comments are straightforward and don't need any discussion (many comments often don't). For instance, I appreciate your reply to the feedback on v6, but by the time it hit my inbox v7 was already there, so it more or less kills the discussion and starts the review cycle afresh. Not a big deal, but overall it would probably be more productive if that reply was sent first, and we could discuss things like sleep, coroutines, caching, global ctx instances, etc... :) Those discussion might then help shape the next patch even more, and result in fewer iterations. Thanks, and happy hacking! -Jeff