[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5] Support vhd type VHD_DIFFERENCING
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH v5] Support vhd type VHD_DIFFERENCING |
Date: |
Tue, 28 Oct 2014 15:04:02 +0000 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Wed, Oct 08, 2014 at 08:42:32PM +0800, Xiaodong Gong wrote:
> Now qemu only supports vhd type VHD_FIXED and VHD_DYNAMIC, so qemu
> can't read snapshot volume of vhd, and can't support other storage
> features of vhd file.
>
> This patch add read parent information in function "vpc_open", read
> bitmap in "vpc_read", and change bitmap in "vpc_write".
>
> Signed-off-by: Xiaodong Gong <address@hidden>
> Reviewed-by: Ding xiao <address@hidden>
> ---
> block/vpc.c | 428
> ++++++++++++++++++++++++++++++++++++++++++++++++++----------
> 1 file changed, 357 insertions(+), 71 deletions(-)
>
> diff --git a/block/vpc.c b/block/vpc.c
> index 4947369..1210542 100644
> --- a/block/vpc.c
> +++ b/block/vpc.c
> @@ -29,17 +29,27 @@
> #if defined(CONFIG_UUID)
> #include <uuid/uuid.h>
> #endif
> +#include <iconv.h>
This isn't used by the patch?
If you need to convert between character encodings use glib functions
instead of iconv:
https://developer.gnome.org/glib/2.26/glib-Character-Set-Conversion.html
>
> /**************************************************************/
>
> #define HEADER_SIZE 512
> +#define DYNAMIC_HEADER_SIZE 1024
> +#define PARENT_LOCATOR_NUM 8
> +#define MACX_PREFIX_LEN 7 /* file:// */
> +#define TBBATMAP_HEAD_SIZE 28
> +
> +#define PLATFORM_MACX 0x5863614d /* big endian */
> +#define PLATFORM_W2RU 0x75723257
> +
> +#define VHD_VERSION(major, minor) (((major) << 16) | ((minor) & 0x0000FFFF))
>
> //#define CACHE
>
> enum vhd_type {
> VHD_FIXED = 2,
> VHD_DYNAMIC = 3,
> - VHD_DIFFERENCING = 4,
> + VHD_DIFF = 4,
> };
>
> // Seconds since Jan 1, 2000 0:00:00 (UTC)
> @@ -138,6 +148,15 @@ typedef struct BDRVVPCState {
> Error *migration_blocker;
> } BDRVVPCState;
>
> +typedef struct vhd_tdbatmap_header {
> + char magic[8]; /* always "tdbatmap" */
> +
> + uint64_t batmap_offset;
> + uint32_t batmap_size;
> + uint32_t batmap_version;
> + uint32_t checksum;
> +} QEMU_PACKED VHDTdBatmapHeader;
> +
> static uint32_t vpc_checksum(uint8_t* buf, size_t size)
> {
> uint32_t res = 0;
> @@ -153,10 +172,107 @@ static uint32_t vpc_checksum(uint8_t* buf, size_t size)
> static int vpc_probe(const uint8_t *buf, int buf_size, const char *filename)
> {
> if (buf_size >= 8 && !strncmp((char *)buf, "conectix", 8))
> - return 100;
> + return 100;
> return 0;
> }
>
> +static int vpc_read_backing_loc(VHDDynDiskHeader *dyndisk_header,
> + BlockDriverState *bs,
> + Error **errp)
> +{
> + BDRVVPCState *s = bs->opaque;
> + int64_t data_offset = 0;
> + int data_length = 0;
> + uint32_t platform;
> + bool done = false;
> + int parent_locator_offset = 0;
> + int i;
> + int ret = 0;
> +
> + for (i = 0; i < PARENT_LOCATOR_NUM; i++) {
> + data_offset =
> + be64_to_cpu(dyndisk_header->parent_locator[i].data_offset);
> + data_length =
> + be32_to_cpu(dyndisk_header->parent_locator[i].data_length);
> + platform = dyndisk_header->parent_locator[i].platform;
be32_to_cpu() missing?
> +
> + /* Extend the location offset */
> + if (parent_locator_offset < data_offset) {
> + parent_locator_offset = data_offset;
> + }
> +
> + if (done) {
> + continue;
> + }
> +
> + /* Skip "file://" in MacX platform */
> + if (platform == PLATFORM_MACX) {
> + data_offset += MACX_PREFIX_LEN;
> + data_length -= MACX_PREFIX_LEN;
> + }
Please check data_length >= MACX_PREFIX_LEN. It's error-prone to allow
data_length to underflow.
> +
> + /* Read location of backing file */
> + if (platform == PLATFORM_MACX || platform == PLATFORM_W2RU) {
> + if (data_offset > s->max_table_entries * s->block_size) {
> + return -1;
> + }
> + if (data_length > BDRV_SECTOR_SIZE) {
> + return -1;
> + }
> + ret = bdrv_pread(bs->file, data_offset, bs->backing_file,
> + data_length);
Please check data_length against bs->backing_file[] size before reading
into it.
> + if (ret < 0) {
> + return ret;
> + }
> + bs->backing_file[data_length] = '\0';
> + }
> +
> + /* Convert location to ACSII string */
> + if (platform == PLATFORM_MACX) {
> + done = true;
> +
> + } else if (platform == PLATFORM_W2RU) {
> + /* Must be UTF16-LE to ASCII */
I guess this is where you wanted to use iconv?
> + char *out, *optr;
> + int j;
> +
> + optr = out = (char *) malloc(data_length + 1);
Please use g_malloc()/g_free() instead of malloc()/free().
> + if (out == NULL) {
> + ret = -1;
> + return ret;
> + }
> + memset(out, 0, data_length + 1);
> +
> + for (j = 0; j < data_length + 1; j++) {
> + out[j] = bs->backing_file[2*j];
> + }
> + out[data_length + 1] = '\0';
> +
> + while (*optr != '\0') {
> + if (*optr == '\\') {
> + *optr = '/';
> + }
> + optr++;
> + }
> +
> + strncpy(bs->backing_file, out, data_length + 1);
> +
> + out = NULL;
> + free(out);
Did you mean:
free(out);
out = NULL;
?
pgpxqA3tO9OCM.pgp
Description: PGP signature