[Drbd-dev] Re: drbd uses wrong API for struct bio

[Lars Marowsky-Bree]

On 2005-01-24T10:10:07, Jens Axboe <axboe at suse.de> wrote:

> drbd_ee_init() doesn't look good. First of all, it makes assumptions
> about what bio_init() would do - it doesn't clear the entire bio to 0.
> This is the type of thing that exposes bugs when we add or change parts
> of the bio stuff, please uncomment that bio_init() call. It does set
> bio->bi_max_vecs, however it needs changing to passing the bdev in so we
> can use bio_add_page() instead. drbd_ee_bio_prepare() looks strange, it
> sets vec length but doesn't assign the page or offset. Again, it should
> use the proper api (bio_add_page()). If it had used bio_init(), it would
> not have to set BIO_UPTODATE manually either.
> 
> bio->bi_next is a pointer, not an integer. Use NULL.
> 
> drbd_blk_run_queue() doesn't look legal, unless it's only ever run on
> drbd's own queue. Even if so, it should remove the plug. I'd suggest
> just using generic_unplug_device().
> 
> drdb should just use bio_iovec(bio) when it uses bio->bi_idx as the
> index anyways.

Ok, I'll go through the code and clean this up now. Thanks!

I'll hassle you with more questions as my changes oops ;-)

> The bio embedding may not be the best idea or the prettiest, but as long
> as it sets ->bi_max_vecs it should be ok. In general the code could
> really do with someone cleaning it up in preperation for submitting to
> mainline...

Yeah, that's happening with 0.8, but this is a 'maintenance' branch,
with all what that entails :-( drbd 0.7 on 2.4 is (unfortunately ;-)
still making money for Linbit, or else I'd guess everyone would be just
too happy to rip it out.


Sincerely,
    Lars Marowsky-Brée <lmb at suse.de>

-- 
High Availability & Clustering
SUSE Labs, Research and Development
SUSE LINUX Products GmbH - A Novell Business