This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


Re: [Xen-devel] [PATCH 2/12] VTPM mini-os: posix IO layer for blkfront i

To: Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH 2/12] VTPM mini-os: posix IO layer for blkfront in mini-os
From: Matthew Fioravante <matthew.fioravante@xxxxxxxxxx>
Date: Mon, 14 Mar 2011 15:07:30 -0400
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Mon, 14 Mar 2011 12:08:49 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20110312010526.GK4922@xxxxxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <4D7AA372.4040506@xxxxxxxxxx> <20110312010526.GK4922@xxxxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv: Gecko/20101208 Thunderbird/3.1.7
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) {
Good catch, actually testing a bitfield with != is a bad idea to begin with anyway.
+   aiocb.aio_dev = dev;
+   aiocb.aio_buf = _xmalloc(blocksize, dev->info.sector_size);
+   aiocb.aio_nbytes = blocksize;
+   aiocb.aio_offset = blknum * blocksize;
+   aiocb.aio_cb = NULL;
+   aiocb.data = NULL;
+      /* read operation */
+      if(!write) {
+        aiocb.aio_cb = NULL;
+        blkfront_read(&aiocb);
+        memcpy(buf,&aiocb.aio_buf[blkoff], bytes);
+      }
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. 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.
+      /* 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?

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? 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.
Apart from that it looks good.


Attachment: smime.p7s
Description: S/MIME Cryptographic Signature

Xen-devel mailing list