[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 4/5] block: cow - used QEMU_PACKED for on-disk s
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 4/5] block: cow - used QEMU_PACKED for on-disk structures |
Date: |
Fri, 20 Sep 2013 08:23:54 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.2 (gnu/linux) |
Jeff Cody <address@hidden> writes:
> On Thu, Sep 19, 2013 at 12:01:24PM -0700, Richard Henderson wrote:
>> On 09/19/2013 11:43 AM, Jeff Cody wrote:
>> > cow_header_v2 is read and written directly from the image file
>> > with bdrv_pread()/bdrv_pwrite(), and as such should be packed to
>> > avoid unintentional padding.
>> >
>> > Also change struct cow_header_v2 to a typedef, and some minor
>> > code style changes to keep checkpatch.pl happy.
>> >
>> > Signed-off-by: Jeff Cody <address@hidden>
>> > ---
>> > block/cow.c | 21 +++++++++++----------
>> > 1 file changed, 11 insertions(+), 10 deletions(-)
>> >
>> > diff --git a/block/cow.c b/block/cow.c
>> > index 909c3e7..9c15afb 100644
>> > --- a/block/cow.c
>> > +++ b/block/cow.c
>> > @@ -32,14 +32,14 @@
>> > #define COW_MAGIC 0x4f4f4f4d /* MOOO */
>> > #define COW_VERSION 2
>> >
>> > -struct cow_header_v2 {
>> > +typedef struct QEMU_PACKED cow_header_v2 {
>> > uint32_t magic;
>> > uint32_t version;
>> > char backing_file[1024];
>> > int32_t mtime;
>> > uint64_t size;
>> > uint32_t sectorsize;
>> > -};
>> > +} COWHeaderV2;
>>
>> This changes the layout of this struct. In particular, there's padding
>> (depending on the host) between mtime and size.
>>
>
> You are right, and that poses a problem for this patch.
>
>> I don't know what the right solution is: COWHeaderV3 with the bug fix,
>> leaving
>> V2 alone; adding an int32_t dummy there where the padding was; nothing,
>> considering the padding to be gone a good thing.
>>
>
> I'm not sure either. I don't think the right thing is to take the
> patch as-is, because that will likely break a lot of existing COW
> images (I just checked, and on x86_64, it is 1056 bytes unpacked, or
> 1048 bytes packed).
>
> Unfortunately, this means that theoretically, image files with this
> format may not be portable, depending on the hosts' compiler and
> alignment. In reality, it likely is not a problem.
>
> I'll drop this one for v2.
Possible solutions:
* Declare format "cow" non-portable. To move a cow to another system,
you have to convert to a portable format.
* Keep using the non-portable header. When opening an existing image,
guess which of the two header variants it got: the padding should be
zero, size and sectorsize sane. Perhaps provide an option to overrule
the guess.
Who's still using format "cow"?
- [Qemu-devel] [PATCH 0/5] block: use QEMU_PACKED for on-disk structures, Jeff Cody, 2013/09/19
- [Qemu-devel] [PATCH 1/5] block: vdi - use QEMU_PACKED for on-disk structures, Jeff Cody, 2013/09/19
- [Qemu-devel] [PATCH 2/5] block: vpc - use QEMU_PACKED for on-disk structures, Jeff Cody, 2013/09/19
- [Qemu-devel] [PATCH 4/5] block: cow - used QEMU_PACKED for on-disk structures, Jeff Cody, 2013/09/19
- Re: [Qemu-devel] [PATCH 4/5] block: cow - used QEMU_PACKED for on-disk structures, Richard Henderson, 2013/09/19
- Re: [Qemu-devel] [PATCH 4/5] block: cow - used QEMU_PACKED for on-disk structures, Jeff Cody, 2013/09/20
- Re: [Qemu-devel] [PATCH 4/5] block: cow - used QEMU_PACKED for on-disk structures,
Markus Armbruster <=
- Re: [Qemu-devel] [PATCH 4/5] block: cow - used QEMU_PACKED for on-disk structures, Jeff Cody, 2013/09/25
- Re: [Qemu-devel] [PATCH 4/5] block: cow - used QEMU_PACKED for on-disk structures, Kevin Wolf, 2013/09/25
- Re: [Qemu-devel] [PATCH 4/5] block: cow - used QEMU_PACKED for on-disk structures, Jeff Cody, 2013/09/25
- Re: [Qemu-devel] [PATCH 4/5] block: cow - used QEMU_PACKED for on-disk structures, Richard Henderson, 2013/09/25
[Qemu-devel] [PATCH 3/5] block: qcow2 - used QEMU_PACKED for on-disk structures, Jeff Cody, 2013/09/19
[Qemu-devel] [PATCH 5/5] block: qed - use QEMU_PACKED for on-disk structures, Jeff Cody, 2013/09/19
Re: [Qemu-devel] [PATCH 0/5] block: use QEMU_PACKED for on-disk structures, Kevin Wolf, 2013/09/20