[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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;
                 }

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.