Re: [PATCH 6/18 V2]: PVH xen: Introduce PVH guest type

[Mukesh Rathor <mukesh.rathor@oracle.com>]

On Mon, 18 Mar 2013 11:54:29 +0000
"Jan Beulich" <JBeulich@suse.com> wrote:

> >>> On 16.03.13 at 01:32, Mukesh Rathor <mukesh.rathor@oracle.com>
> >>> wrote:
> > @@ -277,8 +280,8 @@ struct domain
> >      struct rangeset *iomem_caps;
> >      struct rangeset *irq_caps;
> >  
> > -    /* Is this an HVM guest? */
> > -    bool_t           is_hvm;
> > +    /* !is_pvh && !is_hvm ==> PV, else PVH or HVM */
> > +    enum {hvm_guest=1, pvh_guest} guest_type;
> 
> Even if not used explicitly anywhere right now, please make the PV
> guest case explicit here too (at once avoiding the =1).

Ok.

> Quite likely we'll want to also have the enum have a tag, so we
> can eventually pass value from this enumeration to functions.
 
Ok.

> I'm also mildly puzzled by you using "_guest" suffixes here rather
> than, as usual, a common prefix.
> 
> And of course, please properly format this.

Not sure I follow what needs formatting?
 
> > @@ -718,10 +725,14 @@ void watchdog_domain_destroy(struct domain
> > *d); 
> >  #define VM_ASSIST(_d,_t) (test_bit((_t), &(_d)->vm_assist))
> >  
> > -#define is_hvm_domain(d) ((d)->is_hvm)
> > +#define is_hvm_domain(d) ((d)->guest_type == hvm_guest)
> >  #define is_hvm_vcpu(v)   (is_hvm_domain(v->domain))
> > +#define is_pvh_domain(d) ((d)->guest_type == pvh_guest)
> > +#define is_pvh_vcpu(v)   (is_pvh_domain(v->domain))
> >  #define is_pinned_vcpu(v) ((v)->domain->is_pinned || \
> >                             cpumask_weight((v)->cpu_affinity) == 1)
> > +#define is_hvm_or_pvh_domain(d) (is_hvm_domain(d) ||
> > is_pvh_domain(d)) +#define is_hvm_or_pvh_vcpu(v)
> > (is_hvm_or_pvh_domain(v->domain))
> 
> These surely can have better names, if they're needed at all:
> Wouldn't !is_pv_domain() do what you need?

Nop, that's more confusing, since PVH is a PV domain. So, I suggest
we leave it as is. is_hvm_or_pvh_domain is nicely readable, what name
do you suggest?

Thanks,
Mukesh