[libvirt PATCH v2 13/16] qemu: include nbdkit state in private xml

Jonathon Jongsma posted 16 patches 3 years, 5 months ago
There is a newer version of this series
[libvirt PATCH v2 13/16] qemu: include nbdkit state in private xml
Posted by Jonathon Jongsma 3 years, 5 months ago
Add xml to the private data for a disk source to represent the nbdkit
process so that the state can be re-created if the libvirt daemon is
restarted. Format:

   <nbdkit>
     <pidfile>/path/to/nbdkit.pid</pidfile>
     <socketfile>/path/to/nbdkit.socket</socketfile>
   </nbdkit>

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
---
 src/qemu/qemu_domain.c | 51 ++++++++++++++++++++++++++++++++++++++++++
 src/qemu/qemu_nbdkit.c | 21 +++++++++++++++++
 src/qemu/qemu_nbdkit.h |  4 ++++
 3 files changed, 76 insertions(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f69cfee0cf..cfc030cc9c 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -21,6 +21,7 @@
 
 #include <config.h>
 
+#include "qemu_conf.h"
 #include "qemu_domain.h"
 #include "qemu_alias.h"
 #include "qemu_block.h"
@@ -1816,6 +1817,31 @@ qemuStorageSourcePrivateDataAssignSecinfo(qemuDomainSecretInfo **secinfo,
 }
 
 
+static int
+qemuStorageSourcePrivateDataParseNbdkit(xmlNodePtr node,
+                                        xmlXPathContextPtr ctxt,
+                                        virStorageSource *src)
+{
+    qemuDomainStorageSourcePrivate *srcpriv = qemuDomainStorageSourcePrivateFetch(src);
+    g_autofree char *pidfile = NULL;
+    g_autofree char *socketfile = NULL;
+    VIR_XPATH_NODE_AUTORESTORE(ctxt);
+
+    ctxt->node = node;
+
+    if (!(pidfile = virXPathString("string(./pidfile)", ctxt)))
+        return -1;
+
+    if (!(socketfile = virXPathString("string(./socketfile)", ctxt)))
+        return -1;
+
+    if (!srcpriv->nbdkitProcess)
+        srcpriv->nbdkitProcess = qemuNbdkitProcessLoad(src, pidfile, socketfile);
+
+    return 0;
+}
+
+
 static int
 qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt,
                                   virStorageSource *src)
@@ -1826,6 +1852,7 @@ qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt,
     g_autofree char *httpcookiealias = NULL;
     g_autofree char *tlskeyalias = NULL;
     g_autofree char *thresholdEventWithIndex = NULL;
+    xmlNodePtr nbdkitnode = NULL;
 
     src->nodestorage = virXPathString("string(./nodenames/nodename[@type='storage']/@name)", ctxt);
     src->nodeformat = virXPathString("string(./nodenames/nodename[@type='format']/@name)", ctxt);
@@ -1869,6 +1896,10 @@ qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt,
         virTristateBoolTypeFromString(thresholdEventWithIndex) == VIR_TRISTATE_BOOL_YES)
         src->thresholdEventWithIndex = true;
 
+    if ((nbdkitnode = virXPathNode("nbdkit", ctxt))) {
+        if (qemuStorageSourcePrivateDataParseNbdkit(nbdkitnode, ctxt, src) < 0)
+            return -1;
+    }
     return 0;
 }
 
@@ -1886,6 +1917,23 @@ qemuStorageSourcePrivateDataFormatSecinfo(virBuffer *buf,
 }
 
 
+static void
+qemuStorageSourcePrivateDataFormatNbdkit(qemuNbdkitProcess *nbdkit,
+                                         virBuffer *buf)
+{
+    g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf);
+
+    if (!nbdkit)
+        return;
+
+    virBufferEscapeString(&childBuf, "<pidfile>%s</pidfile>\n",
+                          nbdkit->pidfile);
+    virBufferEscapeString(&childBuf, "<socketfile>%s</socketfile>\n",
+                          nbdkit->socketfile);
+    virXMLFormatElement(buf, "nbdkit", NULL, &childBuf);
+}
+
+
 static int
 qemuStorageSourcePrivateDataFormat(virStorageSource *src,
                                    virBuffer *buf)
@@ -1924,6 +1972,9 @@ qemuStorageSourcePrivateDataFormat(virStorageSource *src,
     if (src->thresholdEventWithIndex)
         virBufferAddLit(buf, "<thresholdEvent indexUsed='yes'/>\n");
 
+    if (srcPriv)
+        qemuStorageSourcePrivateDataFormatNbdkit(srcPriv->nbdkitProcess, buf);
+
     return 0;
 }
 
diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
index 46d4a8a7ac..5a0c80def8 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@ -595,6 +595,27 @@ qemuNbdkitCapsCacheNew(const char *cachedir)
 }
 
 
