[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH RESEND] xen/blkfront: improve protection against issuing unsupported REQ_FUA
Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx> writes: > On 12/01/2014 08:01 AM, Vitaly Kuznetsov wrote: >> Guard against issuing unsupported REQ_FUA and REQ_FLUSH was introduced >> in d11e61583 and was factored out into blkif_request_flush_valid() in >> 0f1ca65ee. However: >> 1) This check in incomplete. In case we negotiated to feature_flush = >> REQ_FLUSH >> and flush_op = BLKIF_OP_FLUSH_DISKCACHE (so FUA is unsupported) FUA >> request >> will still pass the check. >> 2) blkif_request_flush_valid() is misnamed. It is bool but returns true when >> the request is invalid. >> 3) When blkif_request_flush_valid() fails -EIO is being returned. It seems >> that >> -EOPNOTSUPP is more appropriate here. >> Fix all of the above issues. >> >> This patch is based on the original patch by Laszlo Ersek and a comment by >> Jeff Moyer. >> >> Signed-off-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> >> Reviewed-by: Laszlo Ersek <lersek@xxxxxxxxxx> > > Reviewed-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx> > > (although, as I mentioned last time, a companion patch to remove > flush_op would be a good thing to have) > Thanks, it is on my todo list but I'm trying to separate this (potential) bugfix from straight cleanup. > -boris > >> --- >> drivers/block/xen-blkfront.c | 14 ++++++++------ >> 1 file changed, 8 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c >> index 5ac312f..2e6c103 100644 >> --- a/drivers/block/xen-blkfront.c >> +++ b/drivers/block/xen-blkfront.c >> @@ -582,12 +582,14 @@ static inline void flush_requests(struct blkfront_info >> *info) >> notify_remote_via_irq(info->irq); >> } >> -static inline bool blkif_request_flush_valid(struct request *req, >> - struct blkfront_info *info) >> +static inline bool blkif_request_flush_invalid(struct request *req, >> + struct blkfront_info *info) >> { >> return ((req->cmd_type != REQ_TYPE_FS) || >> - ((req->cmd_flags & (REQ_FLUSH | REQ_FUA)) && >> - !info->flush_op)); >> + ((req->cmd_flags & REQ_FLUSH) && >> + !(info->feature_flush & REQ_FLUSH)) || >> + ((req->cmd_flags & REQ_FUA) && >> + !(info->feature_flush & REQ_FUA))); >> } >> /* >> @@ -612,8 +614,8 @@ static void do_blkif_request(struct request_queue *rq) >> blk_start_request(req); >> - if (blkif_request_flush_valid(req, info)) { >> - __blk_end_request_all(req, -EIO); >> + if (blkif_request_flush_invalid(req, info)) { >> + __blk_end_request_all(req, -EOPNOTSUPP); >> continue; >> } >> -- Vitaly _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |