Patchwork To support scsi Virtual Adapter while creating VMDK images

login
register
mail settings
Submitter Praveen_Paladugu@Dell.com
Date Jan. 26, 2012, 11:46 p.m.
Message ID <0F7A61B0A6ABAD4A82DDEF8CCA0ADA2B4E0906614F@AUSX7MCPS301.AMER.DELL.COM>
Download mbox | patch
Permalink /patch/138102/
State New
Headers show

Comments

Praveen_Paladugu@Dell.com - Jan. 26, 2012, 11:46 p.m.
According to VMDK specification, while using virtual scsi adapter, the value of adapterType has to be set to "lsilogic" or "buslogic". When the user adds "scsi" to the list of qemu-img options (Example: qemu-img create -f vmdk foo -o scsi,size=100M) this patch will set the value of adapterType to "lsilogic".

When the "scsi" is not passed, the default "ide" value is used.




Signed-off-by: Praveen K Paladugu <Praveen_paladugu@dell.com>
Andreas Färber - Jan. 27, 2012, 4:19 p.m.
Am 27.01.2012 00:46, schrieb Praveen_Paladugu@Dell.com:
> According to VMDK specification, while using virtual scsi adapter, the value of adapterType has to be set to "lsilogic" or "buslogic". When the user adds "scsi" to the list of qemu-img options (Example: qemu-img create -f vmdk foo -o scsi,size=100M) this patch will set the value of adapterType to "lsilogic".
> 
> When the "scsi" is not passed, the default "ide" value is used.

> Signed-off-by: Praveen K Paladugu <Praveen_paladugu@dell.com>

SoB should be above the patch so that it gets committed. Please cc the
block maintainer, scripts/get_maintainer.pl can help with that.

We're carrying a pretty similar patch from 2009 in OBS:

https://build.opensuse.org/package/view_file?file=qemu-img-vmdk-scsi.patch&package=virt-utils&project=Virtualization&rev=34c58db02d6bfdb1a9dd49fc3c7fa9d4

There were also patches by Soren Hansen and Kevin Wolf back in 2008, by
Pantelis Koukousoulas in 2009, by Aaron Mason and Alex Graf in 2010; are
these all independent or is someone's SoB missing here?

There were some issues raised about BLOCK_FLAG_SCSI that I don't see
addressed here.

Google found these other versions for reference (there may be more):

http://patchwork.ozlabs.org/patch/60730/
http://patchwork.ozlabs.org/patch/60080/
http://copilotco.com/mail-archives/qemu.2008/msg04541.html
http://lists.gnu.org/archive/html/qemu-devel/2009-07/msg02509.html
http://www.mail-archive.com/qemu-devel@nongnu.org/msg15054.html

Regards,
Andreas

