| Message ID | 1411120786-28764-1-git-send-email-lpetrut@cloudbasesolutions.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Sep 19, 2014 at 12:59:46PM +0300, Lucian Petrut wrote: > Note that this patch assumes that all the data blocks are written > right after the BAT. Are there any known files out there where this is not the case? Perhaps an error should be printed if a data block that requires moving does not appear in pagetable[]. That's an indication that the image is not laid out as expected. > +static int vpc_truncate(BlockDriverState *bs, int64_t offset) > +{ > + BDRVVPCState *s = bs->opaque; > + VHDFooter *footer = (VHDFooter *) s->footer_buf; > + VHDDynDiskHeader *dyndisk_header; > + void *buf = NULL; > + int64_t new_total_sectors, old_bat_size, new_bat_size, > + block_offset, new_block_offset, bat_offset; > + int32_t bat_value, data_blocks_required; > + int ret = 0; > + uint16_t cyls = 0; > + uint8_t heads = 0; > + uint8_t secs_per_cyl = 0; > + uint32_t new_num_bat_entries; > + uint64_t index, block_index, new_bat_right_limit; > + > + if (offset & 511) { > + error_report("The new size must be a multiple of 512."); > + return -EINVAL; > + } > + > + if (offset < bs->total_sectors * 512) { > + error_report("Shrinking vhd images is not supported."); > + return -ENOTSUP; > + } > + > + if (cpu_to_be32(footer->type) == VHD_DIFFERENCING) { footer_buf is big-endian so this should be be32_to_cpu() > + error_report("Resizing differencing vhd images is not supported."); > + return -ENOTSUP; > + } > + > + old_bat_size = (s->max_table_entries * 4 + 511) & ~511; > + new_total_sectors = offset / BDRV_SECTOR_SIZE; > + > + for (index = 0; new_total_sectors > (int64_t)cyls * heads * secs_per_cyl; > + index++) { > + if (calculate_geometry(new_total_sectors + index, &cyls, &heads, > + &secs_per_cyl)) { > + return -EFBIG; > + } > + } > + new_total_sectors = (int64_t) cyls * heads * secs_per_cyl; > + new_num_bat_entries = (new_total_sectors + s->block_size / 512) / > + (s->block_size / 512); This expression doesn't just round up, it always adds an extra block. Is this intentional? I expected the numerator for rounding up to be: new_total_sectors + s->block_size / 512 - 1 > + > + if (cpu_to_be32(footer->type) == VHD_DYNAMIC) { be32_to_cpu() > + new_bat_size = (new_num_bat_entries * 4 + 511) & ~511; > + /* Number of blocks required for extending the BAT */ > + data_blocks_required = (new_bat_size - old_bat_size + > + s->block_size - 1) / s->block_size; > + new_bat_right_limit = s->bat_offset + old_bat_size + > + data_blocks_required * > + (s->block_size + s->bitmap_size); > + > + for (block_index = 0; block_index < > + data_blocks_required; block_index++){ > + /* > + * The BAT has to be extended. We'll have to move the first > + * data block(s) to the end of the file, making room for the > + * BAT to expand. Also, the BAT entries have to be updated for > + * the moved blocks. > + */ > + > + block_offset = s->bat_offset + old_bat_size + > + block_index * (s->block_size + s->bitmap_size); > + if (block_offset >= s->free_data_block_offset) { > + /* > + * Do not allocate a new block for the BAT if no data blocks > + * were previously allocated to the vhd image. > + */ > + s->free_data_block_offset += (new_bat_size - old_bat_size); > + break; > + } > + > + if (block_index == 0) { Is this condition correct? I expected !buf. What happens during the second loop iteration when block_index == 1 (we freed buf at the end of the first iteration). If I'm reading the code correctly this results in a NULL pointer dereference. That would mean you never executed this code path. Please create a qemu-iotests test case to exercise the code paths in your patch. There are many examples you can use as a starting point in tests/qemu-iotests/. The basic overview is here: http://qemu-project.org/Documentation/QemuIoTests > + buf = g_malloc(s->block_size + s->bitmap_size); Please use qemu_blockalign()/qemu_vfree() for disk data buffers. If the file is opened with O_DIRECT there are memory alignment constraints. Using qemu_blockalign() avoids the need for a bounce buffer in block/raw-posix.c. (Even if there is no performance concern, it's a good habit to follow consistently so that we do align in the cases where the performance does matter.) > + } > + > + ret = bdrv_pread(bs->file, block_offset, buf, > + s->block_size + s->bitmap_size); > + if (ret < 0) { > + goto out; > + } > + > + new_block_offset = s->free_data_block_offset < new_bat_right_limit ? > + new_bat_right_limit : s->free_data_block_offset; > + bdrv_pwrite_sync(bs->file, new_block_offset, buf, > + s->block_size + s->bitmap_size); > + if (ret < 0) { > + goto out; > + } > + > + for (index = 0; index < s->max_table_entries; index++) { > + if (s->pagetable[index] == block_offset / BDRV_SECTOR_SIZE) { > + s->pagetable[index] = block_offset; The if condition checks for block_offset / BDRV_SECTOR_SIZE, but the assignment stores block_offset (without converting it to sectors). Did you mean: if (s->pagetable[index] == block_offset / BDRV_SECTOR_SIZE) { s->pagetable[index] = new_block_offset / BDRV_SECTOR_SIZE; > + bat_offset = s->bat_offset + (4 * index); > + bat_value = be32_to_cpu(new_block_offset / > + BDRV_SECTOR_SIZE); cpu_to_be32() > + ret = bdrv_pwrite_sync(bs->file, bat_offset, &bat_value, 4); > + if (ret < 0) { > + goto out; > + } > + break; > + } > + } > + > + s->free_data_block_offset = new_block_offset + s->block_size + > + s->bitmap_size; > + g_free(buf); > + buf = NULL; > + } > + > + buf = g_malloc(512); > + memset(buf, 0xFF, 512); > + > + /* Extend the BAT */ > + offset = s->bat_offset + old_bat_size; > + for (index = 0; > + index < (new_bat_size - old_bat_size) / 512; > + index++) { > + ret = bdrv_pwrite_sync(bs->file, offset, buf, 512); You could just bdrv_pwrite() and bdrv_flush() after the loop to make this faster. > + if (ret < 0) { > + goto out; > + } > + offset += 512; > + } > + > + g_free(buf); > + buf = g_malloc(1024); > + > + /* Update the Dynamic Disk Header */ > + ret = bdrv_pread(bs->file, 512, buf, > + 1024); > + if (ret < 0) { > + goto out; > + } > + > + dyndisk_header = (VHDDynDiskHeader *) buf; > + dyndisk_header->max_table_entries = be32_to_cpu(new_num_bat_entries); cpu_to_be32() > + dyndisk_header->checksum = 0; > + dyndisk_header->checksum = be32_to_cpu(vpc_checksum(buf, 1024)); cpu_to_be32() > + ret = bdrv_pwrite_sync(bs->file, 512, buf, 1024); > + if (ret < 0) { > + goto out; > + } > + > + } else { > + s->free_data_block_offset = new_total_sectors * BDRV_SECTOR_SIZE; Don't we have to take into account the header for VHD_FIXED? > + } > + > + footer->cyls = be16_to_cpu(cyls); cpu_to_be16() > + footer->heads = heads; > + footer->secs_per_cyl = secs_per_cyl; > + footer->size = be64_to_cpu(new_total_sectors * BDRV_SECTOR_SIZE); cpu_to_be64() > + footer->checksum = 0; > + footer->checksum = be32_to_cpu(vpc_checksum(s->footer_buf, HEADER_SIZE)); cpu_to_be32() > + > + /* > + * Rewrite the footer, copying to the image header in case of a > + * dynamic vhd. > + */ > + rewrite_footer(bs, (cpu_to_be32(footer->type) != VHD_FIXED)); be32_to_cpu()
Thanks for reviewing this patch! I’ll send a new one, fixing the issues you’ve pointed out, along with some tests. Please see my inline comments bellow. Regards, Lucian Petrut >From: Stefan Hajnoczi >Sent: Monday, September 22, 2014 3:17 PM >To: Petrut Lucian >Cc: qemu-devel@nongnu.org<mailto:qemu-devel@nongnu.org>, Lucian Petrut, jcody@redhat.com<mailto:jcody@redhat.com>, kwolf@redhat.com<mailto:kwolf@redhat.com>, sw@weilnetz.de<mailto:sw@weilnetz.de>, Alessandro Pilotti > > >On Fri, Sep 19, 2014 at 12:59:46PM +0300, Lucian Petrut wrote: >> Note that this patch assumes that all the data blocks are written >> right after the BAT. >Are there any known files out there where this is not the case? >Perhaps an error should be printed if a data block that requires moving >does not appear in pagetable[]. That's an indication that the image is >not laid out as expected. You are right, a check should be added here. I’m not aware of parsers that may have this behavior but better be on the safe side. >> +static int vpc_truncate(BlockDriverState *bs, int64_t offset) >> +{ >> + BDRVVPCState *s = bs->opaque; >> + VHDFooter *footer = (VHDFooter *) s->footer_buf; >> + VHDDynDiskHeader *dyndisk_header; >> + void *buf = NULL; >> + int64_t new_total_sectors, old_bat_size, new_bat_size, >> + block_offset, new_block_offset, bat_offset; >> + int32_t bat_value, data_blocks_required; >> + int ret = 0; >> + uint16_t cyls = 0; >> + uint8_t heads = 0; >> + uint8_t secs_per_cyl = 0; >> + uint32_t new_num_bat_entries; >> + uint64_t index, block_index, new_bat_right_limit; >> + >> + if (offset & 511) { >> + error_report("The new size must be a multiple of 512."); >> + return -EINVAL; >> + } >> + >> + if (offset < bs->total_sectors * 512) { >> + error_report("Shrinking vhd images is not supported."); >> + return -ENOTSUP; >> + } >> + >> + if (cpu_to_be32(footer->type) == VHD_DIFFERENCING) { >footer_buf is big-endian so this should be be32_to_cpu() My bad, I’ll fix the BE related issues. >> + error_report("Resizing differencing vhd images is not supported."); >> + return -ENOTSUP; >> + } >> + >> + old_bat_size = (s->max_table_entries * 4 + 511) & ~511; >> + new_total_sectors = offset / BDRV_SECTOR_SIZE; >> + >> + for (index = 0; new_total_sectors > (int64_t)cyls * heads * secs_per_cyl; >> + index++) { >> + if (calculate_geometry(new_total_sectors + index, &cyls, &heads, >> + &secs_per_cyl)) { >> + return -EFBIG; >> + } >> + } >> + new_total_sectors = (int64_t) cyls * heads * secs_per_cyl; >> + new_num_bat_entries = (new_total_sectors + s->block_size / 512) / >> + (s->block_size / 512); >This expression doesn't just round up, it always adds an extra block. >Is this intentional? >I expected the numerator for rounding up to be: > new_total_sectors + s->block_size / 512 - 1 This was intended, it may just add an extra entry. Also, the number of entries is calculated in the same way in the existing code which creates dynamic VHDs, so I wanted to keep consistency with that as well. >> + >> + if (cpu_to_be32(footer->type) == VHD_DYNAMIC) { >be32_to_cpu() >> + new_bat_size = (new_num_bat_entries * 4 + 511) & ~511; >> + /* Number of blocks required for extending the BAT */ >> + data_blocks_required = (new_bat_size - old_bat_size + >> + s->block_size - 1) / s->block_size; >> + new_bat_right_limit = s->bat_offset + old_bat_size + >> + data_blocks_required * >> + (s->block_size + s->bitmap_size); >> + >> + for (block_index = 0; block_index < >> + data_blocks_required; block_index++){ >> + /* >> + * The BAT has to be extended. We'll have to move the first >> + * data block(s) to the end of the file, making room for the >> + * BAT to expand. Also, the BAT entries have to be updated for >> + * the moved blocks. >> + */ >> + >> + block_offset = s->bat_offset + old_bat_size + >> + block_index * (s->block_size + s->bitmap_size); >> + if (block_offset >= s->free_data_block_offset) { >> + /* >> + * Do not allocate a new block for the BAT if no data blocks >> + * were previously allocated to the vhd image. >> + */ >> + s->free_data_block_offset += (new_bat_size - old_bat_size); >> + break; >> + } >> + >> + if (block_index == 0) { >Is this condition correct? I expected !buf. What happens during >the second loop iteration when block_index == 1 (we freed buf at the end >of the first iteration). Whops, my bad. You are right, I should just use !buf. At first, I did not free this at the end of iteration so I forgot to change this as well. >If I'm reading the code correctly this results in a NULL pointer >dereference. That would mean you never executed this code path. >Please create a qemu-iotests test case to exercise the code paths in >your patch. There are many examples you can use as a starting point in >tests/qemu-iotests/. The basic overview is here: >http://qemu-project.org/Documentation/QemuIoTests Thanks for the link, I’ll add a test for this. >> + buf = g_malloc(s->block_size + s->bitmap_size); >Please use qemu_blockalign()/qemu_vfree() for disk data buffers. If the >file is opened with O_DIRECT there are memory alignment constraints. >Using qemu_blockalign() avoids the need for a bounce buffer in >block/raw-posix.c. >(Even if there is no performance concern, it's a good habit to follow >consistently so that we do align in the cases where the performance does >matter.) You are right, this would be much better. >> + } >> + >> + ret = bdrv_pread(bs->file, block_offset, buf, >> + s->block_size + s->bitmap_size); >> + if (ret < 0) { >> + goto out; >> + } >> + >> + new_block_offset = s->free_data_block_offset < new_bat_right_limit ? >> + new_bat_right_limit : s->free_data_block_offset; >> + bdrv_pwrite_sync(bs->file, new_block_offset, buf, >> + s->block_size + s->bitmap_size); >> + if (ret < 0) { >> + goto out; >> + } >> + >> + for (index = 0; index < s->max_table_entries; index++) { >> + if (s->pagetable[index] == block_offset / BDRV_SECTOR_SIZE) { >> + s->pagetable[index] = block_offset; >The if condition checks for block_offset / BDRV_SECTOR_SIZE, but the >assignment stores block_offset (without converting it to sectors). >Did you mean: > if (s->pagetable[index] == block_offset / BDRV_SECTOR_SIZE) { > s->pagetable[index] = new_block_offset / BDRV_SECTOR_SIZE; You are right here as well. I’ve actually done it this way but forgot to amend the commit, thanks for pointing this out! >> + bat_offset = s->bat_offset + (4 * index); >> + bat_value = be32_to_cpu(new_block_offset / >> + BDRV_SECTOR_SIZE); >cpu_to_be32() >> + ret = bdrv_pwrite_sync(bs->file, bat_offset, &bat_value, 4); >> + if (ret < 0) { >> + goto out; >> + } >> + break; >> + } >> + } >> + >> + s->free_data_block_offset = new_block_offset + s->block_size + >> + s->bitmap_size; >> + g_free(buf); >> + buf = NULL; >> + } >> + >> + buf = g_malloc(512); >> + memset(buf, 0xFF, 512); >> + >> + /* Extend the BAT */ >> + offset = s->bat_offset + old_bat_size; >> + for (index = 0; >> + index < (new_bat_size - old_bat_size) / 512; >> + index++) { >> + ret = bdrv_pwrite_sync(bs->file, offset, buf, 512); >You could just bdrv_pwrite() and bdrv_flush() after the loop to make >this faster. Done. >> + if (ret < 0) { >> + goto out; >> + } >> + offset += 512; >> + } >> + >> + g_free(buf); >> + buf = g_malloc(1024); >> + >> + /* Update the Dynamic Disk Header */ >> + ret = bdrv_pread(bs->file, 512, buf, >> + 1024); >> + if (ret < 0) { >> + goto out; >> + } >> + >> + dyndisk_header = (VHDDynDiskHeader *) buf; >> + dyndisk_header->max_table_entries = be32_to_cpu(new_num_bat_entries); >cpu_to_be32() >> + dyndisk_header->checksum = 0; >> + dyndisk_header->checksum = be32_to_cpu(vpc_checksum(buf, 1024)); >cpu_to_be32() >> + ret = bdrv_pwrite_sync(bs->file, 512, buf, 1024); >> + if (ret < 0) { >> + goto out; >> + } >> + >> + } else { >> + s->free_data_block_offset = new_total_sectors * BDRV_SECTOR_SIZE; >Don't we have to take into account the header for VHD_FIXED? Fixed VHDs, unlike the other VHD types, don’t have a copy of the footer in the file header, nor do they have the “dynamic vhd header”. >> + } >> + >> + footer->cyls = be16_to_cpu(cyls); >cpu_to_be16() >> + footer->heads = heads; >> + footer->secs_per_cyl = secs_per_cyl; >> + footer->size = be64_to_cpu(new_total_sectors * BDRV_SECTOR_SIZE); >cpu_to_be64() >> + footer->checksum = 0; >> + footer->checksum = be32_to_cpu(vpc_checksum(s->footer_buf, HEADER_SIZE)); >cpu_to_be32() >> + >> + /* >> + * Rewrite the footer, copying to the image header in case of a >> + * dynamic vhd. >> + */ >> + rewrite_footer(bs, (cpu_to_be32(footer->type) != VHD_FIXED)); >be32_to_cpu()
On Mon, Sep 22, 2014 at 01:24:16PM +0000, Lucian Petrut wrote: > >> + if (cpu_to_be32(footer->type) == VHD_DIFFERENCING) { > >footer_buf is big-endian so this should be be32_to_cpu() > > My bad, I’ll fix the BE related issues. The existing block/vpc.c code confuses beX_to_cpu() and cpu_to_beX(). I have sent a patch to fix that ("[PATCH] vpc: fix beX_to_cpu() and cpu_to_beX() confusion"). > >> + error_report("Resizing differencing vhd images is not supported."); > >> + return -ENOTSUP; > >> + } > >> + > >> + old_bat_size = (s->max_table_entries * 4 + 511) & ~511; > >> + new_total_sectors = offset / BDRV_SECTOR_SIZE; > >> + > >> + for (index = 0; new_total_sectors > (int64_t)cyls * heads * secs_per_cyl; > >> + index++) { > >> + if (calculate_geometry(new_total_sectors + index, &cyls, &heads, > >> + &secs_per_cyl)) { > >> + return -EFBIG; > >> + } > >> + } > >> + new_total_sectors = (int64_t) cyls * heads * secs_per_cyl; > >> + new_num_bat_entries = (new_total_sectors + s->block_size / 512) / > >> + (s->block_size / 512); > >This expression doesn't just round up, it always adds an extra block. > >Is this intentional? > >I expected the numerator for rounding up to be: > > new_total_sectors + s->block_size / 512 - 1 > > This was intended, it may just add an extra entry. Also, the number > of entries is calculated in the same way in the existing code which > creates dynamic VHDs, so I wanted to keep consistency with that > as well. Okay, if existing code already does it like this it makes sense to be consistent.
diff --git a/block/vpc.c b/block/vpc.c index 055efc4..6a13574 100644 --- a/block/vpc.c +++ b/block/vpc.c @@ -417,7 +417,7 @@ static inline int64_t get_sector_offset(BlockDriverState *bs, * * Returns 0 on success and < 0 on error */ -static int rewrite_footer(BlockDriverState* bs) +static int rewrite_footer(BlockDriverState *bs, bool update_header) { int ret; BDRVVPCState *s = bs->opaque; @@ -427,6 +427,13 @@ static int rewrite_footer(BlockDriverState* bs) if (ret < 0) return ret; + if (update_header) { + ret = bdrv_pwrite_sync(bs->file, 0, s->footer_buf, HEADER_SIZE); + if (ret < 0) { + return ret; + } + } + return 0; } @@ -466,7 +473,7 @@ static int64_t alloc_block(BlockDriverState* bs, int64_t sector_num) // Write new footer (the old one will be overwritten) s->free_data_block_offset += s->block_size + s->bitmap_size; - ret = rewrite_footer(bs); + ret = rewrite_footer(bs, false); if (ret < 0) goto fail; @@ -852,6 +859,180 @@ out: return ret; } + +static int vpc_truncate(BlockDriverState *bs, int64_t offset) +{ + BDRVVPCState *s = bs->opaque; + VHDFooter *footer = (VHDFooter *) s->footer_buf; + VHDDynDiskHeader *dyndisk_header; + void *buf = NULL; + int64_t new_total_sectors, old_bat_size, new_bat_size, + block_offset, new_block_offset, bat_offset; + int32_t bat_value, data_blocks_required; + int ret = 0; + uint16_t cyls = 0; + uint8_t heads = 0; + uint8_t secs_per_cyl = 0; + uint32_t new_num_bat_entries; + uint64_t index, block_index, new_bat_right_limit; + + if (offset & 511) { + error_report("The new size must be a multiple of 512."); + return -EINVAL; + } + + if (offset < bs->total_sectors * 512) { + error_report("Shrinking vhd images is not supported."); + return -ENOTSUP; + } + + if (cpu_to_be32(footer->type) == VHD_DIFFERENCING) { + error_report("Resizing differencing vhd images is not supported."); + return -ENOTSUP; + } + + old_bat_size = (s->max_table_entries * 4 + 511) & ~511; + new_total_sectors = offset / BDRV_SECTOR_SIZE; + + for (index = 0; new_total_sectors > (int64_t)cyls * heads * secs_per_cyl; + index++) { + if (calculate_geometry(new_total_sectors + index, &cyls, &heads, + &secs_per_cyl)) { + return -EFBIG; + } + } + new_total_sectors = (int64_t) cyls * heads * secs_per_cyl; + new_num_bat_entries = (new_total_sectors + s->block_size / 512) / + (s->block_size / 512); + + if (cpu_to_be32(footer->type) == VHD_DYNAMIC) { + new_bat_size = (new_num_bat_entries * 4 + 511) & ~511; + /* Number of blocks required for extending the BAT */ + data_blocks_required = (new_bat_size - old_bat_size + + s->block_size - 1) / s->block_size; + new_bat_right_limit = s->bat_offset + old_bat_size + + data_blocks_required * + (s->block_size + s->bitmap_size); + + for (block_index = 0; block_index < + data_blocks_required; block_index++){ + /* + * The BAT has to be extended. We'll have to move the first + * data block(s) to the end of the file, making room for the + * BAT to expand. Also, the BAT entries have to be updated for + * the moved blocks. + */ + + block_offset = s->bat_offset + old_bat_size + + block_index * (s->block_size + s->bitmap_size); + if (block_offset >= s->free_data_block_offset) { + /* + * Do not allocate a new block for the BAT if no data blocks + * were previously allocated to the vhd image. + */ + s->free_data_block_offset += (new_bat_size - old_bat_size); + break; + } + + if (block_index == 0) { + buf = g_malloc(s->block_size + s->bitmap_size); + } + + ret = bdrv_pread(bs->file, block_offset, buf, + s->block_size + s->bitmap_size); + if (ret < 0) { + goto out; + } + + new_block_offset = s->free_data_block_offset < new_bat_right_limit ? + new_bat_right_limit : s->free_data_block_offset; + bdrv_pwrite_sync(bs->file, new_block_offset, buf, + s->block_size + s->bitmap_size); + if (ret < 0) { + goto out; + } + + for (index = 0; index < s->max_table_entries; index++) { + if (s->pagetable[index] == block_offset / BDRV_SECTOR_SIZE) { + s->pagetable[index] = block_offset; + bat_offset = s->bat_offset + (4 * index); + bat_value = be32_to_cpu(new_block_offset / + BDRV_SECTOR_SIZE); + ret = bdrv_pwrite_sync(bs->file, bat_offset, &bat_value, 4); + if (ret < 0) { + goto out; + } + break; + } + } + + s->free_data_block_offset = new_block_offset + s->block_size + + s->bitmap_size; + g_free(buf); + buf = NULL; + } + + buf = g_malloc(512); + memset(buf, 0xFF, 512); + + /* Extend the BAT */ + offset = s->bat_offset + old_bat_size; + for (index = 0; + index < (new_bat_size - old_bat_size) / 512; + index++) { + ret = bdrv_pwrite_sync(bs->file, offset, buf, 512); + if (ret < 0) { + goto out; + } + offset += 512; + } + + g_free(buf); + buf = g_malloc(1024); + + /* Update the Dynamic Disk Header */ + ret = bdrv_pread(bs->file, 512, buf, + 1024); + if (ret < 0) { + goto out; + } + + dyndisk_header = (VHDDynDiskHeader *) buf; + dyndisk_header->max_table_entries = be32_to_cpu(new_num_bat_entries); + dyndisk_header->checksum = 0; + dyndisk_header->checksum = be32_to_cpu(vpc_checksum(buf, 1024)); + ret = bdrv_pwrite_sync(bs->file, 512, buf, 1024); + if (ret < 0) { + goto out; + } + + } else { + s->free_data_block_offset = new_total_sectors * BDRV_SECTOR_SIZE; + } + + footer->cyls = be16_to_cpu(cyls); + footer->heads = heads; + footer->secs_per_cyl = secs_per_cyl; + footer->size = be64_to_cpu(new_total_sectors * BDRV_SECTOR_SIZE); + footer->checksum = 0; + footer->checksum = be32_to_cpu(vpc_checksum(s->footer_buf, HEADER_SIZE)); + + /* + * Rewrite the footer, copying to the image header in case of a + * dynamic vhd. + */ + rewrite_footer(bs, (cpu_to_be32(footer->type) != VHD_FIXED)); + if (ret < 0) { + goto out; + } + +out: + if (buf) { + g_free(buf); + } + return ret; +} + static int vpc_has_zero_init(BlockDriverState *bs) { BDRVVPCState *s = bs->opaque; @@ -916,6 +1097,7 @@ static BlockDriver bdrv_vpc = { .bdrv_get_info = vpc_get_info, + .bdrv_truncate = vpc_truncate, .create_opts = &vpc_create_opts, .bdrv_has_zero_init = vpc_has_zero_init, };
This patch introduces resize support for dynamic and fixed VHD images. Note that differencing VHD images do not support this operation. In order to resize dynamic VHDs, the BAT region may need to be extended. This may require moving the first data blocks, making room for it to expand. This required updating the according BAT entries for the moved blocks as well, as well as initializing the new BAT entries. In case of fixed VHDs, the only thing that needs to be done is moving and updating the footer. Note that this patch assumes that all the data blocks are written right after the BAT. Signed-off-by: Lucian Petrut <lpetrut@cloudbasesolutions.com> --- block/vpc.c | 186 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 184 insertions(+), 2 deletions(-) Changes from previous version: fixed a few coding style nits underlined by the checkpatch.pl script. I forgot to run this script before submiting the first version, sorry about this.