Re: [Xen-devel] [PATCH v6 04/11] libxl: move libxl__device_from_disk to libxl_internal.c

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2012-05-18 at 16:16 +0100, Roger Pau Monne wrote:
> Ian Campbell wrote:
> > On Fri, 2012-05-18 at 15:55 +0100, Roger Pau Monne wrote:
> >>> @@ -429,6 +429,46 @@ libxl_device_model_version
> >> libxl__device_model_version_running(libxl__gc *gc,
> >>>        return value;
> >>>    }
> >>>
> >>> +int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
> >>> +                                   libxl_device_disk *disk,
> >>> +                                   libxl__device *device)
> >> I think this should be moved to libxl_device instead of
> >> libxl_internal, since it's a device related function.
> >
> > I think it'd be better to leave it in the original file, libxl is so
> > confused about what goes where that it doesn't really matter...
> >
> > If we do want to move it we can do it later as a pure code motion
> > change.
> 
> In my hotplug series I'm moving most libxl_device_{...}_add to provide 
> an internal implementation that takes an ao, making something quite 
> similar to what Stefano does, if I start moving all those to 
> libxl_internal we will fill this file with functions that could be 
> somewhere else (libxl_device).

I didn't propose moving this function tolibxl_internal, quite the
opposite. I proposed leaving it in libxl.c where it is (or rather ,
where the body current effectively is). If and when we want to move it
we can do that as a separate pure code motion change.

>  I understand that the libxl policy is put 
> functions where they seem to belong (device related functions to 
> libxl_device, domain related ones to libxl_dom...),

The policy is basically "confused, historical, no one really knows for
sure".

>  and if they fit in 
> none of this categories then put them in libxl_internal or create a new 
> file.

IMHO libxl_internal should go away and never be used, it just encourages
things to go into a dumping ground file, which is effectively what it
has become.

Ian.


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