Re: [Qemu-devel] [RFC-v2 3/6] vhost-scsi: add -vhost-scsi host device for use with tcm-vhost

[Nicholas A. Bellinger]

On Sun, 2012-08-19 at 11:44 +0300, Michael S. Tsirkin wrote:
> On Sat, Aug 18, 2012 at 05:36:26PM -0700, Nicholas A. Bellinger wrote:
> > On Sat, 2012-08-18 at 22:12 +0300, Michael S. Tsirkin wrote:
> > > On Tue, Aug 14, 2012 at 01:31:14PM -0700, Nicholas A. Bellinger wrote:
> > > > On Mon, 2012-08-13 at 11:53 +0300, Michael S. Tsirkin wrote:
> > > > > On Mon, Aug 13, 2012 at 08:35:14AM +0000, Nicholas A. Bellinger wrote:
> > > > > > From: Stefan Hajnoczi <address@hidden>

<SNIP>

> > > > Mmm, looking at how vhost_net_init + tap.c does this, but am not quite
> > > > what fd needs to be propagated up for virtio-scsi -> vhost-scsi..
> > > > 
> > > > Can you please elaborate on this one a bit more..?
> > > > 
> > > 
> > > The idea is to allow running as a user without access to
> > > /dev/vhost-scsi.
> > > For this, allow passing in the fd of /dev/vhost-scsi through unix domain 
> > > sockets.
> > > 
> > 
> > Ah, that is a pretty neat trick..   So for vhost-scsi code, this would
> > mean something along the lines of the following, yes..?
> 
> Yes but with one correction. See below.
> 
> > Thanks MST!
> 
> > diff --git a/hw/vhost-scsi.c b/hw/vhost-scsi.c
> > index 4206a75..8af8758 100644
> > --- a/hw/vhost-scsi.c
> > +++ b/hw/vhost-scsi.c
> > @@ -21,6 +21,7 @@ struct VHostSCSI {
> >      const char *id;
> >      const char *wwpn;
> >      uint16_t tpgt;
> > +    int vhostfd;
> >      struct vhost_dev dev;
> >      struct vhost_virtqueue vqs[VHOST_SCSI_VQ_NUM];
> >      QLIST_ENTRY(VHostSCSI) list;
> > @@ -114,13 +115,32 @@ void vhost_scsi_stop(VHostSCSI *vs, VirtIODevice 
> > *vdev)
> >  }
> >  
> >  static VHostSCSI *vhost_scsi_add(const char *id, const char *wwpn,
> > -                                 uint16_t tpgt)
> > +                                 uint16_t tpgt, const char *vhostfd_str)
> >  {
> > -    VHostSCSI *vs = g_malloc0(sizeof(*vs));
> > +    VHostSCSI *vs;
> >      int ret;
> >  
> > +    vs = g_malloc0(sizeof(*vs));
> > +    if (!vs) {
> > +        error_report("vhost-scsi: unable to allocate *vs\n");
> > +        return NULL;
> > +    }
> > +    vs->vhostfd = -1;
> > +
> > +    if (vhostfd_str) {
> > +        if (!qemu_isdigit(vhostfd_str[0])) {
> > +            error_report("vhost-scsi: passed vhostfd value is not a 
> > digit\n");
> > +            return NULL;
> 
> This let you use an fd which was open at exec
> but does not allow for fd to be open later in
> case device is hot-plugged.
> 
> See net_handle_fd_param - I think you can just rename it
> qemu_handle_fd_param to avoid code duplication.
> 

OK, so monitor_get_fd() will set this up for the case where the device
is hot-plugged.  That makes alot more sense now..

So renaming net_handle_fd_param -> qemu_handle_fd_param + moving into
cutils.c, and will include as a leading patch for RFC-v3.

Thanks MST!

--nab