From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:58008)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <eblake@redhat.com>) id 1X2L43-0005d8-BK
	for qemu-devel@nongnu.org; Wed, 02 Jul 2014 09:59:15 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <eblake@redhat.com>) id 1X2L3y-0005h2-Pr
	for qemu-devel@nongnu.org; Wed, 02 Jul 2014 09:59:11 -0400
Received: from mx1.redhat.com ([209.132.183.28]:6259)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <eblake@redhat.com>) id 1X2L3y-0005gx-Gp
	for qemu-devel@nongnu.org; Wed, 02 Jul 2014 09:59:06 -0400
Message-ID: <53B41025.8030002@redhat.com>
Date: Wed, 02 Jul 2014 07:59:01 -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>
In-Reply-To: <1403857452-23768-2-git-send-email-cnanakos@grnet.gr>
Content-Type: multipart/signed; micalg=pgp-sha256;
	protocol="application/pgp-signature";
	boundary="4A5IxnJJ6k54ciM2eKlRpAJxPDo4IS8aC"
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)
--4A5IxnJJ6k54ciM2eKlRpAJxPDo4IS8aC
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

On 06/27/2014 02:24 AM, Chrysostomos Nanakos wrote:
> VM Image on Archipelago volume is specified like this:
>=20
> file.driver=3Darchipelago,file.volume=3D<volumename>[,file.mport=3D<map=
perd_port>[,
> file.vport=3D<vlmcd_port>][,file.segment=3D<segment_name>]]
>=20
> 'archipelago' is the protocol.
>=20
> 'mport' is the port number on which mapperd is listening. This is optio=
nal
> and if not specified, QEMU will make Archipelago to use the default por=
t.
>=20
> 'vport' is the port number on which vlmcd is listening. This is optiona=
l
> and if not specified, QEMU will make Archipelago to use the default por=
t.
>=20
> 'segment' is the name of the shared memory segment Archipelago stack is=
 using.
> This is optional and if not specified, QEMU will make Archipelago to us=
e the
> default value, 'archipelago'.
>=20
> Examples:
>=20
> file.driver=3Darchipelago,file.volume=3Dmy_vm_volume
> file.driver=3Darchipelago,file.volume=3Dmy_vm_volume,file.mport=3D123
> file.driver=3Darchipelago,file.volume=3Dmy_vm_volume,file.mport=3D123,
> file.vport=3D1234
> file.driver=3Darchipelago,file.volume=3Dmy_vm_volume,file.mport=3D123,
> file.vport=3D1234,file.segment=3Dmy_segment
>=20
> Signed-off-by: Chrysostomos Nanakos <cnanakos@grnet.gr>
> ---

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.

> +++ b/block/archipelago.c
> @@ -0,0 +1,819 @@
> +/*
> + * QEMU Block driver for Archipelago
> + *
> + * Copyright 2014 GRNET S.A. All rights reserved.

Is it still legally open source if you reserve all rights, or does this
statement contradict the rest of your header?  (Not that you are the
first person to attempt this; the phrase "All rights reserved" appears
in a number of other files in qemu.git, including the mis-spelled
disas/libvixl/LICENCE)

> +
> +            switch (reqdata->op) {
> +            case ARCHIP_OP_READ:
> +                    data =3D xseg_get_data(s->xseg, req);
> +                    segreq =3D reqdata->segreq;

Coding style - indent by 4 spaces, not 8.


> +
> +static int __archipelago_submit_request(BDRVArchipelagoState *s,

Please don't name internal functions with leading __ - that namespace is
reserved for the compiler and libc.

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


--4A5IxnJJ6k54ciM2eKlRpAJxPDo4IS8aC
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/

iQEcBAEBCAAGBQJTtBAlAAoJEKeha0olJ0NquF8H/ioS3GqVgNhUQy7Q4HHyF9Ss
t2B3QWKHhxaZYpUJfxKR1YFFufpTFRLSDpNUGYjYmnhPXQJGwt2qmf8L1o0f6bN9
hwfShBWNP6aap/N+c0nNTqf7McNk3QQoSkkoOd+5CkWH2fkbY7n8at+jJ33mS7zO
KmLS31sB9OGdQ7WAQy840XonRMlLFCe/hrxRpOsUYarMpVVLi81WU3EVgCyzOae8
YAsvrusLppCMEvOKaqxXuzRz0w2KaTwcrumWNVfJvHvWYsfCtZgPNqiA/Ibl6cy3
m3zzVL5vRuXObjkJCg/cIfi+N1tOovm/AeAs51kWW6IV4V9ynQODeZTjiTl5saA=
=Y7ls
-----END PGP SIGNATURE-----

--4A5IxnJJ6k54ciM2eKlRpAJxPDo4IS8aC--