5:30 a.m.
From: Martin Kletzander <mkletzan(a)redhat.com>
We do not guarantee that the disk names will be the same in guest as
they are in the XML, but that should not stop us from trying to be
consistent with the naming. And since we use the same naming as the
linux kernel does, let's stick with it with nvme drives too.
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
src/libxl/libxl_driver.c | 2 +-
src/qemu/qemu_validate.c | 2 +-
src/util/virutil.c | 94 +++++++++++++++++++++++++++++++++++++---
src/util/virutil.h | 7 ++-
src/vbox/vbox_common.c | 2 +-
src/vmx/vmx.c | 2 +-
src/vz/vz_sdk.c | 6 +--
tests/utiltest.c | 38 +++++++++++-----
8 files changed, 126 insertions(+), 27 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 0fb256e5c06d..308c0372aaf4 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -5440,7 +5440,7 @@ libxlDiskPathToID(const char *virtpath)
/* Find any disk prefixes we know about */
for (i = 0; i < G_N_ELEMENTS(drive_prefix); i++) {
if (STRPREFIX(virtpath, drive_prefix[i]) &&
- !virDiskNameParse(virtpath, &disk, &partition)) {
+ !virDiskNameParse(virtpath, NULL, &disk, &partition)) {
fmt = i;
break;
}
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 23a4d70b3441..8acc44747456 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -3445,7 +3445,7 @@ qemuValidateDomainDeviceDefDisk(const virDomainDiskDef *disk,
return -1;
}
- if (virDiskNameParse(disk->dst, &idx, &partition) < 0) {
+ if (virDiskNameParse(disk->dst, NULL, &idx, &partition) < 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("invalid disk target '%1$s'"), disk->dst);
return -1;
diff --git a/src/util/virutil.c b/src/util/virutil.c
index 2abcb282fe52..fb64237692bb 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -314,15 +314,82 @@ virFormatIntPretty(unsigned long long val,
}
+static int
+virDiskNameParseNvme(const char *name, int *controller, int *disk, int *partition)
+{
+ int ctrl = 0;
+ int ns = 0;
+ int part = 0;
+ char *end_ptr = NULL;
+ const char *tmp = STRSKIP(name, "nvme");
+
+ if (!controller)
+ return -1;
+
+ if (!tmp)
+ return -1;
+
+ if (virStrToLong_i(tmp, &end_ptr, 10, &ctrl) < 0)
+ return -1;
+
+ if (*end_ptr != 'n')
+ return -1;
+
+ if (ctrl < 0)
+ return -1;
+
+ tmp = end_ptr + 1;
+
+ if (virStrToLong_i(tmp, &end_ptr, 10, &ns) < 0)
+ return -1;
+
+ if (*end_ptr != '\0' && *end_ptr != 'p')
+ return -1;
+
+ /* NSIDs start from 1, but we need to map them to [0..] for the controller
+ * address. */
+ if (ns < 1)
+ return -1;
+ ns--;
+
+ if (partition) {
+ *partition = 0;
+
+ if (*end_ptr != '\0') {
+ tmp = end_ptr + 1;
+
+ if (virStrToLong_i(tmp, NULL, 10, &part) < 0)
+ return -1;
+
+ /* Partitions on NVMe disks also from 1 */
+ if (part < 1)
+ return -1;
+
+ *partition = part - 1;
+ }
+ }
+
+ *controller = ctrl;
+ *disk = ns;
+
+ return 0;
+}
+
+
/* Translates a device name of the form (regex) /^[fhv]d[a-z]+[0-9]*$/
- * into the corresponding index and partition number
- * (e.g. sda0 => (0,0), hdz2 => (25,2), vdaa12 => (26,12))
+ * or nvme[0-9]+n[0-9]+(p[0-9]+) for NVMe devices
+ * into the corresponding nvme controller (see below), index and partition number
+ * (e.g. sda0 => (0,0,0), hdz2 => (0,25,2), vdaa12 => (0,26,12)) as these do
not
+ * support namespaces and controller is calculated from the disk index
+ * for nvme disks disks: (nvme1n2p3 => (1, 1, 2) as nvme namespaces and
+ * partitions are numbered from 1 and not 0).
* @param name The name of the device
+ * @param nvme_ctrl The disk namespace to be returned
* @param disk The disk index to be returned
* @param partition The partition index to be returned
* @return 0 on success, or -1 on failure
*/
-int virDiskNameParse(const char *name, int *disk, int *partition)
+int virDiskNameParse(const char *name, int *nvme_ctrl, int *disk, int *partition)
{
const char *ptr = NULL;
char *rem;
@@ -331,6 +398,12 @@ int virDiskNameParse(const char *name, int *disk, int *partition)
size_t i;
size_t n_digits;
+ if (STRPREFIX(name, "nvme"))
+ return virDiskNameParseNvme(name, nvme_ctrl, disk, partition);
+
+ if (nvme_ctrl)
+ *nvme_ctrl = 0;
+
for (i = 0; i < G_N_ELEMENTS(drive_prefix); i++) {
if (STRPREFIX(name, drive_prefix[i])) {
ptr = name + strlen(drive_prefix[i]);
@@ -381,6 +454,8 @@ int virDiskNameParse(const char *name, int *disk, int *partition)
/* Translates a device name of the form (regex) /^[fhv]d[a-z]+[0-9]*$/
* into the corresponding index (e.g. sda => 0, hdz => 25, vdaa => 26)
* Note that any trailing string of digits is simply ignored.
+ * Since NVMe disks require controller devices, this function should not be used
+ * for nvme* names unless the caller does not need to know then controller index
* @param name The name of the device
* @return name's index, or -1 on failure
*/
@@ -388,17 +463,24 @@ int virDiskNameToIndex(const char *name)
{
int idx;
- if (virDiskNameParse(name, &idx, NULL) < 0)
+ if (virDiskNameParse(name, NULL, &idx, NULL) < 0)
idx = -1;
return idx;
}
-char *virIndexToDiskName(unsigned int idx, const char *prefix)
+char *virIndexToDiskName(unsigned int nvme_ctrl,
+ unsigned int idx,
+ const char *prefix)
{
- GString *str = g_string_new(NULL);
+ GString *str = NULL;
long long ctr;
+ if (STREQ_NULLABLE(prefix, "nvme"))
+ return g_strdup_printf("nvme%dn%d", nvme_ctrl, idx + 1);
+
+ str = g_string_new(NULL);
+
for (ctr = idx; ctr >= 0; ctr = ctr / 26 - 1)
g_string_prepend_c(str, 'a' + (ctr % 26));
diff --git a/src/util/virutil.h b/src/util/virutil.h
index 6c07aa42c8b5..ca6fd95363b5 100644
--- a/src/util/virutil.h
+++ b/src/util/virutil.h
@@ -51,9 +51,12 @@ unsigned long long
virFormatIntPretty(unsigned long long val,
const char **unit);
-int virDiskNameParse(const char *name, int *disk, int *partition);
+int virDiskNameParse(const char *name, int *nvme_ctrl,
+ int *disk, int *partition);
int virDiskNameToIndex(const char* str);
-char *virIndexToDiskName(unsigned int idx, const char *prefix);
+char *virIndexToDiskName(unsigned int nvme_ctrl,
+ unsigned int idx,
+ const char *prefix);
/* No-op workarounds for functionality missing in mingw. */
#ifndef WITH_GETUID
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 6ffe7cc20bde..95c70671a752 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -447,7 +447,7 @@ vboxGenerateMediumName(PRUint32 storageBus,
return NULL;
}
- name = virIndexToDiskName(total, prefix);
+ name = virIndexToDiskName(0, total, prefix);
return name;
}
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index 0dd03c1a88e7..4a9ad11b42c4 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -2257,7 +2257,7 @@ virVMXGenerateDiskTarget(virDomainDiskDef *def,
return -1;
}
- def->dst = virIndexToDiskName(idx, prefix);
+ def->dst = virIndexToDiskName(0, idx, prefix);
return 0;
}
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index c2226c409dee..684b76ffa057 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -596,16 +596,16 @@ prlsdkGetDiskId(PRL_HANDLE disk, virDomainDiskBus *bus, char **dst)
switch (ifType) {
case PMS_IDE_DEVICE:
*bus = VIR_DOMAIN_DISK_BUS_IDE;
- *dst = virIndexToDiskName(pos, "hd");
+ *dst = virIndexToDiskName(0, pos, "hd");
break;
case PMS_SCSI_DEVICE:
case PMS_UNKNOWN_DEVICE:
*bus = VIR_DOMAIN_DISK_BUS_SCSI;
- *dst = virIndexToDiskName(pos, "sd");
+ *dst = virIndexToDiskName(0, pos, "sd");
break;
case PMS_SATA_DEVICE:
*bus = VIR_DOMAIN_DISK_BUS_SATA;
- *dst = virIndexToDiskName(pos, "sd");
+ *dst = virIndexToDiskName(0, pos, "sd");
break;
default:
virReportError(VIR_ERR_INTERNAL_ERROR,
diff --git a/tests/utiltest.c b/tests/utiltest.c
index b30bfbed70fb..0bef502a84c8 100644
--- a/tests/utiltest.c
+++ b/tests/utiltest.c
@@ -22,19 +22,25 @@ static const char* diskNames[] = {
struct testDiskName
{
const char *name;
+ int ctrl;
int idx;
int partition;
};
static struct testDiskName diskNamesPart[] = {
- {"sda0", 0, 0},
- {"sdb10", 1, 10},
- {"sdc2147483647", 2, 2147483647},
+ {"sda", 0, 0, 0},
+ {"sda0", 0, 0, 0},
+ {"sdb10", 0, 1, 10},
+ {"sdc2147483647", 0, 2, 2147483647},
+ {"nvme0n1", 0, 0, 0},
+ {"nvme0n1p1", 0, 0, 0},
+ {"nvme1n2p3", 1, 1, 2},
};
static const char* diskNamesInvalid[] = {
"sda00", "sda01", "sdb-1",
- "vd2"
+ "vd2",
+ "nvme0n0", "nvme0n1p0",
};
static int
@@ -45,7 +51,7 @@ testIndexToDiskName(const void *data G_GNUC_UNUSED)
for (i = 0; i < G_N_ELEMENTS(diskNames); ++i) {
g_autofree char *diskName = NULL;
- diskName = virIndexToDiskName(i, "sd");
+ diskName = virIndexToDiskName(0, i, "sd");
if (virTestCompareToString(diskNames[i], diskName) < 0) {
return -1;
@@ -66,7 +72,7 @@ testDiskNameToIndex(const void *data G_GNUC_UNUSED)
for (i = 0; i < 100000; ++i) {
g_autofree char *diskName = NULL;
- diskName = virIndexToDiskName(i, "sd");
+ diskName = virIndexToDiskName(0, i, "sd");
idx = virDiskNameToIndex(diskName);
if (idx < 0 || idx != i) {
@@ -85,30 +91,38 @@ static int
testDiskNameParse(const void *data G_GNUC_UNUSED)
{
size_t i;
+ int nvme_ctrl;
int idx;
int partition;
struct testDiskName *disk = NULL;
for (i = 0; i < G_N_ELEMENTS(diskNamesPart); ++i) {
disk = &diskNamesPart[i];
- if (virDiskNameParse(disk->name, &idx, &partition))
+ partition = 0;
+ if (virDiskNameParse(disk->name, &nvme_ctrl, &idx, &partition))
return -1;
+ if (disk->ctrl != nvme_ctrl) {
+ VIR_TEST_DEBUG("\nExpect NVMe controller [%d]", disk->ctrl);
+ VIR_TEST_DEBUG("Actual NVMe controller [%d]", nvme_ctrl);
+ return -1;
+ }
+
if (disk->idx != idx) {
- VIR_TEST_DEBUG("\nExpect [%d]", disk->idx);
- VIR_TEST_DEBUG("Actual [%d]", idx);
+ VIR_TEST_DEBUG("\nExpect index [%d]", disk->idx);
+ VIR_TEST_DEBUG("Actual index [%d]", idx);
return -1;
}
if (disk->partition != partition) {
- VIR_TEST_DEBUG("\nExpect [%d]", disk->partition);
- VIR_TEST_DEBUG("Actual [%d]", partition);
+ VIR_TEST_DEBUG("\nExpect partition [%d]", disk->partition);
+ VIR_TEST_DEBUG("Actual partition [%d]", partition);
return -1;
}
}
for (i = 0; i < G_N_ELEMENTS(diskNamesInvalid); ++i) {
- if (!virDiskNameParse(diskNamesInvalid[i], &idx, &partition)) {
+ if (!virDiskNameParse(diskNamesInvalid[i], &nvme_ctrl, &idx,
&partition)) {
VIR_TEST_DEBUG("Should Fail [%s]", diskNamesInvalid[i]);
return -1;
}
--
2.49.0