WARNING - OLD ARCHIVES

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/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH] Blktap: Userspace file-based image support. (RFC

To: Julian Chesterfield <jac90@xxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] Blktap: Userspace file-based image support. (RFC)
From: Anthony Liguori <aliguori@xxxxxxxxxx>
Date: Mon, 19 Jun 2006 16:56:00 -0500
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Mon, 19 Jun 2006 14:56:29 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <c22f889f9333ac3d7169fd235605f496@xxxxxxxxxxxx>
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/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <C0BCD26E.5C31%julian@xxxxxxxxxxxxx> <c22f889f9333ac3d7169fd235605f496@xxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Thunderbird 1.5.0.4 (X11/20060612)
Julian Chesterfield wrote:

On 19/6/06 8:15 pm, "Anthony Liguori" <aliguori@xxxxxxxxxx> wrote:

Couple general comments on the code:

Please don't introduce more (ab)uses of /proc.  Sure it's just for
debugging but there's no reason to not make that sysfs.

I'm not an expert here, but the nopage handlers that I've seen return
NOPAGE_SIGBUS instead of manually causing a SIGBUS on current.

I think it's better to use C99 initialization than GCC:

  owner:  ...,  =>  .owner = ...,

Some of the indenting is a bit off from Linux CodingStyle.  Stuff like
if( => if ( and some random spaces after an (.

There's some code commented out with C++ comments too.

What's the significance of /**BLKTAP**/ and /**TAPEND**/?

I'm a little surprised to see these conversion tools too.  Wouldn't it
be easier to just add some parameters to qemu-img?

Thanks for the comments anthony. When we initially played with qcow images it was easier to knock-up our own frontend to the plugins for converting between the different image types and testing features like image sparseness. We added an optimisation feature in the xen qcow plugin which would allocate full extents for non backing file based images as well as the asynchronous callback architecture to enable request batching for AIO.

We could certainly adapt qemu-img to use these and other features. Not sure what the best approach for keeping the toolsets in synch between the 2 projects would be though.

It may be worth just bringing up the changes on qemu-devel. I know why you'd want to change the cluster size (it's a pain to work with clusters < block size). I saw another comment about making metadata more coarse. Can you clarify the reasons for that?

I can't imagine there would be that push back in changing the default cluster size in qemu-img from 512 to 4096.. Most OS's are going to write in that granularity anyway I imagine :-)

Regards,

Anthony Liguori


Thanks,
Julian Chesterfield


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel