From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:39306)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <eblake@redhat.com>) id 1X2LYO-0003Oe-T0
	for qemu-devel@nongnu.org; Wed, 02 Jul 2014 10:30:38 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <eblake@redhat.com>) id 1X2LYI-0002Cg-SD
	for qemu-devel@nongnu.org; Wed, 02 Jul 2014 10:30:32 -0400
Received: from mx1.redhat.com ([209.132.183.28]:6062)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <eblake@redhat.com>) id 1X2LYI-0002Ca-Jl
	for qemu-devel@nongnu.org; Wed, 02 Jul 2014 10:30:26 -0400
Message-ID: <53B4177C.4010106@redhat.com>
Date: Wed, 02 Jul 2014 08:30:20 -0600
From: Eric Blake <eblake@redhat.com>
MIME-Version: 1.0
References: <1403857452-23768-1-git-send-email-cnanakos@grnet.gr>
	<1403857452-23768-2-git-send-email-cnanakos@grnet.gr>
	<53B41025.8030002@redhat.com> <53B414B3.3060601@grnet.gr>
In-Reply-To: <53B414B3.3060601@grnet.gr>
Content-Type: multipart/signed; micalg=pgp-sha256;
	protocol="application/pgp-signature";
	boundary="L3wAskMKsG58sfBRgnapweiLdPLwI80Jp"
Subject: Re: [Qemu-devel] [PATCH v6 1/5] block: Support Archipelago as a
	QEMU block backend
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Chrysostomos Nanakos <cnanakos@grnet.gr>, qemu-devel@nongnu.org
Cc: kwolf@redhat.com, stefanha@redhat.com

This is an OpenPGP/MIME signed message (RFC 4880 and 3156)
--L3wAskMKsG58sfBRgnapweiLdPLwI80Jp
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

On 07/02/2014 08:18 AM, Chrysostomos Nanakos wrote:
> On 07/02/2014 04:59 PM, Eric Blake wrote:
>> On 06/27/2014 02:24 AM, Chrysostomos Nanakos wrote:
>>> VM Image on Archipelago volume is specified like this:
>>>
>>> file.driver=3Darchipelago,file.volume=3D<volumename>[,file.mport=3D<m=
apperd_port>[,
>>>
>>> file.vport=3D<vlmcd_port>][,file.segment=3D<segment_name>]]
>>>
>>> 'archipelago' is the protocol.
>>>

>> Just a high-level glance through, and not a thorough review.
>>
>> The command line approach here looks reasonable, although it might be
>> easier to add the QAPI types first (patch 4/5) and then use that type =
in
>> this patch, instead of open-coding things.
>=20
> If I understand well the order of the commit should change? Patch 4/5
> should be first (1/5) and then 1/5 should be 2/5 and so on?

'git rebase -i' can be used to reorder patches; my question is whether
the current patch 4/5 should be hoisted earlier into the series (either
squashed in with the current patch 1, or at least adjacent to it - but
maybe moving it to 2/5 rather than 1/5).  What I'm less sure of is
whether the auto-generated types created by declaring your structure in
the .json file will make life in this patch easier by reusing that
struct, instead of open-coding your QemuOpts.  So it is more of a
question on my end on how much code can be shared between the QemuOpts
of the command line and the QMP additions (since I haven't actually
written a block device myself).  On looking a bit closer, it looks like
you have done things in the correct order.  After all, the command line
parsing was implemented first on most other block devices, then we added
QMP blockdev-add, and its implementation is merely taking a generated
QAPI struct, and converting it into a QemuOpts data structure that can
then be fed to the pre-existing command line code.

At any rate, the important part is that each patch compiles in
isolation, and that you aren't duplicating large portions of code.

--=20
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


--L3wAskMKsG58sfBRgnapweiLdPLwI80Jp
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Public key at http://people.redhat.com/eblake/eblake.gpg
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBCAAGBQJTtBd8AAoJEKeha0olJ0Nqr+sH/igKD76nLpKdL0QdnaggnElT
4HS1Neo6zPWsez7T29JexWBEEUn35JzNAPl668t6sHcjhnzY8ttfPJPikR0Cx/kj
KHpdB8JlvJpsNvoqdhRKWlPa+JAFINn45BYqC2YQM5mwPcB6eWYbNmpJcn8MGJUR
WQkkH7VF6ZJj6qWOVHGPbHC2/LrGQq/qpCJMNFKvoZ+WVnnOdpqkmMcluauHUWUJ
8dLGnBW4wbvxc3oE/N56tNre1c02dgQapS+goNqVXtODQlgwIEMXx3s5ly2d+Ssv
Bb1+pk7+QHmiJCooOnX79A1fby9UUQcdyEV9+BesflU99Qd8/EnTiaJKmh3NQGM=
=Zuu1
-----END PGP SIGNATURE-----

--L3wAskMKsG58sfBRgnapweiLdPLwI80Jp--