[Xen-devel] [RFC] When to use "domain creation flag" or "HVM param"?

[Don Slutz <dslutz@xxxxxxxxxxx>]

Currently Jan Beulich is not happy with the addition of a new domain
creation flag.  Andrew Cooper is not happy with a HVM param.  I am stuck
in the middle.

This is from:

On 02/23/15 11:28, Jan Beulich wrote:
>> I was not sure on the way to go based on the emails.
>
> I think I said so before - we should come to some conclusion among
> maintainers here first. I'm afraid besides Andrew and me no-one
> really voiced any opinion; maybe worth for you to send out a
> separate request-for-comments on this specific aspect...



Below is some of the subsets of the email threads on this:



Message-ID: <54DA5C69.1060409@xxxxxxxxxxxxx>
References: <1412285417-19180-1-git-send-email-dslutz@xxxxxxxxxxx>
<1412285417-19180-5-git-send-email-dslutz@xxxxxxxxxxx>
<542DCA92.1030701@xxxxxxxxxxxxx> <542DD44F.6030101@xxxxxxxxxxxxx>
<54B8F1740200007800055B42@xxxxxxxxxxxxxxxxxxxx>
<54BFE768.3090309@xxxxxxxxxxxxx>
<54C0C39F0200007800057F73@xxxxxxxxxxxxxxxxxxxx> <54C6643B.1@xxxxxxxxxxxxx>
In-Reply-To: <54C6643B.1@xxxxxxxxxxxxx>
Subject: Re: [PATCH v8 4/7] xen: Add vmware_port support
Date: Tue, 10 Feb 2015 14:30:49 -0500
From: Don Slutz <dslutz@xxxxxxxxxxx>
To: Don Slutz <dslutz@xxxxxxxxxxx>, Jan Beulich <JBeulich@xxxxxxxx>
CC: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>, Andrew Cooper
<andrew.cooper3@xxxxxxxxxx>, Ian Campbell <ian.campbell@xxxxxxxxxx>,
George Dunlap <George.Dunlap@xxxxxxxxxxxxx>, Ian Jackson
<ian.jackson@xxxxxxxxxxxxx>, Stefano Stabellini
<stefano.stabellini@xxxxxxxxxxxxx>, Eddie Dong <eddie.dong@xxxxxxxxx>,
Jun Nakajima <jun.nakajima@xxxxxxxxx>, Kevin Tian
<kevin.tian@xxxxxxxxx>, xen-devel@xxxxxxxxxxxxx, Boris Ostrovsky
<boris.ostrovsky@xxxxxxxxxx>, Konrad Rzeszutek Wilk
<konrad.wilk@xxxxxxxxxx>, Keir Fraser <keir@xxxxxxx>, Tim Deegan
<tim@xxxxxxx>

On 01/26/15 10:58, Don Slutz wrote:
> On 01/22/15 03:32, Jan Beulich wrote:
>>>>> On 21.01.15 at 18:52, <dslutz@xxxxxxxxxxx> wrote:
>>> On 01/16/15 05:09, Jan Beulich wrote:
>>>>>>> On 03.10.14 at 00:40, <dslutz@xxxxxxxxxxx> wrote:
>>>>> This is a new domain_create() flag, DOMCRF_vmware_port.  It is
>>>>> passed to domctl as XEN_DOMCTL_CDF_vmware_port.
>>>> Can you explain why a HVM param isn't suitable here?
>>>>
>>> The issue is that you need this flag during construct_vmcb() and
>>> construct_vmcs().  While Intel has vmx_update_exception_bitmap()
>>> AMD does not.  So when HVM param's are setup and/or changed there
>>> currently is no way to adjust AMD's exception bitmap.
>>>
>>> So this is the simpler way.
>> But the less desirable one from a design/consistency perspective.
>> Unless other maintainers disagree, I'd like to see this changed.
>
> Ok, but will wait some time to see if "Unless other maintainers disagree"
>

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?

   -Don Slutz





