Re: [Qemu-devel] [PATCH] Added an option to set the VMDK adapter type

[Kevin Wolf <kwolf@redhat.com>]

Hi Aaron,

Am 04.08.2010 01:46, schrieb Aaron Mason:
> Now that I have half a clue, please find attached a properly formatted
> patch for the above with a signed-off line.  Hopefully attaching it
> won't cause issues as I have winblows on this machine and can't get
> git send-email to work at this time.

Works for me, though it would be even better to attach it for applying
and inlining it additionally to make it easier to quote.

One more round of changes and I think we've got it:

> From 37bed87750573b7ac737c3f2a919b68a06a00513 Mon Sep 17 00:00:00 2001
> From: unknown <aaron.mason@.rlpb.org.au>

You should tell git your real name and mail address in its config file.
You can use git commit --amend --reset-author to update the existing commit.

> Date: Wed, 4 Aug 2010 08:41:38 +1000
> Subject: [PATCH] Added an option to set the VMDK adapter type
> 
> Signed-off-by: Aaron Mason <aaron.mason@thats-too-much.info>
> ---
>  block/vmdk.c |   20 +++++++++++++++++---
>  block_int.h  |    1 +
>  2 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 2d4ba42..ef7733d 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, *adapterType = "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") || 

This line has trailing whitespace.

> +                    !strcmp(options->value.s, "buslogic") ||
> +                    !strcmp(options->value.s, "lsilogic")) {
> +                    adapterType = options->value.s;
> +                } else
> +                    return -1;

Please add braces even for single statements, see CODING_STYLE.

Also better return -EINVAL as the return value is used as a negative
errno to produce the error message.

Kevin