[libvirt] [PATCH] qemu: change monitor.sock from /var/lib/libvirt/qemu/domain-*** to /var/run/libvirt/qemu/domain-***

xinhua.Cao posted 1 patch 6 years, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20171013033652.7292-1-caoxinhua@huawei.com
There is a newer version of this series
src/qemu/qemu_domain.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[libvirt] [PATCH] qemu: change monitor.sock from /var/lib/libvirt/qemu/domain-*** to /var/run/libvirt/qemu/domain-***
Posted by xinhua.Cao 6 years, 6 months ago
directory /var/lib alway is Persistence directory, but in redhat system, /var/run is memory directory.
our running domain xml is saved at /var/run/libvirt/qemu. so if we cold reset system,
the /var/run/libvirt/qemu directory is clear, but /var/lib/libvirt/qemu/domain-*** is saved. so there
have same /var/run/libvirt/qemu/domain-*** directory will be left over at system cold reset.
---
 src/qemu/qemu_domain.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ed27a91..1b42ae5 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1646,7 +1646,7 @@ qemuDomainSetPrivatePathsOld(virQEMUDriverPtr driver,
 
     if (!priv->libDir &&
         virAsprintf(&priv->libDir, "%s/domain-%s",
-                    cfg->libDir, vm->def->name) < 0)
+                    cfg->stateDir, vm->def->name) < 0)
         goto cleanup;
 
     if (!priv->channelTargetDir &&
@@ -1674,7 +1674,7 @@ qemuDomainSetPrivatePaths(virQEMUDriverPtr driver,
         goto cleanup;
 
     if (!priv->libDir &&
-        virAsprintf(&priv->libDir, "%s/domain-%s", cfg->libDir, domname) < 0)
+        virAsprintf(&priv->libDir, "%s/domain-%s", cfg->stateDir, domname) < 0)
         goto cleanup;
 
     if (!priv->channelTargetDir &&
-- 
2.8.3


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: change monitor.sock from /var/lib/libvirt/qemu/domain-*** to /var/run/libvirt/qemu/domain-***
Posted by Martin Kletzander 6 years, 6 months ago
On Fri, Oct 13, 2017 at 11:36:52AM +0800, xinhua.Cao wrote:
>directory /var/lib alway is Persistence directory, but in redhat system, /var/run is memory directory.
>our running domain xml is saved at /var/run/libvirt/qemu. so if we cold reset system,
>the /var/run/libvirt/qemu directory is clear, but /var/lib/libvirt/qemu/domain-*** is saved. so there
>have same /var/run/libvirt/qemu/domain-*** directory will be left over at system cold reset.

Is that he only problem?  I know it sounds right to have that in a
runtime directory, but for me it would also make sense to have that in a
persistent directory.  Either this or any other file that suggests we
did not clean up a domain yet.  I'm not sure we are currently doing it
or that it's a reason for why libDir was chosen, but it might have been.

>---
> src/qemu/qemu_domain.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>index ed27a91..1b42ae5 100644
>--- a/src/qemu/qemu_domain.c
>+++ b/src/qemu/qemu_domain.c
>@@ -1646,7 +1646,7 @@ qemuDomainSetPrivatePathsOld(virQEMUDriverPtr driver,
>
>     if (!priv->libDir &&
>         virAsprintf(&priv->libDir, "%s/domain-%s",
>-                    cfg->libDir, vm->def->name) < 0)
>+                    cfg->stateDir, vm->def->name) < 0)

This is compatibility function that should be left alone so that we
still guess the paths correctly for already launched qemu processes.

>         goto cleanup;
>
>     if (!priv->channelTargetDir &&
>@@ -1674,7 +1674,7 @@ qemuDomainSetPrivatePaths(virQEMUDriverPtr driver,
>         goto cleanup;
>
>     if (!priv->libDir &&
>-        virAsprintf(&priv->libDir, "%s/domain-%s", cfg->libDir, domname) < 0)
>+        virAsprintf(&priv->libDir, "%s/domain-%s", cfg->stateDir, domname) < 0)

More so, in both chunks here you are setting the domain's libDir which
is used for more things than just the monitor.  And, similarly to why
the function above exists, you need to make sure we don't lose the track
of some path just in case we did not save them anywhere in order for
upgrades to go smoothly.  Since this one has two versions, I believe I
managed to save this into the stateXML, but I can't say for sure from
the top of my head.

>         goto cleanup;
>
>     if (!priv->channelTargetDir &&
>-- 
>2.8.3
>
>
>--
>libvir-list mailing list
>libvir-list@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: change monitor.sock from /var/lib/libvirt/qemu/domain-*** to /var/run/libvirt/qemu/domain-***
Posted by Daniel P. Berrange 6 years, 6 months ago
On Fri, Oct 13, 2017 at 09:21:45AM +0200, Martin Kletzander wrote:
> On Fri, Oct 13, 2017 at 11:36:52AM +0800, xinhua.Cao wrote:
> > directory /var/lib alway is Persistence directory, but in redhat system, /var/run is memory directory.
> > our running domain xml is saved at /var/run/libvirt/qemu. so if we cold reset system,
> > the /var/run/libvirt/qemu directory is clear, but /var/lib/libvirt/qemu/domain-*** is saved. so there
> > have same /var/run/libvirt/qemu/domain-*** directory will be left over at system cold reset.
> 
> Is that he only problem?  I know it sounds right to have that in a
> runtime directory, but for me it would also make sense to have that in a
> persistent directory.  Either this or any other file that suggests we
> did not clean up a domain yet.  I'm not sure we are currently doing it
> or that it's a reason for why libDir was chosen, but it might have been.

libDir is almost certainly just a historical accident. Conceptually runDir
makes more sense since this requires no storage across reboots.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] 答复: [PATCH] qemu: change monitor.sock from /var/lib/libvirt/qemu/domain-*** to /var/run/libvirt/qemu/domain-***
Posted by Caoxinhua 6 years, 6 months ago
I also want to change priv->libDir name to priv->runDir. But this might relate a lot of file change.
Currently priv->libDir only save monitor.sock and master-key.aes. All those file need clear at system reset.
So it make sense to change priv->libDir from /var/lib/libvirt/qemu to /var/run/libvirt/qemu

-----邮件原件-----
发件人: Daniel P. Berrange [mailto:berrange@redhat.com] 
发送时间: 2017年10月13日 16:52
收件人: Martin Kletzander
抄送: Caoxinhua; Huangweidong (C); weifuqiang; libvir-list@redhat.com; Yanqiangjun; Wangjing (King, Euler)
主题: Re: [libvirt] [PATCH] qemu: change monitor.sock from /var/lib/libvirt/qemu/domain-*** to /var/run/libvirt/qemu/domain-***

On Fri, Oct 13, 2017 at 09:21:45AM +0200, Martin Kletzander wrote:
> On Fri, Oct 13, 2017 at 11:36:52AM +0800, xinhua.Cao wrote:
> > directory /var/lib alway is Persistence directory, but in redhat system, /var/run is memory directory.
> > our running domain xml is saved at /var/run/libvirt/qemu. so if we 
> > cold reset system, the /var/run/libvirt/qemu directory is clear, but 
> > /var/lib/libvirt/qemu/domain-*** is saved. so there have same /var/run/libvirt/qemu/domain-*** directory will be left over at system cold reset.
> 
> Is that he only problem?  I know it sounds right to have that in a 
> runtime directory, but for me it would also make sense to have that in 
> a persistent directory.  Either this or any other file that suggests 
> we did not clean up a domain yet.  I'm not sure we are currently doing 
> it or that it's a reason for why libDir was chosen, but it might have been.

libDir is almost certainly just a historical accident. Conceptually runDir makes more sense since this requires no storage across reboots.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt]答复: [PATCH] qemu: change monitor.sock from /var/lib/libvirt/qemu/domain-*** to /var/run/libvirt/qemu/domain-***
Posted by Martin Kletzander 6 years, 6 months ago
On Fri, Oct 13, 2017 at 08:56:30AM +0000, Caoxinhua wrote:
>I also want to change priv->libDir name to priv->runDir. But this might relate a lot of file change.
>Currently priv->libDir only save monitor.sock and master-key.aes. All those file need clear at system reset.
>So it make sense to change priv->libDir from /var/lib/libvirt/qemu to /var/run/libvirt/qemu
>

Ah, top-posting...  So OK, we can change this.  But if it is used only
for master.aes and monitor.sock, then there should not be that many
places to be changed.  It is also super-easy to find mistakes by just
building libvirt.

Still, the point about qemuSetPrivatePathsOld() stands, that must not
change due to back-compatibility.

>-----邮件原件-----
>发件人: Daniel P. Berrange [mailto:berrange@redhat.com]
>发送时间: 2017年10月13日 16:52
>收件人: Martin Kletzander
>抄送: Caoxinhua; Huangweidong (C); weifuqiang; libvir-list@redhat.com; Yanqiangjun; Wangjing (King, Euler)
>主题: Re: [libvirt] [PATCH] qemu: change monitor.sock from /var/lib/libvirt/qemu/domain-*** to /var/run/libvirt/qemu/domain-***
>
>On Fri, Oct 13, 2017 at 09:21:45AM +0200, Martin Kletzander wrote:
>> On Fri, Oct 13, 2017 at 11:36:52AM +0800, xinhua.Cao wrote:
>> > directory /var/lib alway is Persistence directory, but in redhat system, /var/run is memory directory.
>> > our running domain xml is saved at /var/run/libvirt/qemu. so if we
>> > cold reset system, the /var/run/libvirt/qemu directory is clear, but
>> > /var/lib/libvirt/qemu/domain-*** is saved. so there have same /var/run/libvirt/qemu/domain-*** directory will be left over at system cold reset.
>>
>> Is that he only problem?  I know it sounds right to have that in a
>> runtime directory, but for me it would also make sense to have that in
>> a persistent directory.  Either this or any other file that suggests
>> we did not clean up a domain yet.  I'm not sure we are currently doing
>> it or that it's a reason for why libDir was chosen, but it might have been.
>
>libDir is almost certainly just a historical accident. Conceptually runDir makes more sense since this requires no storage across reboots.
>
>Regards,
>Daniel
>-- 
>|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
>|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
>|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] 答复: 答复: [PATCH] qemu: change monitor.sock from /var/lib/libvirt/qemu/domain-*** to /var/run/libvirt/qemu/domain-***
Posted by Caoxinhua 6 years, 6 months ago
Ah, top-posting...  So OK, we can change this.  But if it is used only for master.aes and monitor.sock, then there should not be that many places to be changed.  It is also super-easy to find mistakes by just building libvirt.

