From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([140.186.70.92]:34097)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1QY07o-0008R4-6S
	for qemu-devel@nongnu.org; Sat, 18 Jun 2011 14:20:05 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1QY07i-0006zB-R0
	for qemu-devel@nongnu.org; Sat, 18 Jun 2011 14:20:03 -0400
Received: from mail-gy0-f173.google.com ([209.85.160.173]:50224)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1QY07i-0006yl-Da
	for qemu-devel@nongnu.org; Sat, 18 Jun 2011 14:19:58 -0400
Received: by gyg10 with SMTP id 10so160233gyg.4
	for <qemu-devel@nongnu.org>; Sat, 18 Jun 2011 11:19:57 -0700 (PDT)
MIME-Version: 1.0
In-Reply-To: <BANLkTi=C0x1paHiCW5CnzU6tvNFJ2cpfdQ@mail.gmail.com>
References: <BANLkTi=C0x1paHiCW5CnzU6tvNFJ2cpfdQ@mail.gmail.com>
Date: Sat, 18 Jun 2011 19:19:57 +0100
Message-ID: <BANLkTinKCJWopo49ew8OZYava7pa4vFH5A@mail.gmail.com>
From: Stefan Hajnoczi <stefanha@gmail.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
Subject: Re: [Qemu-devel] [PATCH 11/12] VMDK: vmdk_create and options for
	mono flat image
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Fam Zheng <famcool@gmail.com>
Cc: Kevin Wolf <kwolf@redhat.com>, qemu-devel@nongnu.org, Christoph Hellwig <hch@lst.de>

