[PATCH] xl - Special case vhd images validation

[PATCH] xl - Special case vhd images validation

[Kamala Narasimhan]

[-- Attachment #1: Type: text/plain, Size: 428 bytes --]

Description - Special case how we validate vhd image files.  Without this patch when tap:aio:vhd prefixed image files are specified in the config file, disk validation and thus vm creation will fail.

IanJ - Since we decided to not merge disk changes patch 3-5, we need this.

IanC - This patch has the change you suggested to look for DISK_FORMAT_EMPTY.

Signed-off-by: Kamala Narasimhan <kamala.narasimhan@citrix.com>

Kamala

[-- Attachment #2: xl-validate-vhd --]
[-- Type: text/plain, Size: 1690 bytes --]

diff -r 9e87dfb60bc6 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Thu Feb 10 11:49:27 2011 -0500
+++ b/tools/libxl/libxl.c	Thu Feb 10 11:50:25 2011 -0500
@@ -864,18 +864,29 @@ int libxl_vncviewer_exec(libxl_ctx *ctx,
 /******************************************************************************/
 
 static int validate_virtual_disk(libxl_ctx *ctx, char *file_name, 
-    libxl_disk_backend backend_type, libxl_disk_format format)
+    libxl_device_disk *disk)
 {
     struct stat stat_buf;
+    char *delimiter;
 
-    if ((file_name[0] == '\0') && (format == DISK_FORMAT_EMPTY))
+    if (disk->format == DISK_FORMAT_EMPTY)
         return 0;
+
+    if (disk->format == DISK_FORMAT_RAW) {
+        delimiter = strchr(file_name, ':');
+        if (delimiter) { 
+            if (!strncmp(file_name, "vhd:", sizeof("vhd:")-1)) {
+                disk->format = DISK_FORMAT_VHD; 
+                file_name = ++delimiter;
+            }
+        }
+    }
 
     if (stat(file_name, &stat_buf) != 0) {
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to stat %s", file_name);
         return ERROR_INVAL;
     }
-    if (backend_type == DISK_BACKEND_PHY) {
+    if (disk->backend == DISK_BACKEND_PHY) {
         if ( !(S_ISBLK(stat_buf.st_mode)) ) {
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Virtual disk %s is not a block device!\n",
                 file_name);
@@ -899,8 +910,7 @@ int libxl_device_disk_add(libxl_ctx *ctx
     libxl__device device;
     int major, minor, rc;
 
-    rc = validate_virtual_disk(ctx, disk->pdev_path, disk->backend, 
-             disk->format);
+    rc = validate_virtual_disk(ctx, disk->pdev_path, disk);
     if (rc)
         return rc;
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] xl - Special case vhd images validation

[Ian Jackson]

Kamala Narasimhan writes ("[xen-devel][PATCH] xl - Special case vhd images validation"):
> Description - Special case how we validate vhd image files.  Without this patch when tap:aio:vhd prefixed image files are specified in the config file, disk validation and thus vm creation will fail.

> +    if (disk->format == DISK_FORMAT_RAW) {
> +        delimiter = strchr(file_name, ':');
> +        if (delimiter) { 
> +            if (!strncmp(file_name, "vhd:", sizeof("vhd:")-1)) {
> +                disk->format = DISK_FORMAT_VHD; 
> +                file_name = ++delimiter;
> +            }
> +        }
> +    }

This seems to suggest that if you say "tap:raw:" with a filename
starting "vhd:" you get vhd instead.  This is a bit perverse but I
guess acceptable for 4.1 at least.


>          LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to stat %s", file_name);
>          return ERROR_INVAL;
>      }
> -    if (backend_type == DISK_BACKEND_PHY) {
> +    if (disk->backend == DISK_BACKEND_PHY) {

Is this change stray in this patch, or should it be included in one of
the others ?  I'm not sure I follow it, anyway.

Ian.

Re: [PATCH] xl - Special case vhd images validation

[Kamala Narasimhan]

Ian Jackson wrote:
> Kamala Narasimhan writes ("[xen-devel][PATCH] xl - Special case vhd images validation"):
>> Description - Special case how we validate vhd image files.  Without this patch when tap:aio:vhd prefixed image files are specified in the config file, disk validation and thus vm creation will fail.
> 
>> +    if (disk->format == DISK_FORMAT_RAW) {
>> +        delimiter = strchr(file_name, ':');
>> +        if (delimiter) { 
>> +            if (!strncmp(file_name, "vhd:", sizeof("vhd:")-1)) {
>> +                disk->format = DISK_FORMAT_VHD; 
>> +                file_name = ++delimiter;
>> +            }
>> +        }
>> +    }
> 
> This seems to suggest that if you say "tap:raw:" with a filename
> starting "vhd:" you get vhd instead.  This is a bit perverse but I
> guess acceptable for 4.1 at least.
>
We won't need this change if we have gone with patch 3 and further but since we
chose to move those post 4.1, we will need this.

> 
>>          LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to stat %s", file_name);
>>          return ERROR_INVAL;
>>      }
>> -    if (backend_type == DISK_BACKEND_PHY) {
>> +    if (disk->backend == DISK_BACKEND_PHY) {
> 
> Is this change stray in this patch, or should it be included in one of
> the others ?  I'm not sure I follow it, anyway.
> 

I switched the input param for the method as I had to, so we need this as well.

Kamala

Re: [PATCH] xl - Special case vhd images validation

[Kamala Narasimhan]

[-- Attachment #1: Type: text/plain, Size: 434 bytes --]

Resending this patch as the one I sent earlier won't apply given the changes we just made to "[PATCH 2/5] Xl interface change plus changes to code it impacts".

Description - Special case how we validate vhd image files.  Without this patch when tap:aio:vhd prefixed image files are specified in the config file, disk validation and thus vm creation will fail.

Signed-off-by: Kamala Narasimhan <kamala.narasimhan@citrix.com>

Kamala

[-- Attachment #2: xl-validate-vhd --]
[-- Type: text/plain, Size: 1761 bytes --]

diff -r a60ad215181c tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Mon Feb 14 15:15:19 2011 -0500
+++ b/tools/libxl/libxl.c	Mon Feb 14 15:18:33 2011 -0500
@@ -863,19 +863,30 @@ int libxl_vncviewer_exec(libxl_ctx *ctx,
 
 /******************************************************************************/
 
-static int validate_virtual_disk(libxl_ctx *ctx, char *file_name, 
-    libxl_disk_backend backend_type, libxl_disk_format format)
+static int validate_virtual_disk(libxl_ctx *ctx, char *file_name,
+    libxl_device_disk *disk) 
 {
     struct stat stat_buf;
+    char *delimiter;
 
-    if ((file_name[0] == '\0') && (format == DISK_FORMAT_EMPTY))
+    if (disk->format == DISK_FORMAT_EMPTY)
         return 0;
+
+    if (disk->format == DISK_FORMAT_RAW) {
+        delimiter = strchr(file_name, ':');
+        if (delimiter) {
+            if (!strncmp(file_name, "vhd:", sizeof("vhd:")-1)) {
+                disk->format = DISK_FORMAT_VHD;
+                file_name = ++delimiter;
+            }
+        }
+    }
 
     if ( stat(file_name, &stat_buf) != 0 ) {
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to stat %s", file_name);
         return ERROR_INVAL;
     }
-    if (backend_type == DISK_BACKEND_PHY) {
+    if (disk->backend == DISK_BACKEND_PHY) {
         if ( !(S_ISBLK(stat_buf.st_mode)) ) {
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Virtual disk %s is not a block device!\n",
                 file_name);
@@ -899,8 +910,7 @@ int libxl_device_disk_add(libxl_ctx *ctx
     libxl__device device;
     int major, minor, rc;
 
-    rc = validate_virtual_disk(ctx, disk->pdev_path, disk->backend, 
-             disk->format);
+    rc = validate_virtual_disk(ctx, disk->pdev_path, disk); 
     if (rc)
         return rc;
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] xl - Special case vhd images validation

[Ian Jackson]

Kamala Narasimhan writes ("Re: [xen-devel][PATCH] xl - Special case vhd images validation"):
> Description - Special case how we validate vhd image files.  Without this patch when tap:aio:vhd prefixed image files are specified in the config file, disk validation and thus vm creation will fail.

Thanks.  This one looks good to me (although the special case is a bit
unpleasant, I don't think we want to do something more intrusive now)
so I'm happy with it as a followup to your disk interface change
patch.

Ian.