From mboxrd@z Thu Jan  1 00:00:00 1970
From: Mukesh Rathor <mukesh.rathor@oracle.com>
Subject: Re: [PATCH 6/18 V2]: PVH xen: Introduce PVH guest type
Date: Mon, 18 Mar 2013 17:21:22 -0700
Message-ID: <20130318172122.05a084e3@mantra.us.oracle.com>
References: <20130315173240.591f9784@mantra.us.oracle.com>
	<51470E8502000078000C652A@nat28.tlf.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <51470E8502000078000C652A@nat28.tlf.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>
Cc: xen-devel <xen-devel@lists.xen.org>
List-Id: xen-devel@lists.xenproject.org

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