On Tue, Dec 17, 2013 at 08:00:00PM +0800, Fam Zheng wrote: > @@ -1511,48 +1521,55 @@ static int vmdk_create_extent(const char *filename, > int64_t filesize, > header.check_bytes[3] = 0xa; > > /* write all the data */ > - ret = qemu_write_full(fd, &magic, sizeof(magic)); > - if (ret != sizeof(magic)) { > - ret = -errno; > + ret = bdrv_pwrite(bs, 0, &magic, sizeof(magic)); > + if (ret < 0) { > + error_set(errp, QERR_IO_ERROR); > goto exit; > } > - ret = qemu_write_full(fd, &header, sizeof(header)); > + ret = bdrv_pwrite(bs, sizeof(magic), &header, sizeof(header)); > if (ret != sizeof(header)) { > + error_set(errp, QERR_IO_ERROR); > ret = -errno;
This line should be deleted. Also, I noticed you changed ret != sizeof(magic) to ret < 0 for the magic number bdrv_pwrite() but did not change the condition for the header write. Please keep the error handling condition consistent. > goto exit; > } > > - ret = ftruncate(fd, le64_to_cpu(header.grain_offset) << 9); > + ret = bdrv_truncate(bs, (le64_to_cpu(header.grain_offset)) << 9); Why add parentheses around le64_to_cpu()? > if (ret < 0) { > - ret = -errno; > - goto exit; > + error_setg(errp, "Could not truncate file"); goto exit? > } > > /* write grain directory */ > - lseek(fd, le64_to_cpu(header.rgd_offset) << 9, SEEK_SET); > - for (i = 0, tmp = le64_to_cpu(header.rgd_offset) + gd_size; > + gd_buf_size = gd_sectors * BDRV_SECTOR_SIZE * sizeof(*gd_buf); > + gd_buf = g_malloc0(gd_buf_size); > + for (i = 0, tmp = le64_to_cpu(header.rgd_offset) + gd_sectors; > i < gt_count; i++, tmp += gt_size) { > - ret = qemu_write_full(fd, &tmp, sizeof(tmp)); > - if (ret != sizeof(tmp)) { > - ret = -errno; > - goto exit; > - } Was this old code not endian-safe? It appears to be writing native endian values. The new code is different. > @@ -1771,33 +1791,34 @@ static int vmdk_create(const char *filename, > QEMUOptionParameter *options, > total_size / (int64_t)(63 * number_heads * 512), > number_heads, > adapter_type); > - if (split || flat) { > - fd = qemu_open(filename, > - O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE, > - 0644); > + desc_len = strlen(desc); > + /* the descriptor offset = 0x200 */ > + if (!split && !flat) { > + desc_offset = 0x200; > } else { > - fd = qemu_open(filename, > - O_WRONLY | O_BINARY | O_LARGEFILE, > - 0644); > + ret = bdrv_create_file(filename, options, &local_err); Missing error handling if bdrv_create_file() fails. > } > - if (fd < 0) { > - ret = -errno; > + ret = bdrv_file_open(&new_bs, filename, NULL, BDRV_O_RDWR, &local_err); > + if (ret < 0) { > + error_setg_errno(errp, -ret, "Could not write description"); > goto exit; > } > - /* the descriptor offset = 0x200 */ > - if (!split && !flat && 0x200 != lseek(fd, 0x200, SEEK_SET)) { > - ret = -errno; > - goto close_exit; > + ret = bdrv_pwrite(new_bs, desc_offset, desc, desc_len); > + if (ret < 0) { > + error_setg_errno(errp, -ret, "Could not write description"); goto close_exit? > } > - ret = qemu_write_full(fd, desc, strlen(desc)); > - if (ret != strlen(desc)) { > - ret = -errno; > - goto close_exit; > + /* bdrv_pwrite write padding zeros to align to sector, we don't need that > + * for description file */ > + if (desc_offset == 0) { > + ret = bdrv_truncate(new_bs, desc_offset + desc_len); We know desc_offset == 0, so desc_offset (0) + desc_len is really just desc_len.