[PATCH 3/8] qemu-conf: add dbusStateDir

[Laine Stump]

On 2/25/20 6:24 AM, Michal Privoznik wrote:
> On 2/24/20 4:57 PM, Marc-André Lureau wrote:
>> Hi
>>
>> On Thu, Feb 20, 2020 at 10:04 AM Michal Privoznik 
>> <mprivozn at redhat.com> wrote:
>>>
>>> On 1/14/20 2:46 PM, marcandre.lureau at redhat.com wrote:
>>>> From: Marc-André Lureau <marcandre.lureau at redhat.com>
>>>>
>>>> Location of DBus daemon state configuration, socket, pid...
>>>>
>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>
>>>> ---
>>>>    src/qemu/qemu_conf.c | 4 ++++
>>>>    src/qemu/qemu_conf.h | 1 +
>>>>    2 files changed, 5 insertions(+)
>>>>
>>>> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
>>>> index e1fea390c7..ade12dac6c 100644
>>>> --- a/src/qemu/qemu_conf.c
>>>> +++ b/src/qemu/qemu_conf.c
>>>> @@ -144,6 +144,8 @@ virQEMUDriverConfigPtr 
>>>> virQEMUDriverConfigNew(bool privileged)
>>>>
>>>>            cfg->cacheDir = g_strdup_printf("%s/cache/libvirt/qemu", 
>>>> LOCALSTATEDIR);
>>>>
>>>> +        cfg->dbusStateDir = 
>>>> g_strdup_printf("%s/run/libvirt/qemu/dbus", LOCALSTATEDIR);
>>>> +
>>>>            cfg->libDir = g_strdup_printf("%s/lib/libvirt/qemu", 
>>>> LOCALSTATEDIR);
>>>>            cfg->saveDir = g_strdup_printf("%s/save", cfg->libDir);
>>>>            cfg->snapshotDir = g_strdup_printf("%s/snapshot", 
>>>> cfg->libDir);
>>>> @@ -174,6 +176,7 @@ virQEMUDriverConfigPtr 
>>>> virQEMUDriverConfigNew(bool privileged)
>>>>            cfg->stateDir = g_strdup_printf("%s/qemu/run", rundir);
>>>>
>>>>            cfg->swtpmStateDir = g_strdup_printf("%s/swtpm", 
>>>> cfg->stateDir);
>>>> +        cfg->dbusStateDir = g_strdup_printf("%s/dbus", 
>>>> cfg->stateDir);
>>>>
>>>>            cfg->configBaseDir = virGetUserConfigDirectory();
>>>
>>> Instead of doing practically the same in two branches, you can achieve
>>> the same result with just one line if you put the call just below
>>> cfg->slirpStateDir init.
>>>
>>> However, do we need this to be in a special directory instead of per VM
>>> private directory? The way I implemented PR helper was that the socket
>>> it creates and to which qemu connects is stored under vm->priv->libDir
>>> which is initialized in qemuDomainSetPrivatePaths() to:
>>>
>>>     $cfg->libDir/domain-$shortName/
>>>
>>> This way you wouldn't need to care about domain's shortname in the next
>>> patch.
>>
>> Makes sense. I think I based this on slirpStateDir code. Any reason
>> it's not using vm->priv->libdir too?
>>
>
> I don't know. But since there are some releases which would store data 
> under slirpStateDir I don't think we can change this. On daemon 
> restart (with newer version) the new libvirtd wouldn't see the old dir.


It would need to be done carefully, but it's not out of the question: 
There was previously a case where we had been storing the network state 
files in an incorrect location (/var/lib/libvirt) (starting in commit 
23a090ab92 - libvirt-0.6.0 in 2009), and moved them to a more proper 
location (/var/run/libvirt) (commit b9e95491d1 - libvirt 1.2.4 in 2014). 
At the time the move was made, we also added a code to 
networkStateInitialize() that looked for files in the old location and 
moved them to the new location. This code remained in place, at first 
doing its job whenever someone upgraded, and then silently doing nothing 
for a very long time, until 5 years later when it was removed in commit 
43be65a4817f - libvirt 5.1.0. (probably could have been removed much 
earlier than that, but it had just been forgotten.)