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

[Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>]

On Tue, 19 Mar 2013 08:48:53 +0000
"Jan Beulich" <JBeulich@xxxxxxxx> wrote:

> >>> On 19.03.13 at 01:21, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>
> >>> wrote:
> > On Mon, 18 Mar 2013 11:54:29 +0000
> > "Jan Beulich" <JBeulich@xxxxxxxx> wrote:

>And as pointed out elsewhere - your whole series is in need of
>making it conform to coding conventions.

For the most part I looked at existing code in the same or different 
module for coding conventions.

> >> >  #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?
> 
> No. The three kinds should be fully distinct, such that when
> meaning one you can use is_xyz_domain() and when meaning
> two, you can use !is_abc_domain().

> is_hvm_or_pvh_domain() isn't nicely readable to me, in particular
> because this kind of naming doesn't scale. And it's certainly more
> typing than !is_pv_domain().

Since, pvh is a pv domain, I don't like using pv_guest for non PVH PV.
But perhaps I could use the name pv_mmu and have something like
following:

enum guest_type { is_mmu_pv, is_pvh_pv, is_hvm } guest_type;

Then:  is_hvm_or_pvh_domain()  becomes:  !is_mmu_pv().

Alternative to is_mmu_pv: is_pure_pv, is_orig_pv, ....

Let me know what you think.

thanks,
m-

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