Re: [PATCH v8 4/7] xen: Add vmware_port support

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 11/02/15 07:56, Jan Beulich wrote:
>>>> On 10.02.15 at 20:30, <dslutz@verizon.com> wrote:
>> While coding this is up I have hit issues that I need input on:
>>
>> As a HVM_PARAM_ item, I would assume I should be following
>> what HVM_PARAM_VIRIDIAN does.  It has this comment:
>>
>>             case HVM_PARAM_VIRIDIAN:
>>                 /* This should only ever be set once by the tools and
>> read by the guest. */
>>
>> Which is almost true.  However the code allows you to change from 0 to
>> non-zero any time in the life of the DomU.  I am assuming that this is
>> why xc_domain_save() and xc_domain_restore() save and restore this
>> HVM_PARAM_ item.
>>
>> With the enable of vmware_port the same way, I feel it would be a bug
>> to allow the enable after "create" to not also adjust QEMU.  Currently
>> there is no way for the hypervisor to tell QEMU to enable vmware_port
>> handling.  So to avoid adding this code to xen and QEMU, it looks to
>> me that adding code to make this a true write only 1 time would be
>> needed so that you cannot use the hyper call to change later.
>>
>> So, should I extend this change to cover other HVM_PARAM_?
>>
>> Is all this additional code (xc_domain_save(), xc_domain_restore(),
>> write only 1 time) still better then a domain_create() flag?
> I suppose for your case it's indeed the right approach. Which other
> params this may be true for as well I can't immediately say, but I'd
> certainly like to ask for adjustments to others to be in separate
> patches (and perhaps even a separate series), with proper
> rationale for each of them. I guess Andrew will have further input
> for you on this matter...

My recommendation is still to use a creation flag.  The described
problem is exactly the reason why I dislike the use of hvmparams for
booleans like this which really do need to be consistent for the
lifetime of the guest.

I had hoped to see whether I could fix some of this up as part of the
fixes to guest cpuid handling, but that work is still a while off and
not of practical consideration for the short term.

~Andrew