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

2017-10-27 Thread Michal Privoznik
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

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


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

2017-10-27 Thread Daniel P. Berrange
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 :|

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


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

2017-10-27 Thread Michal Privoznik
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

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


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

2017-10-26 Thread Caoxinhua
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:mpriv...@redhat.com] 
发送时间: 2017年10月27日 0:17
收件人: Caoxinhua; libvir-list@redhat.com; jfer...@redhat.com; 
mklet...@redhat.com; berra...@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(>libDir, "%s/domain-%s", cfg->libDir, domname) < 0)
> +virAsprintf(>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

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