On Sat, Jun 4, 2011 at 1:44 AM, Fam Zheng <famcool@gmail.com> wrote:
> + =A0 =A0if (flat) {

The flat and !flat cases are too big, please split them out into functions.

> + =A0 =A0 =A0 =A0const char desc_template[] =3D
> + =A0 =A0 =A0 =A0"# Disk DescriptorFile\n"
> + =A0 =A0 =A0 =A0"version=3D1\n"
> + =A0 =A0 =A0 =A0"CID=3D%x\n"
> + =A0 =A0 =A0 =A0"parentCID=3Dffffffff\n"
> + =A0 =A0 =A0 =A0"createType=3D\"monolithicFlat\"\n"
> + =A0 =A0 =A0 =A0"\n"
> + =A0 =A0 =A0 =A0"# Extent description\n"
> + =A0 =A0 =A0 =A0"RW %" PRId64 " FLAT \"%s\" 0\n"
> + =A0 =A0 =A0 =A0"\n"
> + =A0 =A0 =A0 =A0"# The Disk Data Base \n"
> + =A0 =A0 =A0 =A0"#DDB\n"
> + =A0 =A0 =A0 =A0"\n"
> + =A0 =A0 =A0 =A0"ddb.virtualHWVersion =3D \"%d\"\n"
> + =A0 =A0 =A0 =A0"ddb.geometry.cylinders =3D \"%" PRId64 "\"\n"
> + =A0 =A0 =A0 =A0"ddb.geometry.heads =3D \"16\"\n"
> + =A0 =A0 =A0 =A0"ddb.geometry.sectors =3D \"63\"\n"
> + =A0 =A0 =A0 =A0"ddb.adapterType =3D \"ide\"\n";

This is almost identical to the desc_template[] below.  Please use
createType=3D%s and extent type format specifiers to share this template
instead of copy-pasting it.

> + =A0 =A0 =A0 =A0char ext_filename[1024];
> + =A0 =A0 =A0 =A0strncpy(ext_filename, filename, 1024);
> + =A0 =A0 =A0 =A0ext_filename[1023] =3D '\0';
> + =A0 =A0 =A0 =A0if (backing_file) {
> + =A0 =A0 =A0 =A0 =A0 =A0/* not supporting backing file for flat image */
> + =A0 =A0 =A0 =A0 =A0 =A0return -1;
> + =A0 =A0 =A0 =A0}
> + =A0 =A0 =A0 =A0if (!strcmp(&ext_filename[strlen(ext_filename) - 5], ".v=
mdk"))
> + =A0 =A0 =A0 =A0 =A0 =A0strcpy(&ext_filename[strlen(ext_filename) - 5], =
"-flat.vmdk");
> + =A0 =A0 =A0 =A0else
> + =A0 =A0 =A0 =A0 =A0 =A0strcat(ext_filename, "-flat.vmdk");
> + =A0 =A0 =A0 =A0/* create extent first */
> + =A0 =A0 =A0 =A0fd =3D open(
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0ext_filename,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0O_WRONLY | O_CREAT | O_TRUNC | O_BINARY =
| O_LARGEFILE,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A00644);
> + =A0 =A0 =A0 =A0if (fd < 0)
> + =A0 =A0 =A0 =A0 =A0 =A0return -errno;
> + =A0 =A0 =A0 =A0ret =3D ftruncate(fd, total_size * 512);
> + =A0 =A0 =A0 =A0if (ret) goto exit;
> + =A0 =A0 =A0 =A0close(fd);
> +
> + =A0 =A0 =A0 =A0/* generate descriptor file */
> + =A0 =A0 =A0 =A0snprintf(desc, sizeof(desc), desc_template, (unsigned in=
t)time(NULL),
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 total_size, ext_filename,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (flags & BLOCK_FLAG_COMPAT6 ? 6 : 4),
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 total_size / (int64_t)(63 * 16));
> + =A0 =A0 =A0 =A0fd =3D open(
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0filename,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0O_WRONLY | O_CREAT | O_TRUNC | O_BINARY =
| O_LARGEFILE,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A00644);
> + =A0 =A0 =A0 =A0if (fd < 0)
> + =A0 =A0 =A0 =A0 =A0 =A0return -errno;
> + =A0 =A0 =A0 =A0ret =3D qemu_write_full(fd, desc, strlen(desc));
> + =A0 =A0 =A0 =A0if (ret !=3D strlen(desc)) {
> + =A0 =A0 =A0 =A0 =A0 =A0ret =3D -errno;
> + =A0 =A0 =A0 =A0 =A0 =A0goto exit;
> + =A0 =A0 =A0 =A0}
> + =A0 =A0 =A0 =A0ret =3D 0;
> + =A0 =A0} else {
> + =A0 =A0 =A0 =A0const char desc_template[] =3D
> + =A0 =A0 =A0 =A0"# Disk DescriptorFile\n"
> + =A0 =A0 =A0 =A0"version=3D1\n"
> + =A0 =A0 =A0 =A0"CID=3D%x\n"
> + =A0 =A0 =A0 =A0"parentCID=3Dffffffff\n"
> + =A0 =A0 =A0 =A0"createType=3D\"monolithicSparse\"\n"
> + =A0 =A0 =A0 =A0"\n"
> + =A0 =A0 =A0 =A0"# Extent description\n"
> + =A0 =A0 =A0 =A0"RW %" PRId64 " SPARSE \"%s\"\n"
> + =A0 =A0 =A0 =A0"\n"
> + =A0 =A0 =A0 =A0"# The Disk Data Base \n"
> + =A0 =A0 =A0 =A0"#DDB\n"
> + =A0 =A0 =A0 =A0"\n"
> + =A0 =A0 =A0 =A0"ddb.virtualHWVersion =3D \"%d\"\n"
> + =A0 =A0 =A0 =A0"ddb.geometry.cylinders =3D \"%" PRId64 "\"\n"
> + =A0 =A0 =A0 =A0"ddb.geometry.heads =3D \"16\"\n"
> + =A0 =A0 =A0 =A0"ddb.geometry.sectors =3D \"63\"\n"
> + =A0 =A0 =A0 =A0"ddb.adapterType =3D \"ide\"\n";
> + =A0 =A0 =A0 =A0/* XXX: add support for backing file */

Is this comment still true?  It seems there is support for a backing
file via vmdk_snapshot_create().

Stefan