Skip to contentSkip to conversation

libxl/xl: support for domain 0 bootloader (e.g. pygrub)

PATCH In review Ian Campbell 3 participants 12 messages Jul 12, 2010 – Jul 14, 2010

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 libxl/xl: support for domain 0 bootloader (e.g. pygrub)

This series adds support to libxl/xl for running a domain 0 bootloader (such as pygrub).

#1 lore raw

PATCH 1/1 pygrub: introduce easier to parse output format

libxl would rather like to parse the output of pygrub. Rather than implement an SXP parser in libxl add a --output-format option to pygrub which can select an alternative, simpler to parse, format. Available formats are:

    sxp:	current SXP output format;
    simple:	simple key+value output with \n separating item ( for
		debugging). key and value are separated by a single
		space (and key therefore cannot contain a space);
    simple0:	as simple but with \0 as a separator;

Also add --output-directory to allow temporary files to be placed somewhere other than /var/run/xend/boot.

tools/pygrub/src/pygrub +44 -13
diff -r 4f194a196734 -r 988cf8dff1c0 tools/pygrub/src/pygrub
@@ -630,16 +630,34 @@
 
     return cfg
 
+def format_sxp(kernel, ramdisk, args):
+    s = "linux (kernel %s)" % kernel
+    if ramdisk:
+        s += "(ramdisk %s)" % ramdisk
+    if args:
+        s += "(args \"%s\")" % args
+    return s
+                
+def format_simple(kernel, ramdisk, args, sep):
+    s = ("kernel %s" % kernel) + sep
+    if ramdisk:
+        s += ("ramdisk %s" % ramdisk) + sep
+    if args:
+        s += ("args %s" % args) + sep
+    s += sep
+    return s
+
 if __name__ == "__main__":
     sel = None
     
     def usage():
-        print >> sys.stderr, "Usage: %s [-q|--quiet] [-i|--interactive] [-n|--not-really] [--output=] [--kernel=] [--ramdisk=] [--args=] [--entry=] <image>" %(sys.argv[0],)
+        print >> sys.stderr, "Usage: %s [-q|--quiet] [-i|--interactive] [-n|--not-really] [--output=] [--kernel=] [--ramdisk=] [--args=] [--entry=] [--output-directory=] [--output-format=sxp|simple|simple0] <image>" %(sys.argv[0],)
 
     try:
         opts, args = getopt.gnu_getopt(sys.argv[1:], 'qinh::',
-                                   ["quiet", "interactive", "not-really", 
-                                    "help", "output=", "entry=", "kernel=", 
+                                   ["quiet", "interactive", "not-really", "help", 
+                                    "output=", "output-format=", "output-directory=",
+                                    "entry=", "kernel=", 
                                     "ramdisk=", "args=", "isconfig"])
     except getopt.GetoptError:
         usage()
@@ -655,6 +673,8 @@
     interactive = True
     isconfig = False
     not_really = False
+    output_format = "sxp"
+    output_directory = "/var/run/xend/boot"
 
     # what was passed in
     incfg = { "kernel": None, "ramdisk": None, "args": "" }
@@ -687,6 +707,14 @@
             interactive = False
         elif o in ("--isconfig",):
             isconfig = True
+        elif o in ("--output-format",):
+            if a not in ["sxp", "simple", "simple0"]:
+                print "unkonwn output format %s" % a
+                usage()
+                sys.exit(1)
+            output_format = a
+        elif o in ("--output-directory",):
+            output_directory = a
 
     if output is None or output == "-":
         fd = sys.stdout.fileno()
@@ -723,7 +751,7 @@
     else:
         data = fs.open_file(chosencfg["kernel"]).read()
         (tfd, bootcfg["kernel"]) = tempfile.mkstemp(prefix="boot_kernel.",
-                                                    dir="/var/run/xend/boot")
+                                                    dir=output_directory)
         os.write(tfd, data)
         os.close(tfd)
 
@@ -733,26 +761,29 @@
         else:
             data = fs.open_file(chosencfg["ramdisk"],).read()
             (tfd, bootcfg["ramdisk"]) = tempfile.mkstemp(
-                prefix="boot_ramdisk.", dir="/var/run/xend/boot")
+                prefix="boot_ramdisk.", dir=output_directory)
             os.write(tfd, data)
             os.close(tfd)
     else:
         initrd = None
 
-    sxp = "linux (kernel %s)" % bootcfg["kernel"]
-    if bootcfg["ramdisk"]:
-        sxp += "(ramdisk %s)" % bootcfg["ramdisk"]
+    args = None
     if chosencfg["args"]:
         zfsinfo = fsimage.getbootstring(fs)
-        if zfsinfo is None:
-            sxp += "(args \"%s\")" % chosencfg["args"]
-        else:
+        if zfsinfo is not None:
             e = re.compile("zfs-bootfs=[\w\-\.\:@/]+" )
             (chosencfg["args"],count) = e.subn(zfsinfo, chosencfg["args"])
             if count == 0:
                chosencfg["args"] += " -B %s" % zfsinfo
