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 <stefa...@redhat.com>
> Signed-off-by: Max Reitz <mre...@redhat.com>
> Reviewed-by: Paolo Bonzini <pbonz...@redhat.com>
> ---
> 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
