Patchwork [v2,4/9] block: vhdx - log support struct and defines

login
register
mail settings
Submitter Jeff Cody
Date Aug. 1, 2013, 3:23 a.m.
Message ID <9e47d3ea77357b438ce2164f6e226d216553c58e.1375325971.git.jcody@redhat.com>
Download mbox | patch
Permalink /patch/263892/
State New
Headers show

Comments

Jeff Cody - Aug. 1, 2013, 3:23 a.m.
This adds some magic number defines, and internal structure
definitions for VHDX log replay support.

Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/vhdx.h | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)
Stefan Hajnoczi - Aug. 1, 2013, 3 p.m.
On Wed, Jul 31, 2013 at 11:23:49PM -0400, Jeff Cody wrote:
> @@ -318,6 +323,18 @@ typedef struct VHDXMetadataEntries {
>      uint16_t present;
>  } VHDXMetadataEntries;
>  
> +typedef struct VHDXLogEntries {
> +    uint64_t offset;
> +    uint64_t length;
> +    uint32_t head;
> +    uint32_t tail;
> +} VHDXLogEntries;
> +
> +typedef struct VHDXLogEntryInfo {
> +    uint64_t sector_start;
> +    uint32_t desc_count;
> +} VHDXLogEntryInfo;

An array of VHDXLogEntryInfo structs would be affected by alignment on
x86_64.  &a[1] != (void*)a + sizeof(VHDXLogEntryInfo).

IMO all on-disk structs should be QEMU_PACKED for safety.
Jeff Cody - Aug. 1, 2013, 3:51 p.m.
On Thu, Aug 01, 2013 at 05:00:05PM +0200, Stefan Hajnoczi wrote:
> On Wed, Jul 31, 2013 at 11:23:49PM -0400, Jeff Cody wrote:
> > @@ -318,6 +323,18 @@ typedef struct VHDXMetadataEntries {
> >      uint16_t present;
> >  } VHDXMetadataEntries;
> >  
> > +typedef struct VHDXLogEntries {
> > +    uint64_t offset;
> > +    uint64_t length;
> > +    uint32_t head;
> > +    uint32_t tail;
> > +} VHDXLogEntries;
> > +
> > +typedef struct VHDXLogEntryInfo {
> > +    uint64_t sector_start;
> > +    uint32_t desc_count;
> > +} VHDXLogEntryInfo;
> 
> An array of VHDXLogEntryInfo structs would be affected by alignment on
> x86_64.  &a[1] != (void*)a + sizeof(VHDXLogEntryInfo).
> 
> IMO all on-disk structs should be QEMU_PACKED for safety.

Neither of the two structs above reflect on-disk structs (actually
VHDXLogEntryInfo is currently unused, so I should drop it from this
patch).

I should mention this in the commit message, since the patch context
isn't enough to show it, but the structs introduced in this patch are
below a comment demarcation line that calls out an end to the on-disk
structs:

