Re: [Xen-devel] [PATCH v3 23/24] libxl: Add support for non-PCI passthrough

[Julien Grall <julien.grall@xxxxxxxxxx>]

On 29/01/15 12:31, Stefano Stabellini wrote:
> On Thu, 29 Jan 2015, Julien Grall wrote:
>> Hi Stefano,
>>
>> On 29/01/15 11:12, Stefano Stabellini wrote:
>>>> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
>>>> index 029d2e2..b7ef528 100644
>>>> --- a/tools/libxl/libxl_create.c
>>>> +++ b/tools/libxl/libxl_create.c
>>>> @@ -1430,6 +1430,16 @@ static void domcreate_attach_pci(libxl__egc *egc, 
>>>> libxl__multidev *multidev,
>>>
>>> I think you should at least rename domcreate_attach_pci to something
>>> more generic, like domcreate_attach_dev.
>>
>> Actually I was planning to add a domcreate_attach_dtdev but I forgot
>> about it.
>>
>> What the best approach for this?
> 
> Either one would work. But don't add non-PCI passthrough code to a
> function named domcreate_attach_pci :-)

Right, it was a quick & dirty implementation which I forgot to clean up.

>>>
>>>>          }
>>>>      }
>>>>  
>>>> +    for (i = 0; i < d_config->num_dtdevs; i++) {
>>>> +
>>>> +        ret = libxl__device_dt_add(gc, domid, &d_config->dtdevs[i]);
>>>> +        if (ret < 0) {
>>>> +            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
>>>> +                       "libxl__device_dt_add failed: %d\n", ret);
>>>> +            goto error_out;
>>>> +        }
>>>> +    }
>>>
>>> You are allowed to call xc_* functions from here. The
>>> libxl__device_dt_add wrapper doesn't add much value.
>>
>> I would like to keep the wrapper. It's in sync with the PCI solution and
>> it will avoid refactoring later for add new code.
> 
> But in the PCI case there is a lot of code in the function.
> Regardless if you think it is useful, keep it.

If we plan to implement a proper platform device pass-through we will
have to add lots of code.

Wei, Ian & Ian, do you have any opinions?

Regards,

-- 
Julien Grall

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