Commit 0ba9afc6b28 introduced a logic bug where we will never save the
inactive XML after a blockjob as the variable which was determining
whether to do so is cleared right before. Thus even if we correctly
modify the inactive state it will be rolled back when libvirtd is
restarted.

Reported-by: Thomas Stein <hello@himbee.re>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/qemu/qemu_blockjob.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index fa7e4c8625..f105632a09 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -363,7 +363,7 @@ qemuBlockJobEventProcessLegacy(virQEMUDriverPtr driver,
     if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0)
         VIR_WARN("Unable to save status on vm %s after block job", vm->def->name);

-    if (job->newstate == VIR_DOMAIN_BLOCK_JOB_COMPLETED && vm->newDef) {
+    if (job->state == VIR_DOMAIN_BLOCK_JOB_COMPLETED && vm->newDef) {
         if (virDomainSaveConfig(cfg->configDir, driver->caps, vm->newDef) < 0)
             VIR_WARN("Unable to update persistent definition on vm %s "
                      "after block job", vm->def->name);
-- 
2.21.0

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

On Fri, May 17, 2019 at 10:22:16AM +0200, Peter Krempa wrote:
>Commit 0ba9afc6b28 introduced a logic bug where we will never save the

On second glance, that commit still looks okay to me logic-wise.
Commit c257352797 seems to be the culprit here.

>inactive XML after a blockjob as the variable which was determining
>whether to do so is cleared right before. Thus even if we correctly
>modify the inactive state it will be rolled back when libvirtd is
>restarted.
>
>Reported-by: Thomas Stein <hello@himbee.re>
>Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>---
> src/qemu/qemu_blockjob.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

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

Thank you Peter.

On 2019-05-17 10:22, Peter Krempa wrote:
> Commit 0ba9afc6b28 introduced a logic bug where we will never save the
> inactive XML after a blockjob as the variable which was determining
> whether to do so is cleared right before. Thus even if we correctly
> modify the inactive state it will be rolled back when libvirtd is
> restarted.
> 
> Reported-by: Thomas Stein <hello@himbee.re>
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  src/qemu/qemu_blockjob.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
> index fa7e4c8625..f105632a09 100644
> --- a/src/qemu/qemu_blockjob.c
> +++ b/src/qemu/qemu_blockjob.c
> @@ -363,7 +363,7 @@ qemuBlockJobEventProcessLegacy(virQEMUDriverPtr 
> driver,
>      if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm,
> driver->caps) < 0)
>          VIR_WARN("Unable to save status on vm %s after block job",
> vm->def->name);
> 
> -    if (job->newstate == VIR_DOMAIN_BLOCK_JOB_COMPLETED && vm->newDef) 
> {
> +    if (job->state == VIR_DOMAIN_BLOCK_JOB_COMPLETED && vm->newDef) {
>          if (virDomainSaveConfig(cfg->configDir, driver->caps, 
> vm->newDef) < 0)
>              VIR_WARN("Unable to update persistent definition on vm %s 
> "
>                       "after block job", vm->def->name);

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