Re: [Xen-devel] [v4][PATCH 12/19] tools/libxl: passes rdm reservation policy

[George Dunlap <george.dunlap@xxxxxxxxxxxxx>]

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

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