Xen-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Paul Durrant <pdurrant@amazon.com>
To: <xen-devel@lists.xenproject.org>
Cc: Anthony PERARD <anthony.perard@citrix.com>,
	Paul Durrant <pdurrant@amazon.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>, Wei Liu <wl@xen.org>
Subject: [Xen-devel] [PATCH v3 2/6] libxl_create: make 'soft reset' explicit
Date: Thu, 16 Jan 2020 09:35:58 +0000
Message-ID: <20200116093602.4203-3-pdurrant@amazon.com> (raw)
In-Reply-To: <20200116093602.4203-1-pdurrant@amazon.com>

The 'soft reset' code path in libxl__domain_make() is currently taken if a
valid domid is passed into the function. A subsequent patch will enable
higher levels of the toolstack to determine the domid of newly created or
restored domains and therefore this criteria for choosing 'soft reset'
will no longer be usable.

This patch adds an extra boolean option to libxl__domain_make() to specify
whether it is being invoked in soft reset context and appropriately
modifies callers to choose the right value. To facilitate this, a new
'soft_reset' boolean field is added to struct libxl__domain_create_state
and the 'domid_soft_reset' field is renamed to 'domid' in anticipation of
its wider remit. For the moment do_domain_create() will always set
domid to INVALID_DOMID and hence we can add an assertion into
libxl__domain_create() that, if it is not called in soft reset context,
the passed in domid is exactly that value.

Whilst in the neighbourhood, some checks of 'restore_fd > -1' have been
replaced by 'restore_fd >= 0' to be more conventional and consistent with
checks of 'restore_fd < 0'.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
Cc: Wei Liu <wl@xen.org>
Cc: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl_create.c   | 56 ++++++++++++++++++++++--------------
 tools/libxl/libxl_dm.c       |  2 +-
 tools/libxl/libxl_internal.h |  5 ++--
 3 files changed, 38 insertions(+), 25 deletions(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index bc425fee32..1835a5502c 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -538,7 +538,7 @@ out:
 
 int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config,
                        libxl__domain_build_state *state,
-                       uint32_t *domid)
+                       uint32_t *domid, bool soft_reset)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
     int ret, rc, nb_vm;
@@ -555,14 +555,15 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config,
     libxl_domain_create_info *info = &d_config->c_info;
     libxl_domain_build_info *b_info = &d_config->b_info;
 
+    assert(soft_reset || *domid == INVALID_DOMID);
+
     uuid_string = libxl__uuid2string(gc, info->uuid);
     if (!uuid_string) {
         rc = ERROR_NOMEM;
         goto out;
     }
 
