WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: "Steven Smith" <steven.smith@xxxxxxxxxxxxx>
Subject: RE: [Xen-devel] [Patch 4/7] pvSCSI driver
From: "James Harper" <james.harper@xxxxxxxxxxxxxxxx>
Date: Thu, 28 Feb 2008 09:25:55 +1100
Cc: Jun Kamada <kama@xxxxxxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Wed, 27 Feb 2008 14:26:14 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <20080227140214.GA27558@xxxxxxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <20080218190631.E755.EB2C8575@xxxxxxxxxxxxxx> <20080227111642.GC26424@xxxxxxxxxxxxxxxxxxxxxxxxxx> <AEC6C66638C05B468B556EA548C1A77D0131AEC2@trantor> <20080227140214.GA27558@xxxxxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: Ach5SZEm40nci5QwR1WeSUPCUGiJxwARdgqA
Thread-topic: [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