Jeff Cody <jc...@redhat.com> 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 <jc...@redhat.com> >> > --- >> > 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"?