Re: [PATCH v2 2/4] conf: Add support for http(s) query strings

[Ján Tomko <jtomko@xxxxxxxxxx>]

On a Monday in 2020, Peter Krempa wrote:
> Add a new attribute for holding the query part for http(s) disks.
>
> Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx>
> ---
> docs/formatdomain.html.in                                  | 7 ++++++-
> docs/schemas/domaincommon.rng                              | 6 ++++++
> src/conf/domain_conf.c                                     | 5 +++++
> src/util/virstoragefile.c                                  | 1 +
> src/util/virstoragefile.h                                  | 1 +
> tests/qemuxml2argvdata/disk-network-http.xml               | 2 +-
> .../qemuxml2xmloutdata/disk-network-http.x86_64-latest.xml | 2 +-
> 7 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 6dbf449698..aaeb05961f 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -2847,7 +2847,7 @@
>   &lt;/disk&gt;
>   &lt;disk type='network' device='cdrom'&gt;
>     &lt;driver name='qemu' type='raw'/&gt;
> -    &lt;source protocol="http" name="url_path"&gt;
> +    &lt;source protocol="http" name="url_path" query="foo=bar&amp;amp;baz=flurb&gt;
>       &lt;host name="hostname" port="80"/&gt;
>       &lt;cookies&gt;
>         &lt;cookie name="test"&gt;somevalue&lt;/cookie&gt;
> @@ -3113,6 +3113,11 @@
>               ('tls' <span class="since">Since 4.5.0</span>)
>               </p>
>
> +              <p>For protocols <code>http</code> and <code>https</code> an
> +              optional attribute <code>query</code> specifies the query string.
> +              (<span class="since">Since 6.2.0</span>)

Should this be 6.3.0? Or are we calling this a bugfix?

> +              </p>
> +
>               <p>For "iscsi" (<span class="since">since 1.0.4</span>), the
>               <code>name</code> attribute may include a logical unit number,
>               separated from the target's name by a slash (e.g.,
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 5de17593c1..1807df521c 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -1869,6 +1869,9 @@
>           </choice>
>         </attribute>
>         <attribute name="name"/>
> +        <optional>
> +          <attribute name="query"/>
> +        </optional>
>         <ref name="diskSourceCommon"/>
>         <ref name="diskSourceNetworkHost"/>
>         <optional>
> @@ -1894,6 +1897,9 @@
>           </choice>
>         </attribute>
>         <attribute name="name"/>
> +        <optional>
> +          <attribute name="query"/>
> +        </optional>
>         <ref name="diskSourceCommon"/>
>         <ref name="diskSourceNetworkHost"/>
>         <optional>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 27bc5a797b..914e03c705 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -9482,6 +9482,10 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node,
>     /* config file currently only works with remote disks */
>     src->configFile = virXPathString("string(./config/@file)", ctxt);
>
> +    if (src->protocol == VIR_STORAGE_NET_PROTOCOL_HTTP ||
> +        src->protocol == VIR_STORAGE_NET_PROTOCOL_HTTPS)
> +        src->query = virXMLPropString(node, "query");
> +
>     if (virDomainStorageNetworkParseHosts(node, &src->hosts, &src->nhosts) < 0)
>         return -1;
>
> @@ -24591,6 +24595,7 @@ virDomainDiskSourceFormatNetwork(virBufferPtr attrBuf,
>         path = g_strdup_printf("%s/%s", src->volume, src->path);
>
>     virBufferEscapeString(attrBuf, " name='%s'", path ? path : src->path);
> +    virBufferEscapeString(attrBuf, " query='%s'", src->query);
>
>     if (src->haveTLS != VIR_TRISTATE_BOOL_ABSENT &&
>         !(flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE &&
> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
> index c43e52d1f6..bccef863b4 100644
> --- a/src/util/virstoragefile.c
> +++ b/src/util/virstoragefile.c
> @@ -2418,6 +2418,7 @@ virStorageSourceCopy(const virStorageSource *src,
>     def->compat = g_strdup(src->compat);
>     def->tlsAlias = g_strdup(src->tlsAlias);
>     def->tlsCertdir = g_strdup(src->tlsCertdir);
> +    def->query = g_strdup(src->query);

The string also needs to be freed in virStorageSourceClear.

>     if (src->sliceStorage)
>         def->sliceStorage = virStorageSourceSliceCopy(src->sliceStorage);

Reviewed-by: Ján Tomko <jtomko@xxxxxxxxxx>

Jano

Attachment: signature.asc
Description: PGP signature