From mboxrd@z Thu Jan  1 00:00:00 1970
From: George Dunlap <george.dunlap@eu.citrix.com>
Subject: Re: [v4][PATCH 12/19] tools/libxl: passes rdm
 reservation policy
Date: Wed, 1 Jul 2015 11:22:30 +0100
Message-ID: <5593BF66.9010505@eu.citrix.com>
References: <1435053450-25131-1-git-send-email-tiejun.chen@intel.com>
	<1435053450-25131-13-git-send-email-tiejun.chen@intel.com>
	<CAFLBxZZe8TG-46u1qffcMGY1oF4yknbHk+WH9UXEYqu8HznmBg@mail.gmail.com>
	<55934109.4020504@intel.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: <55934109.4020504@intel.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: "Chen, Tiejun" <tiejun.chen@intel.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>, Stefano Stabellini <stefano.stabellini@eu.citrix.com>, Wei Liu <wei.liu2@citrix.com>, Ian Campbell <ian.campbell@citrix.com>, "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>
List-Id: xen-devel@lists.xenproject.org

On 07/01/2015 02:23 AM, Chen, Tiejun wrote:
>>> @@ -988,6 +988,14 @@ static int do_pci_add(libxl__gc *gc, uint32_t
>>> domid, libxl_device_pci *pcidev, i
>>>
>>>   out:
>>>       if (!libxl_is_stubdom(ctx, domid, NULL)) {
>>> +        if (pcidev->rdm_reserve == LIBXL_RDM_RESERVE_FLAG_RELAXED) {
>>> +            flag = XEN_DOMCTL_DEV_RDM_RELAXED;
>>> +        } else if (pcidev->rdm_reserve ==
>>> LIBXL_RDM_RESERVE_FLAG_STRICT) {
>>> +            flag = XEN_DOMCTL_DEV_RDM_STRICT;
>>> +        } else {
>>> +            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unknown rdm
>>> check flag.");
>>> +            return ERROR_FAIL;
>>> +        }
>>
>> Shouldn't this be in the previous patch?
>>
> 
> This is trying to covert LIBXL_XXX to XEN_XXX passed this policy as a
> hypercall, so I still think this is better to live here. Instead, the
> previous patch is just defining something.

The entire rest of this patch is about xl.  It doesn't make any sense at
all for the previous patch to modify libxl in a way that doesnt'
actually do anything, and then in the current patch modify both xl and
libxl.

What if, for instance, someone had built their own toolstack on top of
libxl, and wanted to backport just the xen/libxl parts of the RMRR
series?  They'd have to backport this patch with the xl changes to get a
functioning system, even though they aren't really using libxl.

 -George