-            sxp += "(args \"%s\")" % (chosencfg["args"])
+        args = chosencfg["args"]
+
+    if output_format == "sxp":
+        ostring = format_sxp(bootcfg["kernel"], bootcfg["ramdisk"], args)
+    elif output_format == "simple":
+        ostring = format_simple(bootcfg["kernel"], bootcfg["ramdisk"], args, "\n")
+    elif output_format == "simple0":
+        ostring = format_simple(bootcfg["kernel"], bootcfg["ramdisk"], args, "\0")
 
     sys.stdout.flush()
-    os.write(fd, sxp)
+    os.write(fd, ostring)
#2 lore raw

PATCH 1/1 xenconsole: do not exit if a pty device is missing

In reply to Ian Campbell

this can just mean we have raced with the bootloader exiting and if we continue we will likely see the real domain console show up.

tools/console/client/main.c +2 -0
diff -r 988cf8dff1c0 -r 24a1b866b2d1 tools/console/client/main.c
@@ -115,6 +115,8 @@
 			 * disambiguate: just read the pty path */
 			pty_path = xs_read(xs, XBT_NULL, path, &len);
 			if (pty_path != NULL) {
+				if (access(pty_path, R_OK|W_OK) != 0)
+					continue;
 				pty_fd = open(pty_path, O_RDWR | O_NOCTTY);
 				if (pty_fd == -1) 
 					err(errno, "Could not open tty `%s'", 
#3 lore raw

PATCH 1/1 libxl: add printf attribute to libxl_xs_write and fixup resulting warnings

In reply to Ian Campbell

There is an unfortunate warning for the empty format string by default which I workaround by adding -Wno-format-zero-length.

tools/libxl/Makefile +1 -1
diff -r 24a1b866b2d1 -r 8545cbf7f513 tools/libxl/Makefile
@@ -11,7 +11,7 @@
 XLUMAJOR = 1.0
 XLUMINOR = 0
 
-CFLAGS += -Werror
+CFLAGS += -Werror -Wno-format-zero-length
 CFLAGS += -I. -fPIC
 CFLAGS += $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) $(CFLAGS_libxenstore) $(CFLAGS_libblktapctl)
 
tools/libxl/libxl.c +4 -4
diff -r 24a1b866b2d1 -r 8545cbf7f513 tools/libxl/libxl.c
@@ -523,7 +523,7 @@
         path = libxl_sprintf(ctx, "/local/domain/0/device-model/%d/state", domid);
         state = libxl_xs_read(ctx, XBT_NULL, path);
         if (state != NULL && !strcmp(state, "paused")) {
-            libxl_xs_write(ctx, XBT_NULL, libxl_sprintf(ctx, "/local/domain/0/device-model/%d/command", domid), "continue", strlen("continue"));
+            libxl_xs_write(ctx, XBT_NULL, libxl_sprintf(ctx, "/local/domain/0/device-model/%d/command", domid), "continue");
             libxl_wait_for_device_model(ctx, domid, "running", NULL, NULL);
         }
     }
@@ -2663,7 +2663,7 @@
     }
 
     if (domid != 0)
