From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=40716 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OoBF7-0004Ku-4P for qemu-devel@nongnu.org; Wed, 25 Aug 2010 04:21:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OoBF5-0005x0-Ti for qemu-devel@nongnu.org; Wed, 25 Aug 2010 04:21:57 -0400 Received: from mx1.redhat.com ([209.132.183.28]:30053) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OoBF5-0005wq-L4 for qemu-devel@nongnu.org; Wed, 25 Aug 2010 04:21:55 -0400 Message-ID: <4C74D292.6060402@redhat.com> Date: Wed, 25 Aug 2010 10:21:38 +0200 From: Kevin Wolf MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH] Introduce NBD named exports. References: <1282611879-28954-1-git-send-email-laurent@vivier.eu> <4C738A01.8090605@redhat.com> <1282688480.22220.14.camel@Quad> In-Reply-To: <1282688480.22220.14.camel@Quad> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Laurent Vivier Cc: qemu-devel Am 25.08.2010 00:21, schrieb Laurent Vivier: > Le mardi 24 ao=C3=BBt 2010 =C3=A0 10:59 +0200, Kevin Wolf a =C3=A9crit = : >> Am 24.08.2010 03:04, schrieb Laurent Vivier: >>> This patch allows to connect Qemu using NBD protocol to an nbd-server >>> using named exports. >>> >>> For instance, if on the host "isoserver", in /etc/nbd-server/config, = you have: >>> >>> [generic] >>> [debian-500-ppc-netinst] >>> exportname =3D /ISO/debian-500-powerpc-netinst.iso >>> [Fedora-10-ppc-netinst] >>> exportname =3D /ISO/Fedora-10-ppc-netinst.iso >>> >>> You can connect to it, using: >>> >>> qemu -cdrom nbd:isoserver:exportname=3Ddebian-500-ppc-netinst >>> qemu -cdrom nbd:isoserver:exportname=3DFedora-10-ppc-netinst >>> >>> NOTE: you need at least nbd-server 2.9.18 >>> Signed-off-by: Laurent Vivier >>> --- >>> block/nbd.c | 57 +++++++++++++++++++-------- >>> nbd.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++= +++------ >>> nbd.h | 5 ++- >>> qemu-doc.texi | 7 +++ >>> qemu-nbd.c | 2 +- >>> 5 files changed, 158 insertions(+), 32 deletions(-) >>> >>> diff --git a/block/nbd.c b/block/nbd.c >>> index a1ec123..ee22b47 100644 >>> --- a/block/nbd.c >>> +++ b/block/nbd.c >>> @@ -42,55 +42,78 @@ typedef struct BDRVNBDState { >>> static int nbd_open(BlockDriverState *bs, const char* filename, int = flags) >>> { >>> BDRVNBDState *s =3D bs->opaque; >>> + >>> + #define EN_OPTSTR ":exportname=3D" >> >> Hm, this makes EN_OPTSTR look like a local variable, but it's actually= a >> macro which is valid after the end of the function. So maybe move it o= ut? >=20 > I agree, but have a look at curl_open(). Seems I should have reviewed the curl patch when it came in. :-) >> Maybe using strlen(EN_OPTSTR) would be clearer? At first I missed the >> fact that sizeof includes the null byte, but maybe it's just me. >=20 > I think sizeof() is resolved at compile time whereas strlen() it is at > runtime. For me, it is better, it like to have a constant. gcc is smart enough to make strlen of a literal string a constant, too. Besides, opening a connection isn't really performance critical. I guess it's a matter of taste, though. So if you like it better as it is, just leave it. >> I don't have a new enough ndbserver to test if it actually works (and = I >=20 > You can test against an old nbdserver to check it is always working ;-) > (or qemu-nbd). Sure, I can test for regressions in the old thing. >> also haven't checked the spec for the new parts), so I'll have to trus= t >> your tests there. >=20 > There is no spec (what is a spec ;-) ?), only a reference implementatio= n > in nbd-client.c and nbd-server.c from http://nbd.sf.net. Oh right, I forgot, the world outside qemu is just as bad. :-) Kevin