From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41235) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZWSkm-0007Ag-60 for qemu-devel@nongnu.org; Mon, 31 Aug 2015 13:20:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZWSkh-0001LB-Sf for qemu-devel@nongnu.org; Mon, 31 Aug 2015 13:20:20 -0400 References: <1439279489-13338-1-git-send-email-wency@cn.fujitsu.com> <1439279489-13338-3-git-send-email-wency@cn.fujitsu.com> From: Eric Blake Message-ID: <55E48CBE.2020901@redhat.com> Date: Mon, 31 Aug 2015 11:19:58 -0600 MIME-Version: 1.0 In-Reply-To: <1439279489-13338-3-git-send-email-wency@cn.fujitsu.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="BCdUPG9Hgrolq6pFLaSiCnpOfeUvAPMRp" Subject: Re: [Qemu-devel] [Patch for-2.5 v2 2/6] support nbd driver in blockdev-add List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wen Congyang , qemu devel , Markus Armbruster , Alberto Garcia , Stefan Hajnoczi Cc: Kevin Wolf , zhanghailiang , qemu block , Jiang Yunhong , Dong Eddie , "Dr. David Alan Gilbert" , Gonglei , Yang Hongyang This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --BCdUPG9Hgrolq6pFLaSiCnpOfeUvAPMRp Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 08/11/2015 01:51 AM, Wen Congyang wrote: > Signed-off-by: Wen Congyang > Signed-off-by: zhanghailiang > Signed-off-by: Gonglei > Reviewed-by: Alberto Garcia > --- > qapi/block-core.json | 17 +++++++++++++++-- > 1 file changed, 15 insertions(+), 2 deletions(-) >=20 > diff --git a/qapi/block-core.json b/qapi/block-core.json > index 7b2efb8..3ed8114 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -1383,7 +1383,7 @@ > 'dmg', 'file', 'ftp', 'ftps', 'host_cdrom', 'host_device',= > 'host_floppy', 'http', 'https', 'null-aio', 'null-co', 'pa= rallels', > 'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'tftp', 'vdi', 'v= hdx', > - 'vmdk', 'vpc', 'vvfat' ] } > + 'vmdk', 'vpc', 'vvfat', 'nbd' ] } Please keep the list alphabetical. Also, this is missing documentation (see how BlockDeviceInfo has listed which releases have added which formats; so it should add a listing of 'nbd' in 2.5). > =20 > ## > # @BlockdevOptionsBase > @@ -1789,6 +1789,19 @@ > '*read-pattern': 'QuorumReadPattern' } } > =20 > ## > +# @BlockdevOptionsNBD > +# > +# Driver specific block device options for NBD > +# > +# @export: #options the NBD export name > +# > +# Since: 2.5 > +## > +{ 'struct': 'BlockdevOptionsNBD', > + 'base': 'InetSocketAddress', > + 'data': { '*export': 'str' } } So does NBD really support a range of ports? Or is it an error to specify 'to'? Perhaps what you should really be doing is making a simpler base class for representing a single-port IP address, then making InetSocketAddress a child class of the simpler one to add in ranging, and make BlockdevOptionsNBD a child class of the simpler one to add in an export name. Also, InetSocketAddress appears to be limited to IPv4 and IPv6 addresses; but what about nbd+unix transport? It feels like you need a flat union with a discriminator that describes what address family (IP vs. unix socket) and then further details based on that discriminator. > + > +## > # @BlockdevOptions > # > # Options for creating a block device. > @@ -1815,7 +1828,7 @@ > 'http': 'BlockdevOptionsFile', > 'https': 'BlockdevOptionsFile', > # TODO iscsi: Wait for structured options > -# TODO nbd: Should take InetSocketAddress for 'host'? > + 'nbd': 'BlockdevOptionsNBD', So while you are indeed literally taking the TODO suggestion, I'm not sure that the literal interpretation is the best. Remember, the current NBD source code accepts: if (is_unix) { /* nbd+unix:///export?socket=3Dpath */ if (uri->server || uri->port || strcmp(qp->p[0].name, "socket")) = { ret =3D -EINVAL; goto out; } qdict_put(options, "path", qstring_from_str(qp->p[0].value)); } else { QString *host; /* nbd[+tcp]://host[:port]/export */ and we want to be able to represent all of those possibilities in QMP. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --BCdUPG9Hgrolq6pFLaSiCnpOfeUvAPMRp Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJV5Iy+AAoJEKeha0olJ0Nq5awH/2I+WmnSDYEFGCM3y4npHZLJ E6uOjVvCS3S2S1XlNVsG6fftQaWDysPMph6WrchjTw4bgcBYB09/fFqFeDitu++x 0HCyVGky7EarrwJ7A+gpdLboR2spjKg6mM1dLvB14Kr9y+fYQtSjJz/HrdokiNJ1 ItGZtIdPKG6WJRXYgxmXz8ZSGg/xuMOjrc4GqwqcNUVW2drzkKgNE5QkT0AHGlvc O0NrS4tAgpycpD2xijDUtt7djnj/YONHOeRh7B4P9DsButYnm2YK8l7O4G0tVRAq JsSFNMOBFH7AY6hMhm2IvFuiiYc1x4dvzmZHIWI959UFKJ35QdLpRw+3f9mREL4= =wnkr -----END PGP SIGNATURE----- --BCdUPG9Hgrolq6pFLaSiCnpOfeUvAPMRp--