[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 4/5] block/vpc: make calculate_geometry spec con
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH 4/5] block/vpc: make calculate_geometry spec conform |
Date: |
Mon, 23 Feb 2015 13:59:19 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 |
On 2015-02-23 at 09:27, Peter Lieven wrote:
The VHD spec [1] allows for total_sectors of 65535 x 16 x 255 (~127GB)
represented by a CHS geometry. If total_sectors is greater
than 65535 x 16 x 255 this geometry is set as a maximum.
Qemu, Hyper-V, VirtualBox and disk2vhd use this special geometry as an
indicator to use the image current size from the footer as disk size.
This patch changes vpc_create to effectively calculate a CxHxS geometry
for the given image size if possible while rounding up if necessary.
If the image size is too big to be represented in CHS we set the maximum
and write the exact requested image size into the footer.
This partly reverts commit 258d2edb, but leaves support for >127G disks
intact.
[1]
http://download.microsoft.com/download/f/f/e/ffef50a5-07dd-4cf8-aaa3-442c0673a029/Virtual%20Hard%20Disk%20Format%20Spec_10_18_06.doc
Signed-off-by: Peter Lieven <address@hidden>
---
block/vpc.c | 45 ++++++++++++++++++++++++---------------------
1 file changed, 24 insertions(+), 21 deletions(-)
diff --git a/block/vpc.c b/block/vpc.c
index 11d3c86..9c5301b 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -46,6 +46,7 @@ enum vhd_type {
#define VHD_TIMESTAMP_BASE 946684800
#define VHD_MAX_SECTORS (65535LL * 255 * 255)
+#define VHD_MAX_GEOMETRY (65535LL * 16 * 255)
// always big-endian
typedef struct vhd_footer {
@@ -218,7 +219,7 @@ static int vpc_open(BlockDriverState *bs, QDict *options,
int flags,
/* Images that have exactly the maximum geometry are probably bigger and
* would be truncated if we adhered to the geometry for them. Rely on
* footer->size for them. */
- if (bs->total_sectors == 65535ULL * 16 * 255) {
+ if (bs->total_sectors == VHD_MAX_GEOMETRY) {
bs->total_sectors = be64_to_cpu(footer->size) / BDRV_SECTOR_SIZE;
}
@@ -642,26 +643,20 @@ static int calculate_geometry(int64_t total_sectors, uint16_t* cyls,
{
uint32_t cyls_times_heads;
- /* Allow a maximum disk size of approximately 2 TB */
- if (total_sectors > 65535LL * 255 * 255) {
- return -EFBIG;
- }
+ total_sectors = MIN(total_sectors, VHD_MAX_GEOMETRY);
- if (total_sectors > 65535 * 16 * 63) {
+ if (total_sectors > 65535LL * 16 * 63) {
While it is >= in the specification, the "else" branch does work fine
with the total_sectors = 65535 * 16 * 63 case. So I'm leaving it up to
you whether to make it really really spec-conform or at least
functionally spec-conform (although the spec says 16 heads, 16191
cylinders, and 255 sectors per cylinder for this case, whereas the
"else" branch leaves us with 16 heads, 65535 cylinders, and 63 sectors
per cylinder).
*secs_per_cyl = 255;
- if (total_sectors > 65535 * 16 * 255) {
- *heads = 255;
- } else {
- *heads = 16;
- }
+ *heads = 16;
cyls_times_heads = total_sectors / *secs_per_cyl;
} else {
*secs_per_cyl = 17;
cyls_times_heads = total_sectors / *secs_per_cyl;
*heads = (cyls_times_heads + 1023) / 1024;
- if (*heads < 4)
+ if (*heads < 4) {
*heads = 4;
+ }
if (cyls_times_heads >= (*heads * 1024) || *heads > 16) {
*secs_per_cyl = 31;
@@ -817,19 +812,27 @@ static int vpc_create(const char *filename, QemuOpts
*opts, Error **errp)
* Calculate matching total_size and geometry. Increase the number of
* sectors requested until we get enough (or fail). This ensures that
* qemu-img convert doesn't truncate images, but rather rounds up.
+ *
+ * If the image size can't be represented by a spec conform CHS geometry,
+ * we set the geometry to 65535 x 16 x 255 (CxHxS) sectors and use
+ * the image size from the VHD footer to calculate total_sectors.
*/
- total_sectors = total_size / BDRV_SECTOR_SIZE;
+ total_sectors = MIN(VHD_MAX_GEOMETRY,
+ DIV_ROUND_UP(total_size, BDRV_SECTOR_SIZE));
DIV_ROUND_UP() is unnecessary, total_size is a multiple of
BDRV_SECTOR_SIZE anyway.
I find the MIN() unnecessary, too, because calculate_geometry() will
take care of that itself.
for (i = 0; total_sectors > (int64_t)cyls * heads * secs_per_cyl; i++) {
- if (calculate_geometry(total_sectors + i, &cyls, &heads,
- &secs_per_cyl))
- {
- ret = -EFBIG;
- goto out;
- }
+ calculate_geometry(total_sectors + i, &cyls, &heads, &secs_per_cyl);
}
- total_sectors = (int64_t) cyls * heads * secs_per_cyl;
- total_size = total_sectors * BDRV_SECTOR_SIZE;
+ if ((int64_t)cyls * heads * secs_per_cyl == VHD_MAX_GEOMETRY) {
+ total_sectors = DIV_ROUND_UP(total_size, BDRV_SECTOR_SIZE);
Again, DIV_ROUND_UP() is unnecessary.
So this is equivalent to total_sectors = total_size / BDRV_SECTOR_SIZE,
which, if you omit the MIN() above, is exactly what total_sectors
already contains, so you can just omit this assignment.
+ /* Allow a maximum disk size of approximately 2 TB */
+ if (total_sectors > VHD_MAX_SECTORS) {
+ return -EFBIG;
No goto out;?
+ }
+ } else {
+ total_sectors = (int64_t)cyls * heads * secs_per_cyl;
+ total_size = total_sectors * BDRV_SECTOR_SIZE;
+ }
An alternative way of writing this would be:
if ((int64_t)cyls * heads * secs_per_cyl < VHD_MAX_GEOMETRY) {
total_sectors = (int64_t)cyls * heads * secs_per_cyl;
}
if (total_sectors > VHD_MAX_SECTORS) {
ret = -EFBIG;
goto out;
}
total_size = total_sectors * BDRV_SECTOR_SIZE;
Or, alternatively, it'd be possible to drop the total_size assignment
and just use total_sectors * BDRV_SECTOR_SIZE in the assignments of
footer->orig_size and footer->size.
Maybe you like that better. I do, but this is not my patch, so it's up
to you.
All in all, apart from the "return -EFBIG" instead of "goto out", the
patch is correct (if you ignore the minor difference for total_sectors
== 65535 * 16 * 63).
Max
/* Prepare the Hard Disk Footer */
memset(buf, 0, 1024);
- [Qemu-devel] [PATCH 0/5] block/vpc optimizations, Peter Lieven, 2015/02/23
- [Qemu-devel] [PATCH 2/5] block/vpc: simplify vpc_read, Peter Lieven, 2015/02/23
- [Qemu-devel] [PATCH 1/5] block/vpc: optimize vpc_co_get_block_status, Peter Lieven, 2015/02/23
- [Qemu-devel] [PATCH 4/5] block/vpc: make calculate_geometry spec conform, Peter Lieven, 2015/02/23
- Re: [Qemu-devel] [PATCH 4/5] block/vpc: make calculate_geometry spec conform,
Max Reitz <=
- [Qemu-devel] [PATCH 3/5] vpc: Ignore geometry for large images, Peter Lieven, 2015/02/23
- [Qemu-devel] [PATCH 5/5] block/vpc: rename footer->size -> footer->current_size, Peter Lieven, 2015/02/23