[block/vpc] dynamic disk header: off-by-one error for "num_bat_entries"

Bug #1870098 reported by Tobias Witek
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
QEMU
Fix Released
Undecided
Unassigned

Bug Description

In current qemu versions (observed in 5.0.0-rc1 as well as 2833ad487cfff7dc33703e4731b75facde1c561e), disk headers for dynamic VPCs are written with an incorrect "block allocation table entries" value.

https://www.microsoft.com/en-us/download/details.aspx?id=23850 (the corresponding spec) states that:

"Max Table Entries
This field holds the maximum entries present in the BAT. This should be equal to the number of blocks in the disk (that is, the disk size divided by the block size)."

Inside the qemu code, the value is "disk size divided by the block size *plus one*".

Calculating "num_bat_entries" as "total_sectors/(block_size / 512)" *should* fix the issue.

Revision history for this message
Kevin Wolf (kwolf-redhat) wrote :

Is there any actual bug resulting from this that you're observing? As I read the spec, having a longer BAT is merely unconventional, not strictly wrong. So if another application fails to deal with such images, it's probably a bug in that application.

Of course, I can't see a reason for making the BAT longer than necessary either. We do, however, need to round up if the disk size is not a multiple of the image block size. So I think what it really should be is:

num_bat_entries = DIV_ROUND_UP(total_sectors, block_size / 512)

If you agree, please let me know if I should submit a patch or if you would like to do that yourself. (See https://wiki.qemu.org/Contribute/SubmitAPatch)

Revision history for this message
Tobias Witek (twitek) wrote :

Ah, sorry, I failed to mention this: Due to this bug, qemu currently cannot create VHDs that are suitable for upload to Azure (because Azure expects disks that are aligned exactly to 1MB).

If it would not be too much trouble for you to submit the patch, I would appreciate that a lot. I've never submitted a patch to qemu and the contribution doc reads somewhat complex, so I'm a bit concerned about dragging a very small patch out longer than strictly necessary.

Thanks a lot!

summary: - [block/vpc] cynamic disk header: off-by-one error for "num_bat_entries"
+ [block/vpc] dynamic disk header: off-by-one error for "num_bat_entries"
Revision history for this message
Kevin Wolf (kwolf-redhat) wrote :

As I don't have your email address, I could not CC you on the patch email. Can you please verify if the following patch on the mailing list fixes your problem?

https://lists.gnu.org/archive/html/qemu-block/2020-04/msg00086.html

Revision history for this message
Tobias Witek (twitek) wrote :

Thanks a lot for looking into it!

Yes, we were able to verify that this patch does fix the problem.

Many thanks!

Revision history for this message
Laurent Vivier (laurent-vivier) wrote :
Changed in qemu:
status: New → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.