Re: [PATCH 2/5] Xl interface change plus changes to code it impacts

[Ian Jackson <Ian.Jackson@eu.citrix.com>]

Kamala Narasimhan writes ("Re: [xen-devel][PATCH 2/5] Xl interface change plus changes to code it impacts"):
> Attached patch should address your concerns.

Thanks for this.  However, there were other changes made between
this most recent version and the previous version, besides the ones
I mentioned in my comments and which you said you'd address.

When you post an updated version of a patch you should state
separately
 - what the patch does to the tree, including intended changes,
   motivation, etc., as you have done (for the commit message)
 - how the patch differs from the previous version, if applicable

Can you explain what the changes you made and why you made them ?
Ideally in general you should use revision control system tools to
make sure that you know what they all are.

But, for your info here is the output of
  diff -w --exclude=\*{~,.o,.d,.opic} -ru A B |grep -v '^Only in'

Most of this is formatting noise, which addresses my comments.

However, you have also:
  * Initialised disk->format and disk->backend somewhere you
    previously didn't
  * Recognised the "tap2" prefix in a place you previously didn't
  * Changed the handling of the "aio" and "raw" prefixes

Normally patch such as this one, which is presented as being mature,
ought not to need unexplained semantic changes at this late stage.

Ian.

diff -w --exclude='*~' --exclude='*.o' --exclude='*.d' --exclude='*.opic' -ru tools/libxl/libxl.c /u/iwj/work/xen-unstable-tools.hg/tools/libxl/libxl.c
--- tools/libxl/libxl.c	2011-02-15 19:13:12.000000000 +0000
+++ /u/iwj/work/xen-unstable-tools.hg/tools/libxl/libxl.c	2011-02-15 19:12:48.000000000 +0000
@@ -931,7 +931,7 @@
     device.kind = DEVICE_VBD;
 
     switch (disk->backend) {
-        case DISK_BACKEND_PHY: {
+        case DISK_BACKEND_PHY: 
             libxl__device_physdisk_major_minor(disk->pdev_path, &major, &minor);
             flexarray_append(back, "physical-device");
             flexarray_append(back, libxl__sprintf(&gc, "%x:%x", major, minor));
@@ -941,9 +941,8 @@
 
             device.backend_kind = DEVICE_VBD;
             break;
-        }
         case DISK_BACKEND_TAP:
-        case DISK_BACKEND_QDISK: {
+        case DISK_BACKEND_QDISK: 
             if (disk->format == DISK_FORMAT_EMPTY)
                 break;
             if (libxl__blktap_enabled(&gc)) {
@@ -972,12 +971,11 @@
                           libxl__device_disk_string_of_format(disk->format), disk->pdev_path));
 
             if (libxl__blktap_enabled(&gc) && 
-                 disk->format != DISK_BACKEND_QDISK)
+                 disk->backend != DISK_BACKEND_QDISK)
                 device.backend_kind = DEVICE_TAP;
             else
                 device.backend_kind = DEVICE_QDISK;
             break;
-        }
         default:
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n", disk->backend);
             rc = ERROR_INVAL;
@@ -1053,7 +1051,7 @@
     char *ret = NULL;
 
     switch (disk->backend) {
-        case DISK_BACKEND_PHY: {
+        case DISK_BACKEND_PHY: 
             if (disk->format != DISK_FORMAT_RAW) {
                 LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "physical block device must"
                     " be raw");
@@ -1063,8 +1061,7 @@
                 disk->pdev_path);
             dev = disk->pdev_path;
             break;
-        }
-        case DISK_BACKEND_TAP: {
+        case DISK_BACKEND_TAP: 
             if (disk->format == DISK_FORMAT_VHD || disk->format == DISK_FORMAT_RAW)
             {
                 if (libxl__blktap_enabled(&gc))
@@ -1091,8 +1088,7 @@
                     "type: %d", disk->backend);
                 break;
             }
-        }
-        case DISK_BACKEND_QDISK: {
+        case DISK_BACKEND_QDISK: 
             if (disk->format != DISK_FORMAT_RAW) {
                 LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally attach a qdisk "
                     "image if the format is not raw");
@@ -1102,15 +1098,12 @@
                 disk->pdev_path);
             dev = disk->pdev_path;
             break;
-
-        }
         case DISK_BACKEND_UNKNOWN:
-        default: {
+        default: 
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend "
                 "type: %d", disk->backend);
             break;
         }
-    }
 
     if (dev != NULL)
         ret = strdup(dev);
diff -w --exclude='*~' --exclude='*.o' --exclude='*.d' --exclude='*.opic' -ru tools/libxl/xl_cmdimpl.c /u/iwj/work/xen-unstable-tools.hg/tools/libxl/xl_cmdimpl.c
--- tools/libxl/xl_cmdimpl.c	2011-02-15 19:13:12.000000000 +0000
+++ /u/iwj/work/xen-unstable-tools.hg/tools/libxl/xl_cmdimpl.c	2011-02-15 19:12:48.000000000 +0000
@@ -452,6 +452,8 @@
     char *p, *end, *tok;
 
     memset(disk, 0, sizeof(*disk));
+    disk->format = DISK_FORMAT_RAW;
+    disk->backend = DISK_BACKEND_TAP;
 
     for(tok = p = buf2, end = buf2 + strlen(buf2) + 1; p < end; p++) {
         switch(state){
@@ -466,7 +468,8 @@
                     state = DSTATE_PHYSPATH;
                     disk->format = DISK_FORMAT_RAW;
                     disk->backend = DISK_BACKEND_TAP;
-                }else if (!strcmp(tok, "tap")) {
+                }else if ((!strcmp(tok, "tap")) ||
+                          (!strcmp(tok, "tap2"))) {
                     state = DSTATE_TAP;
                 }else{
                     fprintf(stderr, "Unknown disk type: %s\n", tok);
@@ -485,9 +488,10 @@
             if ( *p == ':' ) {
                 *p = '\0';
                 if (!strcmp(tok, "aio")) {
-                    disk->format = DISK_FORMAT_RAW;
-                    disk->backend = DISK_BACKEND_TAP;
-                }else if (!strcmp(tok, "vhd")) {
+                    tok = p + 1;
+                    break;
+                }
+                if (!strcmp(tok, "vhd")) {
                     disk->format = DISK_FORMAT_VHD;
                     disk->backend = DISK_BACKEND_TAP;
                 }else if (!strcmp(tok, "qcow")) {
@@ -496,7 +500,11 @@
                 }else if (!strcmp(tok, "qcow2")) {
                     disk->format = DISK_FORMAT_QCOW2;
                     disk->backend = DISK_BACKEND_QDISK;
-                }else {
+                }else if (!strcmp(tok, "raw")) {
+                    disk->format = DISK_FORMAT_RAW;
+                    disk->backend = DISK_BACKEND_TAP;
+                }
+                else {
                     fprintf(stderr, "Unknown tapdisk type: %s\n", tok);
                     return 0;
                 }