Seg fault running xl create with 4.1.0-rc2

Seg fault running xl create with 4.1.0-rc2

[M A Young]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 701 bytes --]

If I run xl create configfile where configfile includes the lines
bootloader = "/usr/bin/pygrub"
disk = [ 'file:/dev/mapper/vg0-partname,xvda1,w' ]

then xl segfaults at the line
     ret = strdup(dev);

of libxl_device_disk_local_attach() in tools/libxl/libxl.c . The problem 
is that dev is not set if libxl__blktap_enabled(&gc) is false or if 
phystype isn't recognized. In the latter case I presume we want to skip 
that line and return NULL, but if libxl__blktap_enabled(&gc) is false we 
should be returning something, at least in the case where 
phystype=PHYSTYPE_FILE, so that we can fall back to qdisk. I think 
something like the attached patch (not yet tested) should work.

 	Michael Young

[-- Attachment #2: Type: TEXT/PLAIN, Size: 706 bytes --]

--- xen-4.1.0/tools/libxl/libxl.c.orig	2011-01-25 19:22:58.000000000 +0000
+++ xen-4.1.0/tools/libxl/libxl.c	2011-01-27 20:23:57.000000000 +0000
@@ -988,13 +988,18 @@
         case PHYSTYPE_VHD:
             if (libxl__blktap_enabled(&gc))
                 dev = libxl__blktap_devpath(&gc, disk->physpath, phystype);
+            else
+                dev = disk->physpath;
             break;
 
         default:
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk physical type: %d\n", phystype);
             break;
     }
-    ret = strdup(dev);
+    if (dev)
+        ret = strdup(dev);
+    else
+        ret = NULL;
     libxl__free_all(&gc);
     return ret;
 }

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

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

Re: Seg fault running xl create with 4.1.0-rc2

[Stefano Stabellini]

On Thu, 27 Jan 2011, M A Young wrote:
> If I run xl create configfile where configfile includes the lines
> bootloader = "/usr/bin/pygrub"
> disk = [ 'file:/dev/mapper/vg0-partname,xvda1,w' ]
> 
> then xl segfaults at the line
>      ret = strdup(dev);
> 
> of libxl_device_disk_local_attach() in tools/libxl/libxl.c . The problem 
> is that dev is not set if libxl__blktap_enabled(&gc) is false or if 
> phystype isn't recognized. In the latter case I presume we want to skip 
> that line and return NULL, but if libxl__blktap_enabled(&gc) is false we 
> should be returning something, at least in the case where 
> phystype=PHYSTYPE_FILE, so that we can fall back to qdisk. I think 
> something like the attached patch (not yet tested) should work.


Thanks for the patch.
I extended it a bit, covering qcow and qcow2 as well:



diff -r 722f7b7678dc tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Fri Jan 28 11:08:49 2011 +0000
+++ b/tools/libxl/libxl.c	Fri Jan 28 16:42:47 2011 +0000
@@ -1021,7 +1021,7 @@ char * libxl_device_disk_local_attach(li
 {
     libxl__gc gc = LIBXL_INIT_GC(ctx);
     const char *dev = NULL;
-    char *ret;
+    char *ret = NULL;
     int phystype = disk->phystype;
     switch (phystype) {
         case PHYSTYPE_PHY: {
@@ -1033,18 +1033,27 @@ char * libxl_device_disk_local_attach(li
             /* let's pretend is tap:aio for the moment */
             phystype = PHYSTYPE_AIO;
         case PHYSTYPE_AIO:
-        case PHYSTYPE_QCOW:
-        case PHYSTYPE_QCOW2:
+            if (!libxl__blktap_enabled(&gc)) {
+                dev = disk->physpath;
+                break;
+            }
         case PHYSTYPE_VHD:
             if (libxl__blktap_enabled(&gc))
                 dev = libxl__blktap_devpath(&gc, disk->physpath, phystype);
+            else
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "tapdisk2 is required to open a vhd disk\n");
+            break;
+        case PHYSTYPE_QCOW:
+        case PHYSTYPE_QCOW2:
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally attach a qcow or qcow2 disk image\n");
             break;
 
         default:
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk physical type: %d\n", phystype);
             break;
     }
-    ret = strdup(dev);
+    if (dev != NULL)
+        ret = strdup(dev);
     libxl__free_all(&gc);
     return ret;
 }

Re: Seg fault running xl create with 4.1.0-rc2

[Ian Jackson]

Stefano Stabellini writes ("Re: [Xen-devel] Seg fault running xl create with 4.1.0-rc2"):
> On Thu, 27 Jan 2011, M A Young wrote:
> > then xl segfaults at the line
> >      ret = strdup(dev);

Well spotted.

> Thanks for the patch.
> I extended it a bit, covering qcow and qcow2 as well:

Thanks, I have applied it.

Ian.