diff mbox series

[v5,06/21] libxl: write qemu arguments into separate xenstore keys

Message ID 20200428040433.23504-7-jandryuk@gmail.com (mailing list archive)
State Superseded
Headers show
Series Add support for qemu-xen runnning in a Linux-based stubdomain | expand

Commit Message

Jason Andryuk April 28, 2020, 4:04 a.m. UTC 
From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

This allows using arguments with spaces, like -append, without
nominating any special "separator" character.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
Changes in v3:
 - previous version of this patch "libxl: use \x1b to separate qemu
   arguments for linux stubdomain" used specific non-printable
   separator, but it was rejected as xenstore doesn't cope well with
   non-printable chars
---
 tools/libxl/libxl_dm.c | 39 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

Comments

Ian Jackson May 14, 2020, 4:25 p.m. UTC | #1 
Jason Andryuk writes ("[PATCH v5 06/21] libxl: write qemu arguments into separate xenstore keys"):
> +static int libxl__write_stub_linux_dmargs(libxl__gc *gc,
> +                                    int dm_domid, int guest_domid,
> +                                    char **args)
> +{
...
> +    vm_path = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("/local/domain/%d/vm", guest_domid));
> +    path = GCSPRINTF("%s/image/dmargs", vm_path);
> +
> +retry_transaction:
> +    t = xs_transaction_start(ctx->xsh);
> +    xs_write(ctx->xsh, t, path, "", 0);
> +    xs_set_permissions(ctx->xsh, t, path, roperm, ARRAY_SIZE(roperm));

This wants to be libxl__xs_mknod I think ?

> +    i = 1;
> +    for (i=1; args[i] != NULL; i++)
> +        xs_write(ctx->xsh, t, GCSPRINTF("%s/%03d", path, i), args[i], strlen(args[i]));

Can you do this with libxl__xs_transaction_* please, and a loop rather
than a goto ?

Why not use libxl__xs_write_checked ?

> +    xs_set_permissions(ctx->xsh, t, GCSPRINTF("%s/rtc/timeoffset", vm_path), roperm, ARRAY_SIZE(roperm));

This line seems out of place.  At least, it is not mentioned in the
commit message.  If it's needed, can you please split it out - and, of
course, then, provide an explanation :-).

> +    if (!xs_transaction_end(ctx->xsh, t, 0))
> +        if (errno == EAGAIN)
> +            goto retry_transaction;
> +    return 0;

Thanks,
Ian.
Jason Andryuk May 17, 2020, 2:29 p.m. UTC | #2 
On Thu, May 14, 2020 at 12:25 PM Ian Jackson <ian.jackson@citrix.com> wrote:
>
> Jason Andryuk writes ("[PATCH v5 06/21] libxl: write qemu arguments into separate xenstore keys"):

> > +    xs_set_permissions(ctx->xsh, t, GCSPRINTF("%s/rtc/timeoffset", vm_path), roperm, ARRAY_SIZE(roperm));
>
> This line seems out of place.  At least, it is not mentioned in the
> commit message.  If it's needed, can you please split it out - and, of
> course, then, provide an explanation :-).

This was copied over from the mini-os version.  It looks like it is
unused by qemu-xen, so it can be dropped.

Thanks,
Jason
diff mbox series

Patch

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 5a7d55686f..bdc23554eb 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2065,6 +2065,40 @@  static int libxl__vfb_and_vkb_from_hvm_guest_config(libxl__gc *gc,
     return 0;
 }
 
+static int libxl__write_stub_linux_dmargs(libxl__gc *gc,
+                                    int dm_domid, int guest_domid,
+                                    char **args)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    int i;
+    char *vm_path;
+    char *path;
+    struct xs_permissions roperm[2];
+    xs_transaction_t t;
+
+    roperm[0].id = 0;
+    roperm[0].perms = XS_PERM_NONE;
+    roperm[1].id = dm_domid;
+    roperm[1].perms = XS_PERM_READ;
+
+    vm_path = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("/local/domain/%d/vm", guest_domid));
+    path = GCSPRINTF("%s/image/dmargs", vm_path);
+
+retry_transaction:
+    t = xs_transaction_start(ctx->xsh);
+    xs_write(ctx->xsh, t, path, "", 0);
+    xs_set_permissions(ctx->xsh, t, path, roperm, ARRAY_SIZE(roperm));
+    i = 1;
+    for (i=1; args[i] != NULL; i++)
+        xs_write(ctx->xsh, t, GCSPRINTF("%s/%03d", path, i), args[i], strlen(args[i]));
+
+    xs_set_permissions(ctx->xsh, t, GCSPRINTF("%s/rtc/timeoffset", vm_path), roperm, ARRAY_SIZE(roperm));
+    if (!xs_transaction_end(ctx->xsh, t, 0))
+        if (errno == EAGAIN)
+            goto retry_transaction;
+    return 0;
+}
+
 static int libxl__write_stub_dmargs(libxl__gc *gc,
                                     int dm_domid, int guest_domid,
                                     char **args)
@@ -2274,7 +2308,10 @@  void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
 
     libxl__store_libxl_entry(gc, guest_domid, "dm-version",
         libxl_device_model_version_to_string(dm_config->b_info.device_model_version));
-    libxl__write_stub_dmargs(gc, dm_domid, guest_domid, args);
+    if (libxl__stubdomain_is_linux(&guest_config->b_info))
+        libxl__write_stub_linux_dmargs(gc, dm_domid, guest_domid, args);
+    else
+        libxl__write_stub_dmargs(gc, dm_domid, guest_domid, args);
     libxl__xs_printf(gc, XBT_NULL,
                      GCSPRINTF("%s/image/device-model-domid",
                                libxl__xs_get_dompath(gc, guest_domid)),