[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v1 Altp2m cleanup 1/3] altp2m cleanup work


  • To: Jan Beulich <JBeulich@xxxxxxxx>
  • From: "Lai, Paul C" <paul.c.lai@xxxxxxxxx>
  • Date: Thu, 23 Jun 2016 18:23:14 +0000
  • Accept-language: en-US
  • Cc: xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, "Sahita, Ravi" <ravi.sahita@xxxxxxxxx>
  • Delivery-date: Thu, 23 Jun 2016 18:23:43 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xen.org>
  • Thread-index: AQHRzWYy+wmaoZXobkS5dw5qJZ7qq5/3V9Yw
  • Thread-topic: [PATCH v1 Altp2m cleanup 1/3] altp2m cleanup work

I'm opposed to moving HVMOP_cmd_min and HVMOP_cmd_max somewhere else.  That 
would make reading/understanding of the macros more difficult.  This practice 
is common.  Also, If min & max are defined elsewhere, it will be more likely to 
lead to mistakes/bugs. 

The use of "_min" and "_max" should be quite clear and is common use in linux 
code; Yes, I know this is xen code and I see it here too.  If there's a better 
way, please propose the better.  Maybe you're suggesting the macro names should 
be all caps: 
  HVMOP_CMD_MIN, HVMOP_CMD_MAX
?  I was following the coding style within the file itself.

I'm open to suggestions,
-Paul

-----Original Message-----
From: Jan Beulich [mailto:JBeulich@xxxxxxxx] 
Sent: Thursday, June 23, 2016 8:45 AM
To: Lai, Paul C <paul.c.lai@xxxxxxxxx>
Cc: Sahita, Ravi <ravi.sahita@xxxxxxxxx>; xen-devel 
<xen-devel@xxxxxxxxxxxxxxxxxxxx>
Subject: Re: [PATCH v1 Altp2m cleanup 1/3] altp2m cleanup work

>>> On 21.06.16 at 18:04, <paul.c.lai@xxxxxxxxx> wrote:
> --- a/xen/include/public/hvm/hvm_op.h
> +++ b/xen/include/public/hvm/hvm_op.h
> @@ -479,6 +479,8 @@ struct xen_hvm_altp2m_op {
>  #define HVMOP_altp2m_set_mem_access       7
>  /* Change a p2m entry to have a different gfn->mfn mapping */
>  #define HVMOP_altp2m_change_gfn           8
> +#define HVMOP_cmd_min                     HVMOP_altp2m_get_domain_state
> +#define HVMOP_cmd_max                     HVMOP_altp2m_change_gfn

I don't see why these would need to be in the public interface. Nor were their 
names chosen to properly describe their purpose.

Jan


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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.