Re: [PATCH v3 5/5] libxl: add options to enable/disable emulated devices

[Ian Campbell <ian.campbell@citrix.com>]

On Thu, 2016-01-21 at 11:01 +0100, Roger Pau Monné wrote:
> El 21/01/16 a les 10.39, Ian Campbell ha escrit:
> > > If we don't have that guarantee I think this is already a bug, and we
> > > should call _setdefault before sending the domain info to the other end.
> > > In fact I have a patch that does exactly that, but I'm unsure if it's
> > > needed because I don't know the policy regarding default values in libxl.
> > 
> > Wei, isn't this (turning the defaults into concrete values) supposed to be
> > taken care of by the JSON mangling which you added?
> 
> Heh, I think you mean the JSON mangling added by Wei.

Correct.

>  In order to 
> propagate the values filled by default in libxl_domain_config I had to 
> add the following patch, which basically calls the _setdefault 
> functions before converting the domain_config into JSON. I'm planning 
> to make it part of this series in the next iteration:

I'll let Wei comment on why this isn't already done.

> > > With the current code, libxl basically limits the set of allowed masks
> > > to what it knows. After the change libxl just becomes a proxy for
> > > transmitting what the user has selected to Xen, and Xen either accepts
> > > or refuses it, basically making Xen the only arbiter that decides which
> > > emulated devices get enabled or not. This means that if we want to make
> > > more emulated devices available to the guest in the future no libxl
> > > changes will be required.
> > 
> > We would need to add a new defbool for the new feature.
> 
> Yes, but I was thinking more in the direction of enabling them, rather 
> than adding new ones.

Which would then require changing the defbool_set_default in libxl after
this change, so you do still need to change libxl.

> > > It also means that HVMlite guests created with current Xen will be
> > > capable of migrating to newer version of Xen, that might have a
> > > different default policy. For example in the future we might want to
> > > enable the lapic by default, so if a guest is created with the current
> > > Xen version it doesn't get a lapic at all, and then when migrated to
> > > newer versions a lapic would magically appear after the migration, which
> > > is not desired.
> > 
> > ... and the reason these details can't be propagated via the Xen blob is
> > that this emul stuff needs to be set exactly once at domain create time I
> > suppose? Changing it to be later binding is considered to be too hard/too
> > big a yak?
> 
> Right, emulated devices are initialised as part of the 
> XEN_DOMCTL_createdomain hypercall. Allowing them to be added later on 
> and introducing a kind of intermediate domain building phase where only 
> a certain set of hypercalls are valid is a possibility that Andrew 
> already pointed out, however this seems like a very big project.

This seems like the right approach to me, but I appreciate you not wanting
to tackle this here and now.

Would it be possible to set the set of "potential" emulated devices at
create time and only "commit" to it after the save state is loaded? That
would essentially mean init-all, load state, de-init those which didn't get
any state loaded? Not as nice as doing it with the split of hypercall
availability, but might be more achievable, since you already have the de-
init code for domain teardown time.

In any case, whatever is chosen as the solution the commit message needs to
go into a fair amount of detail as to why we picked that way of doing
things, particularly if it is a compromise vs doing it properly.

This is important so we can answer "why did we do it this way" in 2 years
time.

> > Even with the set of devices set at domain creation time Xen needs to take
> > care when reading its blob, and not fall apart (from a security PoV, it's
> > allowed to fail the state load) when presented with a save record relating
> > to something which is supposedly disabled. Has this been checked?
> 
> Yes, trying to load a state into a disable device will result in 
> -ENODEV.

Grand.

Ian.

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