Re: [Qemu-devel] [PATCH] Add new block driver for the VDI format (use aio)

[Christoph Hellwig]

On Mon, Jul 27, 2009 at 10:00:34AM +0200, Kevin Wolf wrote:
> Ok, that makes sense. Probably we should remove the #define completely
> then. I mean, why creating images that nobody - not even we ourselves -
> can read?

I agree.  As mentioned during the previous rounds all these ifdef parts
of code that can only be compiled in/out by touching the source code are
really bad.  Either they are good enough to be enabled unconditionally
(or at least through configure if they require a library or similar) or
they are broken / useless enough to not bother.  If virtualbox only
supports 1k block size images and we do aswell there's no point in
carrying around this dead code.

> >> I guess you should remove this block before the patch is included.
> >>   
> > 
> > This is also one of the details were hints of the block driver experts
> > would be helpful as I did not understand this aio_remove / aio_cancel
> > mechanism.
> 
> I wouldn't consider myself an AIO expert and I don't want to tell you
> something wrong, so maybe Christoph would be the right one here?

#if 0 is a horrible way for hints.  Coments with XXX: or TODO: are much
better documentation.  I'll take a look at the aio implementation, but
I'm far from expert on the qemu aio code.

> > I think it would be possible to extend the specification
> > to support compression or encryption.
> > 
> > The official specification (as far as I know it) does not
> > support compression (nor encryption).
> 
> Then remove the entry. The function vdi_write_compressed doesn't exist
> and doesn't even make sense with the current specification. The same
> applies for vdi_set_key.

Seconded, keeping function stubs around just bloats and obsfucates the
code without reason.