[libvirt] [PATCH 5/8] domain: save/restore the state of dbus-daemon running

marcandre.lureau@redhat.com posted 8 patches 6 years ago
There is a newer version of this series
[libvirt] [PATCH 5/8] domain: save/restore the state of dbus-daemon running
Posted by marcandre.lureau@redhat.com 6 years ago
From: Marc-André Lureau <marcandre.lureau@redhat.com>

This avoids trying to start a dbus-daemon when its already running.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 src/qemu/qemu_domain.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 7722a53c62..dda3cb781f 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2916,6 +2916,9 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf,
                           virDomainChrTypeToString(priv->monConfig->type));
     }
 
+    if (priv->dbusDaemonRunning)
+        virBufferAddLit(buf, "<dbusDaemon/>\n");
+
     if (priv->namespaces) {
         ssize_t ns = -1;
 
@@ -3697,6 +3700,8 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt,
         goto error;
     }
 
+    priv->dbusDaemonRunning = virXPathBoolean("boolean(./dbusDaemon)", ctxt) > 0;
+
     if ((node = virXPathNode("./namespaces", ctxt))) {
         xmlNodePtr next;
 
-- 
2.25.0.rc2.1.g09a9a1a997

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [PATCH 5/8] domain: save/restore the state of dbus-daemon running
Posted by Michal Privoznik 5 years, 11 months ago
On 1/14/20 2:46 PM, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> This avoids trying to start a dbus-daemon when its already running.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   src/qemu/qemu_domain.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 7722a53c62..dda3cb781f 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -2916,6 +2916,9 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf,
>                             virDomainChrTypeToString(priv->monConfig->type));
>       }
>   
> +    if (priv->dbusDaemonRunning)
> +        virBufferAddLit(buf, "<dbusDaemon/>\n");
> +
>       if (priv->namespaces) {
>           ssize_t ns = -1;
>   
> @@ -3697,6 +3700,8 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt,
>           goto error;
>       }
>   
> +    priv->dbusDaemonRunning = virXPathBoolean("boolean(./dbusDaemon)", ctxt) > 0;
> +
>       if ((node = virXPathNode("./namespaces", ctxt))) {
>           xmlNodePtr next;
>   
> 

I'd push these a bit down - closer to PR daemon and slirp so that they 
are grouped together.

Michal

Re: [PATCH 5/8] domain: save/restore the state of dbus-daemon running
Posted by Marc-André Lureau 5 years, 11 months ago
Hi

On Thu, Feb 20, 2020 at 10:04 AM Michal Privoznik <mprivozn@redhat.com> wrote:
>
> On 1/14/20 2:46 PM, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > This avoids trying to start a dbus-daemon when its already running.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >   src/qemu/qemu_domain.c | 5 +++++
> >   1 file changed, 5 insertions(+)
> >
> > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> > index 7722a53c62..dda3cb781f 100644
> > --- a/src/qemu/qemu_domain.c
> > +++ b/src/qemu/qemu_domain.c
> > @@ -2916,6 +2916,9 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf,
> >                             virDomainChrTypeToString(priv->monConfig->type));
> >       }
> >
> > +    if (priv->dbusDaemonRunning)
> > +        virBufferAddLit(buf, "<dbusDaemon/>\n");
> > +
> >       if (priv->namespaces) {
> >           ssize_t ns = -1;
> >
> > @@ -3697,6 +3700,8 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt,
> >           goto error;
> >       }
> >
> > +    priv->dbusDaemonRunning = virXPathBoolean("boolean(./dbusDaemon)", ctxt) > 0;
> > +
> >       if ((node = virXPathNode("./namespaces", ctxt))) {
> >           xmlNodePtr next;
> >
> >
>
> I'd push these a bit down - closer to PR daemon and slirp so that they
> are grouped together.

Well, as we introduce DBus bus for the VM, it would be a foundation
for IPC/multi-process communication, not specific to slirp. So I'd
leave it near the top.


Re: [PATCH 5/8] domain: save/restore the state of dbus-daemon running
Posted by Michal Privoznik 5 years, 11 months ago
On 2/24/20 4:58 PM, Marc-André Lureau wrote:
> Hi
> 
> On Thu, Feb 20, 2020 at 10:04 AM Michal Privoznik <mprivozn@redhat.com> wrote:
>>
>> On 1/14/20 2:46 PM, marcandre.lureau@redhat.com wrote:
>>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>
>>> This avoids trying to start a dbus-daemon when its already running.
>>>
>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> ---
>>>    src/qemu/qemu_domain.c | 5 +++++
>>>    1 file changed, 5 insertions(+)
>>>
>>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>>> index 7722a53c62..dda3cb781f 100644
>>> --- a/src/qemu/qemu_domain.c
>>> +++ b/src/qemu/qemu_domain.c
>>> @@ -2916,6 +2916,9 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf,
>>>                              virDomainChrTypeToString(priv->monConfig->type));
>>>        }
>>>
>>> +    if (priv->dbusDaemonRunning)
>>> +        virBufferAddLit(buf, "<dbusDaemon/>\n");
>>> +
>>>        if (priv->namespaces) {
>>>            ssize_t ns = -1;
>>>
>>> @@ -3697,6 +3700,8 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt,
>>>            goto error;
>>>        }
>>>
>>> +    priv->dbusDaemonRunning = virXPathBoolean("boolean(./dbusDaemon)", ctxt) > 0;
>>> +
>>>        if ((node = virXPathNode("./namespaces", ctxt))) {
>>>            xmlNodePtr next;
>>>
>>>
>>
>> I'd push these a bit down - closer to PR daemon and slirp so that they
>> are grouped together.
> 
> Well, as we introduce DBus bus for the VM, it would be a foundation
> for IPC/multi-process communication, not specific to slirp. So I'd
> leave it near the top.
> 

My reasoning was that while in private data the order doesn't matter 
really, but so far we keep internal flags towards the beginning and 
helper daemons related flags towards the end. That is not to say that 
slirp and dbus have something in common. But apparently, our 
parser/formater are out of order anyway. Ideally, we should have the 
same order for parsing/formatting as defined in the struct. But that 
ship sailed long ago.

Michal