-        libxl_xs_write(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/memory/static-max", dompath), "%lu", max_memkb);
+        libxl_xs_write(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/memory/static-max", dompath), "%"PRIu32, max_memkb);
 
     return 0;
 }
@@ -2702,14 +2702,14 @@
     videoram_s = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/memory/videoram", dompath));
     videoram = videoram_s ? atoi(videoram_s) : 0;
 
-    libxl_xs_write(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/memory/target", dompath), "%lu", target_memkb);
+    libxl_xs_write(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/memory/target", dompath), "%"PRIu32, target_memkb);
 
     rc = xc_domain_getinfolist(ctx->xch, domid, 1, &info);
     if (rc != 1 || info.domain != domid)
         return rc;
     xcinfo2xlinfo(&info, &ptr);
     uuid = libxl_uuid2string(ctx, ptr.uuid);
-    libxl_xs_write(ctx, XBT_NULL, libxl_sprintf(ctx, "/vm/%s/memory", uuid), "%lu", target_memkb / 1024);
+    libxl_xs_write(ctx, XBT_NULL, libxl_sprintf(ctx, "/vm/%s/memory", uuid), "%"PRIu32, target_memkb / 1024);
 
     if (enforce || !domid)
         memorykb = target_memkb;
tools/libxl/libxl_dom.c +3 -3
diff -r 24a1b866b2d1 -r 8545cbf7f513 tools/libxl/libxl_dom.c
@@ -247,7 +247,7 @@
         return 1;
     }
     path = libxl_sprintf(si->ctx, "%s/control/shutdown", libxl_xs_get_dompath(si->ctx, si->domid));
-    libxl_xs_write(si->ctx, XBT_NULL, path, "suspend", strlen("suspend"));
+    libxl_xs_write(si->ctx, XBT_NULL, path, "suspend");
     if (si->hvm) {
         unsigned long hvm_pvdrv, hvm_s_state;
         xc_get_hvm_param(si->ctx->xch, si->domid, HVM_PARAM_CALLBACK_IRQ, &hvm_pvdrv);
@@ -275,7 +275,7 @@
     }
     if (!strcmp(state, "suspend")) {
         XL_LOG(si->ctx, XL_LOG_ERROR, "guest didn't suspend in time");
-        libxl_xs_write(si->ctx, XBT_NULL, path, "", 1);
+        libxl_xs_write(si->ctx, XBT_NULL, path, "");
     }
     return 1;
 }
@@ -338,7 +338,7 @@
     char *filename = libxl_sprintf(ctx, "/var/lib/xen/qemu-save.%d", domid);
 
     XL_LOG(ctx, XL_LOG_DEBUG, "Saving device model state to %s", filename);
-    libxl_xs_write(ctx, XBT_NULL, libxl_sprintf(ctx, "/local/domain/0/device-model/%d/command", domid), "save", strlen("save"));
+    libxl_xs_write(ctx, XBT_NULL, libxl_sprintf(ctx, "/local/domain/0/device-model/%d/command", domid), "save");
     libxl_wait_for_device_model(ctx, domid, "paused", NULL, NULL);
 
     c = libxl_write_exactly(ctx, fd, QEMU_SIGNATURE, strlen(QEMU_SIGNATURE),
tools/libxl/libxl_internal.h +1 -1
diff -r 24a1b866b2d1 -r 8545cbf7f513 tools/libxl/libxl_internal.h
@@ -117,7 +117,7 @@
 int libxl_xs_writev(struct libxl_ctx *ctx, xs_transaction_t t,
                     char *dir, char **kvs);
 int libxl_xs_write(struct libxl_ctx *ctx, xs_transaction_t t,
-                   char *path, char *fmt, ...);
+                   char *path, char *fmt, ...) PRINTF_ATTRIBUTE(4, 5);
 char *libxl_xs_get_dompath(struct libxl_ctx *ctx, uint32_t domid); // logs errs
 char *libxl_xs_read(struct libxl_ctx *ctx, xs_transaction_t t, char *path);
 char **libxl_xs_directory(struct libxl_ctx *ctx, xs_transaction_t t, char *path, unsigned int *nb);
#4 lore raw

PATCH 1/1 libxl: add libxl_strdup convenience function

In reply to Ian Campbell

Use in preference to libxl_sprintf(..., "%s", "...")

tools/libxl/libxl.c +12 -12
diff -r 8545cbf7f513 -r 6c6961e09c50 tools/libxl/libxl.c
@@ -1339,7 +1339,7 @@
                 flexarray_set(back, boffset++, "tapdisk-params");
                 flexarray_set(back, boffset++, libxl_sprintf(ctx, "%s:%s", device_disk_string_of_phystype(disk->phystype), disk->physpath));
                 flexarray_set(back, boffset++, "params");
-                flexarray_set(back, boffset++, libxl_sprintf(ctx, "%s", dev));
+                flexarray_set(back, boffset++, libxl_strdup(ctx, dev));
                 backend_type = "phy";
                 device_physdisk_major_minor(dev, &major, &minor);
                 flexarray_set(back, boffset++, "physical-device");
@@ -1467,7 +1467,7 @@
                                                  nic->mac[0], nic->mac[1], nic->mac[2],
                                                  nic->mac[3], nic->mac[4], nic->mac[5]));
     flexarray_set(back, boffset++, "bridge");
-    flexarray_set(back, boffset++, libxl_sprintf(ctx, "%s", nic->bridge));
+    flexarray_set(back, boffset++, libxl_strdup(ctx, nic->bridge));
     flexarray_set(back, boffset++, "handle");
     flexarray_set(back, boffset++, libxl_sprintf(ctx, "%d", nic->devid));
 
@@ -2020,13 +2020,13 @@
 
     info->vnc = vfb->vnc;
     if (vfb->vnclisten)
-        info->vnclisten = libxl_sprintf(ctx, "%s", vfb->vnclisten);
+        info->vnclisten = libxl_strdup(ctx, vfb->vnclisten);
     info->vncdisplay = vfb->vncdisplay;
     info->vncunused = vfb->vncunused;
     if (vfb->vncpasswd)
         info->vncpasswd = vfb->vncpasswd;
     if (vfb->keymap)
-        info->keymap = libxl_sprintf(ctx, "%s", vfb->keymap);
+        info->keymap = libxl_strdup(ctx, vfb->keymap);
     info->sdl = vfb->sdl;
     info->opengl = vfb->opengl;
     for (i = 0; i < num_console; i++) {
@@ -2782,19 +2782,19 @@
     info->xen_version_major = xen_version >> 16;
     info->xen_version_minor = xen_version & 0xFF;
     xc_version(ctx->xch, XENVER_extraversion, &u.xen_extra);
-    info->xen_version_extra = libxl_sprintf(ctx, "%s", u.xen_extra);
+    info->xen_version_extra = libxl_strdup(ctx, u.xen_extra);
 
     xc_version(ctx->xch, XENVER_compile_info, &u.xen_cc);
-    info->compiler = libxl_sprintf(ctx, "%s", u.xen_cc.compiler);
-    info->compile_by = libxl_sprintf(ctx, "%s", u.xen_cc.compile_by);
-    info->compile_domain = libxl_sprintf(ctx, "%s", u.xen_cc.compile_domain);
-    info->compile_date = libxl_sprintf(ctx, "%s", u.xen_cc.compile_date);
+    info->compiler = libxl_strdup(ctx, u.xen_cc.compiler);
+    info->compile_by = libxl_strdup(ctx, u.xen_cc.compile_by);
+    info->compile_domain = libxl_strdup(ctx, u.xen_cc.compile_domain);
+    info->compile_date = libxl_strdup(ctx, u.xen_cc.compile_date);
 
     xc_version(ctx->xch, XENVER_capabilities, &u.xen_caps);
-    info->capabilities = libxl_sprintf(ctx, "%s", u.xen_caps);
+    info->capabilities = libxl_strdup(ctx, u.xen_caps);
 
     xc_version(ctx->xch, XENVER_changeset, &u.xen_chgset);
-    info->changeset = libxl_sprintf(ctx, "%s", u.xen_chgset);
+    info->changeset = libxl_strdup(ctx, u.xen_chgset);
 
     xc_version(ctx->xch, XENVER_platform_parameters, &u.p_parms);
     info->virt_start = u.p_parms.virt_start;
@@ -2802,7 +2802,7 @@
     info->pagesize = xc_version(ctx->xch, XENVER_pagesize, NULL);
 
     xc_version(ctx->xch, XENVER_commandline, &u.xen_commandline);
-    info->commandline = libxl_sprintf(ctx, "%s", u.xen_commandline);
+    info->commandline = libxl_strdup(ctx, u.xen_commandline);
 
     return info;
 }
