Matthew Fioravante, le Mon 14 Mar 2011 15:07:30 -0400, a écrit :
> On 03/11/2011 08:05 PM, Samuel Thibault wrote:
> >Matthew Fioravante, le Fri 11 Mar 2011 17:34:26 -0500, a écrit :
> >>+ /*Make sure we have write permission */
> >>+ if(dev->info.info& VDISK_READONLY || dev->info.mode != O_RDWR) {
> >O_WRONLY too.
> Good catch, actually testing a bitfield with != is a bad idea to begin
> with anyway.
Err, it's not a bitfield, in mini-os it's a {0,1,2} enum.
> >Could you perhaps optimize when buf is actually aligned? That would
> >save a copy.
> >
> This can be done but only if in the current iteration of the loop an
> entire block is being read.
Sure.
> Since aiocb only operates on sectors it'll
> read at minimum a whole sector into buf. If buf isnt big enough to hold
> the data a secondary buffer with a copy operation will have to be done.
Sure.
But I expect people using that interface to tend to allocate big aligned
buffers.
> >>+ /* Write operation */
> >>+ else {
> >>+ /* If we're writing a partial block, we need to read the current
> >>contents first
> >>+ * so we don't overwrite the extra bits with garbage */
> >>+ if(blkoff != 0 || bytes< blocksize) {
> >>+ aiocb.aio_cb = NULL;
> >Maybe blkfront_aio_cb should do it itself? It looks odd to have to do
> >it when reusing an aiocb structure.
> >
> It could, but then that changes the design of aiocb. Was it supposed to
> be a very low level interface for just reading and writing blocks onto
> the disk?
Well, I'd say the whole blkfront itself is a low-level interrface, and
your patch actually provides a higher one :)
This particular change in the design shouldn't break anything, since
aio_cb is actually filled by blkfront itself, so it makes sense that it
cleans it since it expects it to be cleaned.
> Right now you have to set aio_nbytes and aio_offset to a multiple of
> sector size. This could be changed to allow variable sizes.
> Alternatively 2 new fields could be added to specify which portion
> inside a block to operate on.
>
> Can you send a partial block through the xen block frontend and backend
> interface?
No.
> If not we would have to queue up a read and then a write
> internally when the user requests a write. Its possible some users may
> not want this forced behavior of 2 operations.
That's why I wouldn't recommend aio_nbytes/offset to be allowed to
be non-multiples of the sector size. That interface is meant to be
an efficient sector-transfer interface. Your posix layer can handle
flexibility for the user.
Samuel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home