[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [Patch 4/7] pvSCSI driver
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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |