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

[Tim Deegan <tim@xxxxxxx>]

At 15:08 -0500 on 23 Feb (1424700515), Don Slutz wrote:
> 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.

I prefer a new flag, for anything that's fixed for the life of the
domain.  We've already had too many bugs where HVM params changed
when people thought they wouldn't.

Jan, is your objection that we'll run out of XEN_DOMCTL_CDF_* bits?  I
think we can add more flag fields to DOMCTL_createdomain (or a v2) if
that becomes a problem.

Cheers,

Tim.


> 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