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

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/lib/libvirt/qemu/domain-*** directory will be left over at system cold reset. --- src/qemu/qemu_domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ed27a91..3e6fe9b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -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

On 10/16/2017 04:08 AM, 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/lib/libvirt/qemu/domain-*** directory will be left over at system cold reset. --- src/qemu/qemu_domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ed27a91..3e6fe9b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -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 &&
Almost. I see a problem with this patch. Problem is that qemuxml2argvtest needs to be updated. Which is not trivial because state dir is a tempdir (mkdtemp()), and thus '-monitor' part of the command line changes with each test invocation. Michal

Yes,In qemuxml2argvtest case, mymain call qemuTestDriverInit to init qemu driver, qemuTestDriverInit call mkdtemp() to init stateDir, so stateDir is randomly. But '-monitor' part of the command line must be a const value. Can I use a const value "/tmp/lib" to instead of random value at qemuxml2argvtest? -----邮件原件----- 发件人: Michal Privoznik [mailto:mprivozn@redhat.com] 发送时间: 2017年10月27日 0:17 收件人: Caoxinhua; libvir-list@redhat.com; jferlan@redhat.com; mkletzan@redhat.com; berrange@redhat.com 抄送: Yanqiangjun; Huangweidong (C); Wangjing (King, Euler); weifuqiang 主题: Re: [libvirt] [PATCH v2] qemu: change monitor.sock from /var/lib/libvirt/qemu/domain-*** to /var/run/libvirt/qemu/domain-*** On 10/16/2017 04:08 AM, 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/lib/libvirt/qemu/domain-*** directory will be left over at system cold reset. --- src/qemu/qemu_domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ed27a91..3e6fe9b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -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 &&
Almost. I see a problem with this patch. Problem is that qemuxml2argvtest needs to be updated. Which is not trivial because state dir is a tempdir (mkdtemp()), and thus '-monitor' part of the command line changes with each test invocation. Michal

On 10/27/2017 03:47 AM, Caoxinhua wrote:
Yes,In qemuxml2argvtest case, mymain call qemuTestDriverInit to init qemu driver, qemuTestDriverInit call mkdtemp() to init stateDir, so stateDir is randomly. But '-monitor' part of the command line must be a const value. Can I use a const value "/tmp/lib" to instead of random value at qemuxml2argvtest?
We will have to. Looks like we don't need stateDir nor configDir in the test anyway. Michal

On Fri, Oct 27, 2017 at 10:25:23AM +0200, Michal Privoznik wrote:
On 10/27/2017 03:47 AM, Caoxinhua wrote:
Yes,In qemuxml2argvtest case, mymain call qemuTestDriverInit to init qemu driver, qemuTestDriverInit call mkdtemp() to init stateDir, so stateDir is randomly. But '-monitor' part of the command line must be a const value. Can I use a const value "/tmp/lib" to instead of random value at qemuxml2argvtest?
We will have to. Looks like we don't need stateDir nor configDir in the test anyway.
I would suggest '/does-not-exist' so we see a hard fail if we accidentally have something in the test suite that tries to create a file in that dir. Using fixed filenames in /tmp is a big no-no in general. 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 :|

On 10/27/2017 10:57 AM, Daniel P. Berrange wrote:
On Fri, Oct 27, 2017 at 10:25:23AM +0200, Michal Privoznik wrote:
On 10/27/2017 03:47 AM, Caoxinhua wrote:
Yes,In qemuxml2argvtest case, mymain call qemuTestDriverInit to init qemu driver, qemuTestDriverInit call mkdtemp() to init stateDir, so stateDir is randomly. But '-monitor' part of the command line must be a const value. Can I use a const value "/tmp/lib" to instead of random value at qemuxml2argvtest?
We will have to. Looks like we don't need stateDir nor configDir in the test anyway.
I would suggest '/does-not-exist' so we see a hard fail if we accidentally have something in the test suite that tries to create a file in that dir. Using fixed filenames in /tmp is a big no-no in general.
Well, we already use /tmp/lib/ for the monitor path. So sticking with it means we don't have to update all of those ~600 files. Moreover, we don't really create the dirs/files. But sure, having /does-not-exist may be more robust. Michal
participants (4)
-
Caoxinhua
-
Daniel P. Berrange
-
Michal Privoznik
-
xinhua.Cao