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

[libvirt] [PATCH v3] 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/lib/libvirt/qemu/domain-*** directory will be left over at system cold reset.
---
 src/qemu/qemu_domain.c   | 2 +-
 tests/qemuxml2argvtest.c | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 13e77ee..67da8fa 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1713,7 +1713,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 &&
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 03b1bcb..b596bd2 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -620,6 +620,10 @@ mymain(void)
     if (VIR_STRDUP_QUIET(driver.config->memoryBackingDir, "/var/lib/libvirt/qemu/ram") < 0)
         return EXIT_FAILURE;
 
+    VIR_FREE(driver.config->stateDir);
+    if (VIR_STRDUP(driver.config->stateDir, "/tmp/lib") < 0)
+        return EXIT_FAILURE;
+
 # define DO_TEST_FULL(name, migrateFrom, migrateFd, flags,               \
                       parseFlags, gic, ...)                              \
     do {                                                                 \
-- 
2.8.3


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

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

Posted by John Ferlan 6 years, 4 months ago

On 10/27/2017 09:54 PM, 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 +-
>  tests/qemuxml2argvtest.c | 4 ++++
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 

[Looking through my older/unreviewed patch backlog...]

Still something that makes me nervous about this adjustment.

There's already qemuDomainSetPrivatePathsOld to handle finding 'long'
domain names instead of the shortened ones during
qemuDomainObjPrivateXMLParse. I have this recollection of "issues"
cropping up when we changed from long to short domain name that I
wouldn't want to repeat. Might be worth going back in time via git
history searching and trying to compare with patches made (in the event
Martin has completely blocked the memory or forgotten like I have).

If we change to using cfg->stateDir instead of cfg->libDir, then how
would we determine that something "used to" exist in cfg->libDir that we
may need or want to use?

Does PathsOld change to PathsReallyOld?  or perhaps does PathsOld get
augmented to build up libDir using cfg->libDir?   Of course that could
mean checking which to use and trying to make a decision based on that.

> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 13e77ee..67da8fa 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -1713,7 +1713,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 &&
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index 03b1bcb..b596bd2 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -620,6 +620,10 @@ mymain(void)
>      if (VIR_STRDUP_QUIET(driver.config->memoryBackingDir, "/var/lib/libvirt/qemu/ram") < 0)
>          return EXIT_FAILURE;
>  
> +    VIR_FREE(driver.config->stateDir);
> +    if (VIR_STRDUP(driver.config->stateDir, "/tmp/lib") < 0)
> +        return EXIT_FAILURE;
> +

I liked the idea of using "/does-not-exist" here too, but I know it
doesn't work unless all the .args file change.

Maybe we need to develop a way to allow certain path output to change on
the .args to not cause failure for the test just because of it.

John

>  # define DO_TEST_FULL(name, migrateFrom, migrateFd, flags,               \
>                        parseFlags, gic, ...)                              \
>      do {                                                                 \
> 

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

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

Posted by Martin Kletzander 6 years, 4 months ago

[Sorry for replying later, my previous mail got lost, so resending now]

On Thu, Dec 07, 2017 at 06:29:02PM -0500, John Ferlan wrote:
>
>
>On 10/27/2017 09:54 PM, 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 +-
>>  tests/qemuxml2argvtest.c | 4 ++++
>>  2 files changed, 5 insertions(+), 1 deletion(-)
>>
>
>[Looking through my older/unreviewed patch backlog...]
>
>Still something that makes me nervous about this adjustment.
>
>There's already qemuDomainSetPrivatePathsOld to handle finding 'long'
>domain names instead of the shortened ones during
>qemuDomainObjPrivateXMLParse. I have this recollection of "issues"
>cropping up when we changed from long to short domain name that I
>wouldn't want to repeat. Might be worth going back in time via git
>history searching and trying to compare with patches made (in the event
>Martin has completely blocked the memory or forgotten like I have).
>

When I did the switch, I also made sure that we saved the new paths in
the state XML. So we should never need another *Old() function.  I
covered what was needed, but I'm not sure all the paths are covered by
this.  also stuff might have changed. Also, I'm in the middle of a
presentation on a conference, so this is just a quick reply to get you
moving, sorry for no deeper info, I can't recall the rest immediately
now.

>If we change to using cfg->stateDir instead of cfg->libDir, then how
>would we determine that something "used to" exist in cfg->libDir that we
>may need or want to use?
>
>Does PathsOld change to PathsReallyOld?  or perhaps does PathsOld get
>augmented to build up libDir using cfg->libDir?   Of course that could
>mean checking which to use and trying to make a decision based on that.
>
>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>> index 13e77ee..67da8fa 100644
>> --- a/src/qemu/qemu_domain.c
>> +++ b/src/qemu/qemu_domain.c
>> @@ -1713,7 +1713,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 &&
>> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
>> index 03b1bcb..b596bd2 100644
>> --- a/tests/qemuxml2argvtest.c
>> +++ b/tests/qemuxml2argvtest.c
>> @@ -620,6 +620,10 @@ mymain(void)
>>      if (VIR_STRDUP_QUIET(driver.config->memoryBackingDir, "/var/lib/libvirt/qemu/ram") < 0)
>>          return EXIT_FAILURE;
>>
>> +    VIR_FREE(driver.config->stateDir);
>> +    if (VIR_STRDUP(driver.config->stateDir, "/tmp/lib") < 0)
>> +        return EXIT_FAILURE;
>> +
>
>I liked the idea of using "/does-not-exist" here too, but I know it
>doesn't work unless all the .args file change.
>
>Maybe we need to develop a way to allow certain path output to change on
>the .args to not cause failure for the test just because of it.
>
>John
>
>>  # define DO_TEST_FULL(name, migrateFrom, migrateFd, flags,               \
>>                        parseFlags, gic, ...)                              \
>>      do {                                                                 \
>>
>
>--
>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