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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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:
> 
>> >>> On 16.03.13 at 01:32, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>
>> >>> 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;
>> 
>> And of course, please properly format this.
> 
> Not sure I follow what needs formatting?

A number of blanks need to be inserted. And the whole enum
declaration probably doesn't belong on a single line anyway.

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

>> > @@ -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?

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().

Jan


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