Re: [PATCH v3 04/17] libxl: Allow running qemu-xen in stubdomain

[Wei Liu <wei.liu2@citrix.com>]

On Thu, Feb 21, 2019 at 06:06:31PM +0100, Marek Marczykowski-Górecki wrote:
> On Thu, Feb 21, 2019 at 04:01:59PM +0000, Wei Liu wrote:
> > On Mon, Jan 28, 2019 at 10:30:21PM +0100, Marek Marczykowski-Górecki wrote:
> > > Do not prohibit anymore using stubdomain with qemu-xen.
> > > To help distingushing MiniOS and Linux stubdomain, add helper inline
> > > functions libxl__stubdomain_is_linux() and
> > > libxl__stubdomain_is_linux_running(). Those should be used where really
> > > the difference is about MiniOS/Linux, not qemu-xen/qemu-xen-traditional.
> > > 
> > > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> > > 
> > > ---
> > > Changes in v3:
> > >  - new patch, instead of "libxl: Add "stubdomain_version" to
> > >  domain_build_info"
> > >  - helper functions as suggested by Ian Jackson
> > > ---
> > >  tools/libxl/libxl_create.c   |  9 ---------
> > >  tools/libxl/libxl_internal.h | 17 +++++++++++++++++
> > >  2 files changed, 17 insertions(+), 9 deletions(-)
> > > 
> (...)
> > > diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> > > index 459f9bf..b8c698a 100644
> > > --- a/tools/libxl/libxl_internal.h
> > > +++ b/tools/libxl/libxl_internal.h
> > > @@ -2195,6 +2195,23 @@ _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);
> > >  
> > > +static inline
> > > +bool libxl__stubdomain_is_linux_running(libxl__gc *gc, uint32_t domid)
> > > +{
> > > +    /* same logic as in libxl__stubdomain_is_linux */
> > > +    return libxl__device_model_version_running(gc, domid)
> > > +        == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
> > 
> > I don't think the logic is accurate. You're precluding running
> > qemu-xen in a unikernel as stubdom.
> > 
> > I think putting an extra key in xenstore with the underlying platform is
> > more desirable.
> > 
> > > +}
> > > +
> > > +static inline
> > > +bool libxl__stubdomain_is_linux(libxl_domain_build_info *b_info)
> > > +{
> > > +    /* right now qemu-tranditional implies MiniOS stubdomain and qemu-xen
> > > +     * implies Linux stubdomain */
> > > +    return libxl_defbool_val(b_info->device_model_stubdomain) &&
> > > +        b_info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
> > 
> > Subsequently you will need a new field in b_info.
> > 
> > What do you think?
> 
> This is _exactly_ what was in v2 of this patch and Ian suggested to
> change it:
> https://lists.xenproject.org/archives/html/xen-devel/2018-10/msg01317.html

Alright. Ian thought unikernel qemu-xen stubdomain was not coming along
any time soon. I don't disagree. You can keep the patch as-is.

I think in the future if it does come along, we can still easily
distinguish it from the rest.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel