[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: |
Jeff Cody |
Subject: |
Re: [Qemu-devel] [PATCH 4/5] block: cow - used QEMU_PACKED for on-disk structures |
Date: |
Wed, 25 Sep 2013 15:01:47 -0400 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Wed, Sep 25, 2013 at 07:25:20PM +0200, Kevin Wolf wrote:
> Am 25.09.2013 um 17:12 hat Jeff Cody geschrieben:
> > On Fri, Sep 20, 2013 at 08:23:54AM +0200, Markus Armbruster wrote:
> > > 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.
> > >
> >
> > I favor this approach, especially since "cow" is not likely to be used
> > in new environments.
>
> But COW isn't a native qemu format. We should do whatever the format
> really requires, so that new qemu versions handle it correctly - and if
> old qemu versions produced corrupted files, bad luck.
>
It is from UML, right? Is there an official spec that is still around
(most of the links I have found suffer from link rot)? The closest I
could find to a spec were old UML patches for x86_64 that cleaned up
some data types, so that the following was defined:
struct cow_header_v2 {
__u32 magic;
__u32 version;
char backing_file[PATH_LEN_V2];
time_t mtime;
__u64 size;
int sectorsize;
};
That remains ambiguous, although given the era I suppose it could be
argued that 32-bit architecture and alignment is assumed. But if this
is the original spec, then it seems like a non-portable one.
- [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, 2013/09/20
- 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 <=
- 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