[libvirt] [PATCH] qemu: Update a domain's persistent definition file if it's defined

Wang Yechao posted 1 patch 5 years, 4 months ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1545296773-11773-1-git-send-email-wang.yechao255@zte.com.cn
src/qemu/qemu_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[libvirt] [PATCH] qemu: Update a domain's persistent definition file if it's defined
Posted by Wang Yechao 5 years, 4 months ago
From: Li XueLei <li.xuelei1@zte.com.cn>

During making disk snapshot in an active domain, sometimes we
should update the domain's persistent definition.We must check if
the domain is defined, before we update the persistent definition
file.
First,we create a vm.Then,define the vm and undefine the vm.Last create a 
--memspec snapshot for the vm.Now,we will find a persistent definition
file in the config directory if we don't use this patch.

Signed-off-by: Li XueLei <li.xuelei1@zte.com.cn>
---
 src/qemu/qemu_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e32257f..94dcb97 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15330,7 +15330,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver,
 
     if (ret == 0 || !do_transaction) {
         if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0 ||
-            (persist && virDomainSaveConfig(cfg->configDir, driver->caps,
+            (vm->persistent && persist && virDomainSaveConfig(cfg->configDir, driver->caps,
                                             vm->newDef) < 0))
             ret = -1;
     }
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Update a domain's persistent definition file if it's defined
Posted by John Ferlan 5 years, 3 months ago

On 12/20/18 4:06 AM, Wang Yechao wrote:
> From: Li XueLei <li.xuelei1@zte.com.cn>
> 
> During making disk snapshot in an active domain, sometimes we
> should update the domain's persistent definition.We must check if
> the domain is defined, before we update the persistent definition
> file.
> First,we create a vm.Then,define the vm and undefine the vm.Last create a 
> --memspec snapshot for the vm.Now,we will find a persistent definition
> file in the config directory if we don't use this patch.

The formatting is all off, rewritten to make it a bit easier:

During making disk snapshot in an active domain, sometimes we should
update the domain's persistent definition.

We must check if the domain is defined, before we update the persistent
definition file.

First, we create a vm.

Then, define the vm and undefine the vm.

Last create a --memspec snapshot for the vm.

Now, we will find a persistent definition file in the config directory
if we don't use this patch.

...

So I think what you're trying to point out is there is an edge condition
whereby if an active domain is undefined, then it's possible that the
qemuDomainSnapshotCreateDiskActive would call virDomainSaveConfig thus
recreating the domain.

Is that about right?

> 
> Signed-off-by: Li XueLei <li.xuelei1@zte.com.cn>
> ---
>  src/qemu/qemu_driver.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index e32257f..94dcb97 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -15330,7 +15330,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver,
>  
>      if (ret == 0 || !do_transaction) {
>          if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0 ||
> -            (persist && virDomainSaveConfig(cfg->configDir, driver->caps,
> +            (vm->persistent && persist && virDomainSaveConfig(cfg->configDir, driver->caps,
>                                              vm->newDef) < 0))

Wrong indention here. The vm->newDef would need to align under the cfg->
or you could put the virDomainSaveConfig on the next line aligned under
the vm->persistent.

I'll need to look a bit more or hope that perhaps Peter sees this and
has some quick advice.

John

>              ret = -1;
>      }
> 

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