[Qemu-devel] [PATCH v2 0/4] block: use QEMU_PACKED for on-disk structures

[Qemu-devel] [PATCH v2 0/4] block: use QEMU_PACKED for on-disk structures

[Jeff Cody]

v2 changes:
Dropped the "cow" format patch.  This means "cow" is non-portable, but
it keeps behavior the same on x86_64. (Richard Henderson)

Moved QEMU_PACKED to after the struct definition closing brace, to keep it more
stylistically in-line with other QEMU_PACKED useage (Kevin Wolf)

Original description:

Several block image formats did not consistently use packed attributes
when directly reading / writing structures from disk (mainly image format
headers).

These series updates the image formats (see list below), to use
QEMU_PACKED for on-disk structs.  (Some minor code cleanup may also
have ensued, to keep checkpatch.pl happy)


Jeff Cody (4):
  block: vdi - use QEMU_PACKED for on-disk structures
  block: vpc - use QEMU_PACKED for on-disk structures
  block: qcow2 - used QEMU_PACKED for on-disk structures
  block: qed - use QEMU_PACKED for on-disk structures

 block/qcow2.c |  2 +-
 block/qcow2.h |  2 +-
 block/qed.h   |  2 +-
 block/vdi.c   |  2 +-
 block/vpc.c   | 28 ++++++++++++++--------------
 5 files changed, 18 insertions(+), 18 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH v2 1/4] block: vdi - use QEMU_PACKED for on-disk structures

[Jeff Cody]

The header struct VdiHeader is an on-disk structure for the image
format, and as such should be packed.

Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/vdi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/vdi.c b/block/vdi.c
index dcbc27c..b6ec002 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -165,7 +165,7 @@ typedef struct {
     uuid_t uuid_link;
     uuid_t uuid_parent;
     uint64_t unused2[7];
-} VdiHeader;
+} QEMU_PACKED VdiHeader;
 
 typedef struct {
     /* The block map entries are little endian (even in memory). */
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 2/4] block: vpc - use QEMU_PACKED for on-disk structures

[Jeff Cody]

The VHD footer and header structs (vhd_footer and vhd_dyndisk_header)
are on-disk structures for the image format, and as such should be
packed.

