[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v2 4/5] libxl: change xs path for pv qemu



On Mon, 8 Jun 2015, Wei Liu wrote:
> On Thu, Jun 04, 2015 at 12:28:18PM +0100, Stefano Stabellini wrote:
> > If QEMU is ran just to provide PV backends, change the xenstore path to
> > /local/domain/0/device-model/$DOMID/pv.
> > 
> > Add a parameter to libxl__device_model_xs_path to distinguish the device
> > model from the pv backends provider.
> > 
> > Store the device model binary path under
> > /local/domain/$DOMID/device-model on xenstore, so that we can fetch it
> > later and retrieve the list of supported options from
> > /local/domain/0/libxl/$device_model_binary, introduce in the previous
> > path.
> > 
> 
> TBH this protocol works, but it is not very extensible.
> 
> I envisaged we need to assign $emulator_id to different device models
> when I fixed stubdom, but I never got to that since there weren't need
> for multiple emulators.
> 
> That is, as an example:
> 
> /local/domain/$backend_domid/device-model/$domid/$emulator_id/xxx
> 
> That way we can:
> 
> 1. Have something like multidev in libxl to wait for several device
>    models to be ready without writing tedious code for every single one.
> 2. Fit into libxl migration stream, which naturally uses $emulator_id to
>    distinguish different emulators. 
> 
> The downside of this is we need to add an extra option to QEMU to accept
> the emulator id assigned by toolstack.
> 
> Just my two cents.

Actually QEMU already retrieves the ioservid by calling
xc_hvm_create_ioreq_server. I could easily use that as id. I am not sure
how I could get that from libxl though, except by assuming that is going
to be 0, because libxl only knows how to create one QEMU emulator.

As there is no ioservid for pv QEMU, I could still use
/local/domain/$backend_domid/device-model/$domid/pv.

What do you think?



 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> > ---
