[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 2/12] VTPM mini-os: posix IO layer for blkfront in mini-os
I think I totally misunderstood you. With the last case are you just referring to how I keep having to set the callback to NULL everytime? If so I totally agree with you. It should just set it back to NULL when its done. I thought you were talking about how I was handling writes to partial blocks and that somehow the low level blkfront functions should be doing this transparently. On 03/14/2011 03:29 PM, Samuel Thibault wrote: 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 Attachment:
smime.p7s _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |