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

[Jun Kamada <kama@xxxxxxxxxxxxxx>]

Hi James-san,

Thank you for your comments.

On Wed, 27 Feb 2008 23:32:16 +1100
"James Harper" <james.harper@xxxxxxxxxxxxxxxx> wrote:

> > > +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 :)
> 
> > > + 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. 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...

Yes, the __sync_cmd_done() is intended for Reset/Abort command. As
you mentiond above, we need more discussion how to treat Reset/Abort
command. 

I currently think that only a device reset is needed for the moment.
The rings implemented in current code can be shared for read/write
command and reset/abort command in that case.


Best regards,


Jun Kamada



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