| Message ID | 9e47d3ea77357b438ce2164f6e226d216553c58e.1375325971.git.jcody@redhat.com |
|---|---|
| State | New |
| Headers | show |
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.
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 ---- */
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
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
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;
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(-)