From 3d200117bf961ae9c84be64e8a6a8a469983dc74 Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Tue, 11 Jun 2024 10:44:24 +0200 Subject: [PATCH 1/5] qemu_monitor_json: Report error in error paths in SEV related code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit commit 66efdfabd9b2c8527008c600bf66f21528d70889 upstream While working on qemuMonitorJSONGetSEVMeasurement() and qemuMonitorJSONGetSEVInfo() I've noticed that if these functions fail, they do so without appropriate error set. Fill in error reporting. Signed-off-by: Michal Privoznik Reviewed-by: Daniel P. Berrangé Signed-off-by: suryasaimadhu Signed-off-by: PrithivishS --- src/qemu/qemu_monitor_json.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 7f23d034b8..95cfc881b9 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -8023,8 +8023,11 @@ qemuMonitorJSONGetSEVMeasurement(qemuMonitor *mon) if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_OBJECT))) return NULL; - if (!(tmp = virJSONValueObjectGetString(data, "data"))) + if (!(tmp = virJSONValueObjectGetString(data, "data"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-sev-launch-measure reply was missing 'data'")); return NULL; + } return g_strdup(tmp); } @@ -8067,8 +8070,11 @@ qemuMonitorJSONGetSEVInfo(qemuMonitor *mon, if (virJSONValueObjectGetNumberUint(data, "api-major", apiMajor) < 0 || virJSONValueObjectGetNumberUint(data, "api-minor", apiMinor) < 0 || virJSONValueObjectGetNumberUint(data, "build-id", buildID) < 0 || - virJSONValueObjectGetNumberUint(data, "policy", policy) < 0) + virJSONValueObjectGetNumberUint(data, "policy", policy) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-sev reply was missing some data")); return -1; + } return 0; } -- Gitee From 77cd2fdaccb53a952078e9f8404f5ec3e1dea8f6 Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Tue, 11 Jun 2024 12:12:08 +0200 Subject: [PATCH 2/5] conf: Move some members of virDomainSEVDef into virDomainSEVCommonDef MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit commit d2cad18ca3c5dd88c416b6e4dd9c709f527b1dbe upstream Some parts of SEV are to be shared with SEV SNP. In order to reuse XML parsing / formatting code cleanly, let's move those common bits into a new struct (virDomainSEVCommonDef) and adjust rest of the code. [Backport Changes] 1. In src/conf/domain_conf.h, the struct members user_id,secret_header and secret in _virDomainSEVDef were retained. These members were originally introduced in commit cbc574f26c7fa ("conf: qemu: add libvirt support reuse id for Hygon CSV") and 66ab1f1ce7ae3 ("conf: qemu: support provide inject secret for Hygon CSV") as part of the Hygon hardware support changes. Signed-off-by: Michal Privoznik Reviewed-by: Daniel P. Berrangé Signed-off-by: suryasaimadhu Signed-off-by: PrithivishS --- src/conf/domain_conf.c | 55 +++++++++++++++++++++---------- src/conf/domain_conf.h | 12 ++++--- src/conf/schemas/domaincommon.rng | 24 ++++++++------ src/conf/virconftypes.h | 2 ++ src/qemu/qemu_command.c | 8 ++--- src/qemu/qemu_process.c | 12 +++---- src/qemu/qemu_validate.c | 2 +- 7 files changed, 73 insertions(+), 42 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 04f2642109..1c8887e1e4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13518,8 +13518,8 @@ virDomainMemoryTargetDefParseXML(xmlNodePtr node, static int -virDomainSEVDefParseXML(virDomainSEVDef *def, - xmlXPathContextPtr ctxt) +virDomainSEVCommonDefParseXML(virDomainSEVCommonDef *def, + xmlXPathContextPtr ctxt) { int rc; @@ -13527,12 +13527,6 @@ virDomainSEVDefParseXML(virDomainSEVDef *def, &def->kernel_hashes) < 0) return -1; - if (virXPathUIntBase("string(./policy)", ctxt, 16, &def->policy) < 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("failed to get launch security policy")); - return -1; - } - /* the following attributes are platform dependent and if missing, we can * autofill them from domain capabilities later */ @@ -13555,6 +13549,23 @@ virDomainSEVDefParseXML(virDomainSEVDef *def, return -1; } + return 0; +} + + +static int +virDomainSEVDefParseXML(virDomainSEVDef *def, + xmlXPathContextPtr ctxt) +{ + if (virDomainSEVCommonDefParseXML(&def->common, ctxt) < 0) + return -1; + + if (virXPathUIntBase("string(./policy)", ctxt, 16, &def->policy) < 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("failed to get launch security policy")); + return -1; + } + def->dh_cert = virXPathString("string(./dhCert)", ctxt); def->session = virXPathString("string(./session)", ctxt); def->user_id = virXPathString("string(./userid)", ctxt); @@ -26648,6 +26659,24 @@ virDomainKeyWrapDefFormat(virBuffer *buf, virDomainKeyWrapDef *keywrap) } +static void +virDomainSEVCommonDefFormat(virBuffer *attrBuf, + virBuffer *childBuf, + virDomainSEVCommonDef *def) +{ + if (def->kernel_hashes != VIR_TRISTATE_BOOL_ABSENT) + virBufferAsprintf(attrBuf, " kernelHashes='%s'", + virTristateBoolTypeToString(def->kernel_hashes)); + + if (def->haveCbitpos) + virBufferAsprintf(childBuf, "%d\n", def->cbitpos); + + if (def->haveReducedPhysBits) + virBufferAsprintf(childBuf, "%d\n", + def->reduced_phys_bits); +} + + static void virDomainSecDefFormat(virBuffer *buf, virDomainSecDef *sec) { @@ -26664,16 +26693,8 @@ virDomainSecDefFormat(virBuffer *buf, virDomainSecDef *sec) case VIR_DOMAIN_LAUNCH_SECURITY_SEV: { virDomainSEVDef *sev = &sec->data.sev; - if (sev->kernel_hashes != VIR_TRISTATE_BOOL_ABSENT) - virBufferAsprintf(&attrBuf, " kernelHashes='%s'", - virTristateBoolTypeToString(sev->kernel_hashes)); - - if (sev->haveCbitpos) - virBufferAsprintf(&childBuf, "%d\n", sev->cbitpos); + virDomainSEVCommonDefFormat(&attrBuf, &childBuf, &sev->common); - if (sev->haveReducedPhysBits) - virBufferAsprintf(&childBuf, "%d\n", - sev->reduced_phys_bits); virBufferAsprintf(&childBuf, "0x%04x\n", sev->policy); if (sev->dh_cert) virBufferEscapeString(&childBuf, "%s\n", sev->dh_cert); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 180f092c06..c2588a8352 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2865,15 +2865,19 @@ typedef enum { } virDomainLaunchSecurity; -struct _virDomainSEVDef { - char *dh_cert; - char *session; - unsigned int policy; +struct _virDomainSEVCommonDef { bool haveCbitpos; unsigned int cbitpos; bool haveReducedPhysBits; unsigned int reduced_phys_bits; virTristateBool kernel_hashes; +}; + +struct _virDomainSEVDef { + virDomainSEVCommonDef common; + char *dh_cert; + char *session; + unsigned int policy; char *user_id; char *secret_header; char *secret; diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index a2e0119888..160d28344c 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -527,6 +527,19 @@ + + + + + + + + + + + + + sev @@ -537,16 +550,7 @@ - - - - - - - - - - + diff --git a/src/conf/virconftypes.h b/src/conf/virconftypes.h index bcdcb8b825..d175e54898 100644 --- a/src/conf/virconftypes.h +++ b/src/conf/virconftypes.h @@ -210,6 +210,8 @@ typedef struct _virDomainResctrlMonDef virDomainResctrlMonDef; typedef struct _virDomainResourceDef virDomainResourceDef; +typedef struct _virDomainSEVCommonDef virDomainSEVCommonDef; + typedef struct _virDomainSEVDef virDomainSEVDef; typedef struct _virDomainSecDef virDomainSecDef; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c6ce598e4a..10c24f73e3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9811,7 +9811,7 @@ qemuBuildSEVCommandLine(virDomainObj *vm, virCommand *cmd, g_autofree char *secretpath = NULL; VIR_DEBUG("policy=0x%x cbitpos=%d reduced_phys_bits=%d", - sev->policy, sev->cbitpos, sev->reduced_phys_bits); + sev->policy, sev->common.cbitpos, sev->common.reduced_phys_bits); if (sev->user_id) VIR_DEBUG("user_id=%s", sev->user_id); @@ -9829,13 +9829,13 @@ qemuBuildSEVCommandLine(virDomainObj *vm, virCommand *cmd, secretpath = g_strdup_printf("%s/secret.base64", priv->libDir); if (qemuMonitorCreateObjectProps(&props, "sev-guest", "lsec0", - "u:cbitpos", sev->cbitpos, - "u:reduced-phys-bits", sev->reduced_phys_bits, + "u:cbitpos", sev->common.cbitpos, + "u:reduced-phys-bits", sev->common.reduced_phys_bits, "u:policy", sev->policy, "S:user-id", sev->user_id, "S:dh-cert-file", dhpath, "S:session-file", sessionpath, - "T:kernel-hashes", sev->kernel_hashes, + "T:kernel-hashes", sev->common.kernel_hashes, "S:secret-header-file", secretheaderpath, "S:secret-file", secretpath, NULL) < 0) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 5240fbce09..301a351a76 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6811,14 +6811,14 @@ qemuProcessUpdateSEVInfo(virDomainObj *vm) * mandatory on QEMU cmdline */ sevCaps = virQEMUCapsGetSEVCapabilities(qemuCaps); - if (!sev->haveCbitpos) { - sev->cbitpos = sevCaps->cbitpos; - sev->haveCbitpos = true; + if (!sev->common.haveCbitpos) { + sev->common.cbitpos = sevCaps->cbitpos; + sev->common.haveCbitpos = true; } - if (!sev->haveReducedPhysBits) { - sev->reduced_phys_bits = sevCaps->reduced_phys_bits; - sev->haveReducedPhysBits = true; + if (!sev->common.haveReducedPhysBits) { + sev->common.reduced_phys_bits = sevCaps->reduced_phys_bits; + sev->common.haveReducedPhysBits = true; } return 0; diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 5894e2bd26..a5c22ee4be 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1305,7 +1305,7 @@ qemuValidateDomainDef(const virDomainDef *def, return -1; } - if (def->sec->data.sev.kernel_hashes != VIR_TRISTATE_BOOL_ABSENT && + if (def->sec->data.sev.common.kernel_hashes != VIR_TRISTATE_BOOL_ABSENT && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV_GUEST_KERNEL_HASHES)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("SEV measured direct kernel boot is not supported with this QEMU binary")); -- Gitee From 077c3fb43ffe72d418ef674b9d0950aff0bcb18d Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Mon, 17 Jun 2024 11:53:46 +0200 Subject: [PATCH 3/5] conf: Drop needless NULL checks guarding virBufferEscapeString() commit be58733d903fdd4a9a64483f14dbb4af97e526f3 upstream There's no need to guard virBufferEscapeString() with a call to NULL as the very first thing the function does is check all three arguments for NULL. This patch was generated using the following spatch: @@ expression X, Y, E; @@ - if (E) virBufferEscapeString(X, Y, E); Signed-off-by: Michal Privoznik Reviewed-by: Jonathon Jongsma Signed-off-by: suryasaimadhu Signed-off-by: PrithivishS --- src/conf/capabilities.c | 6 ++-- src/conf/domain_conf.c | 57 +++++++++++------------------- src/conf/network_conf.c | 6 ++-- src/conf/node_device_conf.c | 57 ++++++++++++------------------ src/conf/snapshot_conf.c | 5 ++- src/conf/storage_encryption_conf.c | 9 ++--- src/conf/storage_source_conf.c | 3 +- src/conf/virnwfilterbindingdef.c | 3 +- 8 files changed, 55 insertions(+), 91 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 02298e40a3..aca9be7c83 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -688,10 +688,8 @@ virCapabilitiesDomainDataLookupInternal(virCaps *caps, if (domaintype > VIR_DOMAIN_VIRT_NONE) virBufferAsprintf(&buf, "domaintype=%s ", virDomainVirtTypeToString(domaintype)); - if (emulator) - virBufferEscapeString(&buf, "emulator=%s ", emulator); - if (machinetype) - virBufferEscapeString(&buf, "machine=%s ", machinetype); + virBufferEscapeString(&buf, "emulator=%s ", emulator); + virBufferEscapeString(&buf, "machine=%s ", machinetype); if (virBufferCurrentContent(&buf) && !virBufferCurrentContent(&buf)[0]) virBufferAsprintf(&buf, "%s", _("any configuration")); diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1c8887e1e4..7c4df30fc1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5377,8 +5377,7 @@ virDomainDeviceInfoFormat(virBuffer *buf, if (rombar) virBufferAsprintf(buf, " bar='%s'", rombar); } - if (info->romfile) - virBufferEscapeString(buf, " file='%s'", info->romfile); + virBufferEscapeString(buf, " file='%s'", info->romfile); virBufferAddLit(buf, "/>\n"); } @@ -22220,8 +22219,7 @@ virSecurityDeviceLabelDefFormat(virBuffer *buf, virBufferAddLit(buf, "model) - virBufferEscapeString(buf, " model='%s'", def->model); + virBufferEscapeString(buf, " model='%s'", def->model); if (def->labelskip) virBufferAddLit(buf, " labelskip='yes'"); @@ -22416,8 +22414,7 @@ virDomainDiskSourceFormatNetwork(virBuffer *attrBuf, virBufferAsprintf(childBuf, "\n", src->timeout); if (src->protocol == VIR_STORAGE_NET_PROTOCOL_SSH) { - if (src->ssh_known_hosts_file) - virBufferEscapeString(childBuf, "\n", src->ssh_known_hosts_file); + virBufferEscapeString(childBuf, "\n", src->ssh_known_hosts_file); if (src->ssh_keyfile || src->ssh_agent) { virBufferAddLit(childBuf, "idx); - if (model) - virBufferEscapeString(&attrBuf, " model='%s'", model); + virBufferEscapeString(&attrBuf, " model='%s'", model); switch (def->type) { case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: @@ -24605,8 +24601,7 @@ virDomainChrTargetDefFormat(virBuffer *buf, case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN: case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO: - if (def->target.name) - virBufferEscapeString(buf, " name='%s'", def->target.name); + virBufferEscapeString(buf, " name='%s'", def->target.name); if (def->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO && def->state != VIR_DOMAIN_CHR_DEVICE_STATE_DEFAULT && @@ -26013,9 +26008,8 @@ virDomainGraphicsDefFormat(virBuffer *buf, break; } - if (def->data.vnc.keymap) - virBufferEscapeString(buf, " keymap='%s'", - def->data.vnc.keymap); + virBufferEscapeString(buf, " keymap='%s'", + def->data.vnc.keymap); if (def->data.vnc.sharePolicy) virBufferAsprintf(buf, " sharePolicy='%s'", @@ -26030,13 +26024,11 @@ virDomainGraphicsDefFormat(virBuffer *buf, break; case VIR_DOMAIN_GRAPHICS_TYPE_SDL: - if (def->data.sdl.display) - virBufferEscapeString(buf, " display='%s'", - def->data.sdl.display); + virBufferEscapeString(buf, " display='%s'", + def->data.sdl.display); - if (def->data.sdl.xauth) - virBufferEscapeString(buf, " xauth='%s'", - def->data.sdl.xauth); + virBufferEscapeString(buf, " xauth='%s'", + def->data.sdl.xauth); if (def->data.sdl.fullscreen) virBufferAddLit(buf, " fullscreen='yes'"); @@ -26075,9 +26067,8 @@ virDomainGraphicsDefFormat(virBuffer *buf, break; case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: - if (def->data.desktop.display) - virBufferEscapeString(buf, " display='%s'", - def->data.desktop.display); + virBufferEscapeString(buf, " display='%s'", + def->data.desktop.display); if (def->data.desktop.fullscreen) virBufferAddLit(buf, " fullscreen='yes'"); @@ -26130,9 +26121,8 @@ virDomainGraphicsDefFormat(virBuffer *buf, break; } - if (def->data.spice.keymap) - virBufferEscapeString(buf, " keymap='%s'", - def->data.spice.keymap); + virBufferEscapeString(buf, " keymap='%s'", + def->data.spice.keymap); if (def->data.spice.defaultMode != VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY) virBufferAsprintf(buf, " defaultMode='%s'", @@ -26509,11 +26499,9 @@ virDomainResourceDefFormat(virBuffer *buf, if (!def) return; - if (def->partition) - virBufferEscapeString(&childBuf, "%s\n", def->partition); + virBufferEscapeString(&childBuf, "%s\n", def->partition); - if (def->appid) - virBufferEscapeString(&childBuf, "\n", def->appid); + virBufferEscapeString(&childBuf, "\n", def->appid); virXMLFormatElement(buf, "resource", NULL, &childBuf); } @@ -26696,11 +26684,9 @@ virDomainSecDefFormat(virBuffer *buf, virDomainSecDef *sec) virDomainSEVCommonDefFormat(&attrBuf, &childBuf, &sev->common); virBufferAsprintf(&childBuf, "0x%04x\n", sev->policy); - if (sev->dh_cert) - virBufferEscapeString(&childBuf, "%s\n", sev->dh_cert); + virBufferEscapeString(&childBuf, "%s\n", sev->dh_cert); - if (sev->session) - virBufferEscapeString(&childBuf, "%s\n", sev->session); + virBufferEscapeString(&childBuf, "%s\n", sev->session); if (sev->user_id) virBufferEscapeString(&childBuf, "%s\n", sev->user_id); @@ -27956,9 +27942,8 @@ virDomainDefFormatInternalSetRootName(virDomainDef *def, for (i = 0; def->os.initenv && def->os.initenv[i]; i++) virBufferAsprintf(buf, "%s\n", def->os.initenv[i]->name, def->os.initenv[i]->value); - if (def->os.initdir) - virBufferEscapeString(buf, "%s\n", - def->os.initdir); + virBufferEscapeString(buf, "%s\n", + def->os.initdir); if (def->os.inituser) virBufferAsprintf(buf, "%s\n", def->os.inituser); if (def->os.initgroup) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 0449b6f07c..cc8956af53 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -2043,10 +2043,8 @@ virNetworkDNSDefFormat(virBuffer *buf, def->srvs[i].service); virBufferEscapeString(buf, "protocol='%s'", def->srvs[i].protocol); - if (def->srvs[i].domain) - virBufferEscapeString(buf, " domain='%s'", def->srvs[i].domain); - if (def->srvs[i].target) - virBufferEscapeString(buf, " target='%s'", def->srvs[i].target); + virBufferEscapeString(buf, " domain='%s'", def->srvs[i].domain); + virBufferEscapeString(buf, " target='%s'", def->srvs[i].target); if (def->srvs[i].port) virBufferAsprintf(buf, " port='%d'", def->srvs[i].port); if (def->srvs[i].priority) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index f722ab37c6..6c78c9a263 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -176,20 +176,16 @@ virNodeDeviceCapSystemDefFormat(virBuffer *buf, { char uuidstr[VIR_UUID_STRING_BUFLEN]; - if (data->system.product_name) - virBufferEscapeString(buf, "%s\n", - data->system.product_name); + virBufferEscapeString(buf, "%s\n", + data->system.product_name); virBufferAddLit(buf, "\n"); virBufferAdjustIndent(buf, 2); - if (data->system.hardware.vendor_name) - virBufferEscapeString(buf, "%s\n", - data->system.hardware.vendor_name); - if (data->system.hardware.version) - virBufferEscapeString(buf, "%s\n", - data->system.hardware.version); - if (data->system.hardware.serial) - virBufferEscapeString(buf, "%s\n", - data->system.hardware.serial); + virBufferEscapeString(buf, "%s\n", + data->system.hardware.vendor_name); + virBufferEscapeString(buf, "%s\n", + data->system.hardware.version); + virBufferEscapeString(buf, "%s\n", + data->system.hardware.serial); virUUIDFormat(data->system.hardware.uuid, uuidstr); virBufferAsprintf(buf, "%s\n", uuidstr); virBufferAdjustIndent(buf, -2); @@ -197,15 +193,12 @@ virNodeDeviceCapSystemDefFormat(virBuffer *buf, virBufferAddLit(buf, "\n"); virBufferAdjustIndent(buf, 2); - if (data->system.firmware.vendor_name) - virBufferEscapeString(buf, "%s\n", - data->system.firmware.vendor_name); - if (data->system.firmware.version) - virBufferEscapeString(buf, "%s\n", - data->system.firmware.version); - if (data->system.firmware.release_date) - virBufferEscapeString(buf, "%s\n", - data->system.firmware.release_date); + virBufferEscapeString(buf, "%s\n", + data->system.firmware.vendor_name); + virBufferEscapeString(buf, "%s\n", + data->system.firmware.version); + virBufferEscapeString(buf, "%s\n", + data->system.firmware.release_date); virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "\n"); } @@ -225,9 +218,8 @@ virNodeDeviceCapMdevTypesFormat(virBuffer *buf, virMediatedDeviceType *type = mdev_types[i]; virBufferEscapeString(buf, "\n", type->id); virBufferAdjustIndent(buf, 2); - if (type->name) - virBufferEscapeString(buf, "%s\n", - type->name); + virBufferEscapeString(buf, "%s\n", + type->name); virBufferEscapeString(buf, "%s\n", type->device_api); virBufferAsprintf(buf, @@ -451,10 +443,9 @@ virNodeDeviceCapUSBInterfaceDefFormat(virBuffer *buf, data->usb_if.subclass); virBufferAsprintf(buf, "%d\n", data->usb_if.protocol); - if (data->usb_if.description) - virBufferEscapeString(buf, - "%s\n", - data->usb_if.description); + virBufferEscapeString(buf, + "%s\n", + data->usb_if.description); } @@ -466,9 +457,8 @@ virNodeDeviceCapNetDefFormat(virBuffer *buf, virBufferEscapeString(buf, "%s\n", data->net.ifname); - if (data->net.address) - virBufferEscapeString(buf, "
%s
\n", - data->net.address); + virBufferEscapeString(buf, "
%s
\n", + data->net.address); virInterfaceLinkFormat(buf, &data->net.lnk); if (data->net.features) { for (i = 0; i < VIR_NET_DEV_FEAT_LAST; i++) { @@ -530,9 +520,8 @@ virNodeDeviceCapSCSIDefFormat(virBuffer *buf, virBufferAsprintf(buf, "%d\n", data->scsi.target); virBufferAsprintf(buf, "%d\n", data->scsi.lun); - if (data->scsi.type) - virBufferEscapeString(buf, "%s\n", - data->scsi.type); + virBufferEscapeString(buf, "%s\n", + data->scsi.type); } diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index d7fcded302..039ed77b84 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -819,9 +819,8 @@ virDomainSnapshotDefFormatInternal(virBuffer *buf, virBufferAdjustIndent(buf, 2); virBufferEscapeString(buf, "%s\n", def->parent.name); - if (def->parent.description) - virBufferEscapeString(buf, "%s\n", - def->parent.description); + virBufferEscapeString(buf, "%s\n", + def->parent.description); if (def->state) virBufferAsprintf(buf, "%s\n", virDomainSnapshotStateTypeToString(def->state)); diff --git a/src/conf/storage_encryption_conf.c b/src/conf/storage_encryption_conf.c index 1849df5c6c..b86001ec50 100644 --- a/src/conf/storage_encryption_conf.c +++ b/src/conf/storage_encryption_conf.c @@ -317,16 +317,13 @@ virStorageEncryptionInfoDefFormat(virBuffer *buf, { virBufferEscapeString(buf, "cipher_name); virBufferAsprintf(buf, " size='%u'", enc->cipher_size); - if (enc->cipher_mode) - virBufferEscapeString(buf, " mode='%s'", enc->cipher_mode); - if (enc->cipher_hash) - virBufferEscapeString(buf, " hash='%s'", enc->cipher_hash); + virBufferEscapeString(buf, " mode='%s'", enc->cipher_mode); + virBufferEscapeString(buf, " hash='%s'", enc->cipher_hash); virBufferAddLit(buf, "/>\n"); if (enc->ivgen_name) { virBufferEscapeString(buf, "ivgen_name); - if (enc->ivgen_hash) - virBufferEscapeString(buf, " hash='%s'", enc->ivgen_hash); + virBufferEscapeString(buf, " hash='%s'", enc->ivgen_hash); virBufferAddLit(buf, "/>\n"); } } diff --git a/src/conf/storage_source_conf.c b/src/conf/storage_source_conf.c index f974a521b1..003fbf031e 100644 --- a/src/conf/storage_source_conf.c +++ b/src/conf/storage_source_conf.c @@ -1347,8 +1347,7 @@ int virStorageSourcePrivateDataFormatRelPath(virStorageSource *src, virBuffer *buf) { - if (src->relPath) - virBufferEscapeString(buf, "%s\n", src->relPath); + virBufferEscapeString(buf, "%s\n", src->relPath); return 0; } diff --git a/src/conf/virnwfilterbindingdef.c b/src/conf/virnwfilterbindingdef.c index 423ed7a392..fe45c84347 100644 --- a/src/conf/virnwfilterbindingdef.c +++ b/src/conf/virnwfilterbindingdef.c @@ -203,8 +203,7 @@ virNWFilterBindingDefFormatBuf(virBuffer *buf, virBufferAddLit(buf, "\n"); virBufferEscapeString(buf, "\n", def->portdevname); - if (def->linkdevname) - virBufferEscapeString(buf, "\n", def->linkdevname); + virBufferEscapeString(buf, "\n", def->linkdevname); virMacAddrFormat(&def->mac, mac); virBufferAsprintf(buf, "\n", mac); -- Gitee From ccb470dc4a6ec1e4aaeebefd9e490c9526c9ee16 Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Wed, 12 Jun 2024 10:06:57 +0200 Subject: [PATCH 4/5] Drop needless typecast to virDomainLaunchSecurity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit commit a44a43361feea6cf9809cb9ea5873cdf197975fb upstream The sectype member of _virDomainSecDef struct is already declared as of virDomainLaunchSecurity type. There's no need to typecast it to the very same type when passing it to switch(). Signed-off-by: Michal Privoznik Reviewed-by: Daniel P. Berrangé Signed-off-by: suryasaimadhu Signed-off-by: PrithivishS --- src/conf/domain_conf.c | 6 +++--- src/qemu/qemu_command.c | 4 ++-- src/qemu/qemu_firmware.c | 2 +- src/qemu/qemu_namespace.c | 2 +- src/qemu/qemu_process.c | 2 +- src/qemu/qemu_validate.c | 2 +- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7c4df30fc1..6f7c054fea 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3824,7 +3824,7 @@ virDomainSecDefFree(virDomainSecDef *def) if (!def) return; - switch ((virDomainLaunchSecurity) def->sectype) { + switch (def->sectype) { case VIR_DOMAIN_LAUNCH_SECURITY_SEV: g_free(def->data.sev.dh_cert); g_free(def->data.sev.session); @@ -13589,7 +13589,7 @@ virDomainSecDefParseXML(xmlNodePtr lsecNode, &sec->sectype) < 0) return NULL; - switch ((virDomainLaunchSecurity) sec->sectype) { + switch (sec->sectype) { case VIR_DOMAIN_LAUNCH_SECURITY_SEV: if (virDomainSEVDefParseXML(&sec->data.sev, ctxt) < 0) return NULL; @@ -26677,7 +26677,7 @@ virDomainSecDefFormat(virBuffer *buf, virDomainSecDef *sec) virBufferAsprintf(&attrBuf, " type='%s'", virDomainLaunchSecurityTypeToString(sec->sectype)); - switch ((virDomainLaunchSecurity) sec->sectype) { + switch (sec->sectype) { case VIR_DOMAIN_LAUNCH_SECURITY_SEV: { virDomainSEVDef *sev = &sec->data.sev; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 10c24f73e3..97cf5f7eaa 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7055,7 +7055,7 @@ qemuBuildMachineCommandLine(virCommand *cmd, qemuAppendLoadparmMachineParm(&buf, def); if (def->sec) { - switch ((virDomainLaunchSecurity) def->sec->sectype) { + switch (def->sec->sectype) { case VIR_DOMAIN_LAUNCH_SECURITY_SEV: if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT)) { virBufferAddLit(&buf, ",confidential-guest-support=lsec0"); @@ -9872,7 +9872,7 @@ qemuBuildSecCommandLine(virDomainObj *vm, virCommand *cmd, if (!sec) return 0; - switch ((virDomainLaunchSecurity) sec->sectype) { + switch (sec->sectype) { case VIR_DOMAIN_LAUNCH_SECURITY_SEV: return qemuBuildSEVCommandLine(vm, cmd, &sec->data.sev); break; diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c index 31ed6e881b..8ee16daf8d 100644 --- a/src/qemu/qemu_firmware.c +++ b/src/qemu/qemu_firmware.c @@ -1358,7 +1358,7 @@ qemuFirmwareMatchDomain(const virDomainDef *def, } if (def->sec) { - switch ((virDomainLaunchSecurity) def->sec->sectype) { + switch (def->sec->sectype) { case VIR_DOMAIN_LAUNCH_SECURITY_SEV: if (!supportsSEV) { VIR_DEBUG("Domain requires SEV, firmware '%s' doesn't support it", diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c index ff314ce243..821e059480 100644 --- a/src/qemu/qemu_namespace.c +++ b/src/qemu/qemu_namespace.c @@ -651,7 +651,7 @@ qemuDomainSetupLaunchSecurity(virDomainObj *vm, if (!sec) return 0; - switch ((virDomainLaunchSecurity) sec->sectype) { + switch (sec->sectype) { case VIR_DOMAIN_LAUNCH_SECURITY_SEV: VIR_DEBUG("Setting up launch security for SEV"); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 301a351a76..4a8093f9b4 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7054,7 +7054,7 @@ qemuProcessPrepareLaunchSecurityGuestInput(virDomainObj *vm) if (!sec) return 0; - switch ((virDomainLaunchSecurity) sec->sectype) { + switch (sec->sectype) { case VIR_DOMAIN_LAUNCH_SECURITY_SEV: return qemuProcessPrepareSEVGuestInput(vm); case VIR_DOMAIN_LAUNCH_SECURITY_PV: diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index a5c22ee4be..33f0994d14 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1297,7 +1297,7 @@ qemuValidateDomainDef(const virDomainDef *def, return -1; if (def->sec) { - switch ((virDomainLaunchSecurity) def->sec->sectype) { + switch (def->sec->sectype) { case VIR_DOMAIN_LAUNCH_SECURITY_SEV: if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV_GUEST)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", -- Gitee From 20e1a8b896c63dd9c9f92b99e520625677fac732 Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Wed, 12 Jun 2024 09:29:59 +0200 Subject: [PATCH 5/5] src: Convert some _virDomainSecDef::sectype checks to switch() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit commit 7d16c296e3f0ed61443d3095fd6f8951de57f79d upstream In a few instances there is a plain if() check for _virDomainSecDef::sectype. While this works perfectly for now, soon there'll be another type and we can utilize compiler to identify all the places that need adaptation. Switch those if() statements to switch(). [Backport Changes] 1.The enumeration value VIR_DOMAIN_LAUNCH_SECURITY_CVM, a member of the virDomainLaunchSecurity enum, was introduced in libvirt as part of CSV support in commit ddf9053ad7d. However,the switch statements in the upstream patch do not account for this new enum member. This backport ensures that VIR_DOMAIN_LAUNCH_SECURITY_CVM is correctly handled in all the newly added switch statements by adding the appropriate case block in all affected files. This change prevents compiler warnings and enhances the completeness of the code. Signed-off-by: Michal Privoznik Reviewed-by: Daniel P. Berrangé Signed-off-by: TauqirAzam Signed-off-by: PrithivishS --- src/qemu/qemu_cgroup.c | 19 +++++++++++++++---- src/qemu/qemu_driver.c | 17 +++++++++++++++-- src/qemu/qemu_process.c | 18 ++++++++++++++---- src/security/security_dac.c | 34 +++++++++++++++++++++++++++------- 4 files changed, 71 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 47402b3750..1280989a01 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -844,10 +844,21 @@ qemuSetupDevicesCgroup(virDomainObj *vm) return -1; } - if (vm->def->sec && - vm->def->sec->sectype == VIR_DOMAIN_LAUNCH_SECURITY_SEV && - qemuSetupSEVCgroup(vm) < 0) - return -1; + if (vm->def->sec) { + switch (vm->def->sec->sectype) { + case VIR_DOMAIN_LAUNCH_SECURITY_SEV: + if (qemuSetupSEVCgroup(vm) < 0) + return -1; + break; + case VIR_DOMAIN_LAUNCH_SECURITY_PV: + break; + case VIR_DOMAIN_LAUNCH_SECURITY_CVM: + case VIR_DOMAIN_LAUNCH_SECURITY_NONE: + case VIR_DOMAIN_LAUNCH_SECURITY_LAST: + virReportEnumRangeError(virDomainLaunchSecurity, vm->def->sec->sectype); + return -1; + } + } return 0; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index df275c403c..1aab51dea8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19105,10 +19105,23 @@ qemuDomainGetLaunchSecurityInfo(virDomainPtr domain, if (virDomainGetLaunchSecurityInfoEnsureACL(domain->conn, vm->def) < 0) goto cleanup; - if (vm->def->sec && - vm->def->sec->sectype == VIR_DOMAIN_LAUNCH_SECURITY_SEV) { + if (!vm->def->sec) { + ret = 0; + goto cleanup; + } + + switch (vm->def->sec->sectype) { + case VIR_DOMAIN_LAUNCH_SECURITY_SEV: if (qemuDomainGetSEVInfo(vm, params, nparams, flags) < 0) goto cleanup; + break; + case VIR_DOMAIN_LAUNCH_SECURITY_PV: + break; + case VIR_DOMAIN_LAUNCH_SECURITY_CVM: + case VIR_DOMAIN_LAUNCH_SECURITY_NONE: + case VIR_DOMAIN_LAUNCH_SECURITY_LAST: + virReportEnumRangeError(virDomainLaunchSecurity, vm->def->sec->sectype); + return -1; } ret = 0; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 4a8093f9b4..08c1bca53c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6979,11 +6979,21 @@ qemuProcessPrepareDomain(virQEMUDriver *driver, for (i = 0; i < vm->def->nshmems; i++) qemuDomainPrepareShmemChardev(vm->def->shmems[i]); - if (vm->def->sec && - vm->def->sec->sectype == VIR_DOMAIN_LAUNCH_SECURITY_SEV) { - VIR_DEBUG("Updating SEV platform info"); - if (qemuProcessUpdateSEVInfo(vm) < 0) + if (vm->def->sec) { + switch (vm->def->sec->sectype) { + case VIR_DOMAIN_LAUNCH_SECURITY_SEV: + VIR_DEBUG("Updating SEV platform info"); + if (qemuProcessUpdateSEVInfo(vm) < 0) + return -1; + break; + case VIR_DOMAIN_LAUNCH_SECURITY_PV: + break; + case VIR_DOMAIN_LAUNCH_SECURITY_CVM: + case VIR_DOMAIN_LAUNCH_SECURITY_NONE: + case VIR_DOMAIN_LAUNCH_SECURITY_LAST: + virReportEnumRangeError(virDomainLaunchSecurity, vm->def->sec->sectype); return -1; + } } return 0; diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 96aebfce5b..4be9174704 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1977,10 +1977,20 @@ virSecurityDACRestoreAllLabel(virSecurityManager *mgr, rc = -1; } - if (def->sec && - def->sec->sectype == VIR_DOMAIN_LAUNCH_SECURITY_SEV) { - if (virSecurityDACRestoreSEVLabel(mgr, def) < 0) - rc = -1; + if (def->sec) { + switch (def->sec->sectype) { + case VIR_DOMAIN_LAUNCH_SECURITY_SEV: + if (virSecurityDACRestoreSEVLabel(mgr, def) < 0) + rc = -1; + break; + case VIR_DOMAIN_LAUNCH_SECURITY_PV: + break; + case VIR_DOMAIN_LAUNCH_SECURITY_CVM: + case VIR_DOMAIN_LAUNCH_SECURITY_NONE: + case VIR_DOMAIN_LAUNCH_SECURITY_LAST: + virReportEnumRangeError(virDomainLaunchSecurity, def->sec->sectype); + return -1; + } } for (i = 0; i < def->nsysinfo; i++) { @@ -2201,10 +2211,20 @@ virSecurityDACSetAllLabel(virSecurityManager *mgr, return -1; } - if (def->sec && - def->sec->sectype == VIR_DOMAIN_LAUNCH_SECURITY_SEV) { - if (virSecurityDACSetSEVLabel(mgr, def) < 0) + if (def->sec) { + switch (def->sec->sectype) { + case VIR_DOMAIN_LAUNCH_SECURITY_SEV: + if (virSecurityDACSetSEVLabel(mgr, def) < 0) + return -1; + break; + case VIR_DOMAIN_LAUNCH_SECURITY_PV: + break; + case VIR_DOMAIN_LAUNCH_SECURITY_CVM: + case VIR_DOMAIN_LAUNCH_SECURITY_NONE: + case VIR_DOMAIN_LAUNCH_SECURITY_LAST: + virReportEnumRangeError(virDomainLaunchSecurity, def->sec->sectype); return -1; + } } if (virSecurityDACGetImageIds(secdef, priv, &user, &group)) -- Gitee