[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RESEND] Support vhd type VHD_DIFFERENCING
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH RESEND] Support vhd type VHD_DIFFERENCING |
Date: |
Wed, 26 Nov 2014 15:32:49 +0000 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Thu, Nov 06, 2014 at 10:43:50PM +0800, Xiaodong Gong wrote:
> + } else if (platform == PLATFORM_W2RU) {
> + /* Must be UTF16-LE to ASCII */
> + char *out, *optr;
> + int j;
> +
> + optr = out = (char *) g_malloc(data_length + 1);
> + if (out == NULL) {
> + ret = -1;
> + return ret;
> + }
> +
> + 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);
> +
> + g_free(out);
> + out = NULL;
> +
> + done = true;
> + }
Please convert from UTF-16 LE to the local file system character set:
https://developer.gnome.org/glib/stable/glib-Character-Set-Conversion.html
Also, using ->backing_file[] when the data is UTF-16 LE encoded is not
ideal since it halves the maximum size of the string! It would be
better to read into a temporary buffer that is 2 *
sizeof(backing_file[]) big before writing into ->backing_file[].
> @@ -286,6 +411,37 @@ static int vpc_open(BlockDriverState *bs, QDict
> *options, int flags,
> s->free_data_block_offset =
> (s->bat_offset + (s->max_table_entries * 4) + 511) & ~511;
>
> + /* Read tdbatmap header by offset */
> + if (footer->version >= VHD_VERSION(1, 2)) {
Missing be32_to_cpu(footer->version)
> +static int vpc_write(BlockDriverState *bs, int64_t sector_num,
> + const uint8_t *buf, int nb_sectors)
> +{
> + BDRVVPCState *s = bs->opaque;
> + VHDFooter *footer = (VHDFooter *) s->footer_buf;
> + int64_t sectors_per_block = s->block_size >> BDRV_SECTOR_BITS;
> + int64_t offset, sectors;
> + bool diff = true;
> + int ret = 0;
> +
> + switch (be32_to_cpu(footer->type)) {
> + case VHD_FIXED:
> + return bdrv_write(bs->file, sector_num, buf, nb_sectors);
> + case VHD_DYNAMIC:
> + case VHD_DIFF:
> + if (be32_to_cpu(footer->type) == VHD_DYNAMIC) {
> + diff = false;
> }
This can be done with a fall-through case instead of checking
footer->type again:
case VHD_DYNAMIC:
diff = false;
/* fall-through */
case VHD_DIFF:
> + ret = bdrv_pwrite(bs->file, offset, buf,
> + sectors * BDRV_SECTOR_SIZE);
> + if (ret != sectors * BDRV_SECTOR_SIZE) {
> + return -1;
> + }
>
> - nb_sectors -= sectors;
> - sector_num += sectors;
> - buf += sectors * BDRV_SECTOR_SIZE;
> - }
> + if (diff) {
> + ret = write_bitmap(bs, sector_num, sectors);
> + if (ret < 0) {
> + return -1;
> + }
> + }
>
> - return 0;
> + nb_sectors -= sectors;
> + sector_num += sectors;
> + buf += sectors * BDRV_SECTOR_SIZE;
> + }
> + break;
> + default:
> + return -1;
> + }
> + return ret;
In the VHD_DYNAMIC case we must *not* return the number of bytes from
bdrv_pwrite(). Should this be return 0?
pgpELMHhVxRkl.pgp
Description: PGP signature