[PATCH 0/3] Preserve query component of path for HTTP disks
Peter Krempa (3): virStorageSourceParseBackingURI: Preserve query parts of URI qemuBlockStorageSourceGetURI: Properly preserve query component qemuxml2argv: Test the query portion for HTTP disks src/qemu/qemu_block.c | 6 ++++++ src/util/virstoragefile.c | 15 +++++++++++---- .../disk-network-http.x86_64-latest.args | 5 +++-- tests/qemuxml2argvdata/disk-network-http.xml | 2 +- .../disk-network-http.x86_64-latest.xml | 2 +- tests/virstoragetest.c | 4 ++-- 6 files changed, 24 insertions(+), 10 deletions(-) -- 2.24.1
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. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- 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
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@redhat.com> --- 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 :|
Look for the questionmark in the name and move the contents into the query portion so that we format the URI back properly. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 648c3f1026..c51fb2b6ea 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -429,12 +429,18 @@ qemuBlockStorageSourceGetURI(virStorageSourcePtr src) } if (src->path) { + char *query; if (src->volume) { uri->path = g_strdup_printf("/%s/%s", src->volume, src->path); } else { uri->path = g_strdup_printf("%s%s", src->path[0] == '/' ? "" : "/", src->path); } + + if ((query = strchr(uri->path, '?'))) { + uri->query = g_strdup(query + 1); + *query = '\0'; + } } uri->server = g_strdup(src->hosts->name); -- 2.24.1
On Fri, Mar 27, 2020 at 16:51:20 +0100, Peter Krempa wrote:
Look for the questionmark in the name and move the contents into the query portion so that we format the URI back properly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 648c3f1026..c51fb2b6ea 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -429,12 +429,18 @@ qemuBlockStorageSourceGetURI(virStorageSourcePtr src) }
if (src->path) { + char *query; if (src->volume) { uri->path = g_strdup_printf("/%s/%s", src->volume, src->path); } else { uri->path = g_strdup_printf("%s%s", src->path[0] == '/' ? "" : "/", src->path); } + + if ((query = strchr(uri->path, '?'))) { + uri->query = g_strdup(query + 1); + *query = '\0'; + }
This would be quite fragile I'm afraid. We should only do this when we know we got src->path from a URI with a query string, otherwise we could find a random '?' possibly used in a file's name. Jirka
Add a simple test case for preserving the query portion of http disk's path. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemuxml2argvdata/disk-network-http.x86_64-latest.args | 5 +++-- tests/qemuxml2argvdata/disk-network-http.xml | 2 +- tests/qemuxml2xmloutdata/disk-network-http.x86_64-latest.xml | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/qemuxml2argvdata/disk-network-http.x86_64-latest.args b/tests/qemuxml2argvdata/disk-network-http.x86_64-latest.args index ed44424dc3..7b4674a588 100644 --- a/tests/qemuxml2argvdata/disk-network-http.x86_64-latest.args +++ b/tests/qemuxml2argvdata/disk-network-http.x86_64-latest.args @@ -55,8 +55,9 @@ id=virtio-disk2 \ -object secret,id=libvirt-1-storage-httpcookie-secret0,\ data=DrPR9NA6GKJb7qi1KbjHaealKEMVtOWUl2h3yvO5lgIh6cyLHemmlg+h9fcgwREA,\ keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ --blockdev '{"driver":"https","url":"https://example.org:1234/test4.img",\ -"sslverify":false,"cookie-secret":"libvirt-1-storage-httpcookie-secret0",\ +-blockdev '{"driver":"https",\ +"url":"https://example.org:1234/test4.img?par=val&other=ble","sslverify":false,\ +"cookie-secret":"libvirt-1-storage-httpcookie-secret0",\ "node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \ -blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw",\ "file":"libvirt-1-storage"}' \ diff --git a/tests/qemuxml2argvdata/disk-network-http.xml b/tests/qemuxml2argvdata/disk-network-http.xml index 93e6617433..3bcff14b95 100644 --- a/tests/qemuxml2argvdata/disk-network-http.xml +++ b/tests/qemuxml2argvdata/disk-network-http.xml @@ -42,7 +42,7 @@ </disk> <disk type='network' device='disk'> <driver name='qemu' type='raw'/> - <source protocol='https' name='test4.img'> + <source protocol='https' name='test4.img?par=val&other=ble'> <host name='example.org' port='1234'/> <ssl verify='no'/> <cookies> diff --git a/tests/qemuxml2xmloutdata/disk-network-http.x86_64-latest.xml b/tests/qemuxml2xmloutdata/disk-network-http.x86_64-latest.xml index cf36331286..b925f45e28 100644 --- a/tests/qemuxml2xmloutdata/disk-network-http.x86_64-latest.xml +++ b/tests/qemuxml2xmloutdata/disk-network-http.x86_64-latest.xml @@ -49,7 +49,7 @@ </disk> <disk type='network' device='disk'> <driver name='qemu' type='raw'/> - <source protocol='https' name='test4.img'> + <source protocol='https' name='test4.img?par=val&other=ble'> <host name='example.org' port='1234'/> <ssl verify='no'/> <cookies> -- 2.24.1
participants (3)
-
Daniel P. Berrangé -
Jiri Denemark -
Peter Krempa