On Fri, Mar 27, 2020 at 04:51:19PM +0100, Peter Krempa wrote:
> For non-NBD URIs we need to preserve the query part as it may be
> important to refer to the image. If the query doesn't start with
> 'socket=' concatenate it to the name of the image.
I can see that this makes sense for HTTP URIs (and other schemes with
QEMU's curl driver), as the query string can easily be part of the
identifier needed to resolve the image. These query params are not things
which map directly to QEMU driver options for CURL (if there are some
such query params, we should strip them), and thus don't make sense to
map to XML attributes (indeed it is impossible to map them since they
can be completely arbitrary).
For other URIs schemes though, this feels like it opens the door to
passthrough of arbitrary args to QEMU. eg we shouldn't allow this
for gluster/rbd/iscsi/sheepdog/etc drivers - in all of those any
query parameters should be mapped to explicit attributes and not
included in the path name.
>
> Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx>
> ---
> src/util/virstoragefile.c | 15 +++++++++++----
> tests/virstoragetest.c | 4 ++--
> 2 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
> index c43e52d1f6..caf5de2d2c 100644
> --- a/src/util/virstoragefile.c
> +++ b/src/util/virstoragefile.c
> @@ -2818,6 +2818,7 @@ virStorageSourceParseBackingURI(virStorageSourcePtr src,
> {
> g_autoptr(virURI) uri = NULL;
> const char *path = NULL;
> + g_autofree char *pathquery = NULL;
> VIR_AUTOSTRINGLIST scheme = NULL;
>
> if (!(uri = virURIParse(uristr))) {
> @@ -2851,10 +2852,6 @@ virStorageSourceParseBackingURI(virStorageSourcePtr src,
> return -1;
> }
>
> - /* handle socket stored as a query */
> - if (uri->query)
> - src->hosts->socket = g_strdup(STRSKIP(uri->query, "socket="));
> -
> /* uri->path is NULL if the URI does not contain slash after host:
> * transport://host:port */
> if (uri->path)
> @@ -2862,6 +2859,16 @@ virStorageSourceParseBackingURI(virStorageSourcePtr src,
> else
> path = "";
>
> + /* handle socket stored as a query */
> + if (uri->query) {
> + if (STRPREFIX(uri->query, "socket=")) {
> + src->hosts->socket = g_strdup(STRSKIP(uri->query, "socket="));
> + } else {
> + pathquery = g_strdup_printf("%s?%s", path, uri->query);
> + path = pathquery;
> + }
> + }
> +
> /* possibly skip the leading slash */
> if (path[0] == '/')
> path++;
> diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
> index 10d5421150..f0bb46a04c 100644
> --- a/tests/virstoragetest.c
> +++ b/tests/virstoragetest.c
> @@ -1632,7 +1632,7 @@ mymain(void)
> "\"file.url\": \"https://host/folder/esx6.5-rhel7.7-x86%5f64/esx6.5-rhel7.7-x86%5f64-flat.vmdk?dcPath=data&dsName=esx6.5-matrix\",";
> "\"file.timeout\": 2000"
> "}",
> - "<source protocol='https' name='folder/esx6.5-rhel7.7-x86_64/esx6.5-rhel7.7-x86_64-flat.vmdk'>\n"
> + "<source protocol='https' name='folder/esx6.5-rhel7.7-x86_64/esx6.5-rhel7.7-x86_64-flat.vmdk?dcPath=data&dsName=esx6.5-matrix'>\n"
> " <host name='host' port='443'/>\n"
> " <ssl verify='no'/>\n"
> " <cookies>\n"
> @@ -1647,7 +1647,7 @@ mymain(void)
> "\"file.url\": \"https://host/folder/esx6.5-rhel7.7-x86%5f64/esx6.5-rhel7.7-x86%5f64-flat.vmdk?dcPath=data&dsName=esx6.5-matrix\",";
> "\"file.timeout\": 2000"
> "}",
> - "<source protocol='https' name='folder/esx6.5-rhel7.7-x86_64/esx6.5-rhel7.7-x86_64-flat.vmdk'>\n"
> + "<source protocol='https' name='folder/esx6.5-rhel7.7-x86_64/esx6.5-rhel7.7-x86_64-flat.vmdk?dcPath=data&dsName=esx6.5-matrix'>\n"
> " <host name='host' port='443'/>\n"
> " <ssl verify='no'/>\n"
> " <cookies>\n"
> --
> 2.24.1
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
