Opened 11 years ago
Closed 8 years ago
#12465 closed defect (fixed)
VHD: first sectors in newly created Data Block is written incorrectly
Reported by: | Petr Kurtin | Owned by: | |
---|---|---|---|
Component: | virtual disk | Version: | VirtualBox 4.3.4 |
Keywords: | Cc: | ||
Guest type: | other | Host type: | other |
Description
Hello,
in VHD.cpp (vhdWrite function), when you write sector(s) into a newly created Data Block (whose default size is 2Mb), these sectors are not written at the right position and such VHD files are corrupted.
in vhdWrite function, you use:
rc = vdIfIoIntFileWriteUser(pImage->pIfIo, pImage->pStorage, pImage->uCurrentEndOfFile + pImage->cDataBlockBitmapSectors * VHD_SECTOR_SIZE, pIoCtx, cbWrite, vhdAsyncExpansionDataComplete, pExpand);
offset parameter is wrong here, because sectors are always written at the start of the Data Block.
The right offset should be:
uint32_t cSectorInDataBlock = cSector % pImage->cSectorsPerDataBlock; ... pImage->uCurrentEndOfFile + (pImage->cDataBlockBitmapSectors + cSectorInDataBlock) * VHD_SECTOR_SIZE
Thanks,
Petr Kurtin
Change History (5)
comment:2 by , 11 years ago
vhdWrite was completely rewritten for async/sync IO unification (rev. 44252, 8/1/2013) and it contains this bug. When you have an empty VHD file and write a sector, it'll be stored into newly allocated 2Mb data block.
Old version of vhdWrite calculates where this sector should be stored in this 2Mb data block (i.e. cBATEntryIndex position):
/* * Calculate the real offset in the file. */ uVhdOffset = ((uint64_t)pImage->pBlockAllocationTable[cBlockAllocationTableEntry] + pImage->cDataBlockBitmapSectors + cBATEntryIndex) * VHD_SECTOR_SIZE; /* Write data. */ vdIfIoIntFileWriteSync(pImage->pIfIo, pImage->pStorage, uVhdOffset, pvBuf, cbBuf);
New version of vhdWrite writes the sector in the newly created 2Mb data block at the beginning (cBATEntryIndex is ignored):
/* * Write the new block at the current end of the file. */ rc = vdIfIoIntFileWriteUser(pImage->pIfIo, pImage->pStorage, pImage->uCurrentEndOfFile + pImage->cDataBlockBitmapSectors * VHD_SECTOR_SIZE, pIoCtx, cbWrite, vhdAsyncExpansionDataComplete, pExpand);
Repro steps are simple:
- create an empty VHD file
- write 512 bytes to sector no. 7
- read sector no. 7 (=> zeros)
- read sector no. 0 (=> your written data)
Repro pseudocode:
uint32_t cSectorSize = 512; uint32_t cSector = 7; VDWrite((PVBOXHDD)pImage->pDisk, cSector * cSectorSize, data_in , cSectorSize); VDRead ((PVBOXHDD)pImage->pDisk, cSector * cSectorSize, data_out, cSectorSize); VDRead ((PVBOXHDD)pImage->pDisk, 0 * cSectorSize, data_out, cSectorSize);
comment:3 by , 11 years ago
Hi,
thanks for the report. This is indeed a bug in the code but it is not triggered unless you pass VD_OPEN_FLAGS_HONOR_SAME when opening the image. If VD_OPEN_FLAGS_HONOR_SAME is not given the generic storage layer will only do full block writes to previously unallocated blocks, so the bug is not triggered in most cases. Do you have a complete code example?
comment:4 by , 11 years ago
You're right, I don't use full blocking writes (my disk container is based on VHD).
Huh. At which stage is the code executed ? Does it mean that all VHD files will be corrupted by the bug ? I have just checked them with chkdsk /f /r and found no errors.
The VHD files were originally created by Virtual PC 2007.