[PATCH 0/2] tools/libxl: Fixes to domain building

[PATCH 0/2] tools/libxl: Fixes to domain building

[Andrew Cooper]

This series is in preparation for passing more parameters via
XEN_DOMCTL_createdomain.

Andrew Cooper (2):
  tools/libxl: Drop xc_domain_configuration_t from libxl__domain_build_state
  tools/libxl: Don't prepare or save xc_config when soft resetting a domain

 tools/libxl/libxl_create.c   | 53 ++++++++++++++++++++++----------------------
 tools/libxl/libxl_dm.c       |  3 +--
 tools/libxl/libxl_internal.h |  5 +----
 3 files changed, 29 insertions(+), 32 deletions(-)

-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 1/2] tools/libxl: Drop xc_domain_configuration_t from libxl__domain_build_state

[Andrew Cooper]

The data it stores is initialised and exclusively used within
libxl__domain_make(), with the important details written back elsewhere by
libxl__arch_domain_save_config().  Prepare xc_config on libxl__domain_make()'s
stack, and drop the parameter.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl_create.c   | 12 ++++++------
 tools/libxl/libxl_dm.c       |  3 +--
 tools/libxl/libxl_internal.h |  5 +----
 3 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index c4981352..f92c383 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -538,7 +538,7 @@ int libxl__domain_build(libxl__gc *gc,
 }
 
 int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config,
-                       uint32_t *domid, xc_domain_configuration_t *xc_config)
+                       uint32_t *domid)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
     int flags, ret, rc, nb_vm;
@@ -551,6 +551,7 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config,
     xs_transaction_t t = 0;
     xen_domain_handle_t handle;
     libxl_vminfo *vm_list;
+    xc_domain_configuration_t xc_config = {};
 
     /* convenience aliases */
     libxl_domain_create_info *info = &d_config->c_info;
@@ -571,7 +572,7 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config,
     /* Ultimately, handle is an array of 16 uint8_t, same as uuid */
     libxl_uuid_copy(ctx, (libxl_uuid *)handle, &info->uuid);
 
-    ret = libxl__arch_domain_prepare_config(gc, d_config, xc_config);
+    ret = libxl__arch_domain_prepare_config(gc, d_config, &xc_config);
     if (ret < 0) {
         LOGED(ERROR, *domid, "fail to get domain config");
         rc = ERROR_FAIL;
@@ -581,7 +582,7 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config,
     /* Valid domid here means we're soft resetting. */
     if (!libxl_domid_valid_guest(*domid)) {
         ret = xc_domain_create(ctx->xch, info->ssidref, handle, flags, domid,
-                               xc_config);
+                               &xc_config);
         if (ret < 0) {
             LOGED(ERROR, *domid, "domain creation fail");
             rc = ERROR_FAIL;
@@ -589,7 +590,7 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config,
         }
     }
 
-    rc = libxl__arch_domain_save_config(gc, d_config, xc_config);
+    rc = libxl__arch_domain_save_config(gc, d_config, &xc_config);
     if (rc < 0)
         goto out;
 