tools/libxl/libxl_exec.c +1 -1
diff -r 8545cbf7f513 -r 6c6961e09c50 tools/libxl/libxl_exec.c
@@ -100,7 +100,7 @@
     struct libxl_spawn_starting *for_spawn = starting->for_spawn;
 
     if (for_spawn) {
-        for_spawn->what = libxl_sprintf(ctx, "%s", what);
+        for_spawn->what = libxl_strdup(ctx, what);
         if (!for_spawn->what) return ERROR_NOMEM;
     }
 
tools/libxl/libxl_internal.c +11 -1
diff -r 8545cbf7f513 -r 6c6961e09c50 tools/libxl/libxl_internal.c
@@ -138,10 +138,20 @@
     return s;
 }
 
+char *libxl_strdup(struct libxl_ctx *ctx, const char *c)
+{
+    char *s = strdup(c);
+
+    if (s)
+        libxl_ptr_add(ctx, s);
+
+    return s;
+}
+
 char *libxl_dirname(struct libxl_ctx *ctx, const char *s)
 {
     char *c;
-    char *ptr = libxl_sprintf(ctx, "%s", s);
+    char *ptr = libxl_strdup(ctx, s);
 
     c = strrchr(ptr, '/');
     if (!c)
tools/libxl/libxl_internal.h +1 -0
diff -r 8545cbf7f513 -r 6c6961e09c50 tools/libxl/libxl_internal.h
@@ -111,6 +111,7 @@
 void *libxl_zalloc(struct libxl_ctx *ctx, int bytes);
 void *libxl_calloc(struct libxl_ctx *ctx, size_t nmemb, size_t size);
 char *libxl_sprintf(struct libxl_ctx *ctx, const char *fmt, ...) PRINTF_ATTRIBUTE(2, 3);
+char *libxl_strdup(struct libxl_ctx *ctx, const char *c);
 char *libxl_dirname(struct libxl_ctx *ctx, const char *s);
 
 char **libxl_xs_kvs_of_flexarray(struct libxl_ctx *ctx, flexarray_t *array, int length);
#5 lore raw

PATCH 1/1 libxl: fix typo

In reply to Ian Campbell
tools/libxl/libxl_internal.h +1 -1
diff -r 6c6961e09c50 -r 4b41b5e7532c tools/libxl/libxl_internal.h
@@ -176,7 +176,7 @@
  /* higher-level double-fork and separate detach eg as for device models */
 
 struct libxl_spawn_starting {
-    /* put this in your own stateu structure as returned to application */
+    /* put this in your own status structure as returned to application */
     /* all fields are private to libxl_spawn_... */
     pid_t intermediate;
     char *what; /* malloc'd in spawn_spawn */
#6 lore raw

PATCH 1/1 libxl/xl: exec xenconsole in current process, defer decision to fork to caller

In reply to Ian Campbell

Use this to run xenconsole as the foreground process and move the connection to the console in the "create -c" case early enough to be able to view output from the bootloader. This behaviour is consistent with how both "xm console" and "xm create -c" operate.

tools/libxl/libxl.c +5 -5
diff -r 4b41b5e7532c -r bcea013ddd5a tools/libxl/libxl.c
@@ -786,12 +786,12 @@
     return 0;
 }
 
-int libxl_console_attach(struct libxl_ctx *ctx, uint32_t domid, int cons_num)
+int libxl_console_exec(struct libxl_ctx *ctx, uint32_t domid, int cons_num)
 {
-    char *cmd = libxl_sprintf(
-        ctx, "%s/xenconsole %d --num %d",
-        libxl_private_bindir_path(), domid, cons_num);
-    return (system(cmd) != 0) ? ERROR_FAIL : 0;
+    char *p = libxl_sprintf(ctx, "%s/xenconsole", libxl_private_bindir_path());
+    char *domid_s = libxl_sprintf(ctx, "%d", domid);
+    char *cons_num_s = libxl_sprintf(ctx, "%d", cons_num);
+    return execl(p, p, domid_s, "--num", cons_num_s, (void *)NULL) == 0 ? 0 : ERROR_FAIL;
 }
 
 static char ** libxl_build_device_model_args(struct libxl_ctx *ctx,
tools/libxl/libxl.h +1 -1
diff -r 4b41b5e7532c -r bcea013ddd5a tools/libxl/libxl.h
@@ -358,7 +358,7 @@
 int libxl_domain_setmaxmem(struct libxl_ctx *ctx, uint32_t domid, uint32_t target_memkb);
 int libxl_set_memory_target(struct libxl_ctx *ctx, uint32_t domid, uint32_t target_memkb, int enforce);
 
-int libxl_console_attach(struct libxl_ctx *ctx, uint32_t domid, int cons_num);
+int libxl_console_exec(struct libxl_ctx *ctx, uint32_t domid, int cons_num);
 
 int libxl_domain_info(struct libxl_ctx*, struct libxl_dominfo *info_r,
                       uint32_t domid);
tools/libxl/xl_cmdimpl.c +44 -17

113 lines not on this page · raw message

#7 lore raw

PATCH 1/1 libxl: support mapping files rather than carrying paths around

In reply to Ian Campbell

This will allow us to map and then unlink the file and therefore delete the process on process exit or explicit unmap.

Using the mmaped versions of these files required rewriting build_pv to use the xc_dom builder functionality directly rather than through the xc_linux_build "compatibility layer". (The status of the xc_linux_build interface as a compatibility layer seems a bit dubious since all existing callers use it but if anything is going to replace it then libxl seems like the likely candidate).

I'm not thrilled with the definition of the maps lifecycle. This could be solved by adding a helper function to explicitly free the toplevel structure.

tools/libxl/libxl.c +61 -8
diff -r bcea013ddd5a -r 2d4475143f70 tools/libxl/libxl.c
@@ -22,6 +22,7 @@
 #include <sys/types.h>
 #include <fcntl.h>
 #include <sys/select.h>
+#include <sys/mman.h>
 #include <sys/wait.h>
 #include <signal.h>
 #include <unistd.h> /* for write, unlink and close */
@@ -291,12 +292,12 @@
         vments[i++] = "image/ostype";
         vments[i++] = "linux";
         vments[i++] = "image/kernel";
-        vments[i++] = (char*) info->kernel;
+        vments[i++] = (char*) info->kernel.path;
         vments[i++] = "start_time";
         vments[i++] = libxl_sprintf(ctx, "%lu.%02d", start_time.tv_sec,(int)start_time.tv_usec/10000);
-        if (info->u.pv.ramdisk) {
+        if (info->u.pv.ramdisk.path) {
             vments[i++] = "image/ramdisk";
-            vments[i++] = (char*) info->u.pv.ramdisk;
+            vments[i++] = (char*) info->u.pv.ramdisk.path;
         }
         if (info->u.pv.cmdline) {
             vments[i++] = "image/cmdline";
@@ -305,6 +306,10 @@
     }
     ret = build_post(ctx, domid, info, state, vments, localents);
 out:
+    libxl_file_reference_unmap(ctx, &info->kernel);
+    if (!info->hvm)
+	    libxl_file_reference_unmap(ctx, &info->u.pv.ramdisk);
+
     return ret;
 }
 
@@ -338,12 +343,12 @@
         vments[i++] = "image/ostype";
         vments[i++] = "linux";
         vments[i++] = "image/kernel";
-        vments[i++] = (char*) info->kernel;
+        vments[i++] = (char*) info->kernel.path;
         vments[i++] = "start_time";
         vments[i++] = libxl_sprintf(ctx, "%lu.%02d", start_time.tv_sec,(int)start_time.tv_usec/10000);
-        if (info->u.pv.ramdisk) {
+        if (info->u.pv.ramdisk.path) {
             vments[i++] = "image/ramdisk";
-            vments[i++] = (char*) info->u.pv.ramdisk;
+            vments[i++] = (char*) info->u.pv.ramdisk.path;
         }
         if (info->u.pv.cmdline) {
             vments[i++] = "image/cmdline";
@@ -361,6 +366,10 @@
     }
 
 out:
+    libxl_file_reference_unmap(ctx, &info->kernel);
+    if (!info->hvm)
+	    libxl_file_reference_unmap(ctx, &info->u.pv.ramdisk);
+
     esave = errno;
 
     flags = fcntl(fd, F_GETFL);
@@ -1057,9 +1066,9 @@
     b_info.max_vcpus = 1;
     b_info.max_memkb = 32 * 1024;
     b_info.target_memkb = b_info.max_memkb;
-    b_info.kernel = libxl_abs_path(ctx, "ioemu-stubdom.gz", libxl_xenfirmwaredir_path());
+    b_info.kernel.path = libxl_abs_path(ctx, "ioemu-stubdom.gz", libxl_xenfirmwaredir_path());
     b_info.u.pv.cmdline = libxl_sprintf(ctx, " -d %d", info->domid);
-    b_info.u.pv.ramdisk = "";
+    b_info.u.pv.ramdisk.path = "";
     b_info.u.pv.features = "";
     b_info.hvm = 0;
 
@@ -3181,3 +3190,47 @@
     return rc;
 }
 
+int libxl_file_reference_map(struct libxl_ctx *ctx, libxl_file_reference *f)
+{
+	struct stat st_buf;
+	int ret, fd;
+	void *data;
+
+	if (f->mapped)
+		return 0;
+
+	fd = open(f->path, O_RDONLY);
+	if (f < 0)
+		return ERROR_FAIL;
+
+	ret = fstat(fd, &st_buf);
+	if (ret < 0)
+		goto out;
+
+	ret = -1;
+	data = mmap(NULL, st_buf.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
+	if (data == NULL)
+		goto out;
+
+	f->mapped = 1;
+	f->data = data;
+	f->size = st_buf.st_size;
+
+	ret = 0;
+out:
+	close(fd);
+
+	return ret == 0 ? 0 : ERROR_FAIL;
+}
+
+int libxl_file_reference_unmap(struct libxl_ctx *ctx, libxl_file_reference *f)
+{
+	int ret;
+
+	if (!f->mapped)
+		return 0;
+
+	ret = munmap(f->data, f->size);
+
+	return ret == 0 ? 0 : ERROR_FAIL;
+}
tools/libxl/libxl.h +23 -2
diff -r bcea013ddd5a -r 2d4475143f70 tools/libxl/libxl.h
@@ -90,6 +90,24 @@
 } libxl_domain_create_info;
 
 typedef struct {
+    /*
+     * Path is always set if the file refernece is valid. However if
+     * mapped is true then the actual file may already be unlinked.
+     */
+    char *path;
+    int mapped;
+    void *data;
+    size_t size;
+} libxl_file_reference;
+
+/*
+ * Instances of libxl_file_reference contained in this struct which
+ * have been mapped (with libxl_file_reference_map) will be unmapped
+ * by libxl_domain_build/restore. If either of these are never called
+ * then the user is responsible for calling
+ * libxl_file_reference_unmap.
+ */
+typedef struct {
     int max_vcpus;
     int cur_vcpus;
     int tsc_mode;
@@ -98,7 +116,7 @@
     uint32_t video_memkb;
     uint32_t shadow_memkb;
     bool disable_migrate;
-    const char *kernel;
+    libxl_file_reference kernel;
     int hvm;
     union {
         struct {
@@ -115,7 +133,7 @@
         struct {
             uint32_t   slack_memkb;
             const char *cmdline;
-            const char *ramdisk;
+            libxl_file_reference ramdisk;
             const char *features;
         } pv;
     } u;
@@ -308,6 +326,9 @@
 int libxl_domain_shutdown(struct libxl_ctx *ctx, uint32_t domid, int req);
 int libxl_domain_destroy(struct libxl_ctx *ctx, uint32_t domid, int force);
 
+int libxl_file_reference_map(struct libxl_ctx *ctx, libxl_file_reference *f);
+int libxl_file_reference_unmap(struct libxl_ctx *ctx, libxl_file_reference *f);
+
 char *libxl_uuid2string(struct libxl_ctx *ctx, uint8_t uuid[16]);
   /* 0 means ERROR_ENOMEM, which we have logged */
 
tools/libxl/libxl_dom.c +70 -11

104 lines not on this page · raw message

tools/libxl/xl_cmdimpl.c +6 -6
diff -r bcea013ddd5a -r 2d4475143f70 tools/libxl/xl_cmdimpl.c
@@ -196,7 +196,7 @@
     if (c_info->hvm) {
         b_info->shadow_memkb = 0; /* Set later */
         b_info->video_memkb = 8 * 1024;
-        b_info->kernel = "hvmloader";
+        b_info->kernel.path = "hvmloader";
         b_info->hvm = 1;
         b_info->u.hvm.pae = 1;
         b_info->u.hvm.apic = 1;
@@ -366,7 +366,7 @@
     printf("\t(image\n");
     if (c_info->hvm) {
         printf("\t\t(hvm\n");
-        printf("\t\t\t(loader %s)\n", b_info->kernel);
+        printf("\t\t\t(loader %s)\n", b_info->kernel.path);
         printf("\t\t\t(video_memkb %d)\n", b_info->video_memkb);
         printf("\t\t\t(shadow_memkb %d)\n", b_info->shadow_memkb);
         printf("\t\t\t(pae %d)\n", b_info->u.hvm.pae);
@@ -397,9 +397,9 @@
         printf("\t\t)\n");
     } else {
         printf("\t\t(linux %d)\n", b_info->hvm);
-        printf("\t\t\t(kernel %s)\n", b_info->kernel);
+        printf("\t\t\t(kernel %s)\n", b_info->kernel.path);
         printf("\t\t\t(cmdline %s)\n", b_info->u.pv.cmdline);
-        printf("\t\t\t(ramdisk %s)\n", b_info->u.pv.ramdisk);
+        printf("\t\t\t(ramdisk %s)\n", b_info->u.pv.ramdisk.path);
         printf("\t\t)\n");
     }
     printf("\t)\n");
@@ -563,7 +563,7 @@
         b_info->video_memkb = l * 1024;
 
     if (!xlu_cfg_get_string (config, "kernel", &buf))
-        b_info->kernel = strdup(buf);
+        b_info->kernel.path = strdup(buf);
 
     if (c_info->hvm == 1) {
         if (!xlu_cfg_get_long (config, "pae", &l))
@@ -603,7 +603,7 @@
 
         b_info->u.pv.cmdline = cmdline;
         if (!xlu_cfg_get_string (config, "ramdisk", &buf))
-            b_info->u.pv.ramdisk = strdup(buf);
+            b_info->u.pv.ramdisk.path = strdup(buf);
     }
 
     if (!xlu_cfg_get_list (config, "disk", &vbds, 0)) {
tools/ocaml/libs/xl/xl_stubs.c +2 -2
diff -r bcea013ddd5a -r 2d4475143f70 tools/ocaml/libs/xl/xl_stubs.c
@@ -103,7 +103,7 @@
 	c_val->target_memkb = Int64_val(Field(v, 6));
 	c_val->video_memkb = Int64_val(Field(v, 7));
 	c_val->shadow_memkb = Int64_val(Field(v, 8));
-	c_val->kernel = String_val(Field(v, 9));
+	c_val->kernel.path = String_val(Field(v, 9));
 	c_val->hvm = Tag_val(Field(v, 10)) == 0;
 	infopriv = Field(Field(v, 10), 0);
 	if (c_val->hvm) {
@@ -119,7 +119,7 @@
 	} else {
 		c_val->u.pv.slack_memkb = Int64_val(Field(infopriv, 0));
 		c_val->u.pv.cmdline = String_val(Field(infopriv, 1));
-		c_val->u.pv.ramdisk = String_val(Field(infopriv, 2));
+		c_val->u.pv.ramdisk.path = String_val(Field(infopriv, 2));
 		c_val->u.pv.features = String_val(Field(infopriv, 3));
 	}
#8 lore raw

PATCH 1/1 libxl: add function to attach/detach a disk to/from the local VM

In reply to Ian Campbell

Useful if you need to read a guest filesystem (e.g. pygrub).

I'm not overly thrilled with the implementation WRT tap interfaces, particularly WRT to detach. I was unable to find a way to get at the paramters necessary to call tap_ctl_destroy so I assumed for now it that is OK to assume that the tap device is going to be wanted for the actual domain at some point in the immediate future and hence there is no pressing need to destroy it.

tools/libxl/libxl.c +42 -0
diff -r 2d4475143f70 -r 769e3bc13a6b tools/libxl/libxl.c
@@ -1425,6 +1425,48 @@
     return libxl_device_del(ctx, &device, wait);
 }
 
+const char * libxl_device_disk_local_attach(struct libxl_ctx *ctx, libxl_device_disk *disk)
+{
+    char *dev = NULL;
+    int phystype = disk->phystype;
+    switch (phystype) {
+        case PHYSTYPE_PHY: {
+            fprintf(stderr, "attaching PHY disk %s to domain 0\n", disk->physpath);
+            dev = disk->physpath;
+            break;
+        }
+        case PHYSTYPE_FILE:
+            /* let's pretend is tap:aio for the moment */
+            phystype = PHYSTYPE_AIO;
+        case PHYSTYPE_AIO: case PHYSTYPE_QCOW: case PHYSTYPE_QCOW2: case PHYSTYPE_VHD: {
+            const char *msg;
+            if (!tap_ctl_check(&msg)) {
+                const char *type = device_disk_string_of_phystype(phystype);
+                dev = get_blktap2_device(ctx, disk->physpath, type);
+                if (!dev)
+                    dev = make_blktap2_device(ctx, disk->physpath, type);
+            }
+            break;
+        }
+        default:
+            XL_LOG(ctx, XL_LOG_ERROR, "unrecognized disk physical type: %d\n", phystype);
+            break;
+    }
+    return dev;
+}
+
+int libxl_device_disk_local_detach(struct libxl_ctx *ctx, libxl_device_disk *disk)
+{
+    /* Nothing to do for PHYSTYPE_PHY. */
+
+    /*
+     * For other device types assume that the blktap2 process is
+     * needed by the soon to be started domain and do nothing.
+     */
+
+    return 0;
+}
+
 /******************************************************************************/
 int libxl_device_nic_add(struct libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic)
 {
tools/libxl/libxl.h +6 -0
diff -r 2d4475143f70 -r 769e3bc13a6b tools/libxl/libxl.h
@@ -423,6 +423,12 @@
                               libxl_device_disk *disk, libxl_diskinfo *diskinfo);
 int libxl_cdrom_insert(struct libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk);
 
+/*
+ * Make a disk available in this domain. Returns path to a device.
+ */
+const char * libxl_device_disk_local_attach(struct libxl_ctx *ctx, libxl_device_disk *disk);
+int libxl_device_disk_local_detach(struct libxl_ctx *ctx, libxl_device_disk *disk);
+
 typedef struct {
     char *backend;
     uint32_t backend_id;
#9 lore raw

PATCH 1/1 libxl/xl: support running bootloader (e.g. pygrub) in domain 0

In reply to Ian Campbell

Much of the bootloader interaction (including the Solaris and NetBSD portability bits) are translated pretty much directly from the python in tools/python/xen/xend/XendBootloader.py

tools/libxl/Makefile +2 -2
diff -r 769e3bc13a6b -r 5ce5fe094152 tools/libxl/Makefile
@@ -15,9 +15,9 @@
 CFLAGS += -I. -fPIC
 CFLAGS += $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) $(CFLAGS_libxenstore) $(CFLAGS_libblktapctl)
 
-LIBS = $(LDFLAGS_libxenctrl) $(LDFLAGS_libxenguest) $(LDFLAGS_libxenstore) $(LDFLAGS_libblktapctl)
+LIBS = $(LDFLAGS_libxenctrl) $(LDFLAGS_libxenguest) $(LDFLAGS_libxenstore) $(LDFLAGS_libblktapctl) -lutil
 
-LIBXL_OBJS-y = osdeps.o libxl_paths.o
+LIBXL_OBJS-y = osdeps.o libxl_paths.o libxl_bootloader.o
 LIBXL_OBJS = flexarray.o libxl.o libxl_dom.o libxl_exec.o libxl_xshelp.o libxl_device.o libxl_internal.o xenguest.o libxl_utils.o $(LIBXL_OBJS-y)
 
 AUTOINCS= libxlu_cfg_y.h libxlu_cfg_l.h
tools/libxl/libxl.h +20 -1
diff -r 769e3bc13a6b -r 5ce5fe094152 tools/libxl/libxl.h
@@ -132,7 +132,9 @@
         } hvm;
         struct {
             uint32_t   slack_memkb;
-            const char *cmdline;
+            const char *bootloader;
+            const char *bootloader_args;
+            char *cmdline;
             libxl_file_reference ramdisk;
             const char *features;
         } pv;
@@ -329,6 +331,23 @@
 int libxl_file_reference_map(struct libxl_ctx *ctx, libxl_file_reference *f);
 int libxl_file_reference_unmap(struct libxl_ctx *ctx, libxl_file_reference *f);
 
+/*
+ * Run the configured bootloader for a PV domain and update
+ * info->kernel, info->u.pv.ramdisk and info->u.pv.cmdline as
+ * appropriate (any initial values present in these fields must have
+ * been allocated with malloc).
+ *
+ * Is a NOP on non-PV domains or those with no bootloader configured.
+ *
+ * Users should call libxl_file_reference_unmap on the kernel and
+ * ramdisk to cleanup or rely on libxl_domain_{build,restore} to do
+ * it.
+ */
+int libxl_run_bootloader(struct libxl_ctx *ctx,
+                         libxl_domain_build_info *info,
+                         libxl_device_disk *disk,
+                         uint32_t domid);
+
 char *libxl_uuid2string(struct libxl_ctx *ctx, uint8_t uuid[16]);
   /* 0 means ERROR_ENOMEM, which we have logged */
 
tools/libxl/libxl_bootloader.c new file +449 -0

449 lines not on this page · raw message

tools/libxl/xl_cmdimpl.c +22 -0
diff -r 769e3bc13a6b -r 5ce5fe094152 tools/libxl/xl_cmdimpl.c
@@ -363,6 +363,12 @@
     printf("\t(target_memkb %d)\n", b_info->target_memkb);
     printf("\t(nomigrate %d)\n", b_info->disable_migrate);
 
+    if (!c_info->hvm && b_info->u.pv.bootloader) {
+        printf("\t(bootloader %s)\n", b_info->u.pv.bootloader);
+        if (b_info->u.pv.bootloader_args)
+            printf("\t(bootloader_args %s)\n", b_info->u.pv.bootloader_args);
+    }
+
     printf("\t(image\n");
     if (c_info->hvm) {
         printf("\t\t(hvm\n");
@@ -598,6 +604,16 @@
 
         if ((root || extra) && !cmdline) {
             fprintf(stderr, "Failed to allocate memory for cmdline\n");
+            exit(1);
+        }
+
+        if (!xlu_cfg_get_string (config, "bootloader", &buf))
+            b_info->u.pv.bootloader = strdup(buf);
+        if (!xlu_cfg_get_string (config, "bootloader_args", &buf))
+            b_info->u.pv.bootloader_args = strdup(buf);
+
+        if (!b_info->u.pv.bootloader && !b_info->kernel.path) {
+            fprintf(stderr, "Neither kernel nor bootloader specified\n");
             exit(1);
         }
 
@@ -1159,6 +1175,12 @@
             goto error_out;
     }
 
+    ret = libxl_run_bootloader(&ctx, &info2, num_disks > 0 ? &disks[0] : NULL, domid);
+    if (ret) {
+        fprintf(stderr, "failed to run bootloader: %d\n", ret);
+        goto error_out;
+    }
+
     if (!restore_file || !need_daemon) {
         if (dm_info.saved_state) {
             free(dm_info.saved_state);
#10 lore raw
In reply to Ian Campbell

Ian Campbell writes ("[Xen-devel] [PATCH 0 of 9] libxl/xl: support for domain 0 bootloader (e.g. pygrub)"):

Quoted 2 lines

This series adds support to libxl/xl for running a domain 0 bootloader (such as pygrub).

Does anyone have any comments on this ? It all looks reasonable to me but I haven't reviewed every line. If no-one objects I will apply the whole series tomorrow.

Ian.

#11 lore raw
Quoted 9 lines · Ian Jackson

Ian Campbell writes ("[Xen-devel] [PATCH 0 of 9] libxl/xl: support for domain 0 bootloader (e.g. pygrub)"):

Quoted 2 lines

This series adds support to libxl/xl for running a domain 0 bootloader (such as pygrub).

Does anyone have any comments on this ? It all looks reasonable to me but I haven't reviewed every line. If no-one objects I will apply the whole series tomorrow.

Ian.

It's really a necessary feature for xl. I'm happy to test it after you merge it.

Thanks,

Zhigang