From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=34054 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1Oe6PC-0004u9-Pi
	for qemu-devel@nongnu.org; Wed, 28 Jul 2010 09:10:43 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69)
	(envelope-from <kwolf@redhat.com>) id 1Oe6PA-0002Rb-0T
	for qemu-devel@nongnu.org; Wed, 28 Jul 2010 09:10:42 -0400
Received: from mx1.redhat.com ([209.132.183.28]:18989)
	by eggs.gnu.org with esmtp (Exim 4.69)
	(envelope-from <kwolf@redhat.com>) id 1Oe6P9-0002RT-Ls
	for qemu-devel@nongnu.org; Wed, 28 Jul 2010 09:10:39 -0400
Message-ID: <4C502C4F.8070103@redhat.com>
Date: Wed, 28 Jul 2010 15:10:39 +0200
From: Kevin Wolf <kwolf@redhat.com>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] PATCH: Adding options to generate SCSI based VMDK
	images
References: <AANLkTim35MUJLkgc4BqEXguMWR3Q9Fsuohj+84SvJSsY@mail.gmail.com>
In-Reply-To: <AANLkTim35MUJLkgc4BqEXguMWR3Q9Fsuohj+84SvJSsY@mail.gmail.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/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: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Aaron Mason <aaron.mason@thats-too-much.info>
Cc: qemu-devel@nongnu.org

Hi Aaron,

Am 28.07.2010 04:01, schrieb Aaron Mason:
> I tried this once before using command-line parameters and it was
> knocked back.  Someone made the mention of using options so I had a
> look and saw that it was really simple to do.
> 
> The following is a patch from the latest Git snapshot.  I have also
> attached it (in case it gets broken by Gmail) and uploaded it to
> http://milo.thats-too-much.info/qemu-vmdk_gen_scsi.patch (in case the
> mailing list prohibits attachments).

Needs a Signed-off-by line. Also please use "[PATCH]" instead of
"PATCH:" in the subject line so that git am automatically removes it
when applying the patch. You get a correctly formatted mail with git
format-patch.

And yes, your mailer has broken the patch, so adding the attachment was
a good idea (using git send-email would be even better, though).

> diff --git a/block/vmdk.c b/block/vmdk.c
> index 2d4ba42..b60d86f 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -686,9 +686,9 @@ static int vmdk_create(const char *filename,
> QEMUOptionParameter *options)
>          "ddb.geometry.cylinders = \"%" PRId64 "\"\n"
>          "ddb.geometry.heads = \"16\"\n"
>          "ddb.geometry.sectors = \"63\"\n"
> -        "ddb.adapterType = \"ide\"\n";
> +        "ddb.adapterType = \"%s\"\n";
>      char desc[1024];
> -    const char *real_filename, *temp_str;
> +    const char *real_filename, *temp_str, *adapter_type = "ide";
>      int64_t total_size = 0;
>      const char *backing_file = NULL;
>      int flags = 0;
> @@ -702,6 +702,15 @@ static int vmdk_create(const char *filename,
> QEMUOptionParameter *options)
>              backing_file = options->value.s;
>          } else if (!strcmp(options->name, BLOCK_OPT_COMPAT6)) {
>              flags |= options->value.n ? BLOCK_FLAG_COMPAT6: 0;
> +        } else if (!strcmp(options->name, BLOCK_OPT_ADAPTER)) {
> +        if (options->value.s != NULL) {
> +            if (!strcmp(options->value.s, "ide") || \
> +                   !strcmp(options->value.s, "buslogic") || \
> +            !strcmp(options->value.s, "lsilogic")) {
> +            adapter_type = options->value.s;
> +        } else
> +            return -1;
> +        }

qemu uses four spaces for indentation, tabs are not allowed. Also, a \
at the end of a line is not needed.

The logic of the patch looks okay, so please send a v2 with just these
formal things fixed.

Kevin