[Qemu-devel] [PATCH] Add support for vmware vmdk scsi images to qemu-img

[Qemu-devel] [PATCH] Add support for vmware vmdk scsi images to qemu-img

[Pantelis Koukousoulas]

This adds support to qemu-img for creating vmdk scsi images.
(It currently creates only IDE ones).
Original version was from Soren Hansen.

I just updated it for latest qemu.

Tested with latest vmware player (2.5.2) and seems to work well.

Signed-off-by: Pantelis Koukousoulas <pktoss@gmail.com>
---
 block/vmdk.c |   12 ++++++++++--
 block_int.h  |    2 ++
 qemu-img.c   |    1 +
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index f21f02b..6381aa4 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -709,7 +709,7 @@ 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;
     int64_t total_size = 0;
@@ -724,6 +724,8 @@ 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_SCSI)) {
+            flags |= options->value.n ? BLOCK_FLAG_SCSI: 0;
         }
         options++;
     }
@@ -799,7 +801,8 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
     snprintf(desc, sizeof(desc), desc_template, (unsigned int)time(NULL),
              total_size, real_filename,
              (flags & BLOCK_FLAG_COMPAT6 ? 6 : 4),
-             total_size / (int64_t)(63 * 16));
+             total_size / (int64_t)(63 * 16),
+	     (flags & BLOCK_FLAG_SCSI ? "lsilogic" : "ide"));
 
     /* write the descriptor */
     lseek(fd, le64_to_cpu(header.desc_offset) << 9, SEEK_SET);
@@ -843,6 +846,11 @@ static QEMUOptionParameter vmdk_create_options[] = {
         .type = OPT_FLAG,
         .help = "VMDK version 6 image"
     },
+    {
+        .name = BLOCK_OPT_SCSI,
+        .type = OPT_FLAG,
+        .help = "VMDK scsi image"
+    },
     { NULL }
 };
 
diff --git a/block_int.h b/block_int.h
index 830b7e9..0311198 100644
--- a/block_int.h
+++ b/block_int.h
@@ -30,10 +30,12 @@
 #define BLOCK_FLAG_ENCRYPT	1
 #define BLOCK_FLAG_COMPRESS	2
 #define BLOCK_FLAG_COMPAT6	4
+#define BLOCK_FLAG_SCSI         8
 
 #define BLOCK_OPT_SIZE          "size"
 #define BLOCK_OPT_ENCRYPT       "encryption"
 #define BLOCK_OPT_COMPAT6       "compat6"
+#define BLOCK_OPT_SCSI          "scsi"
 #define BLOCK_OPT_BACKING_FILE  "backing_file"
 #define BLOCK_OPT_BACKING_FMT   "backing_fmt"
 #define BLOCK_OPT_CLUSTER_SIZE  "cluster_size"
-- 
1.6.3.3

Re: [Qemu-devel] [PATCH] Add support for vmware vmdk scsi images to qemu-img

[Paul Brook]

On Thursday 30 July 2009, Pantelis Koukousoulas wrote:
> This adds support to qemu-img for creating vmdk scsi images.
> (It currently creates only IDE ones).
> Original version was from Soren Hansen.

Wasn't this (adding generic code for vmware specific nonsense attributes) 
already rejected once?

http://lists.gnu.org/archive/html/qemu-devel/2009-05/msg01131.html

Paul

Re: [Qemu-devel] [PATCH] Add support for vmware vmdk scsi images to qemu-img

[Pantelis Koukousoulas]

On Thu, Jul 30, 2009 at 7:27 PM, Paul Brook<paul@codesourcery.com> wrote:
> On Thursday 30 July 2009, Pantelis Koukousoulas wrote:
>> This adds support to qemu-img for creating vmdk scsi images.
>> (It currently creates only IDE ones).
>> Original version was from Soren Hansen.
>
> Wasn't this (adding generic code for vmware specific nonsense attributes)
> already rejected once?
>
> http://lists.gnu.org/archive/html/qemu-devel/2009-05/msg01131.html

Thanks, I hadn't seen this thread to be honest (don't know how I missed it,
I only found the much older patch from Soren).

I read the thread and it seems the most important reason for the rejection
was the fact that this patch wanted to add a new global option to qemu-img
(-S). My patch doesn't try to do that, it only adds a new "-o scsi=on" to
the vmdk format.

Wrt to scsi not being useful for qemu itself though, I have to agree.
Nevertheless, this patch is very small and trivial and qemu-img usage as
a "swiss-army knife" for converting image format for various VMMs/emulators
is pretty common imho.

If you still think the same reasons for rejection apply, is there any reasonably
small change that could be made to the patch to make it more acceptable?

Thanks,
Pantelis

Re: [Qemu-devel] [PATCH] Add support for vmware vmdk scsi images to qemu-img

[Anthony Liguori]

Pantelis Koukousoulas wrote:
> diff --git a/block_int.h b/block_int.h
> index 830b7e9..0311198 100644
> --- a/block_int.h
> +++ b/block_int.h
> @@ -30,10 +30,12 @@
>  #define BLOCK_FLAG_ENCRYPT	1
>  #define BLOCK_FLAG_COMPRESS	2
>  #define BLOCK_FLAG_COMPAT6	4
> +#define BLOCK_FLAG_SCSI         8

The flag space is limited and I don't want to pollute it with options 
that are of no use to QEMU.  If you were able to limit the impact of 
this patch to just block/vmdk.c, we would be more likely to consider 
it.  That may require a greater refactoring of how options work though.

I wish we never took compat6 :-/

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH] Add support for vmware vmdk scsi images to qemu-img

[Kevin Wolf]

Anthony Liguori schrieb:
> Pantelis Koukousoulas wrote:
>> diff --git a/block_int.h b/block_int.h
>> index 830b7e9..0311198 100644
>> --- a/block_int.h
>> +++ b/block_int.h
>> @@ -30,10 +30,12 @@
>>  #define BLOCK_FLAG_ENCRYPT	1
>>  #define BLOCK_FLAG_COMPRESS	2
>>  #define BLOCK_FLAG_COMPAT6	4
>> +#define BLOCK_FLAG_SCSI         8
> 
> The flag space is limited and I don't want to pollute it with options 
> that are of no use to QEMU.  If you were able to limit the impact of 
> this patch to just block/vmdk.c, we would be more likely to consider 
> it.  That may require a greater refactoring of how options work though.

Well, yes, a refactoring as great as using a local int scsi instead of
defining a global constant to be or-ed into a local int flags... ;-)
Actually I think we could drop these global BLOCK_FLAG_* completely
without losing anything.

And otherwise the patch already is completely local to VMDK.

Kevin