> diff --git a/block.c b/block.c
> index 3f072f6..2a2e725 100644
> --- a/block.c
> +++ b/block.c
> @@ -3690,7 +3690,7 @@ int bdrv_img_create(const char *filename, const char *fmt,
>                      char *options, uint64_t img_size, int flags)
>  {
>      QEMUOptionParameter *param = NULL, *create_options = NULL;
> -    QEMUOptionParameter *backing_fmt, *backing_file, *size;
> +    QEMUOptionParameter *backing_fmt, *backing_file, *size, *scsi;
>      BlockDriverState *bs = NULL;
>      BlockDriver *drv, *proto_drv;
>      BlockDriver *backing_drv = NULL;
> @@ -3797,6 +3797,12 @@ int bdrv_img_create(const char *filename, const char *fmt,
>              goto out;
>          }
>      }
> +    scsi = get_option_parameter(param, BLOCK_OPT_SCSI);
> +    if (scsi && scsi->value.n && strcmp(drv->format_name, "vmdk")) {
> +        error_report("SCSI devices not supported for this file format");
> +        ret = -1;
> +        goto out;
> +   }
>  
>      printf("Formatting '%s', fmt=%s ", filename, fmt);
>      print_option_parameters(param);
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 5623ac1..fc1ac65 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -1377,7 +1377,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";
>  
>      if (filename_decompose(filename, path, prefix, postfix, PATH_MAX)) {
>          return -EINVAL;
> @@ -1390,6 +1390,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;
>          } else if (!strcmp(options->name, BLOCK_OPT_SUBFMT)) {
>              fmt = options->value.s;
>          }
> @@ -1482,7 +1484,8 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
>              parent_desc_line,
>              ext_desc_lines,
>              (flags & BLOCK_FLAG_COMPAT6 ? 6 : 4),
> -            total_size / (int64_t)(63 * 16 * 512));
> +            total_size / (int64_t)(63 * 16 * 512),
> +            (flags & BLOCK_FLAG_SCSI ? "lsilogic" : "ide"));
>      if (split || flat) {
>          fd = open(
>                  filename,
> @@ -1585,6 +1588,12 @@ static QEMUOptionParameter vmdk_create_options[] = {
>              "VMDK flat extent format, can be one of "
>              "{monolithicSparse (default) | monolithicFlat | twoGbMaxExtentSparse | twoGbMaxExtentFlat | streamOptimized} "
>      },
> +    {
> +        .name = BLOCK_OPT_SCSI,
> +        .type = OPT_FLAG,
> +        .help = "Virtual SCSI Adapter"
> +    },
> +
>      { NULL }
>  };
>  
> diff --git a/block_int.h b/block_int.h
> index 311bd2a..5e332fb 100644
> --- a/block_int.h
> +++ b/block_int.h
> @@ -33,6 +33,7 @@
>  
>  #define BLOCK_FLAG_ENCRYPT	1
>  #define BLOCK_FLAG_COMPAT6	4
> +#define BLOCK_FLAG_SCSI         8
>  
>  #define BLOCK_IO_LIMIT_READ     0
>  #define BLOCK_IO_LIMIT_WRITE    1
> @@ -44,6 +45,7 @@
>  #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"
Kevin Wolf - Jan. 27, 2012, 4:34 p.m.
Am 27.01.2012 17:19, schrieb Andreas Färber:
> Am 27.01.2012 00:46, schrieb Praveen_Paladugu@Dell.com:
>> According to VMDK specification, while using virtual scsi adapter, the value of adapterType has to be set to "lsilogic" or "buslogic". When the user adds "scsi" to the list of qemu-img options (Example: qemu-img create -f vmdk foo -o scsi,size=100M) this patch will set the value of adapterType to "lsilogic".
>>
>> When the "scsi" is not passed, the default "ide" value is used.
> 
>> Signed-off-by: Praveen K Paladugu <Praveen_paladugu@dell.com>
> 
> SoB should be above the patch so that it gets committed. Please cc the
> block maintainer, scripts/get_maintainer.pl can help with that.
> 
> We're carrying a pretty similar patch from 2009 in OBS:
> 
> https://build.opensuse.org/package/view_file?file=qemu-img-vmdk-scsi.patch&package=virt-utils&project=Virtualization&rev=34c58db02d6bfdb1a9dd49fc3c7fa9d4
> 
> There were also patches by Soren Hansen and Kevin Wolf back in 2008, by
> Pantelis Koukousoulas in 2009, by Aaron Mason and Alex Graf in 2010; are
> these all independent or is someone's SoB missing here?

I think they are all independent. Aaron's patch came closest, it just
had some coding style issues (I would fix these myself) and was missing
a Signed-off-by (I can't do anything about that).

The other patches (just like this one) all didn't allow both buslogic
and lsilogic, but hardcoded one of them.

Kevin
Praveen_Paladugu@Dell.com - Jan. 27, 2012, 6:07 p.m.
I didn't know other patches were already submitted. I picked up the patch from opensuse distribution. I realized no related code is available in upstream project. So I created one based on the head of the git tree.

Please feel free to ignore this patch, since other/better patches seem to be available. 



Thank you 
Praveen K Paladugu
Dell Linux Engineering

Patch

diff --git a/block.c b/block.c
index 3f072f6..2a2e725 100644
--- a/block.c
+++ b/block.c
@@ -3690,7 +3690,7 @@  int bdrv_img_create(const char *filename, const char *fmt,
                     char *options, uint64_t img_size, int flags)
 {
     QEMUOptionParameter *param = NULL, *create_options = NULL;
-    QEMUOptionParameter *backing_fmt, *backing_file, *size;
+    QEMUOptionParameter *backing_fmt, *backing_file, *size, *scsi;
     BlockDriverState *bs = NULL;
     BlockDriver *drv, *proto_drv;
     BlockDriver *backing_drv = NULL;
@@ -3797,6 +3797,12 @@  int bdrv_img_create(const char *filename, const char *fmt,
             goto out;
         }
     }
+    scsi = get_option_parameter(param, BLOCK_OPT_SCSI);
+    if (scsi && scsi->value.n && strcmp(drv->format_name, "vmdk")) {
+        error_report("SCSI devices not supported for this file format");
+        ret = -1;
+        goto out;
+   }
 
     printf("Formatting '%s', fmt=%s ", filename, fmt);
     print_option_parameters(param);
diff --git a/block/vmdk.c b/block/vmdk.c
index 5623ac1..fc1ac65 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1377,7 +1377,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";
 
     if (filename_decompose(filename, path, prefix, postfix, PATH_MAX)) {
         return -EINVAL;
@@ -1390,6 +1390,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;
         } else if (!strcmp(options->name, BLOCK_OPT_SUBFMT)) {
             fmt = options->value.s;
         }
@@ -1482,7 +1484,8 @@  static int vmdk_create(const char *filename, QEMUOptionParameter *options)
             parent_desc_line,
             ext_desc_lines,
             (flags & BLOCK_FLAG_COMPAT6 ? 6 : 4),
-            total_size / (int64_t)(63 * 16 * 512));
+            total_size / (int64_t)(63 * 16 * 512),
+            (flags & BLOCK_FLAG_SCSI ? "lsilogic" : "ide"));
     if (split || flat) {
         fd = open(
                 filename,
@@ -1585,6 +1588,12 @@  static QEMUOptionParameter vmdk_create_options[] = {
             "VMDK flat extent format, can be one of "
             "{monolithicSparse (default) | monolithicFlat | twoGbMaxExtentSparse | twoGbMaxExtentFlat | streamOptimized} "
     },
+    {
+        .name = BLOCK_OPT_SCSI,
+        .type = OPT_FLAG,
+        .help = "Virtual SCSI Adapter"
+    },
+
     { NULL }
 };
 
diff --git a/block_int.h b/block_int.h
index 311bd2a..5e332fb 100644
--- a/block_int.h
+++ b/block_int.h
@@ -33,6 +33,7 @@ 
 
 #define BLOCK_FLAG_ENCRYPT	1
 #define BLOCK_FLAG_COMPAT6	4
+#define BLOCK_FLAG_SCSI         8
 
 #define BLOCK_IO_LIMIT_READ     0
 #define BLOCK_IO_LIMIT_WRITE    1
@@ -44,6 +45,7 @@ 
 #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"