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

[libvirt] Move esxVMX_IndexToDiskName to util.c



I needed the inverse function to virDiskNameToIndex() for the ESX
driver and added it to esx_vmx.c. The pending VirtualBox 3.1 patch
needs disk index to disk name mapping too. So I want to move
esxVMX_IndexToDiskName() to util.c.

esxVMX_IndexToDiskName() handles indices up to 701. This limit comes
from a gap in the disk name to disk index mapping of
virDiskNameToIndex():

sdzy -> 700
sdzz -> 701
sdaaa -> 728
sdaab -> 729

This line in virDiskNameToIndex() causes this gap:

idx = (idx + i) * 26;

It can be fixed by altering this line to:

idx = (idx + (i < 1 ? 0 : 1)) * 26;

But this change breaks compatibility for indices > 701.

So, I made two patches for either option and ask you for comments.

Matthias
diff --git a/src/esx/esx_vmx.c b/src/esx/esx_vmx.c
index 9a9fe0a..a167513 100644
--- a/src/esx/esx_vmx.c
+++ b/src/esx/esx_vmx.c
@@ -1211,31 +1211,6 @@ esxVMX_ParseSCSIController(virConnectPtr conn, virConfPtr conf, int controller,
 
 
 
-char *
-esxVMX_IndexToDiskName(virConnectPtr conn, int idx, const char *prefix)
-{
-    char *name = NULL;
-
-    if (idx < 0) {
-        ESX_ERROR(conn, VIR_ERR_INTERNAL_ERROR,
-                  "Disk index %d is negative", idx);
-    } else if (idx < 26) {
-        if (virAsprintf(&name, "%s%c", prefix, 'a' + idx) < 0)
-            virReportOOMError(conn);
-    } else if (idx < 702) {
-        if (virAsprintf(&name, "%s%c%c", prefix, 'a' + idx / 26 - 1,
-                        'a' + (idx % 26)) < 0)
-            virReportOOMError(conn);
-    } else {
-        ESX_ERROR(conn, VIR_ERR_INTERNAL_ERROR,
-                  "Disk index %d is too large", idx);
-    }
-
-    return name;
-}
-
-
-
 /*
 struct _virDomainDiskDef {
     int type;               // partly done
@@ -1337,8 +1312,8 @@ esxVMX_ParseDisk(virConnectPtr conn, esxVI_Context *ctx, virConfPtr conf,
             }
 
             (*def)->dst =
-               esxVMX_IndexToDiskName
-                 (conn, controller * 15 + (id < 7 ? id : id - 1), "sd");
+               virIndexToDiskName
+                 (controller * 15 + (id < 7 ? id : id - 1), "sd");
 
             if ((*def)->dst == NULL) {
                 goto failure;
@@ -1371,8 +1346,7 @@ esxVMX_ParseDisk(virConnectPtr conn, esxVI_Context *ctx, virConfPtr conf,
                 goto failure;
             }
 
-            (*def)->dst = esxVMX_IndexToDiskName(conn, controller * 2 + id,
-                                                 "hd");
+            (*def)->dst = virIndexToDiskName(controller * 2 + id, "hd");
 
             if ((*def)->dst == NULL) {
                 goto failure;
@@ -1398,7 +1372,7 @@ esxVMX_ParseDisk(virConnectPtr conn, esxVI_Context *ctx, virConfPtr conf,
                 goto failure;
             }
 
-            (*def)->dst = esxVMX_IndexToDiskName(conn, controller, "fd");
+            (*def)->dst = virIndexToDiskName(controller, "fd");
 
             if ((*def)->dst == NULL) {
                 goto failure;
diff --git a/src/esx/esx_vmx.h b/src/esx/esx_vmx.h
index 4e11ae6..9a66e49 100644
--- a/src/esx/esx_vmx.h
+++ b/src/esx/esx_vmx.h
@@ -28,9 +28,6 @@
 #include "domain_conf.h"
 #include "esx_vi.h"
 
-char *
-esxVMX_IndexToDiskName(virConnectPtr conn, int idx, const char *prefix);
-
 int
 esxVMX_SCSIDiskNameToControllerAndID(virConnectPtr conn, const char *name,
                                      int *controller, int *id);
@@ -53,6 +50,7 @@ esxVMX_AbsolutePathToDatastoreRelatedPath(virConnectPtr conn,
                                           const char *absolutePath);
 
 
+
 /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
  * VMX -> Domain XML
  */
diff --git a/src/util/util.c b/src/util/util.c
index e472e0c..3a85c52 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -1789,7 +1789,7 @@ int virDiskNameToIndex(const char *name) {
         return -1;
 
     for (i = 0; *ptr; i++) {
-        idx = (idx + i) * 26;
+        idx = (idx + (i < 1 ? 0 : 1)) * 26;
 
         if (!c_islower(*ptr))
             return -1;
@@ -1801,6 +1801,36 @@ int virDiskNameToIndex(const char *name) {
     return idx;
 }
 
+char *virIndexToDiskName(int idx, const char *prefix)
+{
+    char *name = NULL;
+    int i, k, offset;
+
+    if (idx < 0) {
+        ReportError(NULL, VIR_ERR_INTERNAL_ERROR,
+                    _("Disk index %d is negative"), idx);
+        return NULL;
+    }
+
+    for (i = 0, k = idx; k >= 0; ++i, k = k / 26 - 1) { }
+
+    offset = strlen(prefix);
+
+    if (VIR_ALLOC_N(name, offset + i + 1)) {
+        virReportOOMError(NULL);
+        return NULL;
+    }
+
+    strcpy(name, prefix);
+    name[offset + i] = '\0';
+
+    for (i = i - 1, k = idx; k >= 0; --i, k = k / 26 - 1) {
+        name[offset + i] = 'a' + (k % 26);
+    }
+
+    return name;
+}
+
 #ifndef AI_CANONIDN
 #define AI_CANONIDN 0
 #endif
diff --git a/src/util/util.h b/src/util/util.h
index 8c9d401..49b27f2 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -186,7 +186,7 @@ void virGenerateMacAddr(const unsigned char *prefix,
                         unsigned char *addr);
 
 int virDiskNameToIndex(const char* str);
-
+char *virIndexToDiskName(int idx, const char *prefix);
 
 int virEnumFromString(const char *const*types,
                       unsigned int ntypes,
diff --git a/tests/esxutilstest.c b/tests/esxutilstest.c
index 5633ee2..dfe8390 100644
--- a/tests/esxutilstest.c
+++ b/tests/esxutilstest.c
@@ -9,8 +9,8 @@
 #include "internal.h"
 #include "memory.h"
 #include "testutils.h"
+#include "util.h"
 #include "esx/esx_util.h"
-#include "esx/esx_vmx.h"
 
 static char *progname;
 
@@ -47,7 +47,7 @@ testIndexToDiskName(const void *data ATTRIBUTE_UNUSED)
     for (i = 0; i < ARRAY_CARDINALITY(names); ++i) {
         VIR_FREE(name);
 
-        name = esxVMX_IndexToDiskName(NULL, i, "sd");
+        name = virIndexToDiskName(i, "sd");
 
         if (STRNEQ(names[i], name)) {
             virtTestDifference(stderr, names[i], name);
diff --git a/src/esx/esx_vmx.c b/src/esx/esx_vmx.c
index 9a9fe0a..a167513 100644
--- a/src/esx/esx_vmx.c
+++ b/src/esx/esx_vmx.c
@@ -1211,31 +1211,6 @@ esxVMX_ParseSCSIController(virConnectPtr conn, virConfPtr conf, int controller,
 
 
 
-char *
-esxVMX_IndexToDiskName(virConnectPtr conn, int idx, const char *prefix)
-{
-    char *name = NULL;
-
-    if (idx < 0) {
-        ESX_ERROR(conn, VIR_ERR_INTERNAL_ERROR,
-                  "Disk index %d is negative", idx);
-    } else if (idx < 26) {
-        if (virAsprintf(&name, "%s%c", prefix, 'a' + idx) < 0)
-            virReportOOMError(conn);
-    } else if (idx < 702) {
-        if (virAsprintf(&name, "%s%c%c", prefix, 'a' + idx / 26 - 1,
-                        'a' + (idx % 26)) < 0)
-            virReportOOMError(conn);
-    } else {
-        ESX_ERROR(conn, VIR_ERR_INTERNAL_ERROR,
-                  "Disk index %d is too large", idx);
-    }
-
-    return name;
-}
-
-
-
 /*
 struct _virDomainDiskDef {
     int type;               // partly done
@@ -1337,8 +1312,8 @@ esxVMX_ParseDisk(virConnectPtr conn, esxVI_Context *ctx, virConfPtr conf,
             }
 
             (*def)->dst =
-               esxVMX_IndexToDiskName
-                 (conn, controller * 15 + (id < 7 ? id : id - 1), "sd");
+               virIndexToDiskName
+                 (controller * 15 + (id < 7 ? id : id - 1), "sd");
 
             if ((*def)->dst == NULL) {
                 goto failure;
@@ -1371,8 +1346,7 @@ esxVMX_ParseDisk(virConnectPtr conn, esxVI_Context *ctx, virConfPtr conf,
                 goto failure;
             }
 
-            (*def)->dst = esxVMX_IndexToDiskName(conn, controller * 2 + id,
-                                                 "hd");
+            (*def)->dst = virIndexToDiskName(controller * 2 + id, "hd");
 
             if ((*def)->dst == NULL) {
                 goto failure;
@@ -1398,7 +1372,7 @@ esxVMX_ParseDisk(virConnectPtr conn, esxVI_Context *ctx, virConfPtr conf,
                 goto failure;
             }
 
-            (*def)->dst = esxVMX_IndexToDiskName(conn, controller, "fd");
+            (*def)->dst = virIndexToDiskName(controller, "fd");
 
             if ((*def)->dst == NULL) {
                 goto failure;
diff --git a/src/esx/esx_vmx.h b/src/esx/esx_vmx.h
index 4e11ae6..9a66e49 100644
--- a/src/esx/esx_vmx.h
+++ b/src/esx/esx_vmx.h
@@ -28,9 +28,6 @@
 #include "domain_conf.h"
 #include "esx_vi.h"
 
-char *
-esxVMX_IndexToDiskName(virConnectPtr conn, int idx, const char *prefix);
-
 int
 esxVMX_SCSIDiskNameToControllerAndID(virConnectPtr conn, const char *name,
                                      int *controller, int *id);
@@ -53,6 +50,7 @@ esxVMX_AbsolutePathToDatastoreRelatedPath(virConnectPtr conn,
                                           const char *absolutePath);
 
 
+
 /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
  * VMX -> Domain XML
  */
diff --git a/src/util/util.c b/src/util/util.c
index e472e0c..f5e4b7c 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -1801,6 +1801,55 @@ int virDiskNameToIndex(const char *name) {
     return idx;
 }
 
+char *virIndexToDiskName(int idx, const char *prefix)
+{
+    char *name = NULL;
+
+    if (idx < 0) {
+        ReportError(NULL, VIR_ERR_INTERNAL_ERROR,
+                    _("Disk index %d is negative"), idx);
+    } else if (idx < 26) {
+        if (virAsprintf(&name, "%s%c", prefix, 'a' + idx) < 0) {
+            virReportOOMError(NULL);
+        }
+    } else if (idx < 702) {
+        if (virAsprintf(&name, "%s%c%c", prefix, 'a' + idx / 26 - 1,
+                        'a' + (idx % 26)) < 0) {
+            virReportOOMError(NULL);
+        }
+    } else {
+        /*
+         * virDiskNameToIndex produces this mapping:
+         *
+         *   ...
+         *    sdzy -> 700
+         *    sdzz -> 701
+         *   sdaaa -> 728
+         *   sdaab -> 729
+         *   ...
+         *
+         * There is a gap in this mapping, there is no disk name for the
+         * indices [702..727].
+         *
+         * The gap is caused by this line in virDiskNameToIndex:
+         *
+         *   idx = (idx + i) * 26;
+         *
+         * Changing it to this line would close the gap:
+         *
+         *   idx = (idx + (i < 1 ? 0 : 1)) * 26;
+         *
+         * But this change cannot be applied, because it would break
+         * compatibility due to the changed mapping: sdaaa would map to 702
+         * instead of 728.
+         */
+        ReportError(NULL, VIR_ERR_INTERNAL_ERROR,
+                    _("Disk index %d is too large"), idx);
+    }
+
+    return name;
+}
+
 #ifndef AI_CANONIDN
 #define AI_CANONIDN 0
 #endif
diff --git a/src/util/util.h b/src/util/util.h
index 8c9d401..49b27f2 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -186,7 +186,7 @@ void virGenerateMacAddr(const unsigned char *prefix,
                         unsigned char *addr);
 
 int virDiskNameToIndex(const char* str);
-
+char *virIndexToDiskName(int idx, const char *prefix);
 
 int virEnumFromString(const char *const*types,
                       unsigned int ntypes,
diff --git a/tests/esxutilstest.c b/tests/esxutilstest.c
index 5633ee2..dfe8390 100644
--- a/tests/esxutilstest.c
+++ b/tests/esxutilstest.c
@@ -9,8 +9,8 @@
 #include "internal.h"
 #include "memory.h"
 #include "testutils.h"
+#include "util.h"
 #include "esx/esx_util.h"
-#include "esx/esx_vmx.h"
 
 static char *progname;
 
@@ -47,7 +47,7 @@ testIndexToDiskName(const void *data ATTRIBUTE_UNUSED)
     for (i = 0; i < ARRAY_CARDINALITY(names); ++i) {
         VIR_FREE(name);
 
-        name = esxVMX_IndexToDiskName(NULL, i, "sd");
+        name = virIndexToDiskName(i, "sd");
 
         if (STRNEQ(names[i], name)) {
             virtTestDifference(stderr, names[i], name);

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