Re: [PATCH v3 3/6] [WIP] libxl: xsrestrict QEMU

[Stefano Stabellini <stefano.stabellini@eu.citrix.com>]

On Thu, 25 Jun 2015, Ian Campbell wrote:
> On Wed, 2015-06-10 at 11:09 +0100, Stefano Stabellini wrote:
> > Check whether QEMU supports the xsrestrict option, by parsing its --help
> > output. Store the result on xenstore for future reference on a per QEMU
> > binary basis, so that device_model_override still works fine with it.
> 
> Is there some way we could avoid needing to do this, e.g. by doing the
> restrict later on via a qmp request, before the guest is unpaused of
> course.

It would be tricky because it needs to be done very early at boot time
in QEMU. Also we would still need to know whether a specific device
model supports this option before actually spawning it. So we would
still have to resort to spawning a "temporary" QEMU beforehand.


> > Replace / with _ in the QEMU binary path before writing it to xenstore,
> > so that it doesn't get confused with xenstore paths.
> > 
> > If QEMU supports xsrestrict and emulator_id, pass xsrestrict=on to it.
> > Statically reserve two emulator_ids, one for device models and another
> > for pv qemus. Use the emulator_ids appropriately.
> > 
> > WIP: direct use of fork is forbidden in libxl
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > 
> > ---
> > Changes in v3:
> > - add emulator_ids
> > - mark as WIP
> > ---
> >  tools/libxl/libxl_dm.c       |   72 ++++++++++++++++++++++++++++++++++++++++++
> >  tools/libxl/libxl_internal.h |    7 ++++
> >  tools/libxl/libxl_utils.c    |   10 ++++++
> >  3 files changed, 89 insertions(+)
> > 
> > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > index 2809ba0..bf77f50 100644
> > --- a/tools/libxl/libxl_dm.c
> > +++ b/tools/libxl/libxl_dm.c
> > @@ -446,6 +446,65 @@ retry:
> >      return 0;
> >  }
> >  
> > +int libxl__check_qemu_supported(libxl__gc *gc, const char *dm, char *opt)
> > +{
> > +    libxl_ctx *ctx = libxl__gc_owner(gc);
> > +    pid_t pid;
> > +    int pipefd[2], status;
> > +    FILE *fp;
> > +    char *buf;
> > +    ssize_t buf_size = 512;
> > +    int ret = 0;
> > +    char *s;
> > +
> > +    s = libxl__strdup(gc, dm);
> > +    libxl__replace_chr(gc, s, '/', '_');
> > +    s = libxl__sprintf(gc, "libxl/%s/%s", s, opt);
> > +    buf = libxl__xs_read(gc, XBT_NULL, s);
> > +    if (buf != NULL)
> > +        return !strcmp(buf, "1");
> > +
> > +    if (access(dm, X_OK) < 0) {
> > +        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
> > +                         "device model %s is not executable", dm);
> > +        return ERROR_FAIL;
> > +    }
> > +
> > +    if (libxl_pipe(ctx, pipefd) < 0)
> > +        return ERROR_FAIL;
> > +
> > +    pid = fork();
> > +    if (pid < 0)
> > +        return ERROR_FAIL;
> > +
> > +    /* child spawn QEMU */
> > +    if (!pid) {
> > +        char *args[] = {(char*)dm, "--help", NULL};
> > +        close(pipefd[0]);
> > +        libxl__exec(gc, -1, pipefd[1], pipefd[1], dm, args, NULL);
> > +        exit(1);
> > +    }
> > +
> > +    /* parent parses the output */
> > +    close(pipefd[1]);
> > +    fp = fdopen(pipefd[0], "r");
> > +    buf = libxl__malloc(gc, buf_size);
> > +    while (fgets(buf, buf_size, fp) != NULL) {
> > +        if (strstr(buf, opt) != NULL) {
> > +            ret = 1;
> > +            goto out;
> > +        }
> > +    }
> > +out:
> > +    close(pipefd[0]);
> > +    waitpid(pid, &status, pid);
> > +    libxl_report_child_exitstatus(ctx, XTL_WARN, dm, pid, status);
> > +
> > +    ret = libxl__xs_write(gc, XBT_NULL, s, "%d", ret);
> > +
> > +    return ret;
> > +}
> > +
> >  static char ** libxl__build_device_model_args_new(libxl__gc *gc,
> >                                          const char *dm, int guest_domid,
> >                                          const libxl_domain_config *guest_config,
> > @@ -931,6 +990,14 @@ end_search:
> >          if (user) {
> >              flexarray_append(dm_args, "-runas");
> >              flexarray_append(dm_args, user);
> > +            if (libxl__check_qemu_supported(gc, dm, "xsrestrict") &&
> > +                libxl__check_qemu_supported(gc, dm, "emulator_id")) {
> > +                flexarray_append(dm_args, "-xenopts");
> > +                flexarray_append(dm_args,
> > +                        GCSPRINTF("xsrestrict=on,emulator_id=%u",
> > +                            (b_info->type == LIBXL_DOMAIN_TYPE_PV) ?
> > +                            QEMU_XEN_PV_ID : QEMU_XEN_DEVICE_MODEL_ID));
> > +            }
> >          }
> >      }
> >      flexarray_append(dm_args, NULL);
> > @@ -1666,6 +1733,11 @@ void libxl__spawn_qdisk_backend(libxl__egc *egc, libxl__dm_spawn_state *dmss)
> >      flexarray_vappend(dm_args, "-monitor", "/dev/null", NULL);
> >      flexarray_vappend(dm_args, "-serial", "/dev/null", NULL);
> >      flexarray_vappend(dm_args, "-parallel", "/dev/null", NULL);
> > +    if (libxl__check_qemu_supported(gc, dm, "emulator_id")) {
> > +        flexarray_append(dm_args, "-xenopts");
> > +        flexarray_append(dm_args,
> > +                GCSPRINTF("emulator_id=%u", QEMU_XEN_PV_ID));
> > +    }
> >      flexarray_append(dm_args, NULL);
> >      args = (char **) flexarray_contents(dm_args);
> >  
> > diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> > index 7d0af40..b4bae2f 100644
> > --- a/tools/libxl/libxl_internal.h
> > +++ b/tools/libxl/libxl_internal.h
> > @@ -106,6 +106,10 @@
> >  #define TAP_DEVICE_SUFFIX "-emu"
> >  #define DISABLE_UDEV_PATH "libxl/disable_udev"
> >  #define DOMID_XS_PATH "domid"
> > +/* Reserved QEMU emulator_ids. For the moment assume max two QEMUs: one
> > + * device model and one PV backends provider. */
> > +#define QEMU_XEN_DEVICE_MODEL_ID  0
> > +#define QEMU_XEN_PV_ID            1
> >  
> >  #define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))
> >  
> > @@ -1505,6 +1509,7 @@ _hidden int libxl__need_xenpv_qemu(libxl__gc *gc,
> >          int nr_vfbs, libxl_device_vfb *vfbs,
> >          int nr_disks, libxl_device_disk *disks,
> >          int nr_channels, libxl_device_channel *channels);
> > +_hidden int libxl__check_qemu_supported(libxl__gc *gc, const char *dm, char *opt);
> >  
> >  /*
> >   * This function will cause the whole libxl process to hang
> > @@ -3554,6 +3559,8 @@ int libxl__string_parse_json(libxl__gc *gc, const libxl__json_object *o,
> >                               char **p);
> >  
> >  int libxl__random_bytes(libxl__gc *gc, uint8_t *buf, size_t len);
> > +/* replace all occurrences of old with new inside s */
> > +void libxl__replace_chr(libxl__gc *gc, char *s, char old, char new);
> >  
> >  /*
> >   * Compile time assertion
> > diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
> > index 67c0b1c..ea08473 100644
> > --- a/tools/libxl/libxl_utils.c
> > +++ b/tools/libxl/libxl_utils.c
> > @@ -1158,6 +1158,16 @@ int libxl__random_bytes(libxl__gc *gc, uint8_t *buf, size_t len)
> >      return ret;
> >  }
> >  
> > +void libxl__replace_chr(libxl__gc *gc, char *s, char old, char new)
> > +{
> > +	int i = 0;
> > +
> > +	for (i = 0; s[i] != '\0'; i++) {
> > +		if (s[i] == old)
> > +			s[i] = new;
> > +	}
> > +}
> > +
> >  /*
> >   * Local variables:
> >   * mode: C
> 
>