Go ahead and make these typedefs as well, with the preferred QEMU
naming convention, so that the packed attribute is used consistently
with the struct.

Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/vpc.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/block/vpc.c b/block/vpc.c
index db61274..b5dca39 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -46,7 +46,7 @@ enum vhd_type {
 #define VHD_TIMESTAMP_BASE 946684800
 
 // always big-endian
-struct vhd_footer {
+typedef struct vhd_footer {
     char        creator[8]; // "conectix"
     uint32_t    features;
     uint32_t    version;
@@ -79,9 +79,9 @@ struct vhd_footer {
     uint8_t     uuid[16];
 
     uint8_t     in_saved_state;
-};
+} QEMU_PACKED VHDFooter;
 
-struct vhd_dyndisk_header {
+typedef struct vhd_dyndisk_header {
     char        magic[8]; // "cxsparse"
 
     // Offset of next header structure, 0xFFFFFFFF if none
@@ -111,7 +111,7 @@ struct vhd_dyndisk_header {
         uint32_t    reserved;
         uint64_t    data_offset;
     } parent_locator[8];
-};
+} QEMU_PACKED VHDDynDiskHeader;
 
 typedef struct BDRVVPCState {
     CoMutex lock;
@@ -160,8 +160,8 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
 {
     BDRVVPCState *s = bs->opaque;
     int i;
-    struct vhd_footer* footer;
-    struct vhd_dyndisk_header* dyndisk_header;
+    VHDFooter *footer;
+    VHDDynDiskHeader *dyndisk_header;
     uint8_t buf[HEADER_SIZE];
     uint32_t checksum;
     int disk_type = VHD_DYNAMIC;
@@ -172,7 +172,7 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
         goto fail;
     }
 
-    footer = (struct vhd_footer*) s->footer_buf;
+    footer = (VHDFooter *) s->footer_buf;
     if (strncmp(footer->creator, "conectix", 8)) {
         int64_t offset = bdrv_getlength(bs->file);
         if (offset < 0) {
@@ -224,7 +224,7 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
             goto fail;
         }
 
-        dyndisk_header = (struct vhd_dyndisk_header *) buf;
+        dyndisk_header = (VHDDynDiskHeader *) buf;
 
         if (strncmp(dyndisk_header->magic, "cxsparse", 8)) {
             ret = -EINVAL;
@@ -446,7 +446,7 @@ static int vpc_read(BlockDriverState *bs, int64_t sector_num,
     int ret;
     int64_t offset;
     int64_t sectors, sectors_per_block;
-    struct vhd_footer *footer = (struct vhd_footer *) s->footer_buf;
+    VHDFooter *footer = (VHDFooter *) s->footer_buf;
 
     if (cpu_to_be32(footer->type) == VHD_FIXED) {
         return bdrv_read(bs->file, sector_num, buf, nb_sectors);
@@ -495,7 +495,7 @@ static int vpc_write(BlockDriverState *bs, int64_t sector_num,
     int64_t offset;
     int64_t sectors, sectors_per_block;
     int ret;
-    struct vhd_footer *footer =  (struct vhd_footer *) s->footer_buf;
+    VHDFooter *footer =  (VHDFooter *) s->footer_buf;
 
     if (cpu_to_be32(footer->type) == VHD_FIXED) {
         return bdrv_write(bs->file, sector_num, buf, nb_sectors);
@@ -597,8 +597,8 @@ static int calculate_geometry(int64_t total_sectors, uint16_t* cyls,
 
 static int create_dynamic_disk(int fd, uint8_t *buf, int64_t total_sectors)
 {
-    struct vhd_dyndisk_header* dyndisk_header =
-        (struct vhd_dyndisk_header*) buf;
+    VHDDynDiskHeader *dyndisk_header =
+        (VHDDynDiskHeader *) buf;
     size_t block_size, num_bat_entries;
     int i;
     int ret = -EIO;
@@ -688,7 +688,7 @@ static int vpc_create(const char *filename, QEMUOptionParameter *options,
                       Error **errp)
 {
     uint8_t buf[1024];
-    struct vhd_footer *footer = (struct vhd_footer *) buf;
+    VHDFooter *footer = (VHDFooter *) buf;
     QEMUOptionParameter *disk_type_param;
     int fd, i;
     uint16_t cyls = 0;
@@ -791,7 +791,7 @@ static int vpc_create(const char *filename, QEMUOptionParameter *options,
 static int vpc_has_zero_init(BlockDriverState *bs)
 {
     BDRVVPCState *s = bs->opaque;
-    struct vhd_footer *footer =  (struct vhd_footer *) s->footer_buf;
+    VHDFooter *footer =  (VHDFooter *) s->footer_buf;
 
     if (cpu_to_be32(footer->type) == VHD_FIXED) {
         return bdrv_has_zero_init(bs->file);
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 3/4] block: qcow2 - used QEMU_PACKED for on-disk structures

[Jeff Cody]

QCowHeader and QCowExtension are structs that reside in the on-disk
image format, and are read and written directly via bdrv_pread()/write(),
and as such should be packed to avoid any unintentional struct padding.

Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/qcow2.c | 2 +-
 block/qcow2.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 318d95d..4a9888c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -52,7 +52,7 @@
 typedef struct {
     uint32_t magic;
     uint32_t len;
-} QCowExtension;
+} QEMU_PACKED QCowExtension;
 
 #define  QCOW2_EXT_MAGIC_END 0
 #define  QCOW2_EXT_MAGIC_BACKING_FORMAT 0xE2792ACA
diff --git a/block/qcow2.h b/block/qcow2.h
index c90e5d6..455e38d 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -86,7 +86,7 @@ typedef struct QCowHeader {
 
     uint32_t refcount_order;
     uint32_t header_length;
-} QCowHeader;
+} QEMU_PACKED QCowHeader;
 
 typedef struct QCowSnapshot {
     uint64_t l1_table_offset;
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 4/4] block: qed - use QEMU_PACKED for on-disk structures

[Jeff Cody]

QEDHeader is read, and written, directly from on-disk images
via bdrv_pread()/write().  To avoid any unintentional padding,
these structs should be packed.

Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/qed.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/qed.h b/block/qed.h
index 2b4dded..5d65bea 100644
--- a/block/qed.h
+++ b/block/qed.h
@@ -100,7 +100,7 @@ typedef struct {
     /* if (features & QED_F_BACKING_FILE) */
     uint32_t backing_filename_offset; /* in bytes from start of header */
     uint32_t backing_filename_size;   /* in bytes */
-} QEDHeader;
+} QEMU_PACKED QEDHeader;
 
 typedef struct {
     uint64_t offsets[0];            /* in bytes */
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 0/4] block: use QEMU_PACKED for on-disk structures

[Richard Henderson]

On 09/25/2013 09:08 AM, Jeff Cody wrote:
> Jeff Cody (4):
>   block: vdi - use QEMU_PACKED for on-disk structures
>   block: vpc - use QEMU_PACKED for on-disk structures
>   block: qcow2 - used QEMU_PACKED for on-disk structures
>   block: qed - use QEMU_PACKED for on-disk structures

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

Re: [Qemu-devel] [PATCH v2 0/4] block: use QEMU_PACKED for on-disk structures

[Kevin Wolf]

Am 25.09.2013 um 18:32 hat Richard Henderson geschrieben:
> On 09/25/2013 09:08 AM, Jeff Cody wrote:
> > Jeff Cody (4):
> >   block: vdi - use QEMU_PACKED for on-disk structures
> >   block: vpc - use QEMU_PACKED for on-disk structures
> >   block: qcow2 - used QEMU_PACKED for on-disk structures
> >   block: qed - use QEMU_PACKED for on-disk structures
> 
> Reviewed-by: Richard Henderson <rth@twiddle.net>

Thanks, applied all to the block branch.

Kevin