[PATCH 0/3] Minor cleanup of create_domain()

[PATCH 0/3] Minor cleanup of create_domain()

[Elliott Mitchell]

Something which has annoyed me for a long while decided to get in front
of me again.  As such getting some cleanup done as the start of
addressing that annoyance.

This is some minor cleanup related to create_domain().  First is simply a
bit of precursor Makefile cleanup.  Next two are things which can be
done earlier in the function and really look like they should be.

Elliott Mitchell (3):
  tools/xl: use Make variable to retrieve xl object filename list
  tools/xl: free loaded configuration data earlier in create_domain()
  tools/xl: move some logic needed only once earlier in create_domain()

 tools/xl/Makefile       |  2 +-
 tools/xl/xl_vmcontrol.c | 36 +++++++++++++++++-------------------
 2 files changed, 18 insertions(+), 20 deletions(-)

-- 
2.47.3

[PATCH 3/3] tools/xl: move some logic needed only once earlier in create_domain()

[Elliott Mitchell]

The value of dom_info->console_autoconnect isn't going to change if a
domain is rebooted.  Nor does this logic need the lock to be held.  As
such moving this earlier makes sense.

The policy domain is set once and then never changed.  Similar to above
simply move before the reboot start point.

Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com>
---
 tools/xl/xl_vmcontrol.c | 32 +++++++++++++++-----------------
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/tools/xl/xl_vmcontrol.c b/tools/xl/xl_vmcontrol.c
index 76c2a07ee8..ae7985122f 100644
--- a/tools/xl/xl_vmcontrol.c
+++ b/tools/xl/xl_vmcontrol.c
@@ -717,7 +717,8 @@ int create_domain(struct domain_create *dom_info)
     int restore_fd = -1;
     int restore_fd_to_close = -1;
     int send_back_fd = -1;
-    const libxl_asyncprogress_how *autoconnect_console_how;
+    const libxl_asyncprogress_how *autoconnect_console_how = NULL;
+    libxl_asyncprogress_how autoconnect_console_how_buf;
     int notify_pipe[2] = { -1, -1 };
     struct save_file_header hdr;
     uint32_t domid_soft_reset = INVALID_DOMID;
@@ -921,6 +922,19 @@ int create_domain(struct domain_create *dom_info)
     if (dryrun_only)
         goto out;
 
+    if (dom_info->console_autoconnect) {
+        if (libxl_pipe(ctx, notify_pipe)) {
+            ret = ERROR_FAIL;
+            goto error_out;
+        }
+        autoconnect_console_how_buf.callback = autoconnect_console;
+        autoconnect_console_how_buf.for_callback = &notify_pipe[1];
+        autoconnect_console_how = &autoconnect_console_how_buf;
+    }
+
+    if (!libxl_domid_valid_guest(d_config.c_info.domid))
+        d_config.c_info.domid = domid_policy;
+
 start:
     assert(domid == INVALID_DOMID);
 
@@ -938,22 +952,6 @@ start:
         }
     }
 
-    libxl_asyncprogress_how autoconnect_console_how_buf;
-    if ( dom_info->console_autoconnect ) {
-        if (libxl_pipe(ctx, notify_pipe)) {
-            ret = ERROR_FAIL;
-            goto error_out;
-        }
-        autoconnect_console_how_buf.callback = autoconnect_console;
-        autoconnect_console_how_buf.for_callback = &notify_pipe[1];
-        autoconnect_console_how = &autoconnect_console_how_buf;
-    }else{
-        autoconnect_console_how = 0;
-    }
-
-    if (!libxl_domid_valid_guest(d_config.c_info.domid))
-        d_config.c_info.domid = domid_policy;
-
     if ( restoring ) {
         libxl_domain_restore_params params;
 
-- 
2.47.3

[PATCH 2/3] tools/xl: free loaded configuration data earlier in create_domain()

[Elliott Mitchell]

Rather than waiting for just before return, release the memory sooner.
This potentially reduces process size due to be reused sooner.  Also
ensures it never gets into the daemon process.
---
 tools/xl/xl_vmcontrol.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/xl/xl_vmcontrol.c b/tools/xl/xl_vmcontrol.c
index 93766f631b..76c2a07ee8 100644
--- a/tools/xl/xl_vmcontrol.c
+++ b/tools/xl/xl_vmcontrol.c
@@ -853,6 +853,8 @@ int create_domain(struct domain_create *dom_info)
         parse_config_data(config_source, config_data, config_len, &d_config);
     }
 
+    free(config_data);
+
     if (!dom_info->ignore_global_affinity_masks) {
         libxl_domain_build_info *b_info = &d_config.b_info;
 
@@ -1200,8 +1202,6 @@ out:
 
     libxl_domain_config_dispose(&d_config);
 
-    free(config_data);
-
     console_child_report(child_console);
 
     if (deathw)
-- 
2.47.3

[PATCH 1/3] tools/xl: use Make variable to retrieve xl object filename list

[Elliott Mitchell]

Irrelevant now, but $^ potentially makes the Makefile more maintainable.
Hopefully some future author finds this valuable.
---
 tools/xl/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/xl/Makefile b/tools/xl/Makefile
index e4eed8be13..8fbd4ae778 100644
--- a/tools/xl/Makefile
+++ b/tools/xl/Makefile
@@ -34,7 +34,7 @@ $(XL_OBJS): CFLAGS += -include $(XEN_ROOT)/tools/config.h # libxl_json.h needs i
 all: xl
 
 xl: $(XL_OBJS)
-	$(CC) $(LDFLAGS) -o $@ $(XL_OBJS) $(LDLIBS_libxenutil) $(LDLIBS_libxenlight) $(LDLIBS_libxentoollog) $(LDLIBS_libxenstore) $(XEN_JSON_LIBS) $(APPEND_LDFLAGS)
+	$(CC) $(LDFLAGS) -o $@ $^ $(LDLIBS_libxenutil) $(LDLIBS_libxenlight) $(LDLIBS_libxentoollog) $(LDLIBS_libxenstore) $(XEN_JSON_LIBS) $(APPEND_LDFLAGS)
 
 .PHONY: install
 install: all
-- 
2.47.3