[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] Add new block driver for the VDI format (use ai
From: |
Stefan Weil |
Subject: |
Re: [Qemu-devel] [PATCH] Add new block driver for the VDI format (use aio) |
Date: |
Fri, 31 Jul 2009 21:53:41 +0200 |
User-agent: |
Mozilla-Thunderbird 2.0.0.22 (X11/20090707) |
Christoph Hellwig schrieb:
> 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.
>
>
>
Hi Christoph
Thanks for the review. Most of your comments were considered in my
latest patch version which I just sent to the list.
I assume that there will be a need for block sizes larger than 1 MiB
in very large images, so I did not remove these parts of the code.
Regards
Stefan
- Re: [Qemu-devel] [PATCH] Add new block driver for the VDI format (use aio), (continued)
- Re: [Qemu-devel] [PATCH] Add new block driver for the VDI format (use aio), Kevin Wolf, 2009/07/24
- Re: [Qemu-devel] [PATCH] Add new block driver for the VDI format (use aio), Stefan Weil, 2009/07/24
- Re: [Qemu-devel] [PATCH] Add new block driver for the VDI format (use aio), Kevin Wolf, 2009/07/27
- Re: [Qemu-devel] [PATCH] Add new block driver for the VDI format (use aio), Jamie Lokier, 2009/07/27
- Re: [Qemu-devel] [PATCH] Add new block driver for the VDI format (use aio), Amit Shah, 2009/07/28
- Re: [Qemu-devel] [PATCH] Add new block driver for the VDI format (use aio), Jamie Lokier, 2009/07/28
- Re: [Qemu-devel] [PATCH] Add new block driver for the VDI format (use aio), Daniel P. Berrange, 2009/07/28
- Re: [Qemu-devel] [PATCH] Add new block driver for the VDI format (use aio), Jamie Lokier, 2009/07/28
- Re: [Qemu-devel] [PATCH] Add new block driver for the VDI format (use aio), Kevin Wolf, 2009/07/28
- Re: [Qemu-devel] [PATCH] Add new block driver for the VDI format (use aio), Christoph Hellwig, 2009/07/31
- Re: [Qemu-devel] [PATCH] Add new block driver for the VDI format (use aio),
Stefan Weil <=
- Re: [Qemu-devel] [PATCH] Add new block driver for the VDI format (use aio), Anthony Liguori, 2009/07/31
- Re: [Qemu-devel] [PATCH] Add new block driver for the VDI format (use aio), Stefan Weil, 2009/07/31
- [Qemu-devel] [PATCH] Add new block driver for the VDI format (only aio supported), Stefan Weil, 2009/07/31
[Qemu-devel] [PATCH] add support for new option of vdi format, Stefan Weil, 2009/07/23