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

[Roger Pau Monnà <roger.pau@xxxxxxxxxx>]

El 21/01/16 a les 12.31, Wei Liu ha escrit:
> On Thu, Jan 21, 2016 at 11:01:43AM +0100, Roger Pau Monné wrote:
>> El 21/01/16 a les 10.39, Ian Campbell ha escrit:
>>> On Wed, 2016-01-20 at 19:33 +0100, Roger Pau Monné wrote:
>>>> El 20/01/16 a les 14.01, Ian Campbell ha escrit:
>>>>> On Wed, 2016-01-20 at 12:57 +0100, Roger Pau Monne wrote:
>>>>>> Allow enabling or disabling emulated devices from the libxl domain
>>>>>> configuration file. For HVM guests with a device model all the
>>>>>> emulated
>>>>>> devices are enabled. For HVM guests without a device model no devices
>>>>>> are
>>>>>> enabled by default, although they can be enabled using the options
>>>>>> provided.
>>>>>> The arbiter of whether a combination is posible or not is always Xen,
>>>>>> libxl
>>>>>> doesn't do any kind of check.
>>>>>>
>>>>>> This set of options is also propagated inside of the libxl migration
>>>>>> record
>>>>>> as part of the contents of the libxl_domain_build_info struct.
>>>>>
>>>>> ... and this is the real motivation for this change, not actually
>>>>> allowing
>>>>> users to control all this AIUI.
>>>>>
>>>>> Did you check that the fields updated using libxl_defbool_setdefault
>>>>> are
>>>>> actually updated in the JSON copy and therefore propagated to the other
>>>>> side of a migration as specific values and not as "pick a default"? I
>>>>> think
>>>>> we don't want these changing on migration. I think/hope all this was
>>>>> automatically handled by the work Wei did in the last release cycle.
>>>>
>>>> No, values populated by the {build/create}_info_setdefault functions are
>>>> not propagated, OTOH values manually set by the user in the config file
>>>> are indeed propagated. Do we have any guarantee that _setdefault is
>>>> always going to behave in the same way?
>>>
>>> No, part of the purpose of defbool and the other "do the default" values is
>>> that we can evolve things over time.
>>>
>>>> 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. 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:
> 
> The requirement of recording decision made in libxl and pass that to the
> receiving end is not new. We had the same problem for uuid, disk and
> some other things.
> 
> The first way of doing it is to update JSON before it is sent -- see
> libxl.c:libxl_retrieve_domain_configuration. It uses the stashed JSON
> file as template and pull in various bits from hypervisor and xenstore.
> Your need of recording what emulated devices are available fits here.
> You just need to provide a way to retrieve those bits in that function.
> 
> Another way of doing it is to update the stashed JSON template before it
> is saved. See libxl_internal.c:libxl__update_domain_configuration. I
> think this might be easier than the first way of doing it.
> 
> You should not export the _setdefault function to xl because it is a
> layer violation.
> 
> Hope this clarify things.

Hello,

Yes, thank you very much, it has indeed clarified things. I've
implemented it inside of libxl__update_domain_configuration without issues.

Roger.


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