-    /* Valid domid here means we're soft resetting. */
-    if (!libxl_domid_valid_guest(*domid)) {
+    if (!soft_reset) {
         struct xen_domctl_createdomain create = {
             .ssidref = info->ssidref,
             .max_vcpus = b_info->max_vcpus,
@@ -611,6 +612,14 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config,
             goto out;
     }
 
+    /*
+     * If soft_reset is set the the domid will have been valid on entry.
+     * If it was not set then xc_domain_create() should have assigned a
+     * valid value. Either way, if we reach this point, domid should be
+     * valid.
+     */
+    assert(libxl_domid_valid_guest(*domid));
+
     ret = xc_cpupool_movedomain(ctx->xch, info->poolid, *domid);
     if (ret < 0) {
         LOGED(ERROR, *domid, "domain move fail");
@@ -1091,13 +1100,14 @@ static void initiate_domain_create(libxl__egc *egc,
     libxl_domain_config *const d_config = dcs->guest_config;
     const int restore_fd = dcs->restore_fd;
 
-    domid = dcs->domid_soft_reset;
+    domid = dcs->domid;
     libxl__domain_build_state_init(&dcs->build_state);
 
     ret = libxl__domain_config_setdefault(gc,d_config,domid);
     if (ret) goto error_out;
 
-    ret = libxl__domain_make(gc, d_config, &dcs->build_state, &domid);
+    ret = libxl__domain_make(gc, d_config, &dcs->build_state, &domid,
+                             dcs->soft_reset);
     if (ret) {
         LOGD(ERROR, domid, "cannot make domain: %d", ret);
         dcs->guest_domid = domid;
@@ -1141,7 +1151,7 @@ static void initiate_domain_create(libxl__egc *egc,
     if (ret)
         goto error_out;
 
-    if (restore_fd >= 0 || dcs->domid_soft_reset != INVALID_DOMID) {
+    if (restore_fd >= 0 || dcs->soft_reset) {
         LOGD(DEBUG, domid, "restoring, not running bootloader");
         domcreate_bootloader_done(egc, &dcs->bl, 0);
     } else  {
@@ -1217,7 +1227,7 @@ static void domcreate_bootloader_done(libxl__egc *egc,
     dcs->sdss.dm.callback = domcreate_devmodel_started;
     dcs->sdss.callback = domcreate_devmodel_started;
 
-    if (restore_fd < 0 && dcs->domid_soft_reset == INVALID_DOMID) {
+    if (restore_fd < 0 && !dcs->soft_reset) {
         rc = libxl__domain_build(gc, d_config, domid, state);
         domcreate_rebuild_done(egc, dcs, rc);
         return;
@@ -1827,7 +1837,7 @@ static int do_domain_create(libxl_ctx *ctx, libxl_domain_config *d_config,
     libxl_domain_config_copy(ctx, &cdcs->dcs.guest_config_saved, d_config);
     cdcs->dcs.restore_fd = cdcs->dcs.libxc_fd = restore_fd;
     cdcs->dcs.send_back_fd = send_back_fd;
-    if (restore_fd > -1) {
+    if (restore_fd >= 0) {
         cdcs->dcs.restore_params = *params;
         rc = libxl__fd_flags_modify_save(gc, cdcs->dcs.restore_fd,
                                          ~(O_NONBLOCK|O_NDELAY), 0,
@@ -1835,7 +1845,8 @@ static int do_domain_create(libxl_ctx *ctx, libxl_domain_config *d_config,
         if (rc < 0) goto out_err;
     }
     cdcs->dcs.callback = domain_create_cb;
-    cdcs->dcs.domid_soft_reset = INVALID_DOMID;
+    cdcs->dcs.domid = INVALID_DOMID;
+    cdcs->dcs.soft_reset = false;
 
     if (cdcs->dcs.restore_params.checkpointed_stream ==
         LIBXL_CHECKPOINTED_STREAM_COLO) {
@@ -1905,7 +1916,7 @@ static void soft_reset_dm_suspended(libxl__egc *egc,
                                     int rc);
 static int do_domain_soft_reset(libxl_ctx *ctx,
                                 libxl_domain_config *d_config,
-                                uint32_t domid_soft_reset,
+                                uint32_t domid,
                                 const libxl_asyncop_how *ao_how,
                                 const libxl_asyncprogress_how
                                 *aop_console_how)
@@ -1933,15 +1944,16 @@ static int do_domain_soft_reset(libxl_ctx *ctx,
     libxl_domain_config_copy(ctx, &srs->cdcs.dcs.guest_config_saved,
                              d_config);
     cdcs->dcs.restore_fd = -1;
-    cdcs->dcs.domid_soft_reset = domid_soft_reset;
+    cdcs->dcs.domid = domid;
+    cdcs->dcs.soft_reset = true;
     cdcs->dcs.callback = domain_create_cb;
     libxl__ao_progress_gethow(&srs->cdcs.dcs.aop_console_how,
                               aop_console_how);
     cdcs->domid_out = &domid_out;
 
-    dom_path = libxl__xs_get_dompath(gc, domid_soft_reset);
+    dom_path = libxl__xs_get_dompath(gc, domid);
     if (!dom_path) {
-        LOGD(ERROR, domid_soft_reset, "failed to read domain path");
+        LOGD(ERROR, domid, "failed to read domain path");
         rc = ERROR_FAIL;
         goto out;
     }
@@ -1950,7 +1962,7 @@ static int do_domain_soft_reset(libxl_ctx *ctx,
                                 GCSPRINTF("%s/store/ring-ref", dom_path),
                                 &xs_store_mfn);
     if (rc) {
-        LOGD(ERROR, domid_soft_reset, "failed to read store/ring-ref.");
+        LOGD(ERROR, domid, "failed to read store/ring-ref.");
         goto out;
     }
     state->store_mfn = xs_store_mfn ? atol(xs_store_mfn): 0;
@@ -1959,7 +1971,7 @@ static int do_domain_soft_reset(libxl_ctx *ctx,
                                 GCSPRINTF("%s/console/ring-ref", dom_path),
                                 &xs_console_mfn);
     if (rc) {
-        LOGD(ERROR, domid_soft_reset, "failed to read console/ring-ref.");
+        LOGD(ERROR, domid, "failed to read console/ring-ref.");
         goto out;
     }
     state->console_mfn = xs_console_mfn ? atol(xs_console_mfn): 0;
@@ -1968,20 +1980,20 @@ static int do_domain_soft_reset(libxl_ctx *ctx,
                                   GCSPRINTF("%s/console/tty", dom_path),
                                   &console_tty);
     if (rc) {
-        LOGD(ERROR, domid_soft_reset, "failed to read console/tty.");
+        LOGD(ERROR, domid, "failed to read console/tty.");
         goto out;
     }
     state->console_tty = libxl__strdup(gc, console_tty);
 
     dss->ao = ao;
-    dss->domid = dss->dsps.domid = domid_soft_reset;
+    dss->domid = dss->dsps.domid = domid;
     dss->dsps.dm_savefile = GCSPRINTF(LIBXL_DEVICE_MODEL_SAVE_FILE".%d",
-                                      domid_soft_reset);
+                                      domid);
 
     rc = libxl__save_emulator_xenstore_data(dss, &srs->toolstack_buf,
                                             &srs->toolstack_len);
     if (rc) {
-        LOGD(ERROR, domid_soft_reset, "failed to save toolstack record.");
+        LOGD(ERROR, domid, "failed to save toolstack record.");
         goto out;
     }
 
@@ -2010,10 +2022,10 @@ static void soft_reset_dm_suspended(libxl__egc *egc,
      * xenstore again with probably different store/console/...
      * channels.
      */
-    xs_release_domain(CTX->xsh, cdcs->dcs.domid_soft_reset);
+    xs_release_domain(CTX->xsh, cdcs->dcs.domid);
 
     srs->dds.ao = ao;
-    srs->dds.domid = cdcs->dcs.domid_soft_reset;
+    srs->dds.domid = cdcs->dcs.domid;
     srs->dds.callback = domain_soft_reset_cb;
     srs->dds.soft_reset = true;
     libxl__domain_destroy(egc, &srs->dds);
@@ -2029,7 +2041,7 @@ static void domain_create_cb(libxl__egc *egc,
 
     *cdcs->domid_out = domid;
 
-    if (dcs->restore_fd > -1) {
+    if (dcs->restore_fd >= 0) {
         flrc = libxl__fd_flags_restore(gc,
                 dcs->restore_fd, dcs->restore_fdfl);
         /*
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index e92e412c1b..f758daf3b6 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2193,7 +2193,7 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
 
     /* fixme: this function can leak the stubdom if it fails */
     ret = libxl__domain_make(gc, dm_config, stubdom_state,
-                             &sdss->pvqemu.guest_domid);
+                             &sdss->pvqemu.guest_domid, false);
     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 3b708fba8f..cb23490c59 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1957,7 +1957,7 @@ _hidden  void libxl__exec(libxl__gc *gc, int stdinfd, int stdoutfd,
 _hidden int libxl__domain_make(libxl__gc *gc,
                                libxl_domain_config *d_config,
                                libxl__domain_build_state *state,
-                               uint32_t *domid);
+                               uint32_t *domid, bool soft_reset);
 
 _hidden int libxl__domain_build(libxl__gc *gc,
                                 libxl_domain_config *d_config,
@@ -4134,7 +4134,8 @@ struct libxl__domain_create_state {
     int restore_fdfl; /* original flags of restore_fd */
     int send_back_fd;
     libxl_domain_restore_params restore_params;
-    uint32_t domid_soft_reset;
+    uint32_t domid;
+    bool soft_reset;
     libxl__domain_create_cb *callback;
     libxl_asyncprogress_how aop_console_how;
     /* private to domain_create */
-- 
2.20.1


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

  parent reply index

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-16  9:35 [Xen-devel] [PATCH v3 0/6] xl/libxl: domid allocation/preservation changes Paul Durrant
2020-01-16  9:35 ` [Xen-devel] [PATCH v3 1/6] libxl: add definition of INVALID_DOMID to the API Paul Durrant
2020-01-16  9:35 ` Paul Durrant [this message]
2020-01-16  9:35 ` [Xen-devel] [PATCH v3 3/6] libxl: add infrastructure to track and query 'retired' domids Paul Durrant
2020-01-16 18:27   ` Ian Jackson
2020-01-17  9:26     ` Durrant, Paul
2020-01-17 11:31       ` Ian Jackson
2020-01-16  9:36 ` [Xen-devel] [PATCH v3 4/6] libxl: allow creation of domains with a specified or random domid Paul Durrant
2020-01-16  9:40   ` Jan Beulich
2020-01-16  9:46     ` Durrant, Paul
2020-01-16  9:59       ` Jan Beulich
2020-01-16 15:53   ` Jason Andryuk
2020-01-16 18:36   ` Ian Jackson
2020-01-17  9:37     ` Durrant, Paul
2020-01-17 11:35       ` Ian Jackson
2020-01-17 12:06         ` Durrant, Paul
2020-01-17 15:30           ` Ian Jackson
2020-01-20  8:18             ` Durrant, Paul
2020-01-16  9:36 ` [Xen-devel] [PATCH v3 5/6] xl.conf: introduce 'domid_policy' Paul Durrant
2020-01-16 18:37   ` Ian Jackson
2020-01-16  9:36 ` [Xen-devel] [PATCH v3 6/6] xl: allow domid to be preserved on save/restore or migrate Paul Durrant
2020-01-16 18:39   ` Ian Jackson
2020-01-16 18:43 ` [Xen-devel] [PATCH v3 0/6] xl/libxl: domid allocation/preservation changes Ian Jackson
2020-01-17  9:11   ` Durrant, Paul

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200116093602.4203-3-pdurrant@amazon.com \
    --to=pdurrant@amazon.com \
    --cc=anthony.perard@citrix.com \
    --cc=ian.jackson@eu.citrix.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Xen-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/xen-devel/0 xen-devel/git/0.git
	git clone --mirror https://lore.kernel.org/xen-devel/1 xen-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 xen-devel xen-devel/ https://lore.kernel.org/xen-devel \
		xen-devel@lists.xenproject.org xen-devel@lists.xen.org
	public-inbox-index xen-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.xenproject.lists.xen-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git