Message-ID: <54E2FF9202000078000607A7@xxxxxxxxxxxxxxxxxxxx>
References: <1412285417-19180-1-git-send-email-dslutz@xxxxxxxxxxx>
 <1412285417-19180-5-git-send-email-dslutz@xxxxxxxxxxx>
 <542DCA92.1030701@xxxxxxxxxxxxx> <542DD44F.6030101@xxxxxxxxxxxxx>
 <54B8F1740200007800055B42@xxxxxxxxxxxxxxxxxxxx>
 <54BFE768.3090309@xxxxxxxxxxxxx>
 <54C0C39F0200007800057F73@xxxxxxxxxxxxxxxxxxxx> <54C6643B.1@xxxxxxxxxxxxx>
 <54DA5C69.1060409@xxxxxxxxxxxxx>
 <54DB193F020000780005ED8F@xxxxxxxxxxxxxxxxxxxx>
 <54DB8B83.9000606@xxxxxxxxxx>
In-Reply-To: <54DB8B83.9000606@xxxxxxxxxx>Subject: Re: [PATCH v8 4/7]
xen: Add vmware_port support
Date: Tue, 17 Feb 2015 07:45:06 +0000
From: Jan Beulich <JBeulich@xxxxxxxx>
To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Don Slutz
<dslutz@xxxxxxxxxxx>
CC: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>, Ian Campbell
<ian.campbell@xxxxxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxxxxx>,
IanJackson <ian.jackson@xxxxxxxxxxxxx>, Stefano Stabellini
<stefano.stabellini@xxxxxxxxxxxxx>, Eddie Dong <eddie.dong@xxxxxxxxx>,
JunNakajima <jun.nakajima@xxxxxxxxx>, Kevin Tian <kevin.tian@xxxxxxxxx>,
xen-devel@xxxxxxxxxxxxx, Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>,
Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>, Keir Fraser
<keir@xxxxxxx>, Tim Deegan <tim@xxxxxxx>

>>> On 11.02.15 at 18:04, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 11/02/15 07:56, Jan Beulich wrote:
>>>>> On 10.02.15 at 20:30, <dslutz@xxxxxxxxxxx> 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.>> I was not sure on the way to go based on the emails.
>
> I think I said so before - we should come to some conclusion among
> maintainers here first. I'm afraid besides Andrew and me no-one
> really voiced any opinion; maybe worth for you to send out a
> separate request-for-comments on this specific aspect...
>>>
>>> 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.

While I can see your point, HVM params are much better scalable
(and more obviously architecture specific) than creation flags...

Jan




Message-ID: <54EB4FE002000078000628F7@xxxxxxxxxxxxxxxxxxxx>
References: <1424127915-27004-1-git-send-email-dslutz@xxxxxxxxxxx>
 <1424127915-27004-6-git-send-email-dslutz@xxxxxxxxxxx>
In-Reply-To: <1424127915-27004-6-git-send-email-dslutz@xxxxxxxxxxx>
Subject: Re: [PATCH v9 05/13] xen: Add vmware_port support
Date: Mon, 23 Feb 2015 15:05:52 +0000
From: Jan Beulich <JBeulich@xxxxxxxx>
To: Don Slutz <dslutz@xxxxxxxxxxx>
CC: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@xxxxxxx>, Suravee
Suthikulpanit <suravee.suthikulpanit@xxxxxxx>, Andrew Cooper
<andrew.cooper3@xxxxxxxxxx>, Ian Campbell <ian.campbell@xxxxxxxxxx>,
George Dunlap <George.Dunlap@xxxxxxxxxxxxx>, Ian Jackson
<ian.jackson@xxxxxxxxxxxxx>, Stefano Stabellini
<stefano.stabellini@xxxxxxxxxxxxx>, Eddie Dong <eddie.dong@xxxxxxxxx>,
Jun Nakajima <jun.nakajima@xxxxxxxxx>, Kevin Tian
<kevin.tian@xxxxxxxxx>, xen-devel@xxxxxxxxxxxxx, Boris Ostrovsky
<boris.ostrovsky@xxxxxxxxxx>, Konrad Rzeszutek Wilk
<konrad.wilk@xxxxxxxxxx>, Keir Fraser <keir@xxxxxxx>, Tim Deegan
<tim@xxxxxxx>

>>> On 17.02.15 at 00:05, <dslutz@xxxxxxxxxxx> wrote:
> This includes adding is_vmware_port_enabled
>
> This is a new domain_create() flag, DOMCRF_vmware_port.  It is
> passed to domctl as XEN_DOMCTL_CDF_vmware_port.

As indicated before, I don't think this is a good use case for a
domain creation flag. Some of the others we have may not be either,
but as I have said quite often recently - let's not make the situation
worse by following bad examples.



   -Don Slutz

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