[libvirt PATCH 1/6] slirp: leave the dbus daemon running on error

[Michal Privoznik]

On 4/21/20 6:50 PM, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Apr 21, 2020 at 6:04 PM Michal Privoznik <mprivozn at redhat.com> wrote:
>>
>> On 4/8/20 7:23 PM, marcandre.lureau at redhat.com wrote:
>>> From: Marc-André Lureau <marcandre.lureau at redhat.com>
>>>
>>> Don't stop the DBus daemon if a slirp helper failed to start, as it
>>> may be shared with other helpers.
>>>
>>> Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>
>>> ---
>>>    src/qemu/qemu_slirp.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/src/qemu/qemu_slirp.c b/src/qemu/qemu_slirp.c
>>> index 09c1247892..49bffa01b8 100644
>>> --- a/src/qemu/qemu_slirp.c
>>> +++ b/src/qemu/qemu_slirp.c
>>> @@ -355,6 +355,6 @@ qemuSlirpStart(qemuSlirpPtr slirp,
>>>            virProcessKillPainfully(pid, true);
>>>        if (pidfile)
>>>            unlink(pidfile);
>>> -    qemuDBusStop(driver, vm);
>>> +    /* leave dbus daemon running, it may be used by others */
>>>        return -1;
>>>    }
>>>
>>
>> I'm not quite sure about this one. Who do you mean by "others"? Other
> 
> Other users of DBus. For now, it's only slirp-helper, but there can be
> already multiple instances.

Ah, I've misunderstood this part. But if there is only one user of the 
DBus (us who are trying to start it now), shouldn't we kill the dbus 
daemon? On the other hand, it will be done by qemuProcessStop() 
eventually. My idea was to track whether the dbus daemon is running 
prior starting it (basically save priv->dbusDaemonRunning before calling 
qemuDBusStart() and then wrap this qemuDBusStop() with 'if 
(was_started)'. I can do the change before push, if you are okay with 
it. I just like functions to tidy up on failure, maybe it's just an 
obsession though.

Michal