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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

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 :-)


> > 
> >>          }
> >>      }
> >>  
> >> +    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.

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