Still, the point about qemuSetPrivatePathsOld() stands, that must not change due to back-compatibility.
> Ah , you are right. Old libvirt version doesn't save libDir at runtime domain xml. so that musn't change at function qemuSetPrivatePathsOld. 

-----邮件原件-----
发件人: Martin Kletzander [mailto:mkletzan@redhat.com] 
发送时间: 2017年10月13日 18:24
收件人: Caoxinhua
抄送: Daniel P. Berrange; Huangweidong (C); weifuqiang; libvir-list@redhat.com; Yanqiangjun; Wangjing (King, Euler)
主题: Re: 答复: [libvirt] [PATCH] qemu: change monitor.sock from /var/lib/libvirt/qemu/domain-*** to /var/run/libvirt/qemu/domain-***

On Fri, Oct 13, 2017 at 08:56:30AM +0000, Caoxinhua wrote:
>I also want to change priv->libDir name to priv->runDir. But this might relate a lot of file change.
>Currently priv->libDir only save monitor.sock and master-key.aes. All those file need clear at system reset.
>So it make sense to change priv->libDir from /var/lib/libvirt/qemu to 
>/var/run/libvirt/qemu
>

Ah, top-posting...  So OK, we can change this.  But if it is used only for master.aes and monitor.sock, then there should not be that many places to be changed.  It is also super-easy to find mistakes by just building libvirt.

