On 1/4/23 04:29, zhenwei pi wrote:
Support a new device type 'crypto'.
Signed-off-by: zhenwei pi <pizhenwei(a)bytedance.com>
---
src/conf/domain_conf.c | 191 +++++++++++++++++++++++++++++++++
src/conf/domain_conf.h | 40 +++++++
src/conf/domain_postparse.c | 1 +
src/conf/domain_validate.c | 18 ++++
src/conf/virconftypes.h | 2 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_command.c | 1 +
src/qemu/qemu_domain.c | 3 +
src/qemu/qemu_domain_address.c | 26 +++++
src/qemu/qemu_driver.c | 5 +
src/qemu/qemu_hotplug.c | 3 +
src/qemu/qemu_validate.c | 22 ++++
12 files changed, 313 insertions(+)
What I'm missing here is qemuxml2xmltest test case. We surely want to
test whether parsing and formatting of the new XML works.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6c088ff295..74448fe627 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -332,6 +332,7 @@ VIR_ENUM_IMPL(virDomainDevice,
"iommu",
"vsock",
"audio",
+ "crypto",
);
VIR_ENUM_IMPL(virDomainDiskDevice,
@@ -1314,6 +1315,22 @@ VIR_ENUM_IMPL(virDomainVsockModel,
"virtio-non-transitional",
);
+VIR_ENUM_IMPL(virDomainCryptoModel,
+ VIR_DOMAIN_CRYPTO_MODEL_LAST,
+ "virtio",
+);
+
+VIR_ENUM_IMPL(virDomainCryptoType,
+ VIR_DOMAIN_CRYPTO_TYPE_LAST,
+ "qemu",
+);
+
+VIR_ENUM_IMPL(virDomainCryptoBackend,
+ VIR_DOMAIN_CRYPTO_BACKEND_LAST,
+ "builtin",
+ "lkcf",
+);
+
VIR_ENUM_IMPL(virDomainDiskDiscard,
VIR_DOMAIN_DISK_DISCARD_LAST,
"default",
@@ -3464,6 +3481,9 @@ void virDomainDeviceDefFree(virDomainDeviceDef *def)
case VIR_DOMAIN_DEVICE_AUDIO:
virDomainAudioDefFree(def->data.audio);
break;
+ case VIR_DOMAIN_DEVICE_CRYPTO:
+ virDomainCryptoDefFree(def->data.crypto);
+ break;
case VIR_DOMAIN_DEVICE_LAST:
case VIR_DOMAIN_DEVICE_NONE:
break;
@@ -3807,6 +3827,10 @@ void virDomainDefFree(virDomainDef *def)
virDomainPanicDefFree(def->panics[i]);
g_free(def->panics);
+ for (i = 0; i < def->ncryptos; i++)
+ virDomainCryptoDefFree(def->cryptos[i]);
+ g_free(def->cryptos);
+
virDomainIOMMUDefFree(def->iommu);
g_free(def->idmap.uidmap);
@@ -4360,6 +4384,8 @@ virDomainDeviceGetInfo(const virDomainDeviceDef *device)
return &device->data.iommu->info;
case VIR_DOMAIN_DEVICE_VSOCK:
return &device->data.vsock->info;
+ case VIR_DOMAIN_DEVICE_CRYPTO:
+ return &device->data.crypto->info;
/* The following devices do not contain virDomainDeviceInfo */
case VIR_DOMAIN_DEVICE_LEASE:
@@ -4462,6 +4488,9 @@ virDomainDeviceSetData(virDomainDeviceDef *device,
case VIR_DOMAIN_DEVICE_AUDIO:
device->data.audio = devicedata;
break;
+ case VIR_DOMAIN_DEVICE_CRYPTO:
+ device->data.crypto = devicedata;
+ break;
case VIR_DOMAIN_DEVICE_NONE:
case VIR_DOMAIN_DEVICE_LAST:
break;
@@ -4673,6 +4702,13 @@ virDomainDeviceInfoIterateFlags(virDomainDef *def,
return rc;
}
+ device.type = VIR_DOMAIN_DEVICE_CRYPTO;
+ for (i = 0; i < def->ncryptos; i++) {
+ device.data.crypto = def->cryptos[i];
+ if ((rc = cb(def, &device, &def->cryptos[i]->info, opaque)) != 0)
+ return rc;
+ }
+
/* If the flag below is set, make sure @cb can handle @info being NULL */
if (iteratorFlags & DOMAIN_DEVICE_ITERATE_MISSING_INFO) {
device.type = VIR_DOMAIN_DEVICE_GRAPHICS;
@@ -4731,6 +4767,7 @@ virDomainDeviceInfoIterateFlags(virDomainDef *def,
case VIR_DOMAIN_DEVICE_IOMMU:
case VIR_DOMAIN_DEVICE_VSOCK:
case VIR_DOMAIN_DEVICE_AUDIO:
+ case VIR_DOMAIN_DEVICE_CRYPTO:
break;
}
#endif
@@ -13417,6 +13454,94 @@ virDomainVsockDefParseXML(virDomainXMLOption *xmlopt,
return g_steal_pointer(&vsock);
}
+
+static virDomainCryptoDef *
+virDomainCryptoDefParseXML(virDomainXMLOption *xmlopt,
+ xmlNodePtr node,
+ xmlXPathContextPtr ctxt,
+ unsigned int flags)
+{
+ virDomainCryptoDef *def;
+ VIR_XPATH_NODE_AUTORESTORE(ctxt)
+ int nbackends;
+ g_autofree xmlNodePtr *backends = NULL;
+ g_autofree char *model = NULL;
+ g_autofree char *backend = NULL;
+ g_autofree char *type = NULL;
+
+ def = g_new0(virDomainCryptoDef, 1);
+
+ if (!(model = virXMLPropString(node, "model"))) {
+ virReportError(VIR_ERR_XML_ERROR, "%s", _("missing crypto device
model"));
+ goto error;
+ }
+
+ if ((def->model = virDomainCryptoModelTypeFromString(model)) < 0) {
Problem with this is that compiler may decide that def->model is
unsigned (because it's declared as: virDomainCryptoModel model.
Now, if virXXXTypeFromString() fails and returns -1, this is then
typecased into unsigned int (or whatever unsigned type compiler decided
on) and < 0 check is never true.
Fortunately, we have a conencient function for getting attribute values
and translating them into enums: virXMLPropEnum().
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown
crypto model '%s'"), model);
+ goto error;
+ }
+
+ if (!(type = virXMLPropString(node, "type"))) {
+ virReportError(VIR_ERR_XML_ERROR, "%s", _("missing crypto device
type"));
+ goto error;
+ }
+
+ if ((def->type = virDomainCryptoTypeTypeFromString(type)) < 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown crypto type
'%s'"), model);
+ goto error;
+ }
+
+ ctxt->node = node;
+
+ if ((nbackends = virXPathNodeSet("./backend", ctxt, &backends)) <
0)
+ goto error;
+
+ if (nbackends != 1) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("only one crypto backend is supported"));
+ goto error;
+ }
+
+ if (!(backend = virXMLPropString(backends[0], "model"))) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing crypto device backend model"));
+ goto error;
+ }
+
+ if ((def->backend = virDomainCryptoBackendTypeFromString(backend)) < 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown crypto backend model '%s'"),
backend);
+ goto error;
+ }
+
+ if (virXMLPropUInt(backends[0], "queues", 10, VIR_XML_PROP_NONE,
&def->queues) < 0) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("parsing crypto device queues failed"));
Nope, this overwrites more specific error message reported by
virXMLPropUInt().
+ goto error;
+ }
+
+ switch ((virDomainCryptoBackend) def->backend) {
+ case VIR_DOMAIN_CRYPTO_BACKEND_BUILTIN:
+ case VIR_DOMAIN_CRYPTO_BACKEND_LKCF:
+ case VIR_DOMAIN_CRYPTO_BACKEND_LAST:
+ break;
+ }
What's the purpose of this statement?
+
+ if (virDomainDeviceInfoParseXML(xmlopt, node, ctxt, &def->info, flags) <
0)
+ goto error;
+
+ if (virDomainVirtioOptionsParseXML(virXPathNode("./driver", ctxt),
+ &def->virtio) < 0)
+ goto error;
+
+ return def;
+
+ error:
+ g_clear_pointer(&def, virDomainCryptoDefFree);
How about declaring @def as g_autoptr() and dropping this label completely?
+ return NULL;
+}
+
+
virDomainDeviceDef *
virDomainDeviceDefParse(const char *xmlStr,
const virDomainDef *def,
@@ -13578,6 +13703,11 @@ virDomainDeviceDefParse(const char *xmlStr,
flags)))
return NULL;
break;
+ case VIR_DOMAIN_DEVICE_CRYPTO:
+ if (!(dev->data.crypto = virDomainCryptoDefParseXML(xmlopt, node, ctxt,
+ flags)))
+ return NULL;
+ break;
case VIR_DOMAIN_DEVICE_NONE:
case VIR_DOMAIN_DEVICE_LAST:
break;
@@ -18670,6 +18800,21 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt,
}
VIR_FREE(nodes);
+ /* Parse the crypto devices */
+ if ((n = virXPathNodeSet("./devices/crypto", ctxt, &nodes)) < 0)
+ return NULL;
+ if (n)
+ def->cryptos = g_new0(virDomainCryptoDef *, n);
+ for (i = 0; i < n; i++) {
+ virDomainCryptoDef *crypto = virDomainCryptoDefParseXML(xmlopt, nodes[i],
+ ctxt, flags);
+ if (!crypto)
+ return NULL;
+
+ def->cryptos[def->ncryptos++] = crypto;
+ }
+ VIR_FREE(nodes);
+
/* Parse the TPM devices */
if ((n = virXPathNodeSet("./devices/tpm", ctxt, &nodes)) < 0)
return NULL;
@@ -21210,6 +21355,7 @@ virDomainDefCheckABIStabilityFlags(virDomainDef *src,
case VIR_DOMAIN_DEVICE_IOMMU:
case VIR_DOMAIN_DEVICE_VSOCK:
case VIR_DOMAIN_DEVICE_AUDIO:
+ case VIR_DOMAIN_DEVICE_CRYPTO:
break;
}
#endif
@@ -24562,6 +24708,47 @@ virDomainRNGDefFree(virDomainRNGDef *def)
}
+static int
+virDomainCryptoDefFormat(virBuffer *buf,
+ virDomainCryptoDef *def,
+ unsigned int flags)
+{
+ const char *model = virDomainCryptoModelTypeToString(def->model);
+ const char *type = virDomainCryptoTypeTypeToString(def->model);
+ const char *backend = virDomainCryptoBackendTypeToString(def->backend);
+ g_auto(virBuffer) driverAttrBuf = VIR_BUFFER_INITIALIZER;
+
+ virBufferAsprintf(buf, "<crypto model='%s'
type='%s'>\n", model, type);
+ virBufferAdjustIndent(buf, 2);
+ virBufferAsprintf(buf, "<backend model='%s'", backend);
+ if (def->queues)
+ virBufferAsprintf(buf, " queues='%d'", def->queues);
+ virBufferAddLit(buf, "/>\n");
+
+ virDomainVirtioOptionsFormat(&driverAttrBuf, def->virtio);
+
+ virXMLFormatElement(buf, "driver", &driverAttrBuf, NULL);
+
+ virDomainDeviceInfoFormat(buf, &def->info, flags);
+
+ virBufferAdjustIndent(buf, -2);
+ virBufferAddLit(buf, "</crypto>\n");
+
+ return 0;
This is all the function returns. Can this be made void instead?
+}
+
+void
+virDomainCryptoDefFree(virDomainCryptoDef *def)
+{
+ if (!def)
+ return;
+
+ virDomainDeviceInfoClear(&def->info);
+ g_free(def->virtio);
+ g_free(def);
+}
+
+
static int
virDomainMemorySourceDefFormat(virBuffer *buf,
virDomainMemoryDef *def)
@@ -27261,6 +27448,10 @@ virDomainDefFormatInternalSetRootName(virDomainDef *def,
return -1;
}
+ for (n = 0; n < def->ncryptos; n++) {
+ if (virDomainCryptoDefFormat(buf, def->cryptos[n], flags))
+ return -1;
+ }
if (def->iommu)
virDomainIOMMUDefFormat(buf, def->iommu);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 1404c55053..9062250d60 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -86,6 +86,7 @@ typedef enum {
VIR_DOMAIN_DEVICE_IOMMU,
VIR_DOMAIN_DEVICE_VSOCK,
VIR_DOMAIN_DEVICE_AUDIO,
+ VIR_DOMAIN_DEVICE_CRYPTO,
VIR_DOMAIN_DEVICE_LAST
} virDomainDeviceType;
@@ -118,6 +119,7 @@ struct _virDomainDeviceDef {
virDomainIOMMUDef *iommu;
virDomainVsockDef *vsock;
virDomainAudioDef *audio;
+ virDomainCryptoDef *crypto;
} data;
};
@@ -2858,6 +2860,34 @@ struct _virDomainVsockDef {
virDomainVirtioOptions *virtio;
};
+typedef enum {
+ VIR_DOMAIN_CRYPTO_MODEL_VIRTIO,
+
+ VIR_DOMAIN_CRYPTO_MODEL_LAST
+} virDomainCryptoModel;
+
+typedef enum {
+ VIR_DOMAIN_CRYPTO_TYPE_QEMU,
+
+ VIR_DOMAIN_CRYPTO_TYPE_LAST
+} virDomainCryptoType;
+
+typedef enum {
+ VIR_DOMAIN_CRYPTO_BACKEND_BUILTIN,
+ VIR_DOMAIN_CRYPTO_BACKEND_LKCF,
+
+ VIR_DOMAIN_CRYPTO_BACKEND_LAST
+} virDomainCryptoBackend;
+
+struct _virDomainCryptoDef {
+ virDomainCryptoModel model;
+ virDomainCryptoType type;
+ virDomainCryptoBackend backend;
+ unsigned int queues;
+ virDomainDeviceInfo info;
+ virDomainVirtioOptions *virtio;
+};
+
struct _virDomainVirtioOptions {
virTristateSwitch iommu;
virTristateSwitch ats;
@@ -3023,6 +3053,9 @@ struct _virDomainDef {
size_t nsysinfo;
virSysinfoDef **sysinfo;
+ size_t ncryptos;
+ virDomainCryptoDef **cryptos;
+
/* At maximum 2 TPMs on the domain if a TPM Proxy is present. */
size_t ntpms;
virDomainTPMDef **tpms;
@@ -3274,6 +3307,7 @@ struct _virDomainXMLPrivateDataCallbacks {
virDomainXMLPrivateDataNewFunc vcpuNew;
virDomainXMLPrivateDataNewFunc chrSourceNew;
virDomainXMLPrivateDataNewFunc vsockNew;
+ virDomainXMLPrivateDataNewFunc cryptoNew;
virDomainXMLPrivateDataNewFunc graphicsNew;
virDomainXMLPrivateDataNewFunc networkNew;
virDomainXMLPrivateDataNewFunc videoNew;
@@ -3440,6 +3474,9 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainIOMMUDef,
virDomainIOMMUDefFree);
virDomainVsockDef *virDomainVsockDefNew(virDomainXMLOption *xmlopt);
void virDomainVsockDefFree(virDomainVsockDef *vsock);
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainVsockDef, virDomainVsockDefFree);
+virDomainCryptoDef *virDomainCryptoDefNew(virDomainXMLOption *xmlopt);
This function is never defined, only declared here.
+void virDomainCryptoDefFree(virDomainCryptoDef *crypto);
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainCryptoDef, virDomainCryptoDefFree);
void virDomainNetTeamingInfoFree(virDomainNetTeamingInfo *teaming);
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainNetTeamingInfo, virDomainNetTeamingInfoFree);
void virDomainNetDefFree(virDomainNetDef *def);
Michal