Re: [Xen-devel] [PATCH 3/3] libxl: libxl__device_from_disk should retrieve backend from xenstore

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Wed, Feb 11, 2015 at 10:18:18AM -0700, Jim Fehlig wrote:
> Wei Liu wrote:
> > On Tue, Feb 10, 2015 at 11:01:46AM +0000, Ian Jackson wrote:
> >   
> >> Wei Liu writes ("[PATCH 3/3] libxl: libxl__device_from_disk should 
> >> retrieve backend from xenstore"):
> >>     
> >>> ... if backend is not set by caller.
> >>>       
> >> Acked-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> >>
> >> as far as it goes, but I think you may want a more radical change -
> >> see below.
> >>
> >>     
> >>> Also change the function to use "goto" idiom while I was there.
> >>>       
> >> (Although usually it would be better to split this kind of thing into
> >> a pre-patch, in this case it's small and easily reviewed.)
> >>
> >> Is the backend type the only missing or potentially-wrong
> >> information ?  ISTM that perhaps the caller might not know the target,
> >> either.
> >>
> >> What should happen if the caller specifies a different target in disk
> >> to the one the device is actually using ?  The documentation should
> >> specify which of the fields are important.
> >>
> >>     
> >
> > I'm not sure because it's not documented.
> >
> > We should take a step back to define the important fields first.
> >
> >   
> >> Maybe libxl_device_disk_remove needs to call libxl_vdev_to_device_disk
> >> and check that the supplied disk struct is plausible somehow.  In that
> >> case it might be nice for the caller to be able to fill in only the
> >> vdev.
> >>
> >>     
> >
> > If so we need to make clear in the documentation. I'm of course fine
> > with this behaviour.
> >
> > Jim, does libvirt (as an example of libxl user) actually cares
> > specifying every fields in that struct? The other user (xl) doesn't seem
> > to care that much.
> >   
> 
> At minimum, libvirt will populate the pdev_path, vdev, backend, and
> format fields. If backend and format (which, in libvirt-speack
> correspond to the 'name' and 'type' attributes on the optional <driver>
> element) are not specified, they are set to LIBXL_DISK_BACKEND_UNKNOWN
> and LIBXL_DISK_FORMAT_RAW respectively.
> 

Since libvirt has a tendency of specifying everything, how come there is
no "name" and "type" in <driver>? Can we actually generate all the
fields needed when attaching a disk and store in libvirt's diskspec?

Wei.

> Regards,
> Jim

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