Still, the point about qemuSetPrivatePathsOld() stands, that must not change due to back-compatibility.

>-----邮件原件-----
>发件人: Daniel P. Berrange [mailto:berrange@redhat.com]
>发送时间: 2017年10月13日 16:52
>收件人: Martin Kletzander
>抄送: Caoxinhua; Huangweidong (C); weifuqiang; libvir-list@redhat.com; 
>Yanqiangjun; Wangjing (King, Euler)
>主题: Re: [libvirt] [PATCH] qemu: change monitor.sock from 
>/var/lib/libvirt/qemu/domain-*** to /var/run/libvirt/qemu/domain-***
>
>On Fri, Oct 13, 2017 at 09:21:45AM +0200, Martin Kletzander wrote:
>> On Fri, Oct 13, 2017 at 11:36:52AM +0800, xinhua.Cao wrote:
>> > directory /var/lib alway is Persistence directory, but in redhat system, /var/run is memory directory.
>> > our running domain xml is saved at /var/run/libvirt/qemu. so if we 
>> > cold reset system, the /var/run/libvirt/qemu directory is clear, 
>> > but
>> > /var/lib/libvirt/qemu/domain-*** is saved. so there have same /var/run/libvirt/qemu/domain-*** directory will be left over at system cold reset.
>>
>> Is that he only problem?  I know it sounds right to have that in a 
>> runtime directory, but for me it would also make sense to have that 
>> in a persistent directory.  Either this or any other file that 
>> suggests we did not clean up a domain yet.  I'm not sure we are 
>> currently doing it or that it's a reason for why libDir was chosen, but it might have been.
>
>libDir is almost certainly just a historical accident. Conceptually runDir makes more sense since this requires no storage across reboots.
>
>Regards,
>Daniel
>--
>|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
>|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
>|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list