Re: [PATCH 14/30] conf: Add support for cookies for HTTP based disks

[Ján Tomko <jtomko@xxxxxxxxxx>]

On a Monday in 2020, Peter Krempa wrote:
> Add possibility to specify one or more cookies for http based disks.
> This patch adds the config parser, storage and validation of the
> cookies.

Cookies are delicious delicacies.

> Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx>
> ---
> docs/formatdomain.html.in                     |  10 ++
> docs/schemas/domaincommon.rng                 |  24 ++++
> src/conf/domain_conf.c                        |  82 +++++++++++++
> src/libvirt_private.syms                      |   1 +
> src/util/virstoragefile.c                     | 115 ++++++++++++++++++
> src/util/virstoragefile.h                     |  15 +++
> .../disk-network-http.xml                     |   8 ++
> 7 files changed, 255 insertions(+)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index dd3a3a1439..dc7a47dd21 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -9340,6 +9340,62 @@ virDomainDiskSourcePoolDefParse(xmlNodePtr node,
> }
>
>
> +static virStorageNetCookieDefPtr

You have Net in the type name

> +virDomainStorageCookieParse(xmlNodePtr node,

no Net in this function name

> +                            xmlXPathContextPtr ctxt)
> +{
> +    VIR_XPATH_NODE_AUTORESTORE(ctxt);
> +    g_autoptr(virStorageNetCookieDef) cookie = NULL;
> +
> +    ctxt->node = node;
> +
> +    cookie = g_new0(virStorageNetCookieDef, 1);
> +
> +    if (!(cookie->name = virXPathString("string(./@name)", ctxt))) {
> +        virReportError(VIR_ERR_XML_ERROR, "%s", _("missing cookie name"));
> +        return NULL;
> +    }
> +
> +    if (!(cookie->value = virXPathString("string(.)", ctxt))) {
> +        virReportError(VIR_ERR_XML_ERROR, _("missing value for cookie '%s'"),
> +                       cookie->name);
> +        return NULL;
> +    }
> +
> +    return g_steal_pointer(&cookie);
> +}
> +
> +
> +static int
> +virDomainStorageCookiesParse(xmlNodePtr node,

no Net here either

> +                             xmlXPathContextPtr ctxt,
> +                             virStorageSourcePtr src)
> +{
> +    VIR_XPATH_NODE_AUTORESTORE(ctxt);
> +    g_autofree xmlNodePtr *nodes = NULL;
> +    ssize_t nnodes;
> +    size_t i;
> +
> +    ctxt->node = node;
> +
> +    if ((nnodes = virXPathNodeSet("./cookie", ctxt, &nodes)) < 0)
> +        return -1;
> +
> +    src->cookies = g_new0(virStorageNetCookieDefPtr, nnodes);
> +    src->ncookies = nnodes;
> +
> +    for (i = 0; i < nnodes; i++) {
> +        if (!(src->cookies[i] = virDomainStorageCookieParse(nodes[i], ctxt)))
> +            return -1;
> +    }
> +
> +    if (virStorageSourceNetCookiesValidate(src) < 0)
> +        return -1;
> +
> +    return 0;
> +}
> +
> +
> static int
> virDomainDiskSourceNetworkParse(xmlNodePtr node,
>                                 xmlXPathContextPtr ctxt,
> @@ -24500,6 +24564,22 @@ virDomainSourceDefFormatSeclabel(virBufferPtr buf,
> }
>
>
> +static void
> +virDomainDiskSourceFormatNetworkCookies(virBufferPtr buf,

Network here for a change

> +                                        virStorageSourcePtr src)
> +{
> +    g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf);
> +    size_t i;
> +
> +    for (i = 0; i < src->ncookies; i++) {
> +        virBufferEscapeString(&childBuf, "<cookie name='%s'>", src->cookies[i]->name);
> +        virBufferEscapeString(&childBuf, "%s</cookie>\n", src->cookies[i]->value);
> +    }
> +
> +    virXMLFormatElement(buf, "cookies", NULL, &childBuf);
> +}
> +
> +
> static int
> virDomainDiskSourceFormatNetwork(virBufferPtr attrBuf,
>                                  virBufferPtr childBuf,
> @@ -24549,6 +24629,8 @@ virDomainDiskSourceFormatNetwork(virBufferPtr attrBuf,
>         virBufferAsprintf(childBuf, "<ssl verify='%s'/>\n",
>                           virTristateBoolTypeToString(src->sslverify));
>
> +    virDomainDiskSourceFormatNetworkCookies(childBuf, src);
> +
>     return 0;
> }
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 511fb88872..73db753652 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -3143,6 +3143,7 @@ virStorageSourceIsEmpty;
> virStorageSourceIsLocalStorage;
> virStorageSourceIsRelative;
> virStorageSourceIsSameLocation;
> +virStorageSourceNetCookiesValidate;
> virStorageSourceNetworkAssignDefaultPorts;
> virStorageSourceNew;
> virStorageSourceNewFromBacking;
> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
> index ca91fc65ba..fb5fff5c5f 100644
> --- a/src/util/virstoragefile.c
> +++ b/src/util/virstoragefile.c
> @@ -2157,6 +2157,118 @@ virStorageSourceSeclabelsCopy(virStorageSourcePtr to,
> }
>
>
> +void
> +virStorageNetCookieDefFree(virStorageNetCookieDefPtr def)
> +{
> +    if (!def)
> +        return;
> +
> +    g_free(def->name);
> +    g_free(def->value);
> +
> +    g_free(def);
> +}
> +
> +
> +static void
> +virStorageSourceCookiesClear(virStorageSourcePtr src)

no Net here

> +{
> +    size_t i;
> +
> +    if (!src || !src->cookies)
> +        return;
> +
> +    for (i = 0; i < src->ncookies; i++)
> +        virStorageNetCookieDefFree(src->cookies[i]);
> +
> +    g_clear_pointer(&src->cookies, g_free);
> +    src->ncookies = 0;
> +}
> +
> +
> +static void
> +virStorageSourceNetCookiesCopy(virStorageSourcePtr to,
> +                               const virStorageSource *from)
> +{
> +    size_t i;
> +
> +    if (from->ncookies == 0)
> +        return;
> +
> +    to->cookies = g_new0(virStorageNetCookieDefPtr, from->ncookies);
> +    to->ncookies = from->ncookies;
> +
> +    for (i = 0; i < from->ncookies; i++) {
> +        to->cookies[i]->name = g_strdup(from->cookies[i]->name);
> +        to->cookies[i]->value = g_strdup(from->cookies[i]->value);
> +    }
> +}
> +
> +

Consider using 'Net' at least for those identifiers that do not have
'Network' in them.

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

Jano

Attachment: signature.asc
Description: PGP signature