/* ----- END VHDX SPECIFICATION STRUCTURES ---- */
Kevin Wolf - Aug. 7, 2013, 3:22 p.m.
Am 01.08.2013 um 05:23 hat Jeff Cody geschrieben:
> This adds some magic number defines, and internal structure
> definitions for VHDX log replay support.
> 
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
>  block/vhdx.h | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/block/vhdx.h b/block/vhdx.h
> index c8d8593..2db6615 100644
> --- a/block/vhdx.h
> +++ b/block/vhdx.h
> @@ -151,7 +151,10 @@ typedef struct QEMU_PACKED VHDXRegionTableEntry {
>  
>  
>  /* ---- LOG ENTRY STRUCTURES ---- */
> +#define VHDX_LOG_MIN_SIZE (1024*1024)

There should be spaces around the operator.

Also, VHDX_LOG_ENTRY_MIN_SIZE? I thought at first that this was the
minimum size of the whole log (or does that happen to be the same and I
just missed it in the spec?)

Kevin
Jeff Cody - Aug. 7, 2013, 5:18 p.m.
On Wed, Aug 07, 2013 at 05:22:45PM +0200, Kevin Wolf wrote:
> Am 01.08.2013 um 05:23 hat Jeff Cody geschrieben:
> > This adds some magic number defines, and internal structure
> > definitions for VHDX log replay support.
> > 
> > Signed-off-by: Jeff Cody <jcody@redhat.com>
> > ---
> >  block/vhdx.h | 21 ++++++++++++++++++++-
> >  1 file changed, 20 insertions(+), 1 deletion(-)
> > 
> > diff --git a/block/vhdx.h b/block/vhdx.h
> > index c8d8593..2db6615 100644
> > --- a/block/vhdx.h
> > +++ b/block/vhdx.h
> > @@ -151,7 +151,10 @@ typedef struct QEMU_PACKED VHDXRegionTableEntry {
> >  
> >  
> >  /* ---- LOG ENTRY STRUCTURES ---- */
> > +#define VHDX_LOG_MIN_SIZE (1024*1024)
> 
> There should be spaces around the operator.
> 

OK.  Checkpatch.pl missed this, I guess it doesn't check macros?

> Also, VHDX_LOG_ENTRY_MIN_SIZE? I thought at first that this was the
> minimum size of the whole log (or does that happen to be the same and I
> just missed it in the spec?)
>

This is indeed the minimum size for the entire log - once you get to
patch 7, you'll see it used.  But an interesting thing about this, is
the original 0.95 spec said that 1MB was also the minimum log *entry*
size as well.  But that was incorrect, and the 1.00 spec notes that a
log entry size min is 4KB (same as the log sector size).

Jeff

Patch

diff --git a/block/vhdx.h b/block/vhdx.h
index c8d8593..2db6615 100644
--- a/block/vhdx.h
+++ b/block/vhdx.h
@@ -151,7 +151,10 @@  typedef struct QEMU_PACKED VHDXRegionTableEntry {
 
 
 /* ---- LOG ENTRY STRUCTURES ---- */
+#define VHDX_LOG_MIN_SIZE (1024*1024)
+#define VHDX_LOG_SECTOR_SIZE 4096
 #define VHDX_LOG_HDR_SIZE 64
+#define VHDX_LOG_SIGNATURE 0x65676f6c
 typedef struct QEMU_PACKED VHDXLogEntryHeader {
     uint32_t    signature;              /* "loge" in ASCII */
     uint32_t    checksum;               /* CRC-32C hash of the 64KB table */
@@ -174,7 +177,8 @@  typedef struct QEMU_PACKED VHDXLogEntryHeader {
 } VHDXLogEntryHeader;
 
 #define VHDX_LOG_DESC_SIZE 32
-
+#define VHDX_LOG_DESC_SIGNATURE 0x63736564
+#define VHDX_LOG_ZERO_SIGNATURE 0x6f72657a
 typedef struct QEMU_PACKED VHDXLogDescriptor {
     uint32_t    signature;              /* "zero" or "desc" in ASCII */
     union  {
@@ -194,6 +198,7 @@  typedef struct QEMU_PACKED VHDXLogDescriptor {
                                            vhdx_log_entry_header */
 } VHDXLogDescriptor;
 
+#define VHDX_LOG_DATA_SIGNATURE 0x61746164
 typedef struct QEMU_PACKED VHDXLogDataSector {
     uint32_t    data_signature;         /* "data" in ASCII */
     uint32_t    sequence_high;          /* 4 MSB of 8 byte sequence_number */
@@ -318,6 +323,18 @@  typedef struct VHDXMetadataEntries {
     uint16_t present;
 } VHDXMetadataEntries;
 
+typedef struct VHDXLogEntries {
+    uint64_t offset;
+    uint64_t length;
+    uint32_t head;
+    uint32_t tail;
+} VHDXLogEntries;
+
+typedef struct VHDXLogEntryInfo {
+    uint64_t sector_start;
+    uint32_t desc_count;
+} VHDXLogEntryInfo;
+
 typedef struct BDRVVHDXState {
     CoMutex lock;
 
@@ -351,6 +368,8 @@  typedef struct BDRVVHDXState {
 
     MSGUID session_guid;
 
+    VHDXLogEntries log;
+
     VHDXParentLocatorHeader parent_header;
     VHDXParentLocatorEntry *parent_entries;