[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

RE: [Xen-devel] [Patch 4/7] pvSCSI driver



> > > > +int do_comfront_cmd_done(struct comfront_info *);
> > > > +
> > > > +static inline int GET_ID_FROM_FREELIST(struct comfront_info
*info)
> > > Why is this all-caps?  It's not a macro.
> > That would just have been cut&pasted from the linux sources... I did
the
> > same in the windows PV drivers :)
> Ah, okay.
> 
> Poking around a little, it looks like the blkfront in upstream Linux
> uses lower-case get_id_from_freelist().  I'm not sure whether it's
> better to be consistent with that or consistent with the other drivers
> in our tree.  I'd probably go with upstream, but either's pretty
> valid.
> 
> > > > +       for (i = info->ring.rsp_cons; i != rp; i++) {
> > > > +
> > > > +               ring_res = RING_GET_RESPONSE(&info->ring, i);
> > > > +
> > > > +               if (info->shadow[ring_res->rqid].cmd ==
> > VSCSIIF_CMND_SCSI) {
> > > > +                       if (scsifront_cmd_done(info, ring_res))
{
> > > > +                               BUG();
> > > > +                       }
> > > > +               } else {
> > > > +                       __sync_cmd_done(info, ring_res);
> > > Can this ever happen?
> > Well... a rogue frontend could pass all manner of crap to the
backend.
> > Best to check for these things.
> This *is* the frontend, unless I'm very, very confused.

Nope. I am the confused party in this case :)

> > The other option for VSCSIIF_CMND_SCSI is a reset, but there is some
> > debate as to whether a frontend using a single device on a physical
> > scsi bus should be allowed to initiate a bus reset...
> Yeah, an LU reset might be a better idea, if the underlying device can
> handle it.

It's a tricky situation, as there are some SCSI errors which do require
a bus reset to resolve...

> Speaking of which, would it be a good idea to expose the tagged
> command queueing stuff?  I'd guess probably not, but I don't really
> understand SCSI well enough to be sure.

I think that that is implicitly exposed anyway, but my understanding of
SCSI is probably about on par with yours, so I'm not absolutely sure.

James


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.