Re: [libvirt PATCH 1/6] slirp: leave the dbus daemon running on error
On 4/21/20 6:50 PM, Marc-André Lureau wrote:
Hi
On Tue, Apr 21, 2020 at 6:04 PM Michal Privoznik <mprivozn(a)redhat.com> wrote:
>
> On 4/8/20 7:23 PM, marcandre.lureau(a)redhat.com wrote:
>> From: Marc-André Lureau <marcandre.lureau(a)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(a)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