[Lguest] [PATCH 4/6] virtio block driver

[Rusty Russell]

On Thu, 2007-09-20 at 15:05 +0200, Jens Axboe wrote:
> On Thu, Sep 20 2007, Rusty Russell wrote:
> > +static void end_dequeued_request(struct request *req,
> > +				 struct request_queue *q, int uptodate)
> > +{
> > +	/* And so the insanity of the block layer infects us here. */
> > +	int nsectors = req->hard_nr_sectors;
> > +
> > +	if (blk_pc_request(req)) {
> > +		nsectors = (req->data_len + 511) >> 9;
> > +		if (!nsectors)
> > +			nsectors = 1;
> > +	}
> > +	if (end_that_request_first(req, uptodate, nsectors))
> > +		BUG();
> > +	add_disk_randomness(req->rq_disk);
> > +	end_that_request_last(req, uptodate);
> > +}
> 
> We have end_queued_request(), lets add end_dequeued_request(). Below.

OK, thanks, I'll throw that in the mix and test...

> > +	vblk->sg[0].page = virt_to_page(&vbr->out_hdr);
> > +	vblk->sg[0].offset = offset_in_page(&vbr->out_hdr);
> > +	vblk->sg[0].length = sizeof(vbr->out_hdr);
> > +	num = blk_rq_map_sg(q, vbr->req, vblk->sg+1);
> 
> This wont work for chained sglists, but I gather (I'm so funny) that it
> wont be an issue for you. How large are your sglists?

Hmm, potentially extremely large.  What do I need to do for chained
sglists?

> > +		if (!do_req(q, vblk, req)) {
> > +			/* Queue full?  Wait. */
> > +			blk_stop_queue(q);
> > +			break;
> > +		}
> 
> Personally I think this bool stuff is foul. You return false/true, but
> still use ! to test. That is just more confusing than the canonical 0/1
> good/bad return imho.

Except "0/1" is not canonical in the kernel.  Arguably, "-errno/0" is
canonical.  OTOH, bool is clear.

if do_req() fails, we assume the queue is full.  I shall change the
comment to that effect.

Thanks!
Rusty.