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

["Andrew Warfield" <andrew.warfield@xxxxxxxxxxxx>]

Excellent comments, thanks.

> 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.
All good -- I'll take a pass through and fix all these this week.

> What's the significance of /**BLKTAP**/ and /**TAPEND**/?
For a while we were maintaining the kernel tap driver as a diff
against blkback, to pick up fixes quickly -- those markers were just
to mark differing regions.  I think the current code has diverged
enough to make this approach untenable.

> I'm a little surprised to see these conversion tools too.  Wouldn't it
> be easier to just add some parameters to qemu-img?
The image tools use our plugins (rather than qemu's) to build disks --
most importantly they adjust the layout to build better-performing
images.  Fair point though, I think julian was going to look and see
if we could get away with just using the qemu tools.

> Pretty interesting stuff, thanks for posting.
Thanks for the feedback!

a.

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