@@ -822,7 +823,6 @@ static void initiate_domain_create(libxl__egc *egc,
 
     /* convenience aliases */
     libxl_domain_config *const d_config = dcs->guest_config;
-    libxl__domain_build_state *const state = &dcs->build_state;
     const int restore_fd = dcs->restore_fd;
 
     domid = dcs->domid_soft_reset;
@@ -957,7 +957,7 @@ static void initiate_domain_create(libxl__egc *egc,
         goto error_out;
     }
 
-    ret = libxl__domain_make(gc, d_config, &domid, &state->config);
+    ret = libxl__domain_make(gc, d_config, &domid);
     if (ret) {
         LOGD(ERROR, domid, "cannot make domain: %d", ret);
         dcs->guest_domid = domid;
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index a3cddce..49678bd 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1953,8 +1953,7 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
     stubdom_state->pv_ramdisk.path = "";
 
     /* fixme: this function can leak the stubdom if it fails */
-    ret = libxl__domain_make(gc, dm_config, &sdss->pvqemu.guest_domid,
-                             &stubdom_state->config);
+    ret = libxl__domain_make(gc, dm_config, &sdss->pvqemu.guest_domid);
     if (ret)
         goto out;
     uint32_t dm_domid = sdss->pvqemu.guest_domid;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 8dd6331..663fcb2 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1145,8 +1145,6 @@ typedef struct {
     xen_vmemrange_t *vmemranges;
     uint32_t num_vmemranges;
 
-    xc_domain_configuration_t config;
-
     xen_pfn_t vuart_gfn;
     evtchn_port_t vuart_port;
 } libxl__domain_build_state;
@@ -1657,8 +1655,7 @@ _hidden  void libxl__exec(libxl__gc *gc, int stdinfd, int stdoutfd,
   * on exit (even error exit), domid may be valid and refer to a domain */
 _hidden int libxl__domain_make(libxl__gc *gc,
                                libxl_domain_config *d_config,
-                               uint32_t *domid,
-                               xc_domain_configuration_t *xc_config);
+                               uint32_t *domid);
 
 _hidden int libxl__domain_build(libxl__gc *gc,
                                 libxl_domain_config *d_config,
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 2/2] tools/libxl: Don't prepare or save xc_config when soft resetting a domain

[Andrew Cooper]

xc_config is only used by xc_domain_create(), but by calling
libxl__arch_domain_{prepare,save}_config() we clobber the real settings with
the default settings.

Move all data and calls relating to xc_domain_create() into the path which
calls it.

As far as I can tell, soft_reset has always been broken for ARM domains using
LIBXL_GIC_VERSION_DEFAULT, which elicits a hard error out of
libxl__arch_domain_save_config().

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>

This patch is far more easily reviewed with `git diff --ignore-all-space`.
---
 tools/libxl/libxl_create.c | 47 +++++++++++++++++++++++-----------------------
 1 file changed, 24 insertions(+), 23 deletions(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index f92c383..60a5542 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -541,7 +541,7 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config,
                        uint32_t *domid)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
-    int flags, ret, rc, nb_vm;
+    int ret, rc, nb_vm;
     const char *dom_type;
     char *uuid_string;
     char *dom_path, *vm_path, *libxl_path;
@@ -549,9 +549,7 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config,
     struct xs_permissions rwperm[1];
     struct xs_permissions noperm[1];
     xs_transaction_t t = 0;
-    xen_domain_handle_t handle;
     libxl_vminfo *vm_list;
-    xc_domain_configuration_t xc_config = {};
 
     /* convenience aliases */
     libxl_domain_create_info *info = &d_config->c_info;
@@ -562,25 +560,28 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config,
         goto out;
     }
 
-    flags = 0;
-    if (info->type != LIBXL_DOMAIN_TYPE_PV) {
-        flags |= XEN_DOMCTL_CDF_hvm_guest;
-        flags |= libxl_defbool_val(info->hap) ? XEN_DOMCTL_CDF_hap : 0;
-        flags |= libxl_defbool_val(info->oos) ? 0 : XEN_DOMCTL_CDF_oos_off;
-    }
+    /* Valid domid here means we're soft resetting. */
+    if (!libxl_domid_valid_guest(*domid)) {
+        int flags = 0;
+        xen_domain_handle_t handle;
+        xc_domain_configuration_t xc_config = {};
+
+        if (info->type != LIBXL_DOMAIN_TYPE_PV) {
+            flags |= XEN_DOMCTL_CDF_hvm_guest;
+            flags |= libxl_defbool_val(info->hap) ? XEN_DOMCTL_CDF_hap : 0;
+            flags |= libxl_defbool_val(info->oos) ? 0 : XEN_DOMCTL_CDF_oos_off;
+        }
 
-    /* Ultimately, handle is an array of 16 uint8_t, same as uuid */
-    libxl_uuid_copy(ctx, (libxl_uuid *)handle, &info->uuid);
+        /* Ultimately, handle is an array of 16 uint8_t, same as uuid */
+        libxl_uuid_copy(ctx, (libxl_uuid *)handle, &info->uuid);
 
-    ret = libxl__arch_domain_prepare_config(gc, d_config, &xc_config);
-    if (ret < 0) {
-        LOGED(ERROR, *domid, "fail to get domain config");
-        rc = ERROR_FAIL;
-        goto out;
-    }
+        ret = libxl__arch_domain_prepare_config(gc, d_config, &xc_config);
+        if (ret < 0) {
+            LOGED(ERROR, *domid, "fail to get domain config");
+            rc = ERROR_FAIL;
+            goto out;
+        }
 
-    /* Valid domid here means we're soft resetting. */
-    if (!libxl_domid_valid_guest(*domid)) {
         ret = xc_domain_create(ctx->xch, info->ssidref, handle, flags, domid,
                                &xc_config);
         if (ret < 0) {
@@ -588,11 +589,11 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config,
             rc = ERROR_FAIL;
             goto out;
         }
-    }
 
-    rc = libxl__arch_domain_save_config(gc, d_config, &xc_config);
-    if (rc < 0)
-        goto out;
+        rc = libxl__arch_domain_save_config(gc, d_config, &xc_config);
+        if (rc < 0)
+            goto out;
+    }
 
     ret = xc_cpupool_movedomain(ctx->xch, info->poolid, *domid);
     if (ret < 0) {
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/2] tools/libxl: Drop xc_domain_configuration_t from libxl__domain_build_state

[Wei Liu]

On Fri, Mar 16, 2018 at 02:06:39PM +0000, Andrew Cooper wrote:
> The data it stores is initialised and exclusively used within
> libxl__domain_make(), with the important details written back elsewhere by
> libxl__arch_domain_save_config().  Prepare xc_config on libxl__domain_make()'s
> stack, and drop the parameter.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

These two patches are superseded by your other series AIUI.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel