| Message ID | 20190723160609.2177-2-paul.durrant@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | stash domain create flags and then use them | expand |
On Tue, Jul 23, 2019 at 05:06:04PM +0100, Paul Durrant wrote: > These are canonical source of data used to set various other flags. If > they are available directly in struct domain then the other flags are no > longer needed. > > This patch simply copies the flags into a new 'createflags' field in > struct domain. Subsequent patches will do the related clean-up work. Thanks! Just one naming comment (which is subject to taste I guess). > diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h > index b40c8fd138..edae372c2b 100644 > --- a/xen/include/xen/sched.h > +++ b/xen/include/xen/sched.h > @@ -308,6 +308,7 @@ enum guest_type { > > struct domain > { > + unsigned int createflags; Can you name this just flags or options or some such (without the create prefix). IMO adding the create prefix makes it look like a field only used during domain creation, while it's not the case. Thanks, Roger.
> -----Original Message----- > From: Roger Pau Monne <roger.pau@citrix.com> > Sent: 25 July 2019 10:23 > To: Paul Durrant <Paul.Durrant@citrix.com> > Cc: xen-devel@lists.xenproject.org; Stefano Stabellini <sstabellini@kernel.org>; Wei Liu <wl@xen.org>; > Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; George Dunlap <George.Dunlap@citrix.com>; Andrew > Cooper <Andrew.Cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Tim (Xen.org) <tim@xen.org>; > Julien Grall <julien.grall@arm.com>; Jan Beulich <jbeulich@suse.com> > Subject: Re: [Xen-devel] [PATCH 1/6] domain: stash xen_domctl_createdomain flags in struct domain > > On Tue, Jul 23, 2019 at 05:06:04PM +0100, Paul Durrant wrote: > > These are canonical source of data used to set various other flags. If > > they are available directly in struct domain then the other flags are no > > longer needed. > > > > This patch simply copies the flags into a new 'createflags' field in > > struct domain. Subsequent patches will do the related clean-up work. > > Thanks! > > Just one naming comment (which is subject to taste I guess). > > > diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h > > index b40c8fd138..edae372c2b 100644 > > --- a/xen/include/xen/sched.h > > +++ b/xen/include/xen/sched.h > > @@ -308,6 +308,7 @@ enum guest_type { > > > > struct domain > > { > > + unsigned int createflags; > > Can you name this just flags or options or some such (without the > create prefix). IMO adding the create prefix makes it look like a > field only used during domain creation, while it's not the case. I guess naming it simply 'flags' would be ok coupled with a comment in the header stating that the field is merely a copy of the domain create flags. Anyone else got opinions on this? Paul > > Thanks, Roger.
On 25.07.2019 12:11, Paul Durrant wrote: >> -----Original Message----- >> From: Roger Pau Monne <roger.pau@citrix.com> >> Sent: 25 July 2019 10:23 >> To: Paul Durrant <Paul.Durrant@citrix.com> >> Cc: xen-devel@lists.xenproject.org; Stefano Stabellini <sstabellini@kernel.org>; Wei Liu <wl@xen.org>; >> Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; George Dunlap <George.Dunlap@citrix.com>; Andrew >> Cooper <Andrew.Cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Tim (Xen.org) <tim@xen.org>; >> Julien Grall <julien.grall@arm.com>; Jan Beulich <jbeulich@suse.com> >> Subject: Re: [Xen-devel] [PATCH 1/6] domain: stash xen_domctl_createdomain flags in struct domain >> >> On Tue, Jul 23, 2019 at 05:06:04PM +0100, Paul Durrant wrote: >>> These are canonical source of data used to set various other flags. If >>> they are available directly in struct domain then the other flags are no >>> longer needed. >>> >>> This patch simply copies the flags into a new 'createflags' field in >>> struct domain. Subsequent patches will do the related clean-up work. >> >> Thanks! >> >> Just one naming comment (which is subject to taste I guess). >> >>> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h >>> index b40c8fd138..edae372c2b 100644 >>> --- a/xen/include/xen/sched.h >>> +++ b/xen/include/xen/sched.h >>> @@ -308,6 +308,7 @@ enum guest_type { >>> >>> struct domain >>> { >>> + unsigned int createflags; >> >> Can you name this just flags or options or some such (without the >> create prefix). IMO adding the create prefix makes it look like a >> field only used during domain creation, while it's not the case. > > I guess naming it simply 'flags' would be ok coupled with a comment > in the header stating that the field is merely a copy of the domain > create flags. Anyone else got opinions on this? We use "flags" too often imo. What about "options" as suggested by Roger, or "settings"? Jan
> -----Original Message----- > From: Jan Beulich <JBeulich@suse.com> > Sent: 25 July 2019 11:25 > To: Paul Durrant <Paul.Durrant@citrix.com> > Cc: Roger Pau Monne <roger.pau@citrix.com>; Julien Grall <julien.grall@arm.com>; Andrew Cooper > <Andrew.Cooper3@citrix.com>; George Dunlap <George.Dunlap@citrix.com>; Ian Jackson > <Ian.Jackson@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>; xen-devel@lists.xenproject.org; > KonradRzeszutek Wilk <konrad.wilk@oracle.com>; Tim (Xen.org) <tim@xen.org>; Wei Liu <wl@xen.org> > Subject: Re: [Xen-devel] [PATCH 1/6] domain: stash xen_domctl_createdomain flags in struct domain > > On 25.07.2019 12:11, Paul Durrant wrote: > >> -----Original Message----- > >> From: Roger Pau Monne <roger.pau@citrix.com> > >> Sent: 25 July 2019 10:23 > >> To: Paul Durrant <Paul.Durrant@citrix.com> > >> Cc: xen-devel@lists.xenproject.org; Stefano Stabellini <sstabellini@kernel.org>; Wei Liu > <wl@xen.org>; > >> Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; George Dunlap <George.Dunlap@citrix.com>; Andrew > >> Cooper <Andrew.Cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Tim (Xen.org) > <tim@xen.org>; > >> Julien Grall <julien.grall@arm.com>; Jan Beulich <jbeulich@suse.com> > >> Subject: Re: [Xen-devel] [PATCH 1/6] domain: stash xen_domctl_createdomain flags in struct domain > >> > >> On Tue, Jul 23, 2019 at 05:06:04PM +0100, Paul Durrant wrote: > >>> These are canonical source of data used to set various other flags. If > >>> they are available directly in struct domain then the other flags are no > >>> longer needed. > >>> > >>> This patch simply copies the flags into a new 'createflags' field in > >>> struct domain. Subsequent patches will do the related clean-up work. > >> > >> Thanks! > >> > >> Just one naming comment (which is subject to taste I guess). > >> > >>> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h > >>> index b40c8fd138..edae372c2b 100644 > >>> --- a/xen/include/xen/sched.h > >>> +++ b/xen/include/xen/sched.h > >>> @@ -308,6 +308,7 @@ enum guest_type { > >>> > >>> struct domain > >>> { > >>> + unsigned int createflags; > >> > >> Can you name this just flags or options or some such (without the > >> create prefix). IMO adding the create prefix makes it look like a > >> field only used during domain creation, while it's not the case. > > > > I guess naming it simply 'flags' would be ok coupled with a comment > > in the header stating that the field is merely a copy of the domain > > create flags. Anyone else got opinions on this? > > We use "flags" too often imo. What about "options" as suggested by > Roger, or "settings"? Alright, let's go with 'options' then. Paul > > Jan
diff --git a/xen/common/domain.c b/xen/common/domain.c index 55aa759b75..d559c8898e 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -333,6 +333,8 @@ struct domain *domain_create(domid_t domid, if ( (d = alloc_domain_struct()) == NULL ) return ERR_PTR(-ENOMEM); + d->createflags = config ? config->flags : 0; + /* Sort out our idea of is_system_domain(). */ d->domain_id = domid; @@ -354,7 +356,7 @@ struct domain *domain_create(domid_t domid, } /* Sort out our idea of is_{pv,hvm}_domain(). All system domains are PV. */ - d->guest_type = ((config && (config->flags & XEN_DOMCTL_CDF_hvm_guest)) + d->guest_type = ((d->createflags & XEN_DOMCTL_CDF_hvm_guest) ? guest_type_hvm : guest_type_pv); TRACE_1D(TRC_DOM0_DOM_ADD, d->domain_id); @@ -431,7 +433,7 @@ struct domain *domain_create(domid_t domid, watchdog_domain_init(d); init_status |= INIT_watchdog; - if ( config->flags & XEN_DOMCTL_CDF_xs_domain ) + if ( d->createflags & XEN_DOMCTL_CDF_xs_domain ) { d->is_xenstore = 1; d->disable_migrate = 1; diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index b40c8fd138..edae372c2b 100644 --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -308,6 +308,7 @@ enum guest_type { struct domain { + unsigned int createflags; domid_t domain_id; unsigned int max_vcpus;
These are canonical source of data used to set various other flags. If they are available directly in struct domain then the other flags are no longer needed. This patch simply copies the flags into a new 'createflags' field in struct domain. Subsequent patches will do the related clean-up work. Signed-off-by: Paul Durrant <paul.durrant@citrix.com> --- Cc: Andrew Cooper <andrew.cooper3@citrix.com> Cc: George Dunlap <George.Dunlap@eu.citrix.com> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Jan Beulich <jbeulich@suse.com> Cc: Julien Grall <julien.grall@arm.com> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Cc: Stefano Stabellini <sstabellini@kernel.org> Cc: Tim Deegan <tim@xen.org> Cc: Wei Liu <wl@xen.org> --- xen/common/domain.c | 6 ++++-- xen/include/xen/sched.h | 1 + 2 files changed, 5 insertions(+), 2 deletions(-)