[PATCH] xl: save domain config (userdata) under correct domid/uuid

[PATCH] xl: save domain config (userdata) under correct domid/uuid

[Ian Jackson]

The automatic tests write:
> Regressions :-(
> 
> Tests which did not succeed and are blocking:
>  test-amd64-amd64-xl          11 guest-localmigrate         fail REGR. vs. 4586
[etc]

I think this is fixed by the patch below, which I have just applied.

Ian.

# HG changeset patch
# User Ian Jackson <Ian.Jackson@eu.citrix.com>
# Date 1294877915 0
# Node ID d839631b604807fe0f978f9eacc1c73f6a9bf0bd
# Parent  20b0f709153e9e36c937caae8ec0b816c8ad1cfd
xl: save domain config (userdata) under correct domid/uuid

Recent changes caused the domain config file to be saved under dom0's
filename in /var/lib/xen.  This was due to the config file being saved
before the domain was created and thus before the domid and uuid were
known.

Fix this by moving the saving code to after creation.

Also, change the "default" initialisation of domid in
xl_cmdimpl.c:create_domain to be domid=-1.  That provides a more
obviously wrong value than 0 (which refers to dom0) so that other bugs
of this kind would be more likely to show up.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Tested-by: Ian Jackson <ian.jackson@eu.citrix.com>

diff -r 20b0f709153e -r d839631b6048 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Wed Jan 12 14:14:13 2011 +0000
+++ b/tools/libxl/xl_cmdimpl.c	Thu Jan 13 00:18:35 2011 +0000
@@ -1437,7 +1437,7 @@ static int create_domain(struct domain_c
         printf_info(-1, &d_config, &d_config.dm_info);
 
 start:
-    domid = 0;
+    domid = -1;
 
     rc = acquire_lock();
     if (rc < 0)
@@ -1446,14 +1446,6 @@ start:
     ret = freemem(&d_config.b_info, &d_config.dm_info);
     if (ret < 0) {
         fprintf(stderr, "failed to free memory for the domain\n");
-        ret = ERROR_FAIL;
-        goto error_out;
-    }
-
-    ret = libxl_userdata_store(&ctx, domid, "xl",
-                                    config_data, config_len);
-    if (ret) {
-        perror("cannot save config file");
         ret = ERROR_FAIL;
         goto error_out;
     }
@@ -1474,6 +1466,14 @@ start:
     }
     if ( ret )
         goto error_out;
+
+    ret = libxl_userdata_store(&ctx, domid, "xl",
+                                    config_data, config_len);
+    if (ret) {
+        perror("cannot save config file");
+        ret = ERROR_FAIL;
+        goto error_out;
+    }
 
     release_lock();

Re: [PATCH] xl: save domain config (userdata) under correct domid/uuid

[Ian Jackson]

I wrote:
> I think this is fixed by the patch below, which I have just applied.

Um, but, I missed a bit.

# HG changeset patch
# User Ian Jackson <Ian.Jackson@eu.citrix.com>
# Date 1294882004 0
# Node ID ce208811f5401dcc6dc39a1e592b102e8d3e7c85
# Parent  d839631b604807fe0f978f9eacc1c73f6a9bf0bd
xl: correct test for domid on error exit from domain_create

The previous changeset 22739:d839631b6048 changed the initialisation
of domid without changing the corresponding cleanup test.  Oops.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

diff -r d839631b6048 -r ce208811f540 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Thu Jan 13 00:18:35 2011 +0000
+++ b/tools/libxl/xl_cmdimpl.c	Thu Jan 13 01:26:44 2011 +0000
@@ -1627,7 +1627,7 @@ start:
 
 error_out:
     release_lock();
-    if (domid)
+    if (domid > 0)
         libxl_domain_destroy(&ctx, domid, 0);
 
 out:

Re: [PATCH] xl: save domain config (userdata) under correct domid/uuid

[Gianni Tedesco]

On Thu, 2011-01-13 at 00:20 +0000, Ian Jackson wrote:
> The automatic tests write:
> > Regressions :-(
> > 
> > Tests which did not succeed and are blocking:
> >  test-amd64-amd64-xl          11 guest-localmigrate         fail REGR. vs. 4586
> [etc]
> 
> I think this is fixed by the patch below, which I have just applied.
> 
> Ian.
> 
> # HG changeset patch
> # User Ian Jackson <Ian.Jackson@eu.citrix.com>
> # Date 1294877915 0
> # Node ID d839631b604807fe0f978f9eacc1c73f6a9bf0bd
> # Parent  20b0f709153e9e36c937caae8ec0b816c8ad1cfd
> xl: save domain config (userdata) under correct domid/uuid
> 
> Recent changes caused the domain config file to be saved under dom0's
> filename in /var/lib/xen.  This was due to the config file being saved
> before the domain was created and thus before the domid and uuid were
> known.
> 
> Fix this by moving the saving code to after creation.
> 
> Also, change the "default" initialisation of domid in
> xl_cmdimpl.c:create_domain to be domid=-1.  That provides a more
> obviously wrong value than 0 (which refers to dom0) so that other bugs
> of this kind would be more likely to show up.
> 
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
> Tested-by: Ian Jackson <ian.jackson@eu.citrix.com>

Nice catch, thanks. It's still a beast of a function.

Gianni