Re: [Xen-devel] [PATCH v11 1/2] libxl: add support for vscsi

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Fri, Apr 08, 2016 at 04:16:46PM +0200, Olaf Hering wrote:
> On Fri, Apr 08, Wei Liu wrote:
> 
> > What Ian wanted was that we need clear correlation of xl configuration
> > syntax with libxl API fields. For example, for a FOO device
> > 
> >    xl.cfg: FOO = [ "bar=baz" ]
> > 
> >    libxl_types.idl:
> > 
> >     libxl_device_FOO = Struct("device_foo", [
> >       ("bar", some_type)
> >       ...
> >       ])
> > 
> > However, I don't think that can be easily achieved in the case of pvscsi
> > because we need to support both legacy Xen-classic kernel and new PVOPS
> > kernel. Olaf, please correct me if I'm wrong.
> 
> I think that has nothing todo with the backend kernel, the domU.cfg has
> always "vscsi=[ 'pdev,vdev' ]". So in this case libxl_device_vscsictrl

OK. We need to continue to support it then.

> should become libxl_device_vscsi.
> 

Currently you have

   libxl_device_vscsi
      libxl_vscsi_pdev -> points to physical device
      libxl_vscsi_hctl -> points to virtual device

   
   libxl_device_vscsictrl
      an array of libxl_device_vscsi

I think this is fine and in line with what usb does.

Wei.

> > > +
> > > +static bool vscsi_fill_ctrl(libxl__gc *gc,
> > > +                            xs_transaction_t t,
> > > +                            const char *fe_path,
> > > +                            const char *dir,
> > > +                            libxl_device_vscsictrl *ctrl)
> > > +{
> > > +    libxl_device_vscsidev dev;
> > > +    char *tmp, *be_path, *devs_path;
> > > +    char **dev_dirs;
> > > +    unsigned int ndev_dirs, dev_dir;
> > > +    bool ok;
> > > +
> > > +    ctrl->devid = atoi(dir);
> > > +
> > > +    be_path = libxl__xs_read(gc, t, GCSPRINTF("%s/%s/backend", fe_path, 
> > > dir));
> > > +    if (!be_path)
> > > +        goto out;
> > > +
> > > +    tmp = libxl__xs_read(gc, t, GCSPRINTF("%s/%s/backend-id", fe_path, 
> > > dir));
> > > +    if (!tmp)
> > > +        goto out;
> > > +    ctrl->backend_domid = atoi(tmp);
> > 
> > Please sanitise input coming from frontend.  You need to check if the
> > backend domid and frontend domid make sense.
> 
> What do you have in mind? Something like in vusb_be_from_xs_fe?
> 
> Olaf

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