From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59619) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XegDt-00088t-Sa for qemu-devel@nongnu.org; Thu, 16 Oct 2014 04:15:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XegDn-0008TW-01 for qemu-devel@nongnu.org; Thu, 16 Oct 2014 04:15:49 -0400 Received: from mx1.redhat.com ([209.132.183.28]:5021) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XegDm-0008TM-3A for qemu-devel@nongnu.org; Thu, 16 Oct 2014 04:15:42 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s9G8FfrC010074 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Thu, 16 Oct 2014 04:15:41 -0400 Date: Thu, 16 Oct 2014 10:15:39 +0200 From: Kevin Wolf Message-ID: <20141016081539.GA3489@noname.redhat.com> References: <1412790915-28605-1-git-send-email-mreitz@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1412790915-28605-1-git-send-email-mreitz@redhat.com> Subject: Re: [Qemu-devel] [PATCH v1.5] nbd: Fix filename generation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: qemu-devel@nongnu.org, Stefan Hajnoczi Am 08.10.2014 um 19:55 hat Max Reitz geschrieben: > Export names may be used with nbd+unix, too, fix nbd_refresh_filename() > accordingly. Also, for nbd+tcp, the documented path schema is > "nbd://host[:port]/export", so use it. Furthermore, as can be seen from > that schema, the port is optional. > > That makes six single cases for how the filename can be formatted; it is > not easy to generalize these cases without the resulting statement being > completely unreadable, thus there is simply one snprintf() per case. > > Finally, taking the options from BDRVNBDState::socket_opts is wrong, > because those will not contain the export name. Just use > BlockDriverState::options instead. > > Reported-by: Stefan Hajnoczi > Signed-off-by: Max Reitz > Reviewed-by: Paolo Bonzini > --- > This is exactly the same patch as patch 1 from my series > "[PATCH 0/3] block: Fix filename generation for blkdebug and nbd", > except I added Paolo's R-b. The reason I split it off from the series is > that is an actual bug fix and therefore I want to get it merged > regardless of when the blkdebug patches are getting reviewed. > --- > block/nbd.c | 44 +++++++++++++++++++++++++++++--------------- > 1 file changed, 29 insertions(+), 15 deletions(-) > > diff --git a/block/nbd.c b/block/nbd.c > index 89775e1..04cc845 100644 > --- a/block/nbd.c > +++ b/block/nbd.c > @@ -342,30 +342,44 @@ static void nbd_attach_aio_context(BlockDriverState *bs, > > static void nbd_refresh_filename(BlockDriverState *bs) > { > - BDRVNBDState *s = bs->opaque; > QDict *opts = qdict_new(); > - const char *path = qemu_opt_get(s->socket_opts, "path"); > - const char *host = qemu_opt_get(s->socket_opts, "host"); > - const char *port = qemu_opt_get(s->socket_opts, "port"); > - const char *export = qemu_opt_get(s->socket_opts, "export"); > + const char *path = qdict_get_try_str(bs->options, "path"); > + const char *host = qdict_get_try_str(bs->options, "host"); > + const char *port = qdict_get_try_str(bs->options, "port"); > + const char *export = qdict_get_try_str(bs->options, "export"); > > qdict_put_obj(opts, "driver", QOBJECT(qstring_from_str("nbd"))); > > - if (path) { > + if (path && export) { > snprintf(bs->exact_filename, sizeof(bs->exact_filename), > - "nbd+unix:%s", path); > - qdict_put_obj(opts, "path", QOBJECT(qstring_from_str(path))); > - } else if (export) { > + "nbd+unix:///%s?socket=%s", export, path); We really need URL encoding for all of the strings we're putting into the URL here and in the other cases in this function. It's a preexisting, independent problem, though, so this patch is still a strict improvement. Thanks, applied to the block branch. Kevin