On Tue, Jun 05, 2007 at 03:05:30PM +0200, Stefan Bader wrote: > This patch adds the new interface to dm-io. And also undoes some formatting, comments, variable name changes etc. that I made when preparing earlier patches for upstream. Please revert those changes and also look at what I changed last time, and make similar changes to this code. Please avoid embedding enum definitions inside structs, for example. - int bi_rw; /* READ|WRITE - not READA */ + int bi_rw; /* READ|WRITE + bio flags */ Do we now support READA? - io_notify_fn fn; /* Callback for asynchronous requests */ - void *context; /* Passed to callback */ + io_notify_fn fn; /* Callback for async req */ + void * context; /* Passed to callback */ Please don't make format changes like that. And things like these need resolving before inclusion: + //spin_lock(&sio->lock); + /* FIXME: This might me unnecessary. */ + /* FIXME: really need this ? */ + unsigned flags; /* Future use... */ Why do we need that *today* rather than adding it in future when we need it? This first patch is large and it would be nice to see it broken down further. For example, get any renaming / function moving out of the way before making substantive code changes. But before that, let's examine and understand the proposed interface changes in dm-io.h. The patch is unfortunately not easy to read in its current form. Adding the new interface in parallel with the old was appropriate previously (change to nature of interface, and we needed to retain the original interface in RHEL) but I don't think it works very well this time. Alasdair -- agk@xxxxxxxxxx -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
