From: Peter Lieven <pl@kamp.de>
To: Fam Zheng <famz@redhat.com>, qemu-devel@nongnu.org
Cc: kwolf@redhat.com, stefanha@redhat.com
Subject: Re: [Qemu-devel] [PATCH v2] vmdk: Allow vmdk_create to work with protocol
Date: Fri, 20 Dec 2013 11:16:08 +0100 [thread overview]
Message-ID: <52B418E8.1040905@kamp.de> (raw)
In-Reply-To: <1387504128-15347-1-git-send-email-famz@redhat.com>
On 20.12.2013 02:48, Fam Zheng wrote:
> This improves vmdk_create to use bdrv_* functions to replace qemu_open
> and other fd functions. The error handling are improved as well. One
> difference is that bdrv_pwrite will round up buffer to sectors, so for
> description file, an extra bdrv_truncate is used in the end to drop
> inding zeros.
>
> Notes:
>
> - A bonus bug fix is correct endian is used in initializing GD entries.
>
> - ROUND_UP and DIV_ROUND_UP are used where possible.
>
> I tested that new code produces exactly the same file as previously.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
>
> ---
> v2: Address Stefan's comments:
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
> block/vmdk.c | 164 ++++++++++++++++++++++++++++++++++-------------------------
> 1 file changed, 95 insertions(+), 69 deletions(-)
>
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 0734bc2..ef078f6 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -1447,23 +1447,33 @@ static int coroutine_fn vmdk_co_write_zeroes(BlockDriverState *bs,
> }
>
> static int vmdk_create_extent(const char *filename, int64_t filesize,
> - bool flat, bool compress, bool zeroed_grain)
> + bool flat, bool compress, bool zeroed_grain,
> + Error **errp)
> {
> int ret, i;
> - int fd = 0;
> + BlockDriverState *bs = NULL;
> VMDK4Header header;
> - uint32_t tmp, magic, grains, gd_size, gt_size, gt_count;
> + Error *local_err;
> + uint32_t tmp, magic, grains, gd_sectors, gt_size, gt_count;
> + uint32_t *gd_buf = NULL;
> + int gd_buf_size;
>
> - fd = qemu_open(filename,
> - O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
> - 0644);
> - if (fd < 0) {
> - return -errno;
> + ret = bdrv_create_file(filename, NULL, &local_err);
> + if (ret < 0) {
> + error_propagate(errp, local_err);
> + goto exit;
> }
> +
> + ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR, &local_err);
> + if (ret < 0) {
> + error_propagate(errp, local_err);
> + goto exit;
> + }
> +
> if (flat) {
> - ret = ftruncate(fd, filesize);
> + ret = bdrv_truncate(bs, filesize);
> if (ret < 0) {
> - ret = -errno;
> + error_setg(errp, "Could not truncate file");
> }
> goto exit;
> }
> @@ -1474,24 +1484,23 @@ static int vmdk_create_extent(const char *filename, int64_t filesize,
> | (compress ? VMDK4_FLAG_COMPRESS | VMDK4_FLAG_MARKER : 0)
> | (zeroed_grain ? VMDK4_FLAG_ZERO_GRAIN : 0);
> header.compressAlgorithm = compress ? VMDK4_COMPRESSION_DEFLATE : 0;
> - header.capacity = filesize / 512;
> + header.capacity = filesize / BDRV_SECTOR_SIZE;
> header.granularity = 128;
> - header.num_gtes_per_gt = 512;
> + header.num_gtes_per_gt = BDRV_SECTOR_SIZE;
>
> - grains = (filesize / 512 + header.granularity - 1) / header.granularity;
> - gt_size = ((header.num_gtes_per_gt * sizeof(uint32_t)) + 511) >> 9;
> - gt_count =
> - (grains + header.num_gtes_per_gt - 1) / header.num_gtes_per_gt;
> - gd_size = (gt_count * sizeof(uint32_t) + 511) >> 9;
> + grains = DIV_ROUND_UP(filesize / BDRV_SECTOR_SIZE, header.granularity);
> + gt_size = DIV_ROUND_UP(header.num_gtes_per_gt * sizeof(uint32_t),
> + BDRV_SECTOR_SIZE);
> + gt_count = DIV_ROUND_UP(grains, header.num_gtes_per_gt);
> + gd_sectors = DIV_ROUND_UP(gt_count * sizeof(uint32_t), BDRV_SECTOR_SIZE);
>
> header.desc_offset = 1;
> header.desc_size = 20;
> header.rgd_offset = header.desc_offset + header.desc_size;
> - header.gd_offset = header.rgd_offset + gd_size + (gt_size * gt_count);
> + header.gd_offset = header.rgd_offset + gd_sectors + (gt_size * gt_count);
> header.grain_offset =
> - ((header.gd_offset + gd_size + (gt_size * gt_count) +
> - header.granularity - 1) / header.granularity) *
> - header.granularity;
> + ROUND_UP(header.gd_offset + gd_sectors + (gt_size * gt_count),
> + header.granularity);
> /* swap endianness for all header fields */
> header.version = cpu_to_le32(header.version);
> header.flags = cpu_to_le32(header.flags);
> @@ -1511,48 +1520,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));
> - if (ret != sizeof(header)) {
> - ret = -errno;
> + ret = bdrv_pwrite(bs, sizeof(magic), &header, sizeof(header));
> + if (ret < 0) {
> + error_set(errp, QERR_IO_ERROR);
> goto exit;
> }
>
> - ret = ftruncate(fd, le64_to_cpu(header.grain_offset) << 9);
> + ret = bdrv_truncate(bs, le64_to_cpu(header.grain_offset) << 9);
> if (ret < 0) {
> - ret = -errno;
> + 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;
> + 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;
> - }
> + gd_buf[i] = cpu_to_le32(tmp);
> + }
> + ret = bdrv_pwrite(bs, le64_to_cpu(header.rgd_offset) * BDRV_SECTOR_SIZE,
> + gd_buf, gd_buf_size);
> + if (ret < 0) {
> + error_set(errp, QERR_IO_ERROR);
> + goto exit;
> }
>
> /* write backup grain directory */
> - lseek(fd, le64_to_cpu(header.gd_offset) << 9, SEEK_SET);
> - for (i = 0, tmp = le64_to_cpu(header.gd_offset) + gd_size;
> + for (i = 0, tmp = le64_to_cpu(header.gd_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;
> - }
> + gd_buf[i] = cpu_to_le32(tmp);
> + }
> + ret = bdrv_pwrite(bs, le64_to_cpu(header.gd_offset) * BDRV_SECTOR_SIZE,
> + gd_buf, gd_buf_size);
> + if (ret < 0) {
> + error_set(errp, QERR_IO_ERROR);
> + goto exit;
> }
>
> ret = 0;
> - exit:
> - qemu_close(fd);
> +exit:
> + if (bs) {
> + bdrv_unref(bs);
> + }
> + g_free(gd_buf);
> return ret;
> }
>
> @@ -1599,7 +1615,9 @@ static int filename_decompose(const char *filename, char *path, char *prefix,
> static int vmdk_create(const char *filename, QEMUOptionParameter *options,
> Error **errp)
> {
> - int fd, idx = 0;
> + int idx = 0;
> + BlockDriverState *new_bs = NULL;
> + Error *local_err;
> char *desc = NULL;
> int64_t total_size = 0, filesize;
> const char *adapter_type = NULL;
> @@ -1616,6 +1634,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
> uint32_t parent_cid = 0xffffffff;
> uint32_t number_heads = 16;
> bool zeroed_grain = false;
> + uint32_t desc_offset = 0, desc_len;
> const char desc_template[] =
> "# Disk DescriptorFile\n"
> "version=1\n"
> @@ -1749,7 +1768,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
> path, desc_filename);
>
> if (vmdk_create_extent(ext_filename, size,
> - flat, compress, zeroed_grain)) {
> + flat, compress, zeroed_grain, errp)) {
> ret = -EINVAL;
> goto exit;
> }
> @@ -1757,7 +1776,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
>
> /* Format description line */
> snprintf(desc_line, sizeof(desc_line),
> - desc_extent_line, size / 512, desc_filename);
> + desc_extent_line, size / BDRV_SECTOR_SIZE, desc_filename);
> g_string_append(ext_desc_lines, desc_line);
> }
> /* generate descriptor file */
> @@ -1768,36 +1787,43 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
> parent_desc_line,
> ext_desc_lines->str,
> (flags & BLOCK_FLAG_COMPAT6 ? 6 : 4),
> - total_size / (int64_t)(63 * number_heads * 512),
> + total_size /
> + (int64_t)(63 * number_heads * BDRV_SECTOR_SIZE),
> 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);
> + if (ret < 0) {
> + error_setg_errno(errp, -ret, "Could not create image file");
> + goto exit;
> + }
> }
> - 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 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_len);
> + if (ret < 0) {
> + error_setg(errp, "Could not truncate file");
> + }
> }
> - ret = 0;
> -close_exit:
> - qemu_close(fd);
> exit:
> + if (new_bs) {
> + bdrv_unref(new_bs);
> + }
> g_free(desc);
> g_string_free(ext_desc_lines, true);
> return ret;
Works great especially with (v3) of the NFS driver ;-)
Tested-by: Peter Lieven <pl@kamp.de>
prev parent reply other threads:[~2013-12-20 10:15 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-12-20 1:48 [Qemu-devel] [PATCH v2] vmdk: Allow vmdk_create to work with protocol Fam Zheng
2013-12-20 8:17 ` Stefan Hajnoczi
2013-12-20 10:16 ` Peter Lieven [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=52B418E8.1040905@kamp.de \
--to=pl@kamp.de \
--cc=famz@redhat.com \
--cc=kwolf@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.