Start a domain whilst undefine it, if starting failed duing ProcessLaunch,
on which period qemu exited unexpectedly, the operation will lead to failure
of undefine the domain until libvirtd restarted. The reason is that libvirtd
will unlock vm during qemuProcessStart, qemuDomainUndefineFlags can get the
lock and set vm->persistent 0 but not remove the "active" domain.

Signed-off-by: Yi Wang <wang.yi59@zte.com.cn>
---
 src/conf/domain_conf.h  | 1 +
 src/qemu/qemu_driver.c  | 6 ++++++
 src/qemu/qemu_process.c | 3 +++
 3 files changed, 10 insertions(+)

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index af15ee8..f339f84 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2468,6 +2468,7 @@ struct _virDomainObj {
     virDomainSnapshotObjPtr current_snapshot;
 
     bool hasManagedSave;
+    bool starting;
 
     void *privateData;
     void (*privateDataFreeFunc)(void *);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6568def..5d9acfc 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7317,6 +7317,12 @@ qemuDomainUndefineFlags(virDomainPtr dom,
     if (!(vm = qemuDomObjFromDomain(dom)))
         return -1;
 
+    if (vm->starting) {
+        virReportError(VIR_ERR_OPERATION_INVALID,
+                       "%s", _("cannot undefine during domain starting up"));
+        goto cleanup;
+    }
+
     cfg = virQEMUDriverGetConfig(driver);
 
     if (virDomainUndefineFlagsEnsureACL(dom->conn, vm->def) < 0)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 525521a..7b708be 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5847,6 +5847,8 @@ qemuProcessStart(virConnectPtr conn,
     if (!migrateFrom && !snapshot)
         flags |= VIR_QEMU_PROCESS_START_NEW;
 
+    vm->starting = true;
+
     if (qemuProcessInit(driver, vm, updatedCPU,
                         asyncJob, !!migrateFrom, flags) < 0)
         goto cleanup;
@@ -5892,6 +5894,7 @@ qemuProcessStart(virConnectPtr conn,
     ret = 0;
 
  cleanup:
+    vm->starting = false;
     qemuProcessIncomingDefFree(incoming);
     return ret;
 
-- 
1.8.3.1


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

On Sat, Jul 22, 2017 at 04:55:49 -0400, Yi Wang wrote:
> Start a domain whilst undefine it, if starting failed duing ProcessLaunch,
> on which period qemu exited unexpectedly, the operation will lead to failure
> of undefine the domain until libvirtd restarted. The reason is that libvirtd
> will unlock vm during qemuProcessStart, qemuDomainUndefineFlags can get the
> lock and set vm->persistent 0 but not remove the "active" domain.

Shouldn't the startup code handle that? It definitely works when
starting a transient domain, so making it transient while the startup
code is executed should be the same case.

Since we copy the definition prior to startup, there really should not
be any problem in making the VM transient while it's being started.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 07/24/2017 04:06 AM, Peter Krempa wrote:
> On Sat, Jul 22, 2017 at 04:55:49 -0400, Yi Wang wrote:
>> Start a domain whilst undefine it, if starting failed duing ProcessLaunch,
>> on which period qemu exited unexpectedly, the operation will lead to failure
>> of undefine the domain until libvirtd restarted. The reason is that libvirtd
>> will unlock vm during qemuProcessStart, qemuDomainUndefineFlags can get the
>> lock and set vm->persistent 0 but not remove the "active" domain.
> 
> Shouldn't the startup code handle that? It definitely works when
> starting a transient domain, so making it transient while the startup
> code is executed should be the same case.
> 
> Since we copy the definition prior to startup, there really should not
> be any problem in making the VM transient while it's being started.
> 
> 

FWIW:

This patch started as:

https://www.redhat.com/archives/libvir-list/2017-June/msg01081.html

I reviewed earlier this month:

https://www.redhat.com/archives/libvir-list/2017-July/msg00278.html

but responses to the initial review and a couple followups by the
submitter got unthreaded... A trail of a few breadcrumbs:

https://www.redhat.com/archives/libvir-list/2017-July/msg00387.html
https://www.redhat.com/archives/libvir-list/2017-July/msg00762.html
https://www.redhat.com/archives/libvir-list/2017-July/msg00864.html

In any case, the crux of the issue is that during startup the domain obj
lock is released right around the time we go to start the monitor (and
the vmagent) - an extra reference is taken to avoid obj deletion. In
another thread there's an Undefine run. That thread gets the lock, finds
a persistent domain and then clears that persistent bit at the end,
calls RemoveInactive. By that time, the domain startup thread gets the
obj lock back. The Undefine running caused some issues, causing the
start to go into it's failure path, but that fails to remove the domain.

The command used to start/undefine buried in one response is

   virsh start win7 &; sleep 0.2; virsh undefine win7


John

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

On 07/22/2017 04:55 AM, Yi Wang wrote:
> Start a domain whilst undefine it, if starting failed duing ProcessLaunch,
> on which period qemu exited unexpectedly, the operation will lead to failure
> of undefine the domain until libvirtd restarted. The reason is that libvirtd
> will unlock vm during qemuProcessStart, qemuDomainUndefineFlags can get the
> lock and set vm->persistent 0 but not remove the "active" domain.
> 
> Signed-off-by: Yi Wang <wang.yi59@zte.com.cn>
> ---
>  src/conf/domain_conf.h  | 1 +
>  src/qemu/qemu_driver.c  | 6 ++++++
>  src/qemu/qemu_process.c | 3 +++
>  3 files changed, 10 insertions(+)
> 

Can you apply a couple of recent patches, see:

https://www.redhat.com/archives/libvir-list/2017-August/msg00389.html

and see if those would resolve what you're seeing... without these
changes of course...

Tks -

John

> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index af15ee8..f339f84 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2468,6 +2468,7 @@ struct _virDomainObj {
>      virDomainSnapshotObjPtr current_snapshot;
>  
>      bool hasManagedSave;
> +    bool starting;
>  
>      void *privateData;
>      void (*privateDataFreeFunc)(void *);
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 6568def..5d9acfc 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -7317,6 +7317,12 @@ qemuDomainUndefineFlags(virDomainPtr dom,
>      if (!(vm = qemuDomObjFromDomain(dom)))
>          return -1;
>  
> +    if (vm->starting) {
> +        virReportError(VIR_ERR_OPERATION_INVALID,
> +                       "%s", _("cannot undefine during domain starting up"));
> +        goto cleanup;
> +    }
> +
>      cfg = virQEMUDriverGetConfig(driver);
>  
>      if (virDomainUndefineFlagsEnsureACL(dom->conn, vm->def) < 0)
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 525521a..7b708be 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -5847,6 +5847,8 @@ qemuProcessStart(virConnectPtr conn,
>      if (!migrateFrom && !snapshot)
>          flags |= VIR_QEMU_PROCESS_START_NEW;
>  
> +    vm->starting = true;
> +
>      if (qemuProcessInit(driver, vm, updatedCPU,
>                          asyncJob, !!migrateFrom, flags) < 0)
>          goto cleanup;
> @@ -5892,6 +5894,7 @@ qemuProcessStart(virConnectPtr conn,
>      ret = 0;
>  
>   cleanup:
> +    vm->starting = false;
>      qemuProcessIncomingDefFree(incoming);
>      return ret;
>  
> 

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