From 8cd7196974d9bf56434e78eecc289694d95af4f0 Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Mon, 3 Jun 2019 17:31:13 +0200 Subject: [PATCH] conf: Format and parse NVMe type disk To simplify implementation, some restrictions are added. For instance, an NVMe disk can't go to any bus but virtio and has to be type of 'disk' and can't have startupPolicy set. Signed-off-by: Michal Privoznik Reviewed-by: Cole Robinson --- src/conf/domain_conf.c | 97 ++++++++++++++++++++++++++ src/libvirt_private.syms | 1 + src/libxl/xen_xl.c | 1 + src/qemu/qemu_block.c | 2 + src/qemu/qemu_command.c | 1 + src/qemu/qemu_driver.c | 4 ++ src/qemu/qemu_migration.c | 5 ++ src/util/virstoragefile.c | 57 +++++++++++++++ src/util/virstoragefile.h | 17 +++++ tests/qemuxml2xmloutdata/disk-nvme.xml | 1 + tests/qemuxml2xmltest.c | 1 + 11 files changed, 187 insertions(+) create mode 120000 tests/qemuxml2xmloutdata/disk-nvme.xml diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index cfec86a24d..f60aa7371f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5141,6 +5141,11 @@ virDomainDiskDefPostParse(virDomainDiskDefPtr disk, return -1; } + if (disk->src->type == VIR_STORAGE_TYPE_NVME) { + if (disk->src->nvme->managed == VIR_TRISTATE_BOOL_ABSENT) + disk->src->nvme->managed = VIR_TRISTATE_BOOL_YES; + } + if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && virDomainDiskDefAssignAddress(xmlopt, disk, def) < 0) { return -1; @@ -6025,6 +6030,15 @@ virDomainDiskDefValidate(const virDomainDef *def, return -1; } + if (disk->src->type == VIR_STORAGE_TYPE_NVME) { + /* NVMe namespaces start from 1 */ + if (disk->src->nvme->namespace == 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("NVMe namespace can't be zero")); + return -1; + } + } + for (next = disk->src; next; next = next->backingStore) { if (virSecurityDeviceLabelDefValidateXML(next->seclabels, next->nseclabels, @@ -9290,6 +9304,68 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node, } +static int +virDomainDiskSourceNVMeParse(xmlNodePtr node, + xmlXPathContextPtr ctxt, + virStorageSourcePtr src) +{ + g_autoptr(virStorageSourceNVMeDef) nvme = NULL; + g_autofree char *type = NULL; + g_autofree char *namespace = NULL; + g_autofree char *managed = NULL; + xmlNodePtr address; + + nvme = g_new0(virStorageSourceNVMeDef, 1); + + if (!(type = virXMLPropString(node, "type"))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing 'type' attribute to disk source")); + return -1; + } + + if (STRNEQ(type, "pci")) { + virReportError(VIR_ERR_XML_ERROR, + _("unsupported source type '%s'"), + type); + return -1; + } + + if (!(namespace = virXMLPropString(node, "namespace"))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing 'namespace' attribute to disk source")); + return -1; + } + + if (virStrToLong_ull(namespace, NULL, 10, &nvme->namespace) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("malformed namespace '%s'"), + namespace); + return -1; + } + + if ((managed = virXMLPropString(node, "managed"))) { + if ((nvme->managed = virTristateBoolTypeFromString(managed)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("malformed managed value '%s'"), + managed); + return -1; + } + } + + if (!(address = virXPathNode("./address", ctxt))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("NVMe disk source is missing address")); + return -1; + } + + if (virPCIDeviceAddressParseXML(address, &nvme->pciAddr) < 0) + return -1; + + src->nvme = g_steal_pointer(&nvme); + return 0; +} + + static int virDomainDiskSourcePRParse(xmlNodePtr node, xmlXPathContextPtr ctxt, @@ -9388,6 +9464,10 @@ virDomainStorageSourceParse(xmlNodePtr node, if (virDomainDiskSourcePoolDefParse(node, &src->srcpool) < 0) return -1; break; + case VIR_STORAGE_TYPE_NVME: + if (virDomainDiskSourceNVMeParse(node, ctxt, src) < 0) + return -1; + break; case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, @@ -24115,6 +24195,19 @@ virDomainDiskSourceFormatNetwork(virBufferPtr attrBuf, } +static void +virDomainDiskSourceNVMeFormat(virBufferPtr attrBuf, + virBufferPtr childBuf, + const virStorageSourceNVMeDef *nvme) +{ + virBufferAddLit(attrBuf, " type='pci'"); + virBufferAsprintf(attrBuf, " managed='%s'", + virTristateBoolTypeToString(nvme->managed)); + virBufferAsprintf(attrBuf, " namespace='%llu'", nvme->namespace); + virPCIDeviceAddressFormat(childBuf, nvme->pciAddr, false); +} + + static int virDomainDiskSourceFormatPrivateData(virBufferPtr buf, virStorageSourcePtr src, @@ -24191,6 +24284,10 @@ virDomainDiskSourceFormat(virBufferPtr buf, break; + case VIR_STORAGE_TYPE_NVME: + virDomainDiskSourceNVMeFormat(&attrBuf, &childBuf, src->nvme); + break; + case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 50e63b2492..19d2c67689 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3095,6 +3095,7 @@ virStorageSourceNetworkAssignDefaultPorts; virStorageSourceNew; virStorageSourceNewFromBacking; virStorageSourceNewFromBackingAbsolute; +virStorageSourceNVMeDefFree; virStorageSourceParseRBDColonString; virStorageSourcePoolDefFree; virStorageSourcePoolModeTypeFromString; diff --git a/src/libxl/xen_xl.c b/src/libxl/xen_xl.c index 8112214b5f..ebcea41d1c 100644 --- a/src/libxl/xen_xl.c +++ b/src/libxl/xen_xl.c @@ -1642,6 +1642,7 @@ xenFormatXLDiskSrc(virStorageSourcePtr src, char **srcstr) break; case VIR_STORAGE_TYPE_VOLUME: + case VIR_STORAGE_TYPE_NVME: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: break; diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 629a09b897..757023f3e3 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1060,6 +1060,7 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src, break; case VIR_STORAGE_TYPE_VOLUME: + case VIR_STORAGE_TYPE_NVME: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: return NULL; @@ -2331,6 +2332,7 @@ qemuBlockStorageSourceCreateGetStorageProps(virStorageSourcePtr src, case VIR_STORAGE_TYPE_BLOCK: case VIR_STORAGE_TYPE_DIR: case VIR_STORAGE_TYPE_VOLUME: + case VIR_STORAGE_TYPE_NVME: return 0; case VIR_STORAGE_TYPE_NONE: diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0d67e8b501..f93d86d8d4 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1127,6 +1127,7 @@ qemuGetDriveSourceString(virStorageSourcePtr src, break; case VIR_STORAGE_TYPE_VOLUME: + case VIR_STORAGE_TYPE_NVME: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: break; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7a7f388767..c4f63184e5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14838,6 +14838,7 @@ qemuDomainSnapshotPrepareDiskExternalInactive(virDomainSnapshotDiskDefPtr snapdi case VIR_STORAGE_TYPE_DIR: case VIR_STORAGE_TYPE_VOLUME: + case VIR_STORAGE_TYPE_NVME: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, @@ -14854,6 +14855,7 @@ qemuDomainSnapshotPrepareDiskExternalInactive(virDomainSnapshotDiskDefPtr snapdi case VIR_STORAGE_TYPE_NETWORK: case VIR_STORAGE_TYPE_DIR: case VIR_STORAGE_TYPE_VOLUME: + case VIR_STORAGE_TYPE_NVME: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, @@ -14921,6 +14923,7 @@ qemuDomainSnapshotPrepareDiskExternalActive(virDomainSnapshotDiskDefPtr snapdisk case VIR_STORAGE_TYPE_DIR: case VIR_STORAGE_TYPE_VOLUME: + case VIR_STORAGE_TYPE_NVME: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, @@ -15048,6 +15051,7 @@ qemuDomainSnapshotPrepareDiskInternal(virDomainDiskDefPtr disk, case VIR_STORAGE_TYPE_DIR: case VIR_STORAGE_TYPE_VOLUME: + case VIR_STORAGE_TYPE_NVME: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 2251742b3c..29d228a8d9 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -227,6 +227,7 @@ qemuMigrationDstPrecreateDisk(virConnectPtr conn, case VIR_STORAGE_TYPE_BLOCK: case VIR_STORAGE_TYPE_DIR: + case VIR_STORAGE_TYPE_NVME: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, @@ -1288,6 +1289,10 @@ qemuMigrationSrcIsSafe(virDomainDefPtr def, /* But network disks are safe again. */ continue; + case VIR_STORAGE_TYPE_NVME: + unsafe = true; + break; + case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_BLOCK: case VIR_STORAGE_TYPE_DIR: diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 14e1efc9e8..480f0d7db4 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -56,6 +56,7 @@ VIR_ENUM_IMPL(virStorage, "dir", "network", "volume", + "nvme", ); VIR_ENUM_IMPL(virStorageFileFormat, @@ -2092,6 +2093,49 @@ virStoragePRDefCopy(virStoragePRDefPtr src) } +static virStorageSourceNVMeDefPtr +virStorageSourceNVMeDefCopy(const virStorageSourceNVMeDef *src) +{ + virStorageSourceNVMeDefPtr ret = NULL; + + ret = g_new0(virStorageSourceNVMeDef, 1); + + ret->namespace = src->namespace; + ret->managed = src->managed; + virPCIDeviceAddressCopy(&ret->pciAddr, &src->pciAddr); + return ret; +} + + +static bool +virStorageSourceNVMeDefIsEqual(const virStorageSourceNVMeDef *a, + const virStorageSourceNVMeDef *b) +{ + if (!a && !b) + return true; + + if (!a || !b) + return false; + + if (a->namespace != b->namespace || + a->managed != b->managed || + !virPCIDeviceAddressEqual(&a->pciAddr, &b->pciAddr)) + return false; + + return true; +} + + +void +virStorageSourceNVMeDefFree(virStorageSourceNVMeDefPtr def) +{ + if (!def) + return; + + VIR_FREE(def); +} + + virSecurityDeviceLabelDefPtr virStorageSourceGetSecurityLabelDef(virStorageSourcePtr src, const char *model) @@ -2290,6 +2334,9 @@ virStorageSourceCopy(const virStorageSource *src, !(def->pr = virStoragePRDefCopy(src->pr))) return NULL; + if (src->nvme) + def->nvme = virStorageSourceNVMeDefCopy(src->nvme); + if (virStorageSourceInitiatorCopy(&def->initiator, &src->initiator) < 0) return NULL; @@ -2348,6 +2395,10 @@ virStorageSourceIsSameLocation(virStorageSourcePtr a, } } + if (a->type == VIR_STORAGE_TYPE_NVME && + !virStorageSourceNVMeDefIsEqual(a->nvme, b->nvme)) + return false; + return true; } @@ -2430,6 +2481,9 @@ virStorageSourceIsLocalStorage(const virStorageSource *src) case VIR_STORAGE_TYPE_NETWORK: case VIR_STORAGE_TYPE_VOLUME: + /* While NVMe disks are local, they are not accessible via src->path. + * Therefore, we have to return false here. */ + case VIR_STORAGE_TYPE_NVME: case VIR_STORAGE_TYPE_LAST: case VIR_STORAGE_TYPE_NONE: return false; @@ -2515,6 +2569,7 @@ virStorageSourceClear(virStorageSourcePtr def) VIR_FREE(def->compat); virStorageEncryptionFree(def->encryption); virStoragePRDefFree(def->pr); + virStorageSourceNVMeDefFree(def->nvme); virStorageSourceSeclabelsClear(def); virStoragePermsFree(def->perms); VIR_FREE(def->timestamps); @@ -3770,6 +3825,7 @@ virStorageSourceUpdatePhysicalSize(virStorageSourcePtr src, /* We shouldn't get VOLUME, but the switch requires all cases */ case VIR_STORAGE_TYPE_VOLUME: + case VIR_STORAGE_TYPE_NVME: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: return -1; @@ -4226,6 +4282,7 @@ virStorageSourceIsRelative(virStorageSourcePtr src) case VIR_STORAGE_TYPE_NETWORK: case VIR_STORAGE_TYPE_VOLUME: + case VIR_STORAGE_TYPE_NVME: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: return false; diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index c357803e50..42ca49bc64 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -30,6 +30,7 @@ #include "virutil.h" #include "virsecret.h" #include "virenum.h" +#include "virpci.h" /* Minimum header size required to probe all known formats with * virStorageFileProbeFormat, or obtain metadata from a known format. @@ -51,6 +52,7 @@ typedef enum { VIR_STORAGE_TYPE_DIR, VIR_STORAGE_TYPE_NETWORK, VIR_STORAGE_TYPE_VOLUME, + VIR_STORAGE_TYPE_NVME, VIR_STORAGE_TYPE_LAST } virStorageType; @@ -230,6 +232,16 @@ struct _virStorageSourceInitiatorDef { char *iqn; /* Initiator IQN */ }; +typedef struct _virStorageSourceNVMeDef virStorageSourceNVMeDef; +typedef virStorageSourceNVMeDef *virStorageSourceNVMeDefPtr; +struct _virStorageSourceNVMeDef { + unsigned long long namespace; + int managed; /* enum virTristateBool */ + virPCIDeviceAddress pciAddr; + + /* Don't forget to update virStorageSourceNVMeDefCopy */ +}; + typedef struct _virStorageDriverData virStorageDriverData; typedef virStorageDriverData *virStorageDriverDataPtr; @@ -261,6 +273,8 @@ struct _virStorageSource { bool encryptionInherited; virStoragePRDefPtr pr; + virStorageSourceNVMeDefPtr nvme; /* type == VIR_STORAGE_TYPE_NVME */ + virStorageSourceInitiatorDef initiator; virObjectPtr privateData; @@ -416,6 +430,9 @@ bool virStoragePRDefIsManaged(virStoragePRDefPtr prd); bool virStorageSourceChainHasManagedPR(virStorageSourcePtr src); +void virStorageSourceNVMeDefFree(virStorageSourceNVMeDefPtr def); +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virStorageSourceNVMeDef, virStorageSourceNVMeDefFree); + virSecurityDeviceLabelDefPtr virStorageSourceGetSecurityLabelDef(virStorageSourcePtr src, const char *model); diff --git a/tests/qemuxml2xmloutdata/disk-nvme.xml b/tests/qemuxml2xmloutdata/disk-nvme.xml new file mode 120000 index 0000000000..ea9eb267ac --- /dev/null +++ b/tests/qemuxml2xmloutdata/disk-nvme.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/disk-nvme.xml \ No newline at end of file diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index ce56b2926b..55bbd924fb 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -330,6 +330,7 @@ mymain(void) DO_TEST("disk-network-sheepdog", NONE); DO_TEST("disk-network-vxhs", NONE); DO_TEST("disk-network-tlsx509", NONE); + DO_TEST("disk-nvme", QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_QCOW2_LUKS); DO_TEST("disk-scsi", QEMU_CAPS_SCSI_LSI, QEMU_CAPS_SCSI_MEGASAS, QEMU_CAPS_SCSI_MPTSAS1068, QEMU_CAPS_SCSI_DISK_WWN); DO_TEST("disk-virtio-scsi-reservations",