[PATCH] xen: Don't add dom0 twice on driver reload

Jim Fehlig posted 1 patch 2 weeks ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20200911175300.26239-1-jfehlig@suse.com
src/libxl/libxl_driver.c | 46 +++++++++++++++++++++++-----------------
1 file changed, 26 insertions(+), 20 deletions(-)

[PATCH] xen: Don't add dom0 twice on driver reload

Posted by Jim Fehlig 2 weeks ago
When the xen driver loads, it probes libxl for some info about dom0 and
adds it to the virDomainObjList. The driver then looks for any domains
in stateDir and if they are still alive adds them to the list as well.
This logic is a bit flawed wrt handling driver reload and causes the
following error

  internal error: unexpected domain Domain-0 already exists

A simple fix is to load all domains from stateDir first and then only
add dom0 if needed.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
 src/libxl/libxl_driver.c | 46 +++++++++++++++++++++++-----------------
 1 file changed, 26 insertions(+), 20 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 161a6882f3..083738871d 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -604,27 +604,33 @@ libxlAddDom0(libxlDriverPrivatePtr driver)
         goto cleanup;
     }
 
-    if (!(def = virDomainDefNew()))
-        goto cleanup;
+    /*
+     * On a driver reload dom0 will already exist. On host restart it must
+     * created.
+     */
+    if ((vm = virDomainObjListFindByID(driver->domains, 0)) == NULL) {
+        if (!(def = virDomainDefNew()))
+            goto cleanup;
 
-    def->id = 0;
-    def->virtType = VIR_DOMAIN_VIRT_XEN;
-    def->name = g_strdup("Domain-0");
+        def->id = 0;
+        def->virtType = VIR_DOMAIN_VIRT_XEN;
+        def->name = g_strdup("Domain-0");
 
-    def->os.type = VIR_DOMAIN_OSTYPE_XEN;
+        def->os.type = VIR_DOMAIN_OSTYPE_XEN;
 
-    if (virUUIDParse("00000000-0000-0000-0000-000000000000", def->uuid) < 0)
-        goto cleanup;
+        if (virUUIDParse("00000000-0000-0000-0000-000000000000", def->uuid) < 0)
+            goto cleanup;
 
-    if (!(vm = virDomainObjListAdd(driver->domains, def,
-                                   driver->xmlopt,
-                                   0,
-                                   NULL)))
-        goto cleanup;
-    def = NULL;
+        if (!(vm = virDomainObjListAdd(driver->domains, def,
+                                       driver->xmlopt,
+                                       0,
+                                       NULL)))
+            goto cleanup;
+
+        vm->persistent = 1;
+        virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_BOOTED);
+    }
 
-    vm->persistent = 1;
-    virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_BOOTED);
     if (virDomainDefSetVcpusMax(vm->def, d_info.vcpu_max_id + 1, driver->xmlopt))
         goto cleanup;
 
@@ -783,10 +789,6 @@ libxlStateInitialize(bool privileged,
     if (!(libxl_driver->xmlopt = libxlCreateXMLConf(libxl_driver)))
         goto error;
 
-    /* Add Domain-0 */
-    if (libxlAddDom0(libxl_driver) < 0)
-        goto error;
-
     /* Load running domains first. */
     if (virDomainObjListLoadAllConfigs(libxl_driver->domains,
                                        cfg->stateDir,
@@ -796,6 +798,10 @@ libxlStateInitialize(bool privileged,
                                        NULL, NULL) < 0)
         goto error;
 
+    /* Add Domain-0 */
+    if (libxlAddDom0(libxl_driver) < 0)
+        goto error;
+
     libxlReconnectDomains(libxl_driver);
 
     /* Then inactive persistent configs */
-- 
2.28.0


Re: [PATCH] xen: Don't add dom0 twice on driver reload

Posted by Ján Tomko 4 days ago
On a Friday in 2020, Jim Fehlig wrote:
>When the xen driver loads, it probes libxl for some info about dom0 and
>adds it to the virDomainObjList. The driver then looks for any domains
>in stateDir and if they are still alive adds them to the list as well.
>This logic is a bit flawed wrt handling driver reload and causes the
>following error
>
>  internal error: unexpected domain Domain-0 already exists
>
>A simple fix is to load all domains from stateDir first and then only
>add dom0 if needed.
>
>Signed-off-by: Jim Fehlig <jfehlig@suse.com>
>---
> src/libxl/libxl_driver.c | 46 +++++++++++++++++++++++-----------------
> 1 file changed, 26 insertions(+), 20 deletions(-)
>

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

Jano