> >  tools/libxl/libxl.c          |    2 +-
> >  tools/libxl/libxl_device.c   |    2 +-
> >  tools/libxl/libxl_dm.c       |   20 ++++++++++++--------
> >  tools/libxl/libxl_dom.c      |   12 ++++++------
> >  tools/libxl/libxl_internal.c |   19 +++++++++++++++----
> >  tools/libxl/libxl_internal.h |    5 ++---
> >  tools/libxl/libxl_pci.c      |   14 +++++++-------
> >  7 files changed, 44 insertions(+), 30 deletions(-)
> > 
> > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> > index 516713e..bca4c88 100644
> > --- a/tools/libxl/libxl.c
> > +++ b/tools/libxl/libxl.c
> > @@ -1035,7 +1035,7 @@ int libxl_domain_unpause(libxl_ctx *ctx, uint32_t 
> > domid)
> >      if (type == LIBXL_DOMAIN_TYPE_HVM) {
> >          uint32_t dm_domid = libxl_get_stubdom_id(ctx, domid);
> >  
> > -        path = libxl__device_model_xs_path(gc, dm_domid, domid, "/state");
> > +        path = libxl__device_model_xs_path(gc, false, dm_domid, domid, 
> > "/state");
> >          state = libxl__xs_read(gc, XBT_NULL, path);
> >          if (state != NULL && !strcmp(state, "paused")) {
> >              libxl__qemu_traditional_cmd(gc, domid, "continue");
> > diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> > index 93bb41e..aadcd08 100644
> > --- a/tools/libxl/libxl_device.c
> > +++ b/tools/libxl/libxl_device.c
> > @@ -1190,7 +1190,7 @@ int libxl__wait_for_device_model_deprecated(libxl__gc 
> > *gc,
> >      char *path;
> >      uint32_t dm_domid = libxl_get_stubdom_id(CTX, domid);
> >  
> > -    path = libxl__device_model_xs_path(gc, dm_domid, domid, "/state");
> > +    path = libxl__device_model_xs_path(gc, false, dm_domid, domid, 
> > "/state");
> >      return libxl__xenstore_child_wait_deprecated(gc, domid,
> >                                       LIBXL_DEVICE_MODEL_START_TIMEOUT,
> >                                       "Device Model", path, state, spawning,
> > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > index b37a84a..29ef8ae 100644
> > --- a/tools/libxl/libxl_dm.c
> > +++ b/tools/libxl/libxl_dm.c
> > @@ -1267,9 +1267,9 @@ void libxl__spawn_stub_dm(libxl__egc *egc, 
> > libxl__stub_dm_spawn_state *sdss)
> >  retry_transaction:
> >      t = xs_transaction_start(ctx->xsh);
> >      xs_mkdir(ctx->xsh, t,
> > -             libxl__device_model_xs_path(gc, dm_domid, guest_domid, ""));
> > +             libxl__device_model_xs_path(gc, false, dm_domid, guest_domid, 
> > ""));
> >      xs_set_permissions(ctx->xsh, t,
> > -                       libxl__device_model_xs_path(gc, dm_domid,
> > +                       libxl__device_model_xs_path(gc, false, dm_domid,
> >                                                     guest_domid, ""),
> >                         perm, ARRAY_SIZE(perm));
> >      if (!xs_transaction_end(ctx->xsh, t, 0))
> > @@ -1430,7 +1430,7 @@ static void stubdom_pvqemu_cb(libxl__egc *egc,
> >      sdss->xswait.what = GCSPRINTF("Stubdom %u for %u startup",
> >                                    dm_domid, sdss->dm.guest_domid);
> >      sdss->xswait.path =
> > -        libxl__device_model_xs_path(gc, dm_domid, sdss->dm.guest_domid,
> > +        libxl__device_model_xs_path(gc, true, dm_domid, 
> > sdss->dm.guest_domid,
> >                                      "/state");
> >      sdss->xswait.timeout_ms = LIBXL_STUBDOM_START_TIMEOUT * 1000;
> >      sdss->xswait.callback = stubdom_xswait_cb;
> > @@ -1566,7 +1566,8 @@ void libxl__spawn_local_dm(libxl__egc *egc, 
> > libxl__dm_spawn_state *dmss)
> >          free(path);
> >      }
> >  
> > -    path = libxl__device_model_xs_path(gc, LIBXL_TOOLSTACK_DOMID, domid, 
> > "");
> > +    path = libxl__device_model_xs_path(gc, b_info->type == 
> > LIBXL_DOMAIN_TYPE_PV,
> > +                   LIBXL_TOOLSTACK_DOMID, domid, "");
> >      rwperm[0].id = LIBXL_TOOLSTACK_DOMID;
> >      rwperm[0].perms = XS_PERM_NONE;
> >      rwperm[1].id = domid;
> > @@ -1595,6 +1596,8 @@ void libxl__spawn_local_dm(libxl__egc *egc, 
> > libxl__dm_spawn_state *dmss)
> >      const char *dom_path = libxl__xs_get_dompath(gc, domid);
> >      spawn->pidpath = GCSPRINTF("%s/%s", dom_path, 
> > "image/device-model-pid");
> >  
> > +    libxl__xs_write(gc, XBT_NULL, GCSPRINTF("%s/%s", dom_path, 
> > "device-model"), "%s", dm);
> > +
> >      if (vnc && vnc->passwd) {
> >          /* This xenstore key will only be used by qemu-xen-traditionnal.
> >           * The code to supply vncpasswd to qemu-xen is later. */
> > @@ -1619,8 +1622,9 @@ retry_transaction:
> >          LIBXL__LOG(CTX, XTL_DEBUG, "  %s", *arg);
> >  
> >      spawn->what = GCSPRINTF("domain %d device model", domid);
> > -    spawn->xspath = libxl__device_model_xs_path(gc, LIBXL_TOOLSTACK_DOMID,
> > -                                                domid, "/state");
> > +    spawn->xspath = libxl__device_model_xs_path(gc,
> > +            b_info->type == LIBXL_DOMAIN_TYPE_PV, LIBXL_TOOLSTACK_DOMID,
> > +            domid, "/state");
> >      spawn->timeout_ms = LIBXL_DEVICE_MODEL_START_TIMEOUT * 1000;
> >      spawn->pidpath = GCSPRINTF("%s/image/device-model-pid", dom_path);
> >      spawn->midproc_cb = libxl__spawn_record_pid;
> > @@ -1748,7 +1752,7 @@ void libxl__spawn_qdisk_backend(libxl__egc *egc, 
> > libxl__dm_spawn_state *dmss)
> >      dmss->build_state->saved_state = 0;
> >  
> >      dmss->spawn.what = GCSPRINTF("domain %u Qdisk backend", domid);
> > -    dmss->spawn.xspath = GCSPRINTF("device-model/%u/state", domid);
> > +    dmss->spawn.xspath = libxl__device_model_xs_path(gc, true, 0, domid, 
> > "/state");
> >      dmss->spawn.timeout_ms = LIBXL_DEVICE_MODEL_START_TIMEOUT * 1000;
> >      /*
> >       * We cannot save Qemu pid anywhere in the xenstore guest dir,
> > @@ -1831,7 +1835,7 @@ out:
> >  
> >  int libxl__destroy_device_model(libxl__gc *gc, uint32_t domid)
> >  {
> > -    char *path = libxl__device_model_xs_path(gc, LIBXL_TOOLSTACK_DOMID,
> > +    char *path = libxl__device_model_xs_path(gc, false, 
> > LIBXL_TOOLSTACK_DOMID,
> >                                               domid, "");
> >      if (!xs_rm(CTX->xsh, XBT_NULL, path))
> >          LOG(ERROR, "xs_rm failed for %s", path);
> > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> > index f408646..8e254a7 100644
> > --- a/tools/libxl/libxl_dom.c
> > +++ b/tools/libxl/libxl_dom.c
> > @@ -1016,7 +1016,7 @@ int libxl__qemu_traditional_cmd(libxl__gc *gc, 
> > uint32_t domid,
> >  {
> >      char *path = NULL;
> >      uint32_t dm_domid = libxl_get_stubdom_id(CTX, domid);
> > -    path = libxl__device_model_xs_path(gc, dm_domid, domid, "/command");
> > +    path = libxl__device_model_xs_path(gc, false, dm_domid, domid, 
> > "/command");
> >      return libxl__xs_write(gc, XBT_NULL, path, "%s", cmd);
> >  }
> >  
> > @@ -1034,7 +1034,7 @@ static inline char *restore_helper(libxl__gc *gc, 
> > uint32_t dm_domid,
> >                                     uint32_t domid,
> >                                     uint64_t phys_offset, char *node)
> >  {
> > -    return libxl__device_model_xs_path(gc, dm_domid, domid,
> > +    return libxl__device_model_xs_path(gc, false, dm_domid, domid,
> >                                         "/physmap/%"PRIx64"/%s",
> >                                         phys_offset, node);
> >  }
> > @@ -1146,9 +1146,9 @@ static void 
> > domain_suspend_switch_qemu_xen_traditional_logdirty
> >  
> >      if (!lds->cmd_path) {
> >          uint32_t dm_domid = libxl_get_stubdom_id(CTX, domid);
> > -        lds->cmd_path = libxl__device_model_xs_path(gc, dm_domid, domid,
> > +        lds->cmd_path = libxl__device_model_xs_path(gc, false, dm_domid, 
> > domid,
> >                                                      "/logdirty/cmd");
> > -        lds->ret_path = libxl__device_model_xs_path(gc, dm_domid, domid,
> > +        lds->ret_path = libxl__device_model_xs_path(gc, false, dm_domid, 
> > domid,
> >                                                      "/logdirty/ret");
> >      }
> >      lds->cmd = enable ? "enable" : "disable";
> > @@ -1672,7 +1672,7 @@ static inline char *physmap_path(libxl__gc *gc, 
> > uint32_t dm_domid,
> >                                   uint32_t domid,
> >                                   char *phys_offset, char *node)
> >  {
> > -    return libxl__device_model_xs_path(gc, dm_domid, domid,
> > +    return libxl__device_model_xs_path(gc, false, dm_domid, domid,
> >                                         "/physmap/%s/%s",
> >                                         phys_offset, node);
> >  }
> > @@ -1694,7 +1694,7 @@ int libxl__toolstack_save(uint32_t domid, uint8_t 
> > **buf,
> >      dm_domid = libxl_get_stubdom_id(CTX, domid);
> >  
> >      entries = libxl__xs_directory(gc, 0,
> > -                libxl__device_model_xs_path(gc, dm_domid, domid, 
> > "/physmap"),
> > +                libxl__device_model_xs_path(gc, false, dm_domid, domid, 
> > "/physmap"),
> >                  &num);
> >      count = num;
> >  
> > diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
> > index c2c67bd..f8a59d2 100644
> > --- a/tools/libxl/libxl_internal.c
> > +++ b/tools/libxl/libxl_internal.c
> > @@ -567,14 +567,25 @@ void libxl__update_domain_configuration(libxl__gc *gc,
> >      dst->b_info.video_memkb = src->b_info.video_memkb;
> >  }
> >  
> > -char *libxl__device_model_xs_path(libxl__gc *gc, uint32_t dm_domid,
> > +char *libxl__device_model_xs_path(libxl__gc *gc, bool pvqemu, uint32_t 
> > dm_domid,
> >                                    uint32_t domid, const char *format,  ...)
> >  {
> >      char *s, *fmt;
> >      va_list ap;
> > -
> > -    fmt = GCSPRINTF("/local/domain/%u/device-model/%u%s", dm_domid,
> > -                    domid, format);
> > +    char *dm;
> > +    int xsrestrict = 0;
> > +
> > +    dm = libxl__xs_read(gc, XBT_NULL, 
> > GCSPRINTF("/local/domain/%u/device-model", domid));
> > +    if (dm)
> > +        xsrestrict = libxl__check_qemu_supported(gc, dm, "xsrestrict");
> > +
> > +    if (!xsrestrict || !pvqemu) {
> > +        fmt = GCSPRINTF("/local/domain/%u/device-model/%u%s", dm_domid,
> > +                domid, format);
> > +    } else {
> > +        fmt = GCSPRINTF("/local/domain/%u/device-model/%u/pv%s", dm_domid,
> > +                domid, format);
> > +    }
> >  
> >      va_start(ap, format);
> >      s = libxl__vsprintf(gc, fmt, ap);
> > diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> > index b11297b..c154827 100644
> > --- a/tools/libxl/libxl_internal.h
> > +++ b/tools/libxl/libxl_internal.h
> > @@ -1826,9 +1826,8 @@ _hidden libxl__json_object 
> > *libxl__json_parse(libxl__gc *gc_opt, const char *s);
> >  _hidden int libxl__device_model_version_running(libxl__gc *gc, uint32_t 
> > domid);
> >    /* Return the system-wide default device model */
> >  _hidden libxl_device_model_version libxl__default_device_model(libxl__gc 
> > *gc);
> > -_hidden char *libxl__device_model_xs_path(libxl__gc *gc, uint32_t dm_domid,
> > -                                          uint32_t domid,
> > -                                          const char *format, ...) 
> > PRINTF_ATTRIBUTE(4, 5);
> > +_hidden char *libxl__device_model_xs_path(libxl__gc *gc, bool pvqemu, 
> > uint32_t dm_domid,
> > +                                  uint32_t domid, const char *format,  
> > ...);
> >  
> >  /* Check how executes hotplug script currently */
> >  int libxl__hotplug_settings(libxl__gc *gc, xs_transaction_t t);
> > diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
> > index e0743f8..07f6209 100644
> > --- a/tools/libxl/libxl_pci.c
> > +++ b/tools/libxl/libxl_pci.c
> > @@ -853,9 +853,9 @@ static int qemu_pci_add_xenstore(libxl__gc *gc, 
> > uint32_t domid,
> >      uint32_t dm_domid;
> >  
> >      dm_domid = libxl_get_stubdom_id(CTX, domid);
> > -    path = libxl__device_model_xs_path(gc, dm_domid, domid, "/state");
> > +    path = libxl__device_model_xs_path(gc, false, dm_domid, domid, 
> > "/state");
> >      state = libxl__xs_read(gc, XBT_NULL, path);
> > -    path = libxl__device_model_xs_path(gc, dm_domid, domid, "/parameter");
> > +    path = libxl__device_model_xs_path(gc, false, dm_domid, domid, 
> > "/parameter");
> >      if (pcidev->vdevfn) {
> >          libxl__xs_write(gc, XBT_NULL, path, PCI_BDF_VDEVFN","PCI_OPTIONS,
> >                          pcidev->domain, pcidev->bus, pcidev->dev,
> > @@ -870,9 +870,9 @@ static int qemu_pci_add_xenstore(libxl__gc *gc, 
> > uint32_t domid,
> >      libxl__qemu_traditional_cmd(gc, domid, "pci-ins");
> >      rc = libxl__wait_for_device_model_deprecated(gc, domid, NULL, NULL,
> >                                        pci_ins_check, state);
> > -    path = libxl__device_model_xs_path(gc, dm_domid, domid, "/parameter");
> > +    path = libxl__device_model_xs_path(gc, false, dm_domid, domid, 
> > "/parameter");
> >      vdevfn = libxl__xs_read(gc, XBT_NULL, path);
> > -    path = libxl__device_model_xs_path(gc, dm_domid, domid, "/state");
> > +    path = libxl__device_model_xs_path(gc, false, dm_domid, domid, 
> > "/state");
> >      if ( rc < 0 )
> >          LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> >                     "qemu refused to add device: %s", vdevfn);
> > @@ -1178,9 +1178,9 @@ static int qemu_pci_remove_xenstore(libxl__gc *gc, 
> > uint32_t domid,
> >  
> >      dm_domid = libxl_get_stubdom_id(CTX, domid);
> >  
> > -    path = libxl__device_model_xs_path(gc, dm_domid, domid, "/state");
> > +    path = libxl__device_model_xs_path(gc, false, dm_domid, domid, 
> > "/state");
> >      state = libxl__xs_read(gc, XBT_NULL, path);
> > -    path = libxl__device_model_xs_path(gc, dm_domid, domid, "/parameter");
> > +    path = libxl__device_model_xs_path(gc, false, dm_domid, domid, 
> > "/parameter");
> >      libxl__xs_write(gc, XBT_NULL, path, PCI_BDF, pcidev->domain,
> >                      pcidev->bus, pcidev->dev, pcidev->func);
> >  
> > @@ -1198,7 +1198,7 @@ static int qemu_pci_remove_xenstore(libxl__gc *gc, 
> > uint32_t domid,
> >              return ERROR_FAIL;
> >          }
> >      }
> > -    path = libxl__device_model_xs_path(gc, dm_domid, domid, "/state");
> > +    path = libxl__device_model_xs_path(gc, false, dm_domid, domid, 
> > "/state");
> >      xs_write(ctx->xsh, XBT_NULL, path, state, strlen(state));
> >  
> >      return 0;
> > -- 
> > 1.7.10.4
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.