[libvirt] [PATCH 02/35] qemu: domain: Format storage source node names into private data

[John Ferlan]

On 04/25/2018 11:15 AM, Peter Krempa wrote:
> Save and restore node names if we know them in the status XML so that we
> don't need to recalculate them or don't lose them in some cases.
> 
> Signed-off-by: Peter Krempa <pkrempa at redhat.com>
> ---
>  src/qemu/qemu_domain.c                    | 12 ++++++++++++
>  tests/qemustatusxml2xmldata/modern-in.xml |  4 ++++
>  2 files changed, 16 insertions(+)

So, what would cause a need for recalculation?  Or losing them?

I assume it's "expensive" to query, but then I wonder what trap we could
fall into by saving them. If we've needed to recalculate or lost them
before, what's to say that same issue doesn't happen by saving them?

I also assume they don't change after we've saved them, right? I'm not
opposed to this, it's the OK so how are they to be used and then why if
they could be lost or needed recalculation would we want to save them?
Also, what assurances do we have then when they are to be saved that
they aren't in a condition that caused this patch to be generated.

If the only way to get the 'real' value is ask QEMU, then why not ask?
It's not like an alias which we generated it's IIUC something QEMU
generates and wishes to be referenced in that manner.

Maybe it's just the wording of the commit that raised the questions ;-)

John

> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 326c939c85..2c5de9508b 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -1937,6 +1937,9 @@ static int
>  qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt,
>                                    virStorageSourcePtr src)
>  {
> +    src->nodestorage = virXPathString("string(./nodenames/nodename[@type='storage']/@name)", ctxt);
> +    src->nodeformat = virXPathString("string(./nodenames/nodename[@type='format']/@name)", ctxt);
> +
>      if (virStorageSourcePrivateDataParseRelPath(ctxt, src) < 0)
>          return -1;
> 
> @@ -1948,6 +1951,15 @@ static int
>  qemuStorageSourcePrivateDataFormat(virStorageSourcePtr src,
>                                     virBufferPtr buf)
>  {
> +    if (src->nodestorage || src->nodeformat) {
> +        virBufferAddLit(buf, "<nodenames>\n");
> +        virBufferAdjustIndent(buf, 2);
> +        virBufferEscapeString(buf, "<nodename type='storage' name='%s'/>\n", src->nodestorage);
> +        virBufferEscapeString(buf, "<nodename type='format' name='%s'/>\n", src->nodeformat);
> +        virBufferAdjustIndent(buf, -2);
> +        virBufferAddLit(buf, "</nodenames>\n");
> +    }
> +
>      if (virStorageSourcePrivateDataFormatRelPath(src, buf) < 0)
>          return -1;
> 
> diff --git a/tests/qemustatusxml2xmldata/modern-in.xml b/tests/qemustatusxml2xmldata/modern-in.xml
> index c1e57618b6..d57e1f605f 100644
> --- a/tests/qemustatusxml2xmldata/modern-in.xml
> +++ b/tests/qemustatusxml2xmldata/modern-in.xml
> @@ -309,6 +309,10 @@
>            <format type='qcow2'/>
>            <source file='/var/lib/libvirt/images/base.qcow2'>
>              <privateData>
> +              <nodenames>
> +                <nodename type='storage' name='test-storage'/>
> +                <nodename type='format' name='test-format'/>
> +              </nodenames>
>                <relPath>base.qcow2</relPath>
>              </privateData>
>            </source>
>