diff mbox series

[v2,1/6] domain: stash xen_domctl_createdomain flags in struct domain

Message ID 20190725133920.40673-2-paul.durrant@citrix.com (mailing list archive)
State New, archived
Headers show
Series stash domain create flags and then use them | expand

Commit Message

Paul Durrant July 25, 2019, 1:39 p.m. UTC
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 'options' 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>

v2:
 - Rename 'createflags' to 'options'
---
 xen/common/domain.c     | 6 ++++--
 xen/include/xen/sched.h | 1 +
 2 files changed, 5 insertions(+), 2 deletions(-)

Comments

George Dunlap July 29, 2019, 9:20 a.m. UTC | #1
On 7/25/19 2:39 PM, 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 'options' field in
> struct domain. Subsequent patches will do the related clean-up work.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>

This looks great, thanks:

Acked-by: George Dunlap <george.dunlap@citrix.com>
Roger Pau Monné July 29, 2019, 9:24 a.m. UTC | #2
On Thu, Jul 25, 2019 at 02:39:15PM +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 'options' field in
> struct domain. Subsequent patches will do the related clean-up work.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks.
Jan Beulich July 30, 2019, 9:17 a.m. UTC | #3
On 25.07.2019 15:39, Paul Durrant wrote:
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -306,6 +306,7 @@ enum guest_type {
>   
>   struct domain
>   {
> +    unsigned int     options;         /* copy of createdomain flags */
>      domid_t          domain_id;
>   
>      unsigned int     max_vcpus;

I was about to commit this when I noticed the placement here:
I think it would be pretty good to retain domain_id as the
first field. I'll be happy to make that adjustment while
committing, as long as you are fine with me doing so.

Jan
Paul Durrant July 30, 2019, 9:24 a.m. UTC | #4
> -----Original Message-----
> From: Jan Beulich <JBeulich@suse.com>
> Sent: 30 July 2019 10:17
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: xen-devel@lists.xenproject.org; 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>; Konrad Rzeszutek Wilk
> <konrad.wilk@oracle.com>; Tim (Xen.org) <tim@xen.org>; Wei Liu <wl@xen.org>
> Subject: Re: [Xen-devel] [PATCH v2 1/6] domain: stash xen_domctl_createdomain flags in struct domain
> 
> On 25.07.2019 15:39, Paul Durrant wrote:
> > --- a/xen/include/xen/sched.h
> > +++ b/xen/include/xen/sched.h
> > @@ -306,6 +306,7 @@ enum guest_type {
> >
> >   struct domain
> >   {
> > +    unsigned int     options;         /* copy of createdomain flags */
> >      domid_t          domain_id;
> >
> >      unsigned int     max_vcpus;
> 
> I was about to commit this when I noticed the placement here:
> I think it would be pretty good to retain domain_id as the
> first field. I'll be happy to make that adjustment while
> committing, as long as you are fine with me doing so.

Yes, sure. I guess put it just below domid instead.

  Paul

> 
> Jan
Jan Beulich July 30, 2019, 9:25 a.m. UTC | #5
On 30.07.2019 11:17, Jan Beulich wrote:
> On 25.07.2019 15:39, Paul Durrant wrote:
>> --- a/xen/include/xen/sched.h
>> +++ b/xen/include/xen/sched.h
>> @@ -306,6 +306,7 @@ enum guest_type {
>>    
>>    struct domain
>>    {
>> +    unsigned int     options;         /* copy of createdomain flags */
>>       domid_t          domain_id;
>>    
>>       unsigned int     max_vcpus;
> 
> I was about to commit this when I noticed the placement here:
> I think it would be pretty good to retain domain_id as the
> first field. I'll be happy to make that adjustment while
> committing, as long as you are fine with me doing so.

And I realize I should have said where I'd want to put it: I
think it wants to go next to guest_type, so that once that
field goes away structure layout (i.e. holes and overall size)
is the same again.

Jan
Paul Durrant July 30, 2019, 9:53 a.m. UTC | #6
> -----Original Message-----
> From: Jan Beulich <JBeulich@suse.com>
> Sent: 30 July 2019 10:26
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: JulienGrall <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; Konrad Rzeszutek Wilk
> <konrad.wilk@oracle.com>; Tim (Xen.org) <tim@xen.org>; Wei Liu <wl@xen.org>
> Subject: Re: [Xen-devel] [PATCH v2 1/6] domain: stash xen_domctl_createdomain flags in struct domain
> 
> On 30.07.2019 11:17, Jan Beulich wrote:
> > On 25.07.2019 15:39, Paul Durrant wrote:
> >> --- a/xen/include/xen/sched.h
> >> +++ b/xen/include/xen/sched.h
> >> @@ -306,6 +306,7 @@ enum guest_type {
> >>
> >>    struct domain
> >>    {
> >> +    unsigned int     options;         /* copy of createdomain flags */
> >>       domid_t          domain_id;
> >>
> >>       unsigned int     max_vcpus;
> >
> > I was about to commit this when I noticed the placement here:
> > I think it would be pretty good to retain domain_id as the
> > first field. I'll be happy to make that adjustment while
> > committing, as long as you are fine with me doing so.
> 
> And I realize I should have said where I'd want to put it: I
> think it wants to go next to guest_type, so that once that
> field goes away structure layout (i.e. holes and overall size)
> is the same again.

Ok, I don't really mind.

  Paul

> 
> Jan
diff mbox series

Patch

diff --git a/xen/common/domain.c b/xen/common/domain.c
index bc56a51815..eef486af05 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -331,6 +331,8 @@  struct domain *domain_create(domid_t domid,
     if ( (d = alloc_domain_struct()) == NULL )
         return ERR_PTR(-ENOMEM);
 
+    d->options = config ? config->flags : 0;
+
     /* Sort out our idea of is_system_domain(). */
     d->domain_id = domid;
 
@@ -352,7 +354,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->options & XEN_DOMCTL_CDF_hvm_guest)
                      ? guest_type_hvm : guest_type_pv);
 
     TRACE_1D(TRC_DOM0_DOM_ADD, d->domain_id);
@@ -429,7 +431,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->options & 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 c197e93d73..cff2990b10 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -306,6 +306,7 @@  enum guest_type {
 
 struct domain
 {
+    unsigned int     options;         /* copy of createdomain flags */
     domid_t          domain_id;
 
     unsigned int     max_vcpus;