From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55255) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1axYVc-0007Lm-1m for qemu-devel@nongnu.org; Tue, 03 May 2016 07:29:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1axYVQ-0004J8-8o for qemu-devel@nongnu.org; Tue, 03 May 2016 07:28:50 -0400 References: <1461706338-20219-1-git-send-email-mreitz@redhat.com> <1461706338-20219-17-git-send-email-mreitz@redhat.com> <20160502153621.GK4882@noname.redhat.com> From: Max Reitz Message-ID: <57288B44.5020500@redhat.com> Date: Tue, 3 May 2016 13:28:04 +0200 MIME-Version: 1.0 In-Reply-To: <20160502153621.GK4882@noname.redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="aaO5vFI7jtlldfS42dAjXUcHeedEk0cVe" Subject: Re: [Qemu-devel] [PATCH 16/19] block/nbd: Implement bdrv_dirname() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-block@nongnu.org, qemu-devel@nongnu.org, Alberto Garcia , Eric Blake This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --aaO5vFI7jtlldfS42dAjXUcHeedEk0cVe Content-Type: multipart/mixed; boundary="DBc849A11Ki1blRbvKQLiSAbwAKMv71th" From: Max Reitz To: Kevin Wolf Cc: qemu-block@nongnu.org, qemu-devel@nongnu.org, Alberto Garcia , Eric Blake Message-ID: <57288B44.5020500@redhat.com> Subject: Re: [PATCH 16/19] block/nbd: Implement bdrv_dirname() References: <1461706338-20219-1-git-send-email-mreitz@redhat.com> <1461706338-20219-17-git-send-email-mreitz@redhat.com> <20160502153621.GK4882@noname.redhat.com> In-Reply-To: <20160502153621.GK4882@noname.redhat.com> --DBc849A11Ki1blRbvKQLiSAbwAKMv71th Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable On 02.05.2016 17:36, Kevin Wolf wrote: > Am 26.04.2016 um 23:32 hat Max Reitz geschrieben: >> The idea behind this implementation is that the export name might be >> interpreted as a path (which is the only apparent interpretation of >> relative filenames for NBD paths). >> >> The default implementation of bdrv_dirname() would handle that just fi= ne >> for nbd+tcp, but not for nbd+unix, because in that case, the last >> element of the path is the Unix socket path and not the export name. >> Therefore, we need to implement an own bdrv_dirname() which uses the >> legacy NBD URL which has the export name at the end. >> >> Signed-off-by: Max Reitz >> --- >> block/nbd.c | 29 +++++++++++++++++++++++++++++ >> 1 file changed, 29 insertions(+) >> >> diff --git a/block/nbd.c b/block/nbd.c >> index f7ea3b3..64534bb 100644 >> --- a/block/nbd.c >> +++ b/block/nbd.c >> @@ -469,6 +469,32 @@ static void nbd_refresh_filename(BlockDriverState= *bs, QDict *options) >> bs->full_open_options =3D opts; >> } >> =20 >> +static char *nbd_dirname(BlockDriverState *bs, Error **errp) >> +{ >> + const char *path =3D qdict_get_try_str(bs->options, "path"); >> + const char *host =3D qdict_get_try_str(bs->options, "host"); >> + const char *port =3D qdict_get_try_str(bs->options, "port"); >> + const char *export =3D qdict_get_try_str(bs->options, "export"); >> + >> + if (path && export) { >> + return g_strdup_printf("nbd:unix:%s:exportname=3D%s/", path, = export); >> + } else if (path && !export) { >> + return g_strdup_printf("nbd:unix:%s:exportname=3D", path); >> + } else if (!path && export && port) { >> + return g_strdup_printf("nbd://%s:%s/%s/", host, port, export)= ; >> + } else if (!path && export && !port) { >> + return g_strdup_printf("nbd://%s/%s/", host, export); >> + } else if (!path && !export && port) { >> + return g_strdup_printf("nbd://%s:%s/", host, port); >> + } else if (!path && !export && !port) { >> + return g_strdup_printf("nbd://%s/", host); >> + } >=20 > Many different cases already, and it's only going to get worse. Is it? I'm not sure we can support things like TLS here at all, so I think this should be about it, even in the future. > Wouldn'= t > it be better to store saddr in BDRVNBDState (so that we don't have to > parse the options a second time here) and to go for the full version > here always? Then it's one case for tcp and one for unix and that's it.= I think we can do that even without saddr, but I guess I need to see how this patch interacts with my "blockdev-add for NBD" series anyway. Thanks for the suggestion, I'll act on it, Max --DBc849A11Ki1blRbvKQLiSAbwAKMv71th-- --aaO5vFI7jtlldfS42dAjXUcHeedEk0cVe 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 iQEcBAEBCAAGBQJXKItEAAoJEDuxQgLoOKyt2ycH/21WWsUWnxwh2WlrAcgcmVkT Y1wnITCnXbcuDsO/dswR7rvQFbzxyRBHyVJbg0eFWLJZgwu2F7C51B5zEyFIEiDY 4G47vTgcGI5ct5M8u7itM0REkXO1xHLxR94eY9Nbop9inyPJuR6g7y2sfJ5V80lG knC8WwjYBlFACnuNXXqFqAskP6uphRrHApJB6oE/CqSQwbGLDztURVFMk5ST/R3A L+YZ4oLkg3sYRBcF7I903TgbrM4b/FgzkFwLNsNcfdP5CkCduLiSYhKhIQCGUTtZ RyQ/2vvWiW7mFF0C82msPaZtBWLfLjuHLv5QHb6Iwc2EgLucfHs1NTAd0Yvj0WY= =eXjD -----END PGP SIGNATURE----- --aaO5vFI7jtlldfS42dAjXUcHeedEk0cVe--