+qemuNbdkitProcess *
+qemuNbdkitProcessLoad(virStorageSource *source,
+                      const char *pidfile,
+                      const char *socketfile)
+{
+    int rc;
+    qemuNbdkitProcess *nbdkit = g_new0(qemuNbdkitProcess, 1);
+
+    nbdkit->pidfile = g_strdup(pidfile);
+    nbdkit->socketfile = g_strdup(socketfile);
+    nbdkit->source = virObjectRef(source);
+    nbdkit->user = -1;
+    nbdkit->group = -1;
+
+    if ((rc = virPidFileReadPath(nbdkit->pidfile, &nbdkit->pid)) < 0)
+        VIR_WARN("Failed to read pidfile %s", nbdkit->pidfile);
+
+    return nbdkit;
+}
+
+
 static qemuNbdkitProcess *
 qemuNbdkitProcessNew(qemuNbdkitCaps *caps,
                      virStorageSource *source,
diff --git a/src/qemu/qemu_nbdkit.h b/src/qemu/qemu_nbdkit.h
index 4fda5113a2..e84fd2eacc 100644
--- a/src/qemu/qemu_nbdkit.h
+++ b/src/qemu/qemu_nbdkit.h
@@ -83,4 +83,8 @@ void qemuNbdkitProcessFree(qemuNbdkitProcess *proc);
 
 int qemuNbdkitProcessSetupCgroup(qemuNbdkitProcess *proc, virCgroup *cgroup);
 
+qemuNbdkitProcess * qemuNbdkitProcessLoad(virStorageSource *source,
+                                          const char *pidfile,
+                                          const char *socketfile);
+
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuNbdkitProcess, qemuNbdkitProcessFree);
-- 
2.37.1
Re: [libvirt PATCH v2 13/16] qemu: include nbdkit state in private xml
Posted by Peter Krempa 3 years, 4 months ago
On Wed, Aug 31, 2022 at 13:40:58 -0500, Jonathon Jongsma wrote:
> Add xml to the private data for a disk source to represent the nbdkit
> process so that the state can be re-created if the libvirt daemon is
> restarted. Format:
> 
>    <nbdkit>
>      <pidfile>/path/to/nbdkit.pid</pidfile>
>      <socketfile>/path/to/nbdkit.socket</socketfile>
>    </nbdkit>
> 
> Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
> ---
>  src/qemu/qemu_domain.c | 51 ++++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_nbdkit.c | 21 +++++++++++++++++
>  src/qemu/qemu_nbdkit.h |  4 ++++
>  3 files changed, 76 insertions(+)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index f69cfee0cf..cfc030cc9c 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -21,6 +21,7 @@
>  
>  #include <config.h>
>  
> +#include "qemu_conf.h"

Neither I nor the compiler found something which would require adding
this include.

>  #include "qemu_domain.h"
>  #include "qemu_alias.h"
>  #include "qemu_block.h"
> @@ -1816,6 +1817,31 @@ qemuStorageSourcePrivateDataAssignSecinfo(qemuDomainSecretInfo **secinfo,
>  }
>  
>  
> +static int
> +qemuStorageSourcePrivateDataParseNbdkit(xmlNodePtr node,
> +                                        xmlXPathContextPtr ctxt,
> +                                        virStorageSource *src)
> +{
> +    qemuDomainStorageSourcePrivate *srcpriv = qemuDomainStorageSourcePrivateFetch(src);
> +    g_autofree char *pidfile = NULL;
> +    g_autofree char *socketfile = NULL;
> +    VIR_XPATH_NODE_AUTORESTORE(ctxt);
> +
> +    ctxt->node = node;
> +
> +    if (!(pidfile = virXPathString("string(./pidfile)", ctxt)))
> +        return -1;
> +
> +    if (!(socketfile = virXPathString("string(./socketfile)", ctxt)))
> +        return -1;
> +
> +    if (!srcpriv->nbdkitProcess)

Looks like you can move this condition to the top to avoid parsing
anything if it's not going to be used.

> +        srcpriv->nbdkitProcess = qemuNbdkitProcessLoad(src, pidfile, socketfile);

I'm not entirely a fan of trying to read the pidfile in the parser but
doing it elsewhere would probably be worse.

> +
> +    return 0;
> +}
> +
> +
>  static int
>  qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt,
>                                    virStorageSource *src)
Re: [libvirt PATCH v2 13/16] qemu: include nbdkit state in private xml
Posted by Peter Krempa 3 years, 4 months ago
On Tue, Sep 27, 2022 at 15:26:26 +0200, Peter Krempa wrote:
> On Wed, Aug 31, 2022 at 13:40:58 -0500, Jonathon Jongsma wrote:
> > Add xml to the private data for a disk source to represent the nbdkit
> > process so that the state can be re-created if the libvirt daemon is
> > restarted. Format:
> > 
> >    <nbdkit>
> >      <pidfile>/path/to/nbdkit.pid</pidfile>
> >      <socketfile>/path/to/nbdkit.socket</socketfile>
> >    </nbdkit>
> > 
> > Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
> > ---
> >  src/qemu/qemu_domain.c | 51 ++++++++++++++++++++++++++++++++++++++++++
> >  src/qemu/qemu_nbdkit.c | 21 +++++++++++++++++
> >  src/qemu/qemu_nbdkit.h |  4 ++++
> >  3 files changed, 76 insertions(+)

One more thing. This patch should go before the patch that actually
starts using the nbdkit processes, to ensure that the data is always in
teh status XML.