From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38942) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xj8Jh-0003BU-2Z for qemu-devel@nongnu.org; Tue, 28 Oct 2014 11:04:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xj8Jb-0007VV-Dx for qemu-devel@nongnu.org; Tue, 28 Oct 2014 11:04:12 -0400 Received: from mail-la0-x232.google.com ([2a00:1450:4010:c03::232]:55471) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xj8Jb-0007VJ-25 for qemu-devel@nongnu.org; Tue, 28 Oct 2014 11:04:07 -0400 Received: by mail-la0-f50.google.com with SMTP id s18so789744lam.37 for ; Tue, 28 Oct 2014 08:04:05 -0700 (PDT) Date: Tue, 28 Oct 2014 15:04:02 +0000 From: Stefan Hajnoczi Message-ID: <20141028150402.GG22805@stefanha-thinkpad.redhat.com> References: <1412772152-2427-1-git-send-email-gordongong0350@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="7lMq7vMTJT4tNk0a" Content-Disposition: inline In-Reply-To: <1412772152-2427-1-git-send-email-gordongong0350@gmail.com> Subject: Re: [Qemu-devel] [PATCH v5] Support vhd type VHD_DIFFERENCING List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Xiaodong Gong Cc: kwolf@redhat.com, ssdxiao@163.com, qemu-devel@nongnu.org, stefanha@redhat.com --7lMq7vMTJT4tNk0a Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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. >=20 > This patch add read parent information in function "vpc_open", read > bitmap in "vpc_read", and change bitmap in "vpc_write". >=20 > Signed-off-by: Xiaodong Gong > Reviewed-by: Ding xiao > --- > block/vpc.c | 428 ++++++++++++++++++++++++++++++++++++++++++++++++++----= ------ > 1 file changed, 357 insertions(+), 71 deletions(-) >=20 > 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 > #endif > +#include 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 > =20 > /**************************************************************/ > =20 > #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) & 0x0000F= FFF)) > =20 > //#define CACHE > =20 > enum vhd_type { > VHD_FIXED =3D 2, > VHD_DYNAMIC =3D 3, > - VHD_DIFFERENCING =3D 4, > + VHD_DIFF =3D 4, > }; > =20 > // Seconds since Jan 1, 2000 0:00:00 (UTC) > @@ -138,6 +148,15 @@ typedef struct BDRVVPCState { > Error *migration_blocker; > } BDRVVPCState; > =20 > +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 =3D 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 *filen= ame) > { > if (buf_size >=3D 8 && !strncmp((char *)buf, "conectix", 8)) > - return 100; > + return 100; > return 0; > } > =20 > +static int vpc_read_backing_loc(VHDDynDiskHeader *dyndisk_header, > + BlockDriverState *bs, > + Error **errp) > +{ > + BDRVVPCState *s =3D bs->opaque; > + int64_t data_offset =3D 0; > + int data_length =3D 0; > + uint32_t platform; > + bool done =3D false; > + int parent_locator_offset =3D 0; > + int i; > + int ret =3D 0; > + > + for (i =3D 0; i < PARENT_LOCATOR_NUM; i++) { > + data_offset =3D > + be64_to_cpu(dyndisk_header->parent_locator[i].data_offset); > + data_length =3D > + be32_to_cpu(dyndisk_header->parent_locator[i].data_length); > + platform =3D dyndisk_header->parent_locator[i].platform; be32_to_cpu() missing? > + > + /* Extend the location offset */ > + if (parent_locator_offset < data_offset) { > + parent_locator_offset =3D data_offset; > + } > + > + if (done) { > + continue; > + } > + > + /* Skip "file://" in MacX platform */ > + if (platform =3D=3D PLATFORM_MACX) { > + data_offset +=3D MACX_PREFIX_LEN; > + data_length -=3D MACX_PREFIX_LEN; > + } Please check data_length >=3D MACX_PREFIX_LEN. It's error-prone to allow data_length to underflow. > + > + /* Read location of backing file */ > + if (platform =3D=3D PLATFORM_MACX || platform =3D=3D PLATFORM_W2= RU) { > + if (data_offset > s->max_table_entries * s->block_size) { > + return -1; > + } > + if (data_length > BDRV_SECTOR_SIZE) { > + return -1; > + } > + ret =3D 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] =3D '\0'; > + } > + > + /* Convert location to ACSII string */ > + if (platform =3D=3D PLATFORM_MACX) { > + done =3D true; > + > + } else if (platform =3D=3D PLATFORM_W2RU) { > + /* Must be UTF16-LE to ASCII */ I guess this is where you wanted to use iconv? > + char *out, *optr; > + int j; > + > + optr =3D out =3D (char *) malloc(data_length + 1); Please use g_malloc()/g_free() instead of malloc()/free(). > + if (out =3D=3D NULL) { > + ret =3D -1; > + return ret; > + } > + memset(out, 0, data_length + 1); > + > + for (j =3D 0; j < data_length + 1; j++) { > + out[j] =3D bs->backing_file[2*j]; > + } > + out[data_length + 1] =3D '\0'; > + > + while (*optr !=3D '\0') { > + if (*optr =3D=3D '\\') { > + *optr =3D '/'; > + } > + optr++; > + } > + > + strncpy(bs->backing_file, out, data_length + 1); > + > + out =3D NULL; > + free(out); Did you mean: free(out); out =3D NULL; ? --7lMq7vMTJT4tNk0a Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJUT7BiAAoJEJykq7OBq3PICcQH/jDNpZjzR+SIOjBa/yraGltn m8FWlWIqm4Mh1cPXZHzu9qUzWsI1CVojhBS+YiOiencjZupVYVEB4n/pYhKvRhT5 8tzQWWVPZbiPA7oOQsam2nMwG3Ir2hDFMT0c6YVWMKDb3+jVO+HBGQtyf0xDrGcd aylrIr4/NIZ154oADNa8ga9S1ZImDYOnn04Yvn40OYbZy9rpzpbN3KyyxBl5WQ5C VIZBsc1aodk9g7uyjsYExPYFiEAYbkKFsnouHa3s4Ln0VZlfk2Wkaa3tKQ/J3KXx sjWbTAWfJozLxX3xqTegMSHixF44PBjXq50iESeZ0S5VjbugxjXM22DJcraMVe8= =mA0I -----END PGP SIGNATURE----- --7lMq7vMTJT4tNk0a--