[libvirt][PATCH v10 4/5] conf: Introduce SGX EPC element into device memory xml
From: Lin Yang
...
512
...
Signed-off-by: Lin Yang
---
docs/formatdomain.rst | 9 +++-
docs/schemas/domaincommon.rng | 1 +
src/conf/domain_conf.c| 6 +++
src/conf/domain_conf.h| 1 +
src/conf/domain_validate.c| 16 ++
src/qemu/qemu_alias.c | 3 ++
src/qemu/qemu_command.c | 1 +
src/qemu/qemu_domain.c| 38 +-
src/qemu/qemu_domain_address.c| 6 +++
src/qemu/qemu_driver.c| 1 +
src/qemu/qemu_process.c | 2 +
src/qemu/qemu_validate.c | 8 +++
src/security/security_apparmor.c | 1 +
src/security/security_dac.c | 2 +
src/security/security_selinux.c | 2 +
tests/qemuxml2argvdata/sgx-epc.xml| 36 +
.../sgx-epc.x86_64-latest.xml | 52 +++
tests/qemuxml2xmltest.c | 2 +
18 files changed, 172 insertions(+), 15 deletions(-)
create mode 100644 tests/qemuxml2argvdata/sgx-epc.xml
create mode 100644 tests/qemuxml2xmloutdata/sgx-epc.x86_64-latest.xml
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index e2f99c60a6..ee9328ca36 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -7912,6 +7912,11 @@ Example: usage of the memory devices
524288
+
+
+ 16384
+
+
...
@@ -7920,7 +7925,9 @@ Example: usage of the memory devices
1.2.14` Provide ``nvdimm`` model that adds a Non-Volatile DIMM module.
:since:`Since 3.2.0` Provide ``virtio-pmem`` model to add a paravirtualized
persistent memory device. :since:`Since 7.1.0` Provide ``virtio-mem`` model
- to add paravirtualized memory device. :since:`Since 7.9.0`
+ to add paravirtualized memory device. :since:`Since 7.9.0` Provide
+ ``sgx-epc`` model to add a SGX enclave page cache (EPC) memory to the guest.
+ :since:`Since 8.0.0`
``access``
An optional attribute ``access`` ( :since:`since 3.2.0` ) that provides
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 64a797de46..0aca97618f 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -6641,6 +6641,7 @@
nvdimm
virtio-pmem
virtio-mem
+ sgx-epc
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 58e696416d..1745ecff7f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1415,6 +1415,7 @@ VIR_ENUM_IMPL(virDomainMemoryModel,
"nvdimm",
"virtio-pmem",
"virtio-mem",
+ "sgx-epc",
);
VIR_ENUM_IMPL(virDomainShmemModel,
@@ -5606,6 +5607,7 @@ virDomainMemoryDefPostParse(virDomainMemoryDef *mem,
case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM:
case VIR_DOMAIN_MEMORY_MODEL_DIMM:
+case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC:
case VIR_DOMAIN_MEMORY_MODEL_NONE:
case VIR_DOMAIN_MEMORY_MODEL_LAST:
break;
@@ -14558,6 +14560,7 @@ virDomainMemorySourceDefParseXML(xmlNodePtr node,
def->nvdimmPath = virXPathString("string(./path)", ctxt);
break;
+case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC:
case VIR_DOMAIN_MEMORY_MODEL_NONE:
case VIR_DOMAIN_MEMORY_MODEL_LAST:
break;
@@ -14626,6 +14629,7 @@ virDomainMemoryTargetDefParseXML(xmlNodePtr node,
case VIR_DOMAIN_MEMORY_MODEL_NONE:
case VIR_DOMAIN_MEMORY_MODEL_DIMM:
case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM:
+case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC:
case VIR_DOMAIN_MEMORY_MODEL_LAST:
break;
}
@@ -16415,6 +16419,7 @@ virDomainMemoryFindByDefInternal(virDomainDef *def,
continue;
break;
+case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC:
case VIR_DOMAIN_MEMORY_MODEL_NONE:
case VIR_DOMAIN_MEMORY_MODEL_LAST:
break;
@@ -25851,6 +25856,7 @@ virDomainMemorySourceDefFormat(virBuffer *buf,
virBufferEscapeString(, "%s\n", def->nvdimmPath);
break;
+case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC:
case VIR_DOMAIN_MEMORY_MODEL_NONE:
case VIR_DOMAIN_MEMORY_MODEL_LAST:
break;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 0731007355..2b12e9d1ef 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2518,6 +2518,7 @@ typedef enum {
VIR_DOMAIN_MEMORY_MODEL_NVDIMM, /* nvdimm memory device */
VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM, /* virtio-pmem memory device */
VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM, /* virtio-mem memory device */
+VIR_DOMAIN_MEMORY_MODEL_SGX_EPC, /* SGX enclave page cache */
VIR_DOMAIN_MEMORY_MODEL_LAST
} virDomainMemoryModel;
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index
[libvirt][PATCH v10 3/5] Add unit test for domaincapsdata sgx
Signed-off-by: Haibin Huang --- src/conf/domain_capabilities.c| 2 -- tests/domaincapsdata/bhyve_basic.x86_64.xml | 1 + tests/domaincapsdata/bhyve_fbuf.x86_64.xml| 1 + tests/domaincapsdata/bhyve_uefi.x86_64.xml| 1 + tests/domaincapsdata/empty.xml| 1 + tests/domaincapsdata/libxl-xenfv.xml | 1 + tests/domaincapsdata/libxl-xenpv.xml | 1 + tests/domaincapsdata/qemu_2.11.0-q35.x86_64.xml | 1 + tests/domaincapsdata/qemu_2.11.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_2.11.0.s390x.xml| 1 + tests/domaincapsdata/qemu_2.11.0.x86_64.xml | 1 + tests/domaincapsdata/qemu_2.12.0-q35.x86_64.xml | 1 + tests/domaincapsdata/qemu_2.12.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_2.12.0-virt.aarch64.xml | 1 + tests/domaincapsdata/qemu_2.12.0.aarch64.xml | 1 + tests/domaincapsdata/qemu_2.12.0.ppc64.xml| 1 + tests/domaincapsdata/qemu_2.12.0.s390x.xml| 1 + tests/domaincapsdata/qemu_2.12.0.x86_64.xml | 1 + tests/domaincapsdata/qemu_2.4.0-q35.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.4.0-tcg.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.4.0.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.5.0-q35.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.5.0-tcg.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.5.0.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.6.0-q35.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.6.0-tcg.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.6.0-virt.aarch64.xml | 1 + tests/domaincapsdata/qemu_2.6.0.aarch64.xml | 1 + tests/domaincapsdata/qemu_2.6.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_2.6.0.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.7.0-q35.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.7.0-tcg.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.7.0.s390x.xml | 1 + tests/domaincapsdata/qemu_2.7.0.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.8.0-q35.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.8.0-tcg.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.8.0.s390x.xml | 1 + tests/domaincapsdata/qemu_2.8.0.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.9.0-q35.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.9.0-tcg.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.9.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_2.9.0.s390x.xml | 1 + tests/domaincapsdata/qemu_2.9.0.x86_64.xml| 1 + tests/domaincapsdata/qemu_3.0.0-q35.x86_64.xml| 1 + tests/domaincapsdata/qemu_3.0.0-tcg.x86_64.xml| 1 + tests/domaincapsdata/qemu_3.0.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_3.0.0.s390x.xml | 1 + tests/domaincapsdata/qemu_3.0.0.x86_64.xml| 1 + tests/domaincapsdata/qemu_3.1.0-q35.x86_64.xml| 1 + tests/domaincapsdata/qemu_3.1.0-tcg.x86_64.xml| 1 + tests/domaincapsdata/qemu_3.1.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_3.1.0.x86_64.xml| 1 + tests/domaincapsdata/qemu_4.0.0-q35.x86_64.xml| 1 + tests/domaincapsdata/qemu_4.0.0-tcg.x86_64.xml| 1 + tests/domaincapsdata/qemu_4.0.0-virt.aarch64.xml | 1 + tests/domaincapsdata/qemu_4.0.0.aarch64.xml | 1 + tests/domaincapsdata/qemu_4.0.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_4.0.0.s390x.xml | 1 + tests/domaincapsdata/qemu_4.0.0.x86_64.xml| 1 + tests/domaincapsdata/qemu_4.1.0-q35.x86_64.xml| 1 + tests/domaincapsdata/qemu_4.1.0-tcg.x86_64.xml| 1 + tests/domaincapsdata/qemu_4.1.0.x86_64.xml| 1 + tests/domaincapsdata/qemu_4.2.0-q35.x86_64.xml| 1 + tests/domaincapsdata/qemu_4.2.0-tcg.x86_64.xml| 1 + tests/domaincapsdata/qemu_4.2.0-virt.aarch64.xml | 1 + tests/domaincapsdata/qemu_4.2.0.aarch64.xml | 1 + tests/domaincapsdata/qemu_4.2.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_4.2.0.s390x.xml | 1 + tests/domaincapsdata/qemu_4.2.0.x86_64.xml| 1 + tests/domaincapsdata/qemu_5.0.0-q35.x86_64.xml| 1 + tests/domaincapsdata/qemu_5.0.0-tcg.x86_64.xml| 1 + tests/domaincapsdata/qemu_5.0.0-virt.aarch64.xml | 1 + tests/domaincapsdata/qemu_5.0.0.aarch64.xml | 1 + tests/domaincapsdata/qemu_5.0.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_5.0.0.x86_64.xml| 1 + tests/domaincapsdata/qemu_5.1.0-q35.x86_64.xml| 1 + tests/domaincapsdata/qemu_5.1.0-tcg.x86_64.xml| 1 + tests/domaincapsdata/qemu_5.1.0.sparc.xml | 1 + tests/domaincapsdata/qemu_5.1.0.x86_64.xml| 1 + tests/domaincapsdata/qemu_5.2.0-q35.x86_64.xml| 1 + tests/domaincapsdata/qemu_5.2.0-tcg.x86_64.xml| 1 + tests/domaincapsdata/qemu_5.2.0-virt.aarch64.xml | 1 + tests/domaincapsdata/qemu_5.2.0.aarch64.xml | 1 + tests/domaincapsdata/qemu_5.2.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_5.2.0.s390x.xml | 1 + tests/domaincapsdata/qemu_5.2.0.x86_64.xml| 1 + tests/domaincapsdata/qemu_6.0.0-q35.x86_64.xml| 1 +
[libvirt][PATCH v10 1/5] qemu: provide support to query the SGX capability
QEMU version >= 6.2.0 provides support for creating enclave on
SGX x86 platform using Software Guard Extensions (SGX) feature.
This patch adds support to query the SGX capability from the qemu.
Signed-off-by: Haibin Huang
---
src/conf/domain_capabilities.c| 10 ++
src/conf/domain_capabilities.h| 13 ++
src/libvirt_private.syms | 1 +
src/qemu/qemu_capabilities.c | 113 ++
src/qemu/qemu_capabilities.h | 4 +
src/qemu/qemu_capspriv.h | 4 +
src/qemu/qemu_monitor.c | 10 ++
src/qemu/qemu_monitor.h | 3 +
src/qemu/qemu_monitor_json.c | 72 +++
src/qemu/qemu_monitor_json.h | 9 ++
.../caps_6.2.0.x86_64.replies | 22 +++-
.../caps_6.2.0.x86_64.xml | 5 +
.../caps_7.0.0.x86_64.replies | 22 +++-
.../caps_7.0.0.x86_64.xml | 5 +
14 files changed, 285 insertions(+), 8 deletions(-)
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index c394a7a390..1170fd26df 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -78,6 +78,16 @@ virSEVCapabilitiesFree(virSEVCapability *cap)
}
+void
+virSGXCapabilitiesFree(virSGXCapability *cap)
+{
+if (!cap)
+return;
+
+VIR_FREE(cap);
+}
+
+
static void
virDomainCapsDispose(void *obj)
{
diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
index 1d2f4ac7a5..973d543ce8 100644
--- a/src/conf/domain_capabilities.h
+++ b/src/conf/domain_capabilities.h
@@ -191,6 +191,13 @@ struct _virSEVCapability {
unsigned int max_es_guests;
};
+typedef struct _virSGXCapability virSGXCapability;
+typedef virSGXCapability *virSGXCapabilityPtr;
+struct _virSGXCapability {
+bool flc;
+unsigned int epc_size;
+};
+
typedef enum {
VIR_DOMAIN_CAPS_FEATURE_IOTHREADS = 0,
VIR_DOMAIN_CAPS_FEATURE_VMCOREINFO,
@@ -227,6 +234,7 @@ struct _virDomainCaps {
virDomainCapsFeatureGIC gic;
virSEVCapability *sev;
+virSGXCapability *sgx;
/* add new domain features here */
virTristateBool features[VIR_DOMAIN_CAPS_FEATURE_LAST];
@@ -275,3 +283,8 @@ void
virSEVCapabilitiesFree(virSEVCapability *capabilities);
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virSEVCapability, virSEVCapabilitiesFree);
+
+void
+virSGXCapabilitiesFree(virSGXCapability *capabilities);
+
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virSGXCapability, virSGXCapabilitiesFree);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ba3462d849..0e74e4f20e 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -219,6 +219,7 @@ virDomainCapsEnumSet;
virDomainCapsFormat;
virDomainCapsNew;
virSEVCapabilitiesFree;
+virSGXCapabilitiesFree;
# conf/domain_conf.h
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 1b28c3f161..0e43dd2466 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -663,6 +663,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
"device.json+hotplug", /* QEMU_CAPS_DEVICE_JSON */
"hvf", /* QEMU_CAPS_HVF */
"virtio-mem-pci.prealloc", /*
QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI_PREALLOC */
+ "sgx-epc", /* QEMU_CAPS_SGX_EPC */
);
@@ -744,6 +745,8 @@ struct _virQEMUCaps {
virSEVCapability *sevCapabilities;
+virSGXCapability *sgxCapabilities;
+
/* Capabilities which may differ depending on the accelerator. */
virQEMUCapsAccel kvm;
virQEMUCapsAccel hvf;
@@ -1398,6 +1401,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
{ "virtio-vga-gl", QEMU_CAPS_VIRTIO_VGA_GL },
{ "s390-pv-guest", QEMU_CAPS_S390_PV_GUEST },
{ "virtio-mem-pci", QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI },
+{ "sgx-epc", QEMU_CAPS_SGX_EPC },
};
@@ -1962,6 +1966,22 @@ virQEMUCapsSEVInfoCopy(virSEVCapability **dst,
}
+static int
+virQEMUCapsSGXInfoCopy(virSGXCapabilityPtr *dst,
+ virSGXCapabilityPtr src)
+{
+g_autoptr(virSGXCapability) tmp = NULL;
+
+tmp = g_new0(virSGXCapability, 1);
+
+tmp->flc = src->flc;
+tmp->epc_size = src->epc_size;
+
+*dst = g_steal_pointer();
+return 0;
+}
+
+
static void
virQEMUCapsAccelCopyMachineTypes(virQEMUCapsAccel *dst,
virQEMUCapsAccel *src)
@@ -2043,6 +2063,12 @@ virQEMUCaps *virQEMUCapsNewCopy(virQEMUCaps *qemuCaps)
qemuCaps->sevCapabilities) < 0)
return NULL;
+
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SGX_EPC) &&
+virQEMUCapsSGXInfoCopy(>sgxCapabilities,
+ qemuCaps->sgxCapabilities) < 0)
+return NULL;
+
return g_steal_pointer();
}
@@ -2606,6 +2632,13 @@ virQEMUCapsGetSEVCapabilities(virQEMUCaps *qemuCaps)
}
+virSGXCapabilityPtr
[libvirt][PATCH v10 5/5] Update default CPU location in qemu QOM tree
From: Lin Yang
---
src/qemu/qemu_monitor_json.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 811db233c4..8c7f088775 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -48,7 +48,7 @@
VIR_LOG_INIT("qemu.qemu_monitor_json");
-#define QOM_CPU_PATH "/machine/unattached/device[0]"
+#define QOM_CPU_PATH "/machine/cpu[0]"
#define LINE_ENDING "\r\n"
@@ -1679,7 +1679,7 @@ int qemuMonitorJSONSystemReset(qemuMonitor *mon)
* A s390 cpu entry could look like this
* { "arch": "s390",
*"cpu-index": 0,
- *"qom-path": "/machine/unattached/device[0]",
+ *"qom-path": "/machine/cpu[0]",
*"thread_id": 3081,
*"cpu-state": "operating" }
*
@@ -1711,7 +1711,7 @@ qemuMonitorJSONExtractCPUS390Info(virJSONValue *jsoncpu,
* [{ "arch": "x86",
*"current": true,
*"CPU": 0,
- *"qom_path": "/machine/unattached/device[0]",
+ *"qom_path": "/machine/cpu[0]",
*"pc": -2130415978,
*"halted": true,
*"thread_id": 2631237,
@@ -1726,7 +1726,7 @@ qemuMonitorJSONExtractCPUS390Info(virJSONValue *jsoncpu,
* "thread-id": 0,
* "socket-id": 0
*},
- *"qom-path": "/machine/unattached/device[0]",
+ *"qom-path": "/machine/cpu[0]",
*"thread-id": 2631237,
*...},
*{...}
@@ -1737,7 +1737,7 @@ qemuMonitorJSONExtractCPUS390Info(virJSONValue *jsoncpu,
*"props": {
* "core-id": 0
*},
- *"qom-path": "/machine/unattached/device[0]",
+ *"qom-path": "/machine/cpu[0]",
*"thread-id": 1237,
*"cpu-state": "operating",
*...},
@@ -7853,7 +7853,7 @@ qemuMonitorQueryHotpluggableCpusFree(struct
qemuMonitorQueryHotpluggableCpusEntr
* "socket-id": 0
*},
*"vcpus-count": 1,
- *"qom-path": "/machine/unattached/device[0]",
+ *"qom-path": "/machine/cpu[0]",
*"type": "qemu64-x86_64-cpu"
* },
* {...}
--
2.17.1
[libvirt][PATCH v10 2/5] conf: expose SGX feature in domain capabilities
Extend hypervisor capabilities to include sgx feature. When available,
the hypervisor supports launching an VM with SGX on Intel platfrom.
The SGX feature tag privides additional details like section size and
sgx1 or sgx2.
Signed-off-by: Haibin Huang
---
docs/formatdomaincaps.html.in | 26 ++
docs/schemas/domaincaps.rng| 22 +-
src/conf/domain_capabilities.c | 21 +
src/qemu/qemu_capabilities.c | 24
4 files changed, 92 insertions(+), 1 deletion(-)
diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in
index 35b8bf3def..d932e6df80 100644
--- a/docs/formatdomaincaps.html.in
+++ b/docs/formatdomaincaps.html.in
@@ -598,6 +598,10 @@
cbitpos47/cbitpos
reduced-phys-bits1/reduced-phys-bits
/sev
+sgx
+ flcno/flc
+ epc_size1/epc_size
+/sgx
/features
/domainCapabilities
@@ -689,5 +693,27 @@
This value may be configurable in the firmware for some hosts.
+SGX capabilities
+
+Intel Software Guard Extensions (Intel SGX) capabilities are exposed
under
+the sgx element.
+Intel SGX helps protect data in use via unique application isolation
technology.
+Protect selected code and data from modification using hardened enclaves
with
+Intel SGX.
+
+
+ For more details on the SGX feature, please follow resources in the
+ SGX developer's document store. In order to use SGX with libvirt have
+ a look at SGX in domain
XML
+
+
+
+ flc
+ FLC (Flexible Launch Control), not strictly part of SGX2, but was
not part
+ of original SGX hardware either.
+ epc_size
+ The size of the SGX enclave page cache (called EPC).
+
+
diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng
index 9cbc2467ab..5ace30ae0d 100644
--- a/docs/schemas/domaincaps.rng
+++ b/docs/schemas/domaincaps.rng
@@ -270,6 +270,9 @@
+
+
+
@@ -330,7 +333,24 @@
-
+
+
+
+
+
+
+
+
+
+KiB
+
+
+
+
+
+
+
+
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index 1170fd26df..2e9f0ec225 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -100,6 +100,7 @@ virDomainCapsDispose(void *obj)
virObjectUnref(caps->cpu.custom);
virCPUDefFree(caps->cpu.hostModel);
virSEVCapabilitiesFree(caps->sev);
+virSGXCapabilitiesFree(caps->sgx);
values = >os.loader.values;
for (i = 0; i < values->nvalues; i++)
@@ -618,6 +619,25 @@ virDomainCapsFeatureSEVFormat(virBuffer *buf,
return;
}
+static void
+virDomainCapsFeatureSGXFormat(virBuffer *buf,
+ const virSGXCapability *sgx)
+{
+if (!sgx) {
+return; // will delete in test patch
+virBufferAddLit(buf, "\n");
+} else {
+return; // will delete in test patch
+virBufferAddLit(buf, "\n");
+virBufferAdjustIndent(buf, 2);
+virBufferAsprintf(buf, "%s\n", sgx->flc ? "yes" : "no");
+virBufferAsprintf(buf, "%d\n",
sgx->epc_size);
+virBufferAdjustIndent(buf, -2);
+virBufferAddLit(buf, "\n");
+}
+
+return;
+}
static void
virDomainCapsFormatFeatures(const virDomainCaps *caps,
@@ -638,6 +658,7 @@ virDomainCapsFormatFeatures(const virDomainCaps *caps,
}
virDomainCapsFeatureSEVFormat(, caps->sev);
+virDomainCapsFeatureSGXFormat(, caps->sgx);
virXMLFormatElement(buf, "features", NULL, );
}
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 0e43dd2466..745d203241 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -6632,6 +6632,29 @@ virQEMUCapsFillDomainFeatureS390PVCaps(virQEMUCaps
*qemuCaps,
}
}
+/**
+ * virQEMUCapsFillDomainFeatureiSGXCaps:
+ * @qemuCaps: QEMU capabilities
+ * @domCaps: domain capabilities
+ *
+ * Take the information about SGX capabilities that has been obtained
+ * using the 'query-sgx-capabilities' QMP command and stored in @qemuCaps
+ * and convert it to a form suitable for @domCaps.
+ */
+static void
+virQEMUCapsFillDomainFeatureSGXCaps(virQEMUCaps *qemuCaps,
+virDomainCaps *domCaps)
+{
+virSGXCapability *cap = qemuCaps->sgxCapabilities;
+
+if (!cap)
+return;
+
+domCaps->sgx = g_new0(virSGXCapability, 1);
+
+domCaps->sgx->flc = cap->flc;
+domCaps->sgx->epc_size = cap->epc_size;
+}
int
virQEMUCapsFillDomainCaps(virQEMUCaps *qemuCaps,
@@ -6684,6 +6707,7 @@ virQEMUCapsFillDomainCaps(virQEMUCaps *qemuCaps,
virQEMUCapsFillDomainFeatureGICCaps(qemuCaps, domCaps);
virQEMUCapsFillDomainFeatureSEVCaps(qemuCaps, domCaps);
virQEMUCapsFillDomainFeatureS390PVCaps(qemuCaps, domCaps);
+
[libvirt][PATCH v10 0/5] Support query and use SGX
This patch series provides support for enabling Intel's Software Guard Extensions (SGX) feature in guest VM. Giving the SGX support in QEMU had been merged. Intel SGX is a set of instructions that increases the security of application code and data, giving them more protection from disclosure or modification. Developers can partition sensitive information into enclaves, which are areas of execution in memory with more security protection. It depends on QEMU fixing[1], which will move cpu QOM object from /machine/unattached/device[nn] to /machine/cpu[nn]. It requires libvirt to change the default cpu QOM object location once QEMU patch gets accepted, but it is out of this SGX patch scope. The typical flow looks below at very high level: 1. Calls virConnectGetDomainCapabilities API to domain capabilities that includes the following SGX information. ... N ... 2. User requests to start a guest calling virCreateXML() with SGX requirement. It does not support NUMA yet, since latest QEMU 6.2 release does not support NUMA. It should contain ... N ... [1] https://lists.nongnu.org/archive/html/qemu-devel/2022-01/msg03534.html Haibin Huang (3): qemu: provide support to query the SGX capability conf: expose SGX feature in domain capabilities Add unit test for domaincapsdata sgx Lin Yang (2): conf: Introduce SGX EPC element into device memory xml Update default CPU location in qemu QOM tree docs/formatdomain.rst | 9 +- docs/formatdomaincaps.html.in | 26 docs/schemas/domaincaps.rng | 22 ++- docs/schemas/domaincommon.rng | 1 + src/conf/domain_capabilities.c| 29 src/conf/domain_capabilities.h| 13 ++ src/conf/domain_conf.c| 6 + src/conf/domain_conf.h| 1 + src/conf/domain_validate.c| 16 ++ src/libvirt_private.syms | 1 + src/qemu/qemu_alias.c | 3 + src/qemu/qemu_capabilities.c | 137 ++ src/qemu/qemu_capabilities.h | 4 + src/qemu/qemu_capspriv.h | 4 + src/qemu/qemu_command.c | 1 + src/qemu/qemu_domain.c| 38 +++-- src/qemu/qemu_domain_address.c| 6 + src/qemu/qemu_driver.c| 1 + src/qemu/qemu_monitor.c | 10 ++ src/qemu/qemu_monitor.h | 3 + src/qemu/qemu_monitor_json.c | 84 ++- src/qemu/qemu_monitor_json.h | 9 ++ src/qemu/qemu_process.c | 2 + src/qemu/qemu_validate.c | 8 + src/security/security_apparmor.c | 1 + src/security/security_dac.c | 2 + src/security/security_selinux.c | 2 + tests/domaincapsdata/bhyve_basic.x86_64.xml | 1 + tests/domaincapsdata/bhyve_fbuf.x86_64.xml| 1 + tests/domaincapsdata/bhyve_uefi.x86_64.xml| 1 + tests/domaincapsdata/empty.xml| 1 + tests/domaincapsdata/libxl-xenfv.xml | 1 + tests/domaincapsdata/libxl-xenpv.xml | 1 + .../domaincapsdata/qemu_2.11.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_2.11.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_2.11.0.s390x.xml| 1 + tests/domaincapsdata/qemu_2.11.0.x86_64.xml | 1 + .../domaincapsdata/qemu_2.12.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_2.12.0-tcg.x86_64.xml | 1 + .../qemu_2.12.0-virt.aarch64.xml | 1 + tests/domaincapsdata/qemu_2.12.0.aarch64.xml | 1 + tests/domaincapsdata/qemu_2.12.0.ppc64.xml| 1 + tests/domaincapsdata/qemu_2.12.0.s390x.xml| 1 + tests/domaincapsdata/qemu_2.12.0.x86_64.xml | 1 + .../domaincapsdata/qemu_2.4.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_2.4.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_2.4.0.x86_64.xml| 1 + .../domaincapsdata/qemu_2.5.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_2.5.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_2.5.0.x86_64.xml| 1 + .../domaincapsdata/qemu_2.6.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_2.6.0-tcg.x86_64.xml | 1 + .../qemu_2.6.0-virt.aarch64.xml | 1 + tests/domaincapsdata/qemu_2.6.0.aarch64.xml | 1 + tests/domaincapsdata/qemu_2.6.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_2.6.0.x86_64.xml| 1 + .../domaincapsdata/qemu_2.7.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_2.7.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_2.7.0.s390x.xml | 1 + tests/domaincapsdata/qemu_2.7.0.x86_64.xml| 1 + .../domaincapsdata/qemu_2.8.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_2.8.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_2.8.0.s390x.xml |
[libvirt PATCH 4/4] maint: remove unnecessary virutil.h includes
Signed-off-by: Ján Tomko --- src/esx/esx_vi.c | 1 - src/nwfilter/nwfilter_ebiptables_driver.c | 1 - src/openvz/openvz_conf.c | 1 - src/util/virdnsmasq.c | 1 - src/util/virfirewalld.c | 1 - src/vmware/vmware_conf.c | 1 - tests/testutilsqemu.c | 1 - 7 files changed, 7 deletions(-) diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index bc43fa0af1..96ce827555 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -35,7 +35,6 @@ #include "esx_vi_methods.h" #include "esx_util.h" #include "virstring.h" -#include "virutil.h" #define VIR_FROM_THIS VIR_FROM_ESX diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 058f2ec559..54065a0f75 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -43,7 +43,6 @@ #include "configmake.h" #include "virstring.h" #include "virfirewall.h" -#include "virutil.h" #define VIR_FROM_THIS VIR_FROM_NWFILTER diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index afdd97e3c9..191c79e1e2 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -40,7 +40,6 @@ #include "vircommand.h" #include "virstring.h" #include "virhostcpu.h" -#include "virutil.h" #define VIR_FROM_THIS VIR_FROM_OPENVZ diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c index 68aaa83b2a..fd4efa802c 100644 --- a/src/util/virdnsmasq.c +++ b/src/util/virdnsmasq.c @@ -34,7 +34,6 @@ #include "datatypes.h" #include "virbitmap.h" #include "virdnsmasq.h" -#include "virutil.h" #include "vircommand.h" #include "viralloc.h" #include "virerror.h" diff --git a/src/util/virfirewalld.c b/src/util/virfirewalld.c index f467756f26..c909901833 100644 --- a/src/util/virfirewalld.c +++ b/src/util/virfirewalld.c @@ -28,7 +28,6 @@ #include "virfirewalldpriv.h" #include "viralloc.h" #include "virerror.h" -#include "virutil.h" #include "virlog.h" #include "virgdbus.h" #include "virenum.h" diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index bce690bbdf..ebba435cc4 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -32,7 +32,6 @@ #include "vmware_conf.h" #include "virstring.h" #include "virlog.h" -#include "virutil.h" #define VIR_FROM_THIS VIR_FROM_VMWARE diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index cf66c12622..646ef415d1 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -12,7 +12,6 @@ # include "qemu/qemu_capspriv.h" # include "virstring.h" # include "virfilecache.h" -# include "virutil.h" # define VIR_FROM_THIS VIR_FROM_QEMU -- 2.34.1
[libvirt PATCH 3/4] virParseVersionString: rename to virStringParseVersion
Signed-off-by: Ján Tomko
---
src/bhyve/bhyve_driver.c | 2 +-
src/ch/ch_conf.c | 2 +-
src/esx/esx_vi.c | 8
src/libvirt_private.syms | 2 +-
src/lxc/lxc_driver.c | 2 +-
src/nwfilter/nwfilter_ebiptables_driver.c | 4 ++--
src/openvz/openvz_conf.c | 2 +-
src/util/virdnsmasq.c | 2 +-
src/util/virfirewalld.c | 2 +-
src/util/virstring.c | 7 ---
src/util/virstring.h | 4 ++--
src/vbox/vbox_common.c| 2 +-
src/vmware/vmware_conf.c | 2 +-
src/vz/vz_utils.c | 2 +-
tests/testutilsqemu.c | 2 +-
tests/utiltest.c | 2 +-
tools/virt-host-validate-common.c | 2 +-
17 files changed, 25 insertions(+), 24 deletions(-)
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index f291f12e50..f85f879abb 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -267,7 +267,7 @@ bhyveConnectGetVersion(virConnectPtr conn, unsigned long
*version)
uname();
-if (virParseVersionString(ver.release, version, true) < 0) {
+if (virStringParseVersion(version, ver.release, true) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unknown release: %s"), ver.release);
return -1;
diff --git a/src/ch/ch_conf.c b/src/ch/ch_conf.c
index be12934dcd..88a23a59cd 100644
--- a/src/ch/ch_conf.c
+++ b/src/ch/ch_conf.c
@@ -204,7 +204,7 @@ chExtractVersion(virCHDriver *driver)
return -1;
}
-if (virParseVersionString(tmp, , true) < 0) {
+if (virStringParseVersion(, tmp, true) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to parse cloud-hypervisor version: %s"), tmp);
return -1;
diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
index 80ed6199e3..bc43fa0af1 100644
--- a/src/esx/esx_vi.c
+++ b/src/esx/esx_vi.c
@@ -869,8 +869,8 @@ esxVI_Context_Connect(esxVI_Context *ctx, const char *url,
return -1;
}
-if (virParseVersionString(ctx->service->about->apiVersion,
- >apiVersion, true) < 0) {
+if (virStringParseVersion(>apiVersion,
+ ctx->service->about->apiVersion, true) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Could not parse VI API version '%s'"),
ctx->service->about->apiVersion);
@@ -884,8 +884,8 @@ esxVI_Context_Connect(esxVI_Context *ctx, const char *url,
return -1;
}
-if (virParseVersionString(ctx->service->about->version,
- >productVersion, true) < 0) {
+if (virStringParseVersion(>productVersion,
+ ctx->service->about->version, true) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Could not parse product version '%s'"),
ctx->service->about->version);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 38506c52c6..2d1896a995 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3302,7 +3302,6 @@ virStorageFileParseBackingStoreStr;
# util/virstring.h
-virParseVersionString;
virSkipSpaces;
virSkipSpacesAndBackslash;
virSkipSpacesBackwards;
@@ -3319,6 +3318,7 @@ virStringIsPrintable;
virStringMatch;
virStringMatchesNameSuffix;
virStringParsePort;
+virStringParseVersion;
virStringParseYesNo;
virStringReplace;
virStringSearch;
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 7bc39120ee..8000871f7a 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1656,7 +1656,7 @@ static int lxcConnectGetVersion(virConnectPtr conn,
unsigned long *version)
if (virConnectGetVersionEnsureACL(conn) < 0)
return -1;
-if (virParseVersionString(ver.release, version, true) < 0) {
+if (virStringParseVersion(version, ver.release, true) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, _("Unknown release: %s"),
ver.release);
return -1;
}
diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c
b/src/nwfilter/nwfilter_ebiptables_driver.c
index 0c420dd91f..058f2ec559 100644
--- a/src/nwfilter/nwfilter_ebiptables_driver.c
+++ b/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -3656,7 +3656,7 @@ ebiptablesDriverProbeCtdir(void)
}
/* following Linux lxr, the logic was inverted in 2.6.39 */
-if (virParseVersionString(utsname.release, , true) < 0) {
+if (virStringParseVersion(, utsname.release, true) < 0) {
VIR_ERROR(_("Could not determine kernel version from string %s"),
utsname.release);
return;
@@ -3689,7 +3689,7 @@ ebiptablesDriverProbeStateMatchQuery(virFirewall *fw
G_GNUC_UNUSED,
* 'iptables v1.4.16'
*/
if (!(tmp =
[libvirt PATCH 2/4] util: virParseVersionString: move to virstring.c
Signed-off-by: Ján Tomko
---
src/libvirt_private.syms | 2 +-
src/util/virstring.c | 47
src/util/virstring.h | 4
src/util/virutil.c | 46 ---
src/util/virutil.h | 3 ---
5 files changed, 52 insertions(+), 50 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index bc6fa191bf..38506c52c6 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3302,6 +3302,7 @@ virStorageFileParseBackingStoreStr;
# util/virstring.h
+virParseVersionString;
virSkipSpaces;
virSkipSpacesAndBackslash;
virSkipSpacesBackwards;
@@ -3546,7 +3547,6 @@ virMemoryLimitIsSet;
virMemoryLimitTruncate;
virMemoryMaxValue;
virParseOwnershipIds;
-virParseVersionString;
virPipe;
virPipeNonBlock;
virPipeQuiet;
diff --git a/src/util/virstring.c b/src/util/virstring.c
index cee56debca..a7ce566963 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -1019,3 +1019,50 @@ int virStringParseYesNo(const char *str, bool *result)
return 0;
}
+
+
+/**
+ * virParseVersionString:
+ * @str: const char pointer to the version string
+ * @version: unsigned long pointer to output the version number
+ * @allowMissing: true to treat 3 like 3.0.0, false to error out on
+ * missing minor or micro
+ *
+ * Parse an unsigned version number from a version string. Expecting
+ * 'major.minor.micro' format, ignoring an optional suffix.
+ *
+ * The major, minor and micro numbers are encoded into a single version number:
+ *
+ * 100 * major + 1000 * minor + micro
+ *
+ * Returns the 0 for success, -1 for error.
+ */
+int
+virParseVersionString(const char *str, unsigned long *version,
+ bool allowMissing)
+{
+unsigned int major, minor = 0, micro = 0;
+char *tmp;
+
+if (virStrToLong_ui(str, , 10, ) < 0)
+return -1;
+
+if (!allowMissing && *tmp != '.')
+return -1;
+
+if ((*tmp == '.') && virStrToLong_ui(tmp + 1, , 10, ) < 0)
+return -1;
+
+if (!allowMissing && *tmp != '.')
+return -1;
+
+if ((*tmp == '.') && virStrToLong_ui(tmp + 1, , 10, ) < 0)
+return -1;
+
+if (major > UINT_MAX / 100 || minor > 999 || micro > 999)
+return -1;
+
+*version = 100 * major + 1000 * minor + micro;
+
+return 0;
+}
diff --git a/src/util/virstring.h b/src/util/virstring.h
index 45f07ddd7a..1dbeb7445f 100644
--- a/src/util/virstring.h
+++ b/src/util/virstring.h
@@ -135,3 +135,7 @@ int virStringParsePort(const char *str,
int virStringParseYesNo(const char *str,
bool *result)
G_GNUC_WARN_UNUSED_RESULT;
+
+int virParseVersionString(const char *str,
+ unsigned long *version,
+ bool allowMissing);
diff --git a/src/util/virutil.c b/src/util/virutil.c
index 8a6efd4d5c..fe5500726e 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -231,52 +231,6 @@ virScaleInteger(unsigned long long *value, const char
*suffix,
}
-/**
- * virParseVersionString:
- * @str: const char pointer to the version string
- * @version: unsigned long pointer to output the version number
- * @allowMissing: true to treat 3 like 3.0.0, false to error out on
- * missing minor or micro
- *
- * Parse an unsigned version number from a version string. Expecting
- * 'major.minor.micro' format, ignoring an optional suffix.
- *
- * The major, minor and micro numbers are encoded into a single version number:
- *
- * 100 * major + 1000 * minor + micro
- *
- * Returns the 0 for success, -1 for error.
- */
-int
-virParseVersionString(const char *str, unsigned long *version,
- bool allowMissing)
-{
-unsigned int major, minor = 0, micro = 0;
-char *tmp;
-
-if (virStrToLong_ui(str, , 10, ) < 0)
-return -1;
-
-if (!allowMissing && *tmp != '.')
-return -1;
-
-if ((*tmp == '.') && virStrToLong_ui(tmp + 1, , 10, ) < 0)
-return -1;
-
-if (!allowMissing && *tmp != '.')
-return -1;
-
-if ((*tmp == '.') && virStrToLong_ui(tmp + 1, , 10, ) < 0)
-return -1;
-
-if (major > UINT_MAX / 100 || minor > 999 || micro > 999)
-return -1;
-
-*version = 100 * major + 1000 * minor + micro;
-
-return 0;
-}
-
/**
* Format @val as a base-10 decimal number, in the
* buffer @buf of size @buflen. To allocate a suitable
diff --git a/src/util/virutil.h b/src/util/virutil.h
index 6bb55918ad..6adebde242 100644
--- a/src/util/virutil.h
+++ b/src/util/virutil.h
@@ -44,9 +44,6 @@ int virScaleInteger(unsigned long long *value, const char
*suffix,
unsigned long long scale, unsigned long long limit)
ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT;
-int virParseVersionString(const char *str, unsigned long *version,
- bool allowMissing);
-
char *virFormatIntDecimal(char *buf, size_t buflen, int val)
[libvirt PATCH 1/4] maint: add required includes
Some files do not include what they use and rely on virutil.h to pull in the necessary header files. Fix it. Signed-off-by: Ján Tomko --- src/nwfilter/nwfilter_ebiptables_driver.c | 1 + src/util/virfirewalld.c | 1 + 2 files changed, 2 insertions(+) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 63ba69f794..0c420dd91f 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -22,6 +22,7 @@ #include +#include #include #include #include diff --git a/src/util/virfirewalld.c b/src/util/virfirewalld.c index 4795bf7925..5a0a45f324 100644 --- a/src/util/virfirewalld.c +++ b/src/util/virfirewalld.c @@ -32,6 +32,7 @@ #include "virlog.h" #include "virgdbus.h" #include "virenum.h" +#include "virstring.h" #define VIR_FROM_THIS VIR_FROM_FIREWALLD -- 2.34.1
[libvirt PATCH 0/4] move virParseVersionString to virstring.c
And clean up some includes while doing it. Ján Tomko (4): maint: add required includes util: virParseVersionString: move to virstring.c virParseVersionString: rename to virStringParseVersion maint: remove unnecessary virutil.h includes src/bhyve/bhyve_driver.c | 2 +- src/ch/ch_conf.c | 2 +- src/esx/esx_vi.c | 9 ++--- src/libvirt_private.syms | 2 +- src/lxc/lxc_driver.c | 2 +- src/nwfilter/nwfilter_ebiptables_driver.c | 6 +-- src/openvz/openvz_conf.c | 3 +- src/util/virdnsmasq.c | 3 +- src/util/virfirewalld.c | 4 +- src/util/virstring.c | 48 +++ src/util/virstring.h | 4 ++ src/util/virutil.c| 46 -- src/util/virutil.h| 3 -- src/vbox/vbox_common.c| 2 +- src/vmware/vmware_conf.c | 3 +- src/vz/vz_utils.c | 2 +- tests/testutilsqemu.c | 3 +- tests/utiltest.c | 2 +- tools/virt-host-validate-common.c | 2 +- 19 files changed, 73 insertions(+), 75 deletions(-) -- 2.34.1
Re: The unix domain socket remains even after the VM is destroyed
On Tue, Jan 25, 2022 at 02:18:29PM -0500, Masayoshi Mizuma wrote:
Hello,
I found an issue that libvirt isn't close an unix domain socket to connect to
the qemu monitor
even after the VM is destroyed.
This issue happens since commit 695bdb3841 ("src: ensure GSource background unref
happens in correct event loop")
on the system whose glib version is 2.56.
I would appreciate it if you could give any ideas to solve the issue.
My very rough and fast guess is that this happens due to our workaround
while running on a system on which the fix was actually backported. You
can remove the workaround (which is implemented for glib < 2.64.0), keep
only the new code and see if it reproduces. I know it's not very
scientific, but we already faced so many workarounds for old buggy glib
that it's tiresome to try and fix all that when it goes away with update
of glib. If the above does not help, then I would look at what could
actually happening (maybe the unref does not get scheduled because the
event loop it is requested on does not run any more, but that's just a
guess).
The socket is allocated in qemuMonitorOpenUnix(), and used by the
monitor->socket
and monitor->watch:
qemuMonitorOpen
qemuMonitorOpenUnix
if ((monfd = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) {
qemuMonitorOpenInternal
mon->socket = g_socket_new_from_fd(fd, );
qemuMonitorRegister
mon->watch = g_socket_create_source(mon->socket,
Usually, the socket is closed when the reference counter of the glib object
(mon->socket and mon->watch) gets 0:
qemuMonitorClose
qemuMonitorUnregister
vir_g_source_unref(mon->watch, mon->context);
g_source_set_callback(idle, virEventGLibSourceUnrefIdle, src, NULL);
virEventGLibSourceUnrefIdle
g_source_unref(src); <== unref monitor->watch
g_object_unref(mon->socket); <== unref monitor->socket
It seems that the callback virEventGLibSourceUnrefIdle() to unref the
monitor->watch doesn't
work when qemuMonitorUnregister() is called via qemuProcessStop(), so the
socket isn't closed.
I'm not sure why the callback doesn't work at that time. I suspect that the VM
is closing
so the main loop of the monitor doesn't work any more.
We can close the socket to add g_socket_close() before unref the mon->socket,
however,
it may remain a memory leak because of mon->watch (GSource object), so probably
it isn't a good idea to close the socket.
We can unref the mon->watch to set NULL to the second argument of
vir_g_source_unref()
because the default main loop still works at that time, however, I'm not
sure it's an appropriate way to avoid the gobject issue which the commit
solves...
I found this issue on the qemu monitor, and probably the qemu agent has the
same issue
because the socket procedure is almost the same as the monitor.
I would appreciate it if you could give any ideas to solve this issue.
Following is to observe the callback working:
---
diff --git a/src/util/glibcompat.c b/src/util/glibcompat.c
index eb6dcc0111..b8b1770424 100644
--- a/src/util/glibcompat.c
+++ b/src/util/glibcompat.c
@@ -24,6 +24,9 @@
#include "glibcompat.h"
+#include "virlog.h"
+
+VIR_LOG_INIT("util.glibcompat");
/*
* Note that because of the GLIB_VERSION_MAX_ALLOWED constant in
* config-post.h, allowing use of functions from newer GLib via
@@ -244,6 +247,7 @@ virEventGLibSourceUnrefIdle(gpointer data)
GSource *src = data;
g_source_unref(src);
+VIR_DEBUG("unrefed: %p", src);
return FALSE;
}
@@ -257,6 +261,7 @@ void vir_g_source_unref(GSource *src, GMainContext *ctx)
g_source_attach(idle, ctx);
g_source_unref(idle);
+VIR_DEBUG("unref registered: %p ctx: %p", src, ctx);
}
#endif
---
Case the mon->watch (0x28008af0) is unreffed correctly
(via qemuMonitorUpdateWatch()):
18:54:15.403+: 16845: debug : qemuMonitorEmitResume:1159 :
mon=0x683ac020
18:54:15.403+: 16845: debug : qemuProcessHandleResume:713 : Transitioned
guest test-vm into running state, reason 'booted', event detail 0
18:54:15.404+: 16845: debug : vir_g_source_unref:264 : unref registered:
0x28008af0 ctx: 0x780169a0
18:54:15.404+: 16845: debug : qemuMonitorJSONIOProcessLine:222 : Line [{"return": {},
"id": "libvirt-10"}]
18:54:15.404+: 16845: info : qemuMonitorJSONIOProcessLine:242 : QEMU_MONITOR_RECV_REPLY:
mon=0x683ac020 reply={"return": {}, "id": "libvirt-10"}
18:54:15.404+: 16845: debug : vir_g_source_unref:264 : unref registered:
0x2819a260 ctx: 0x780169a0
18:54:15.404+: 16845: debug : virEventGLibSourceUnrefIdle:250 : unrefed:
0x28008af0
Case the mon->watch (0x7802bb30) isn't unreffed
(via qemuProcessStop()):
18:54:15.642+: 16589: debug : qemuProcessStop:8008 : Shutting down
vm=0xd40edec0 name=test-vm id=3 pid=16842, reason=destroyed, asyncJob=none,
flags=0x0
18:54:15.642+: 16589: debug : qemuDomainLogAppendMessage:6733 : Append log
message (vm='test-vm'
[libvirt PATCH] syntax-check: https: list the HTTP-only sites
Instead of listing the sites that surely support HTTPS, list the ones that don't. Signed-off-by: Ján Tomko --- build-aux/syntax-check.mk | 66 --- 1 file changed, 54 insertions(+), 12 deletions(-) diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index 4d396699c9..d5cdb3c70e 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -874,20 +874,62 @@ sc_prohibit_obj_free_apis_in_virsh: halt='avoid using public virXXXFree in virsh, use virsh-prefixed wrappers instead' \ $(_sc_search_regexp) -https_sites = www.libvirt.org -https_sites += libvirt.org -https_sites += security.libvirt.org -https_sites += qemu.org -https_sites += www.qemu.org -https_sites += wiki.qemu.org -https_sites += linux-kvm.org -https_sites += www.linux-kvm.org - -https_re= ($(subst $(space),|,$(https_sites))) +# Links in various schemas +http_sites = libvirt.org.*\/schemas\/ +http_sites += \.dtd +http_sites += libosinfo +http_sites += localhost +http_sites += rdf:resource +http_sites += schemas.dmtf.org +http_sites += schemas.microsoft.com +http_sites += schemas.xmlsoap.org +http_sites += www.inkscape.org +http_sites += www.innotek.de +http_sites += www.w3.org +http_sites += xmlns + +# Links in licenses +http_sites += scripts.sil.org +http_sites += www.gnu.org\/licenses\/ +http_sites += www.sun.com + +# Example links +http_sites += example.com +http_sites += example.org +http_sites += herp.derp + +# HTTP-only sites +http_sites += 0pointer.de +http_sites += mah.everybody.org +http_sites += mingw.org +http_sites += munin.projects.linpro.no +http_sites += netcat.sourceforge.net +http_sites += snooze.inria.fr +http_sites += www.nimbusproject.org +http_sites += www.odin.com +http_sites += www.sflow.net +http_sites += xmlsoft.org +http_sites += etallen.com + +# dead sites +http_sites += blog.lystor.org.ua +http_sites += blog.mes-stats.fr +http_sites += cc1.ifj.edu.pl +http_sites += www.javvin.com + +# 404 links +http_sites += publib.boulder.ibm.com +http_sites += kerneltrap.org +http_sites += valloric.github.io +http_sites += www.microsoft.com +http_sites += xenbits.xen.org +http_sites += lovezutto.googlepages.com + +http_re= ($(subst $(space),|,$(http_sites))) sc_prohibit_http_urls: - @prohibit='http://$(https_re)' \ - exclude="/schemas/" \ + @prohibit='http://\w' \ + exclude="$(http_re)" \ halt='Links must use https:// protocol' \ $(_sc_search_regexp) -- 2.34.1
[libvirt PATCH 3/4] libxl: assume LIBXL_HAVE_SRM_V2
Introduced in Xen 4.6.0 by:
commit 3a9ace0147d48af49ffd34628f9510f248f2f588
tools/libxc+libxl+xl: Restore v2 streams
Signed-off-by: Ján Tomko
---
src/libxl/libxl_conf.h | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index b74fedc2c8..e0662d90b3 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -149,11 +149,7 @@ struct _libxlDriverPrivate {
};
#define LIBXL_SAVE_MAGIC "libvirt-xml\n \0 \r"
-#ifdef LIBXL_HAVE_SRM_V2
-# define LIBXL_SAVE_VERSION 2
-#else
-# define LIBXL_SAVE_VERSION 1
-#endif
+#define LIBXL_SAVE_VERSION 2
typedef struct _libxlSavefileHeader libxlSavefileHeader;
struct _libxlSavefileHeader {
--
2.34.1
[libvirt PATCH 4/4] libxl: assume LIBXL_HAVE_PVUSB
Introduced in Xen 4.7 by commit: commit bf7628f087b212052a0e9f024044b2790c33f820 libxl: add pvusb API Signed-off-by: Ján Tomko --- src/libxl/libxl_conf.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h index e0662d90b3..b74f455b69 100644 --- a/src/libxl/libxl_conf.h +++ b/src/libxl/libxl_conf.h @@ -204,14 +204,12 @@ libxlMakeVfb(virPortAllocatorRange *graphicsports, int libxlMakePCI(virDomainHostdevDef *hostdev, libxl_device_pci *pcidev); -#ifdef LIBXL_HAVE_PVUSB int libxlMakeUSBController(virDomainControllerDef *controller, libxl_device_usbctrl *usbctrl); int libxlMakeUSB(virDomainHostdevDef *hostdev, libxl_device_usbdev *usbdev); -#endif virDomainXMLOption * libxlCreateXMLConf(libxlDriverPrivate *driver); -- 2.34.1
[libvirt PATCH 1/4] libxl: assume LIBXL_HAVE_DEVICE_BACKEND_DOMNAME
Introduced in 4.3.0 by xen commit:
commit ef496b81f0336f09968a318e7f81151dd4f5a0cc
libxl: postpone backend name resolution
Signed-off-by: Ján Tomko
---
src/libxl/libxl_conf.c | 20 ++--
1 file changed, 2 insertions(+), 18 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 5d87b999f2..f062f8e958 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -1191,16 +1191,8 @@ libxlMakeDisk(virDomainDiskDef *l_disk,
libxl_device_disk *x_disk)
return -1;
}
-if (l_disk->domain_name) {
-#ifdef LIBXL_HAVE_DEVICE_BACKEND_DOMNAME
+if (l_disk->domain_name)
x_disk->backend_domname = g_strdup(l_disk->domain_name);
-#else
-virReportError(VIR_ERR_XML_DETAIL, "%s",
-_("this version of libxenlight does not "
- "support backend domain name"));
-return -1;
-#endif
-}
return 0;
}
@@ -1408,16 +1400,8 @@ libxlMakeNic(virDomainDef *def,
goto cleanup;
}
-if (l_nic->domain_name) {
-#ifdef LIBXL_HAVE_DEVICE_BACKEND_DOMNAME
+if (l_nic->domain_name)
x_nic->backend_domname = g_strdup(l_nic->domain_name);
-#else
-virReportError(VIR_ERR_XML_DETAIL, "%s",
-_("this version of libxenlight does not "
- "support backend domain name"));
-goto cleanup;
-#endif
-}
/*
* Set bandwidth.
--
2.34.1
[libvirt PATCH 2/4] libxl: remove LIBXL_ATTR_UNUSED
Unused as of: commit 446d09149802677546449fa2dd253f3ebce377ac libxl: pass driver config to libxlMakeDomBuildInfo All other usage of LIBXL_HAVE_DEVICE_CHANNEL was removed by: commit e58004d70aceb560fba64803e566b8be3ef93940 Xen: Remove unneeded LIBXL_HAVE_* ifdefs Signed-off-by: Ján Tomko --- src/libxl/libxl_conf.h | 5 - 1 file changed, 5 deletions(-) diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h index f833daf669..b74fedc2c8 100644 --- a/src/libxl/libxl_conf.h +++ b/src/libxl/libxl_conf.h @@ -220,11 +220,6 @@ libxlMakeUSB(virDomainHostdevDef *hostdev, libxl_device_usbdev *usbdev); virDomainXMLOption * libxlCreateXMLConf(libxlDriverPrivate *driver); -#ifdef LIBXL_HAVE_DEVICE_CHANNEL -# define LIBXL_ATTR_UNUSED -#else -# define LIBXL_ATTR_UNUSED G_GNUC_UNUSED -#endif int libxlBuildDomainConfig(virPortAllocatorRange *graphicsports, virDomainDef *def, -- 2.34.1
[libvirt PATCH 0/4] libxl: clean up more LIBXL_HAVE constants
We still were checking for some that were introduced before Xen 4.9 Ján Tomko (4): libxl: assume LIBXL_HAVE_DEVICE_BACKEND_DOMNAME libxl: remove LIBXL_ATTR_UNUSED libxl: assume LIBXL_HAVE_SRM_V2 libxl: assume LIBXL_HAVE_PVUSB src/libxl/libxl_conf.c | 20 ++-- src/libxl/libxl_conf.h | 13 + 2 files changed, 3 insertions(+), 30 deletions(-) -- 2.34.1
[libvirt PATCH] tests: refactor testSELinuxLoadDef
Since its introduction in
commit 907a39e735d256b8428ed4c77009d1f713aea19b
Add a test suite for validating SELinux labelling
this function did not return NULL on OOM.
Since we abort on OOM now, switch testSELinuxMungePath to void,
return NULL explicitly on XML parsing failure and remove
the (now pointless) cleanup label.
Signed-off-by: Ján Tomko
---
tests/securityselinuxlabeltest.c | 35 +++-
1 file changed, 12 insertions(+), 23 deletions(-)
diff --git a/tests/securityselinuxlabeltest.c b/tests/securityselinuxlabeltest.c
index 09902e1c54..b62162fe9f 100644
--- a/tests/securityselinuxlabeltest.c
+++ b/tests/securityselinuxlabeltest.c
@@ -82,16 +82,12 @@ testUserXattrEnabled(void)
return ret;
}
-static int
+static void
testSELinuxMungePath(char **path)
{
-char *tmp;
-
-tmp = g_strdup_printf("%s/securityselinuxlabeldata%s", abs_builddir,
*path);
-
-VIR_FREE(*path);
+char *tmp = g_strdup_printf("%s/securityselinuxlabeldata%s", abs_builddir,
*path);
+g_free(*path);
*path = tmp;
-return 0;
}
static int
@@ -154,7 +150,7 @@ testSELinuxLoadFileList(const char *testname,
static virDomainDef *
testSELinuxLoadDef(const char *testname)
{
-char *xmlfile = NULL;
+g_autofree char *xmlfile = NULL;
virDomainDef *def = NULL;
size_t i;
@@ -163,15 +159,14 @@ testSELinuxLoadDef(const char *testname)
if (!(def = virDomainDefParseFile(xmlfile, driver.xmlopt,
NULL, 0)))
-goto cleanup;
+return NULL;
for (i = 0; i < def->ndisks; i++) {
if (def->disks[i]->src->type != VIR_STORAGE_TYPE_FILE &&
def->disks[i]->src->type != VIR_STORAGE_TYPE_BLOCK)
continue;
-if (testSELinuxMungePath(>disks[i]->src->path) < 0)
-goto cleanup;
+testSELinuxMungePath(>disks[i]->src->path);
}
for (i = 0; i < def->nserials; i++) {
@@ -182,23 +177,17 @@ testSELinuxLoadDef(const char *testname)
continue;
if (def->serials[i]->source->type == VIR_DOMAIN_CHR_TYPE_UNIX) {
-if (testSELinuxMungePath(>serials[i]->source->data.nix.path)
< 0)
-goto cleanup;
+testSELinuxMungePath(>serials[i]->source->data.nix.path);
} else {
-if (testSELinuxMungePath(>serials[i]->source->data.file.path)
< 0)
-goto cleanup;
+testSELinuxMungePath(>serials[i]->source->data.file.path);
}
}
-if (def->os.kernel &&
-testSELinuxMungePath(>os.kernel) < 0)
-goto cleanup;
-if (def->os.initrd &&
-testSELinuxMungePath(>os.initrd) < 0)
-goto cleanup;
+if (def->os.kernel)
+testSELinuxMungePath(>os.kernel);
+if (def->os.initrd)
+testSELinuxMungePath(>os.initrd);
- cleanup:
-VIR_FREE(xmlfile);
return def;
}
--
2.34.1
Re: [libvirt PATCH v5 3/7] ch_driver, ch_domain: vcpupin callback in ch driver
On 1/25/22 17:19, Praveen K Paladugu wrote:
> From: Vineeth Pillai
>
> Signed-off-by: Vineeth Pillai
> Signed-off-by: Praveen K Paladugu
> ---
> src/ch/ch_domain.c | 30 +
> src/ch/ch_domain.h | 7 ++-
> src/ch/ch_driver.c | 145
> src/ch/ch_monitor.c | 2 +-
> 4 files changed, 181 insertions(+), 3 deletions(-)
>
> diff --git a/src/ch/ch_domain.c b/src/ch/ch_domain.c
> index 6f0cec8c6e..6fde7122f7 100644
> --- a/src/ch/ch_domain.c
> +++ b/src/ch/ch_domain.c
> @@ -20,6 +20,7 @@
>
> #include
>
> +#include "datatypes.h"
> #include "ch_domain.h"
> #include "domain_driver.h"
> #include "viralloc.h"
> @@ -416,3 +417,32 @@ virCHDomainGetMachineName(virDomainObj *vm)
>
> return ret;
> }
> +
> +/**
> + * virCHDomainObjFromDomain:
> + * @domain: Domain pointer that has to be looked up
> + *
> + * This function looks up @domain and returns the appropriate virDomainObjPtr
> + * that has to be released by calling virDomainObjEndAPI().
> + *
> + * Returns the domain object with incremented reference counter which is
> locked
> + * on success, NULL otherwise.
> + */
> +virDomainObj *
> +virCHDomainObjFromDomain(virDomainPtr domain)
> +{
> +virDomainObj *vm;
> +virCHDriver *driver = domain->conn->privateData;
> +char uuidstr[VIR_UUID_STRING_BUFLEN];
> +
> +vm = virDomainObjListFindByUUID(driver->domains, domain->uuid);
> +if (!vm) {
> +virUUIDFormat(domain->uuid, uuidstr);
> +virReportError(VIR_ERR_NO_DOMAIN,
> + _("no domain with matching uuid '%s' (%s)"),
> + uuidstr, domain->name);
> +return NULL;
> +}
> +
> +return vm;
> +}
I'm not against moving this function, but:
1) it needs to be done in a separate commit
2) don't forget to remove the function from its original place (ch_driver.c)
> diff --git a/src/ch/ch_domain.h b/src/ch/ch_domain.h
> index cb94905b94..60a761ac84 100644
> --- a/src/ch/ch_domain.h
> +++ b/src/ch/ch_domain.h
> @@ -97,5 +97,8 @@ virCHDomainGetVcpuPid(virDomainObj *vm,
> bool
> virCHDomainHasVcpuPids(virDomainObj *vm);
>
> -char *
> -virCHDomainGetMachineName(virDomainObj *vm);
> +char
> +*virCHDomainGetMachineName(virDomainObj *vm);
> +
> +virDomainObj
> +*virCHDomainObjFromDomain(virDomain *domain);
> diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
> index 53e0872207..956189e83f 100644
> --- a/src/ch/ch_driver.c
> +++ b/src/ch/ch_driver.c
> @@ -25,6 +25,7 @@
> #include "ch_driver.h"
> #include "ch_monitor.h"
> #include "ch_process.h"
> +#include "domain_cgroup.h"
> #include "datatypes.h"
> #include "driver.h"
> #include "viraccessapicheck.h"
> @@ -1129,6 +1130,148 @@ chDomainGetVcpus(virDomainPtr dom,
> return ret;
> }
>
> +static int
> +chDomainPinVcpuLive(virDomainObj *vm,
> +virDomainDef *def,
> +int vcpu,
> +virCHDriver *driver,
> +virCHDriverConfig *cfg,
> +virBitmap *cpumap)
> +{
> +g_autoptr(virBitmap) tmpmap = NULL;
> +g_autoptr(virCgroup) cgroup_vcpu = NULL;
> +virDomainVcpuDef *vcpuinfo;
> +virCHDomainObjPrivate *priv = vm->privateData;
> +
> +g_autofree char *str = NULL;
> +
> +if (!virCHDomainHasVcpuPids(vm)) {
> +virReportError(VIR_ERR_OPERATION_INVALID,
> + "%s", _("cpu affinity is not supported"));
> +return -1;
> +}
> +
> +if (!(vcpuinfo = virDomainDefGetVcpu(def, vcpu))) {
> +virReportError(VIR_ERR_INVALID_ARG,
> + _("vcpu %d is out of range of live cpu count %d"),
> + vcpu, virDomainDefGetVcpusMax(def));
> +return -1;
> +}
> +
> +if (!(tmpmap = virBitmapNewCopy(cpumap)))
> +return -1;
> +
> +if (!(str = virBitmapFormat(cpumap)))
> +return -1;
> +
> +if (vcpuinfo->online) {
> +/* Configure the corresponding cpuset cgroup before set affinity. */
> +if (virCgroupHasController(priv->cgroup,
> VIR_CGROUP_CONTROLLER_CPUSET)) {
> +if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_VCPU,
> vcpu,
> + false, _vcpu) < 0)
> +return -1;
> +if (virDomainCgroupSetupCpusetCpus(cgroup_vcpu, cpumap) < 0)
> +return -1;
> +}
> +
> +if (virProcessSetAffinity(virCHDomainGetVcpuPid(vm, vcpu), cpumap,
> false) < 0)
> +return -1;
> +}
> +
> +virBitmapFree(vcpuinfo->cpumask);
> +vcpuinfo->cpumask = tmpmap;
> +tmpmap = NULL;
g_steal_pointer() in other words.
> +
> +if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0)
> +return -1;
> +
> +return 0;
> +}
> +
> +
> +static int
> +chDomainPinVcpuFlags(virDomainPtr dom,
> + unsigned int vcpu,
> + unsigned char *cpumap,
> + int maplen,
> +
Re: [libvirt PATCH v5 6/7] ch_process: Setup emulator and iothread settings
On 1/25/22 17:19, Praveen K Paladugu wrote:
> using virCHProcessSetupPid
>
> Signed-off-by: Praveen K Paladugu
> ---
> src/ch/ch_monitor.c | 60 +++
> src/ch/ch_monitor.h | 2 ++
> src/ch/ch_process.c | 77 -
> 3 files changed, 138 insertions(+), 1 deletion(-)
>
> diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
> index 6c1d889a82..2a7cffb696 100644
> --- a/src/ch/ch_monitor.c
> +++ b/src/ch/ch_monitor.c
> @@ -930,3 +930,63 @@ virCHMonitorGetInfo(virCHMonitor *mon, virJSONValue
> **info)
> {
> return virCHMonitorGet(mon, URL_VM_INFO, info);
> }
> +
> +/**
> + * virCHMonitorGetIOThreads:
> + * @mon: Pointer to the monitor
> + * @iothreads: Location to return array of IOThreadInfo data
> + *
> + * Retrieve the list of iothreads defined/running for the machine
> + *
> + * Returns count of IOThreadInfo structures on success
> + *-1 on error.
> + */
> +int virCHMonitorGetIOThreads(virCHMonitor *mon,
> + virDomainIOThreadInfo ***iothreads)
> +{
> +size_t nthreads = 0, niothreads = 0;
> +int thd_index;
> +virDomainIOThreadInfo **iothreadinfolist = NULL, *iothreadinfo = NULL;
> +
> +*iothreads = NULL;
> +nthreads = virCHMonitorRefreshThreadInfo(mon);
> +
> +iothreadinfolist = g_new0(virDomainIOThreadInfo*, nthreads);
> +
> +for (thd_index = 0; thd_index < nthreads; thd_index++) {
> +virBitmap *map = NULL;
> +if (mon->threads[thd_index].type == virCHThreadTypeIO) {
> +iothreadinfo = g_new0(virDomainIOThreadInfo, 1);
> +
> +iothreadinfo->iothread_id = mon->threads[thd_index].ioInfo.tid;
> +
> +if (!(map = virProcessGetAffinity(iothreadinfo->iothread_id)))
> +goto cleanup;
> +
> +if (virBitmapToData(map, &(iothreadinfo->cpumap),
> +&(iothreadinfo->cpumaplen)) < 0) {
> +virBitmapFree(map);
> +goto cleanup;
> +}
> +virBitmapFree(map);
> +/* Append to iothreadinfolist */
> +iothreadinfolist[niothreads] = iothreadinfo;
> +niothreads++;
> +}
> +}
> +VIR_DELETE_ELEMENT_INPLACE(iothreadinfolist,
> + niothreads, nthreads);
I'm sorry, but what are you trying to say here? It looks like you're
trying to turn @iothreadinfolist into a null terminated array. Well, for
that all you need to to pass nthreads + 1 into g_new0() above.
> +*iothreads = iothreadinfolist;
> +VIR_DEBUG("niothreads = %ld", niothreads);
> +return niothreads;
> +
> +cleanup:
This needs to be named 'error' because the control gets here only in
error cases. The label is not used in successful path.
> +if (iothreadinfolist) {
> +for (thd_index = 0; thd_index < niothreads; thd_index++)
> +VIR_FREE(iothreadinfolist[thd_index]);
Life's not that simple. The argument is type of virDomainIOThreadInfo*
and as such might have ->cpumap member allocated. You need to call
virDomainIOThreadInfoFree() instead of plain VIR_FREE().
> +VIR_FREE(iothreadinfolist);
This one is okay, because this is just an array of pointers.
> +}
> +if (iothreadinfo)
> +VIR_FREE(iothreadinfo);
And again, no need for the if() and plain VIR_FREE() is not enough.
> +return -1;
> +}
Michal
Re: [libvirt PATCH v5 2/7] ch: methods for cgroup mgmt in ch driver
On 1/25/22 17:19, Praveen K Paladugu wrote:
> From: Vineeth Pillai
>
> Signed-off-by: Vineeth Pillai
> Signed-off-by: Praveen K Paladugu
> ---
> src/ch/ch_conf.c| 2 +
> src/ch/ch_conf.h| 4 +-
> src/ch/ch_domain.c | 34 +
> src/ch/ch_domain.h | 11 +-
> src/ch/ch_monitor.c | 96 ++
> src/ch/ch_monitor.h | 54 +++-
> src/ch/ch_process.c | 308 ++--
> src/ch/ch_process.h | 3 +
> 8 files changed, 492 insertions(+), 20 deletions(-)
>
> diff --git a/src/ch/ch_domain.c b/src/ch/ch_domain.c
> index a746d0f5fd..6f0cec8c6e 100644
> --- a/src/ch/ch_domain.c
> +++ b/src/ch/ch_domain.c
> @@ -319,6 +319,40 @@ chValidateDomainDeviceDef(const virDomainDeviceDef *dev,
> _("Serial can only be enabled for a PTY"));
> return -1;
> }
> +return 0;
> +}
> +
> +int
> +virCHDomainRefreshThreadInfo(virDomainObj *vm)
> +{
> +size_t maxvcpus = virDomainDefGetVcpusMax(vm->def);
> +virCHMonitorThreadInfo *info = NULL;
> +size_t nthreads, ncpus = 0;
We like one variable per line more, because it allows smaller diffs
should this area of code be ever changed.
> +size_t i;
> +
> +nthreads = virCHMonitorGetThreadInfo(virCHDomainGetMonitor(vm),
> + true, );
> +
> +for (i = 0; i < nthreads; i++) {
> +virCHDomainVcpuPrivate *vcpupriv;
> +virDomainVcpuDef *vcpu;
> +virCHMonitorCPUInfo *vcpuInfo;
> +
> +if (info[i].type != virCHThreadTypeVcpu)
> +continue;
> +
> +/* TODO: hotplug support */
> +vcpuInfo = [i].vcpuInfo;
> +vcpu = virDomainDefGetVcpu(vm->def, vcpuInfo->cpuid);
> +vcpupriv = CH_DOMAIN_VCPU_PRIVATE(vcpu);
> +vcpupriv->tid = vcpuInfo->tid;
> +ncpus++;
> +}
> +
> +/* TODO: Remove the warning when hotplug is implemented.*/
> +if (ncpus != maxvcpus)
> +VIR_WARN("Mismatch in the number of cpus, expected: %ld, actual:
> %ld",
> + maxvcpus, ncpus);
>
> return 0;
> }
> diff --git a/src/ch/ch_domain.h b/src/ch/ch_domain.h
> index 4d0b5479b8..cb94905b94 100644
> --- a/src/ch/ch_domain.h
> +++ b/src/ch/ch_domain.h
> @@ -53,11 +53,13 @@ typedef struct _virCHDomainObjPrivate
> virCHDomainObjPrivate;
> struct _virCHDomainObjPrivate {
> struct virCHDomainJobObj job;
>
> -virChrdevs *chrdevs;
> -virCHDriver *driver;
> -virCHMonitor *monitor;
> char *machineName;
> virBitmap *autoCpuset;
> +virBitmap *autoNodeset;
> +virCHDriver *driver;
> +virCHMonitor *monitor;
> +virCgroup *cgroup;
> +virChrdevs *chrdevs;
Looks like you can't make up your mind about the order. I remember
seeing the order of the members being changed over and over. Any reason
for that?
> };
>
> #define CH_DOMAIN_PRIVATE(vm) \
> @@ -87,7 +89,8 @@ void
> virCHDomainObjEndJob(virDomainObj *obj);
>
> int
> -virCHDomainRefreshVcpuInfo(virDomainObj *vm);
> +virCHDomainRefreshThreadInfo(virDomainObj *vm);
> +
> pid_t
> virCHDomainGetVcpuPid(virDomainObj *vm,
>unsigned int vcpuid);
> diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
> index a19f0c7e33..d984bf9822 100644
> --- a/src/ch/ch_monitor.c
> +++ b/src/ch/ch_monitor.c
> @@ -41,6 +41,7 @@ VIR_LOG_INIT("ch.ch_monitor");
>
> static virClass *virCHMonitorClass;
> static void virCHMonitorDispose(void *obj);
> +static void virCHMonitorThreadInfoFree(virCHMonitor * mon);
>
> static int virCHMonitorOnceInit(void)
> {
> @@ -578,6 +579,7 @@ static void virCHMonitorDispose(void *opaque)
> virCHMonitor *mon = opaque;
>
> VIR_DEBUG("mon=%p", mon);
> +virCHMonitorThreadInfoFree(mon);
> virObjectUnref(mon->vm);
> }
>
> @@ -743,6 +745,100 @@ virCHMonitorGet(virCHMonitor *mon, const char
> *endpoint, virJSONValue **response
> return ret;
> }
>
> +static void
> +virCHMonitorThreadInfoFree(virCHMonitor *mon)
> +{
> +mon->nthreads = 0;
> +if (mon->threads)
> +VIR_FREE(mon->threads);
Since VIR_FREE() is really just g_clear_pointer() there's no point in
checking the pointer. In fact, glib will do that for us.
> +}
> +
> +static size_t
> +virCHMonitorRefreshThreadInfo(virCHMonitor *mon)
> +{
> +virCHMonitorThreadInfo *info = NULL;
> +g_autofree pid_t *tids = NULL;
> +virDomainObj *vm = mon->vm;
> +size_t ntids = 0;
> +size_t i;
> +
> +
> +virCHMonitorThreadInfoFree(mon);
> +if (virProcessGetPids(vm->pid, , ) < 0) {
> +mon->threads = NULL;
No need to set to NULL. It was done by virCHMonitorThreadInfoFree() above.
> +return 0;
> +}
> +
> diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c
> index 49976d769e..1a0730a4d1 100644
> --- a/src/ch/ch_process.c
> +++ b/src/ch/ch_process.c
> +/**
> + * virCHProcessSetupPid:
> + *
> + * This function sets resource properties (affinity, cgroups,
> + * scheduler) for any PID associated
Re: [libvirt PATCH v5 5/7] ch_driver: add numatune callbacks for CH driver
On 1/25/22 17:19, Praveen K Paladugu wrote:
> From: Vineeth Pillai
>
> Signed-off-by: Vineeth Pillai
> Signed-off-by: Praveen K Paladugu
> ---
> src/ch/ch_driver.c | 260 +
> 1 file changed, 260 insertions(+)
>
> diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
> index 32ae15f940..d257c025ef 100644
> --- a/src/ch/ch_driver.c
> +++ b/src/ch/ch_driver.c
> @@ -43,6 +43,7 @@
> #include "viruri.h"
> #include "virutil.h"
> #include "viruuid.h"
> +#include "virnuma.h"
>
> #define VIR_FROM_THIS VIR_FROM_CH
>
> @@ -1302,6 +1303,263 @@ chDomainPinVcpu(virDomainPtr dom,
>VIR_DOMAIN_AFFECT_LIVE);
> }
>
> +#define CH_NB_NUMA_PARAM 2
> +
> +static int
> +chDomainGetNumaParameters(virDomainPtr dom,
> + virTypedParameterPtr params,
> + int *nparams,
> + unsigned int flags)
> +{
> +size_t i;
> +virDomainObj *vm = NULL;
> +virDomainNumatuneMemMode tmpmode = VIR_DOMAIN_NUMATUNE_MEM_STRICT;
> +virCHDomainObjPrivate *priv;
> +g_autofree char *nodeset = NULL;
> +int ret = -1;
> +virDomainDef *def = NULL;
> +bool live = false;
> +virBitmap *autoNodeset = NULL;
> +
> +virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
> + VIR_DOMAIN_AFFECT_CONFIG |
> + VIR_TYPED_PARAM_STRING_OKAY, -1);
> +
> +if (!(vm = virCHDomainObjFromDomain(dom)))
> +return -1;
> +priv = vm->privateData;
> +
> +if (virDomainGetNumaParametersEnsureACL(dom->conn, vm->def) < 0)
> +goto cleanup;
> +
> +if (!(def = virDomainObjGetOneDefState(vm, flags, )))
> +goto cleanup;
> +
> +if (live)
> +autoNodeset = priv->autoNodeset;
> +
> +if ((*nparams) == 0) {
> +*nparams = CH_NB_NUMA_PARAM;
> +ret = 0;
> +goto cleanup;
> +}
> +
> +for (i = 0; i < CH_NB_NUMA_PARAM && i < *nparams; i++) {
> +virMemoryParameterPtr param = [i];
> +
> +switch (i) {
> +case 0: /* fill numa mode here */
> +ignore_value(virDomainNumatuneGetMode(def->numa, -1, ));
> +
> +if (virTypedParameterAssign(param, VIR_DOMAIN_NUMA_MODE,
> +VIR_TYPED_PARAM_INT, tmpmode) < 0)
> +goto cleanup;
> +
> +break;
> +
> +case 1: /* fill numa nodeset here */
> +nodeset = virDomainNumatuneFormatNodeset(def->numa, autoNodeset,
> -1);
> +
> +if (!nodeset ||
> +virTypedParameterAssign(param, VIR_DOMAIN_NUMA_NODESET,
> +VIR_TYPED_PARAM_STRING, nodeset) < 0)
> +goto cleanup;
> +
> +nodeset = NULL;
> +break;
> +
> +/* coverity[dead_error_begin] */
I don't think this is correct. Does coverity really complain?
> +default:
> +break;
> +/* should not hit here */
> +}
> +}
> +
> +if (*nparams > CH_NB_NUMA_PARAM)
> +*nparams = CH_NB_NUMA_PARAM;
> +ret = 0;
> +
> + cleanup:
> +virDomainObjEndAPI();
> +return ret;
> +}
> +
> +static int
> +chDomainSetNumaParamsLive(virDomainObj *vm,
> + virBitmap *nodeset)
> +{
> +g_autoptr(virCgroup) cgroup_temp = NULL;
> +virCHDomainObjPrivate *priv = vm->privateData;
> +g_autofree char *nodeset_str = NULL;
> +virDomainNumatuneMemMode mode;
> +size_t i = 0;
> +
> +
> +if (virDomainNumatuneGetMode(vm->def->numa, -1, ) == 0 &&
> +mode != VIR_DOMAIN_NUMATUNE_MEM_STRICT) {
> +virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> + _("change of nodeset for running domain requires
> strict numa mode"));
> +return -1;
> +}
> +
> +if (!virNumaNodesetIsAvailable(nodeset))
> +return -1;
> +
> +/* Ensure the cpuset string is formatted before passing to cgroup */
> +if (!(nodeset_str = virBitmapFormat(nodeset)))
> +return -1;
> +
> +if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0,
> + false, _temp) < 0 ||
> +virCgroupSetCpusetMems(cgroup_temp, nodeset_str) < 0)
> +return -1;
> +
> +
> +for (i = 0; i < virDomainDefGetVcpusMax(vm->def); i++) {
> +virDomainVcpuDef *vcpu = virDomainDefGetVcpu(vm->def, i);
> +
> +if (!vcpu->online)
> +continue;
> +
> +if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_VCPU, i,
> + false, _temp) < 0 ||
> +virCgroupSetCpusetMems(cgroup_temp, nodeset_str) < 0)
> +return -1;
> +
> +return 0;
> +}
> +
If anything, coverity should complain about the block below. Because of
this return 0 ^^^ the block below is effectivelly a dead code.
> +for (i = 0; i < vm->def->niothreadids; i++) {
> +if (virCgroupNewThread(priv->cgroup,
Re: [libvirt PATCH v5 1/7] qemu, hypervisor: refactor some cgroup mgmt methods
On 1/25/22 17:19, Praveen K Paladugu wrote:
> Refactor some cgroup management methods from qemu into hypervisor.
> These methods will be shared with ch driver for cgroup management.
>
> Signed-off-by: Praveen K Paladugu
> ---
> src/hypervisor/domain_cgroup.c | 457 -
> src/hypervisor/domain_cgroup.h | 72 ++
> src/libvirt_private.syms | 14 +-
> src/qemu/qemu_cgroup.c | 413 +
> src/qemu/qemu_cgroup.h | 11 -
> src/qemu/qemu_driver.c | 14 +-
> src/qemu/qemu_hotplug.c| 7 +-
> src/qemu/qemu_process.c| 24 +-
> 8 files changed, 580 insertions(+), 432 deletions(-)
>
> diff --git a/src/hypervisor/domain_cgroup.c b/src/hypervisor/domain_cgroup.c
> index 61b54f071c..05c53ff7d1 100644
> --- a/src/hypervisor/domain_cgroup.c
> +++ b/src/hypervisor/domain_cgroup.c
> @@ -22,11 +22,12 @@
>
> #include "domain_cgroup.h"
> #include "domain_driver.h"
> -
> +#include "util/virnuma.h"
> +#include "virlog.h"
> #include "virutil.h"
>
> #define VIR_FROM_THIS VIR_FROM_DOMAIN
> -
> +VIR_LOG_INIT("domain.cgroup");
>
> int
> virDomainCgroupSetupBlkio(virCgroup *cgroup, virDomainBlkiotune blkio)
> @@ -269,3 +270,455 @@ virDomainCgroupSetMemoryLimitParameters(virCgroup
> *cgroup,
>
> return 0;
> }
> +
> +
> +int
> +virDomainCgroupSetupBlkioCgroup(virDomainObj *vm,
> +virCgroup *cgroup)
> +{
> +
No need to start a function with an empty line.
> +if (!virCgroupHasController(cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) {
> +if (vm->def->blkio.weight || vm->def->blkio.ndevices) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("Block I/O tuning is not available on this
> host"));
> +return -1;
> +} else {
> +return 0;
Here and in the rest of the patch: the else branch is not necessary and
in fact is not in the original qemu code.
> +}
> +}
> +
> +return virDomainCgroupSetupBlkio(cgroup, vm->def->blkio);
> +}
> +
> +
> +
> +int
> +virDomainCgroupInitCgroup(const char *prefix,
> + virDomainObj * vm,
> + size_t nnicindexes,
> + int *nicindexes,
> + virCgroup * cgroup,
> + int cgroupControllers,
> + unsigned int maxThreadsPerProc,
> + bool privileged,
> + char *machineName)
> +{
> +if (!privileged)
> +return 0;
> +
> +if (!virCgroupAvailable())
> +return 0;
> +
> +virCgroupFree(cgroup);
> +cgroup = NULL;
> +
This doesn't do what you think it does. This merely clears the local
variable, but doesn't affect the variable in caller. Therefore, @cgroup
really needs to be a double pointer. And this can then be just:
g_clear_pointer(cgroup, virCgroupFree);
Subsequently, virDomainCgroupSetupCgroup() will need to take a double
pointer too.
> +if (!vm->def->resource)
> +vm->def->resource = g_new0(virDomainResourceDef, 1);
> +
> +if (!vm->def->resource->partition)
> +vm->def->resource->partition = g_strdup("/machine");
> +
> +if (!g_path_is_absolute(vm->def->resource->partition)) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("Resource partition '%s' must start with '/'"),
> + vm->def->resource->partition);
> +return -1;
> +}
> +
> +if (virCgroupNewMachine(machineName,
> +prefix,
> +vm->def->uuid,
> +NULL,
> +vm->pid,
> +false,
> +nnicindexes, nicindexes,
> +vm->def->resource->partition,
> +cgroupControllers,
> +maxThreadsPerProc, ) < 0) {
> +if (virCgroupNewIgnoreError())
> +return 0;
> +
> +return -1;
> +}
> +
> +return 0;
> +}
> +
> +
> +int
> +virDomainCgroupSetupCgroup(const char *prefix,
> + virDomainObj * vm,
> + size_t nnicindexes,
> + int *nicindexes,
> + virCgroup * cgroup,
> + int cgroupControllers,
> + unsigned int maxThreadsPerProc,
> + bool privileged,
> + char *machineName)
> +{
> +if (!vm->pid) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Cannot setup cgroups until process is started"));
> +return -1;
> +}
> +
> +if (virDomainCgroupInitCgroup(prefix,
> + vm,
> + nnicindexes,
> +
Re: [libvirt PATCH v5 0/7] cgroup and thread management in ch driver
On 1/25/22 17:19, Praveen K Paladugu wrote: > This patchset adds support for cgroup management of ch threads. This version > correctly manages cgroups for vcpu and emulator threads created by ch. cgroup > management for iothreads is not yet supported. > > Along with cgroup management, this patchset also enables support for pinning > vcpu and emulator threads to selected host cpus. > And also does a lot of formatting changes back and forth. I'm not fond of that really. If you want to clean up the formatting please do so in a separate patch(set). > src/ch/ch_conf.c | 2 + > src/ch/ch_conf.h | 4 +- > src/ch/ch_domain.c | 64 > src/ch/ch_domain.h | 18 +- > src/ch/ch_driver.c | 590 + > src/ch/ch_monitor.c| 156 + > src/ch/ch_monitor.h| 56 +++- > src/ch/ch_process.c| 385 - > src/ch/ch_process.h| 3 + > src/hypervisor/domain_cgroup.c | 457 - > src/hypervisor/domain_cgroup.h | 72 > src/libvirt_private.syms | 14 +- > src/qemu/qemu_cgroup.c | 413 +-- > src/qemu/qemu_cgroup.h | 11 - > src/qemu/qemu_driver.c | 14 +- > src/qemu/qemu_hotplug.c| 7 +- > src/qemu/qemu_process.c| 24 +- > 17 files changed, 1835 insertions(+), 455 deletions(-) > Nevertheless, I'm fixing all the issues I've raised and merging. Reviewed-by: Michal Privoznik Michal
Re: [libvirt PATCH v5 7/7] ch_driver: emulator threadinfo & pinning callbacks
On 1/25/22 17:19, Praveen K Paladugu wrote:
> Signed-off-by: Praveen K Paladugu
> ---
> src/ch/ch_driver.c | 154 +
> 1 file changed, 154 insertions(+)
>
> diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
> index d257c025ef..d60ff468f0 100644
> --- a/src/ch/ch_driver.c
> +++ b/src/ch/ch_driver.c
> @@ -1303,6 +1303,158 @@ chDomainPinVcpu(virDomainPtr dom,
>VIR_DOMAIN_AFFECT_LIVE);
> }
>
> +
> +
> +static int
> +chDomainGetEmulatorPinInfo(virDomainPtr dom,
> + unsigned char *cpumaps,
> + int maplen,
> + unsigned int flags)
> +{
> +virDomainObj *vm = NULL;
> +virDomainDef *def;
> +virCHDomainObjPrivate *priv;
> +bool live;
> +int ret = -1;
> +virBitmap *cpumask = NULL;
> +g_autoptr(virBitmap) bitmap = NULL;
> +virBitmap *autoCpuset = NULL;
> +
> +virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
> + VIR_DOMAIN_AFFECT_CONFIG, -1);
> +
> +if (!(vm = chDomObjFromDomain(dom)))
This needs to be virCHDomainObjFromDomain(). And in the other function too.
> +goto cleanup;
> +
> +if (virDomainGetEmulatorPinInfoEnsureACL(dom->conn, vm->def) < 0)
> +goto cleanup;
> +
> +if (!(def = virDomainObjGetOneDefState(vm, flags, )))
> +goto cleanup;
> +
> +if (live) {
> +priv = vm->privateData;
> +autoCpuset = priv->autoCpuset;
> +}
> +if (def->cputune.emulatorpin) {
> +cpumask = def->cputune.emulatorpin;
> +} else if (def->cpumask) {
> +cpumask = def->cpumask;
> +} else if (vm->def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO
> &&
> + autoCpuset) {
> +cpumask = autoCpuset;
> +} else {
> +if (!(bitmap = virHostCPUGetAvailableCPUsBitmap()))
> +goto cleanup;
> +cpumask = bitmap;
> +}
> +
> +virBitmapToDataBuf(cpumask, cpumaps, maplen);
> +
> +ret = 1;
> +
> + cleanup:
> +virDomainObjEndAPI();
> +return ret;
> +}
> +
Michal
Re: [PATCH] domain_cgroup: Don't put semicolon at the end of VIR_GET_LIMIT_PARAMETER macro
On 1/28/22 6:15 AM, Michal Privoznik wrote: In domain_cgroup.c there's VIR_GET_LIMIT_PARAMETER macro which has a semicolon at the end of its declaration. Well, remove it so that the places where macro is used have to put the semicolon explicitly. This helps with automatic reformatting (at least in vim). Signed-off-by: Michal Privoznik Reviewed-by: Laine Stump
Re: [libvirt PATCH v3 1/3] scripts: Check spelling
On Fri, Jan 21, 2022 at 10:41:48 +0100, Tim Wiederhake wrote:
> This is a wrapper for codespell [1], a spell checker for source code.
> Codespell does not compare words to a dictionary, but rather works by
> checking words against a list of common typos, making it produce fewer
> false positives than other solutions.
>
> The script in this patch works around the lack of per-directory ignore
> lists and some oddities regarding capitalization in ignore lists.
> The ".codespellrc" file is used to coarsly filter out translation and
> git files, as scanning those makes up for roughly 50% of the run time
> otherwise.
>
> [1] (https://github.com/codespell-project/codespell/)
>
> Signed-off-by: Tim Wiederhake
> ---
> .codespellrc | 2 +
> scripts/check-spelling.py | 135 ++
> 2 files changed, 137 insertions(+)
> create mode 100644 .codespellrc
> create mode 100755 scripts/check-spelling.py
>
> diff --git a/.codespellrc b/.codespellrc
> new file mode 100644
> index 00..0c45be445b
> --- /dev/null
> +++ b/.codespellrc
> @@ -0,0 +1,2 @@
> +[codespell]
> +skip = .git/,*.po
This doesn't seem to work for me, after patch 2/3 I'm getting the
following:
>>> MALLOC_PERTURB_=230 /bin/python3
>>> /home/pipo/libvirt/scripts/check-spelling.py --ignore-untracked
― ✀ ―
stdout:
(".git/logs/HEAD", "lenght"), # line 187, "length"?
(".git/logs/HEAD", "programatic"), # line 424, "programmatic"?
(".git/logs/HEAD", "programatic"), # line 570, "programmatic"?
(".git/logs/HEAD", "programatic"), # line 583, "programmatic"?
(".git/logs/HEAD", "programatic"), # line 714, "programmatic"?
(".git/logs/HEAD", "programatic"), # line 727, "programmatic"?
(".git/logs/HEAD", "programatic"), # line 802, "programmatic"?
(".git/logs/HEAD", "programatic"), # line 810, "programmatic"?
(".git/logs/HEAD", "programatic"), # line 816, "programmatic"?
(".git/logs/HEAD", "programatic"), # line 935, "programmatic"?
(".git/logs/HEAD", "programatic"), # line 944, "programmatic"?
(".git/logs/HEAD", "programatic"), # line 1005, "programmatic"?
(".git/logs/HEAD", "programatic"), # line 1007, "programmatic"?
(".git/logs/HEAD", "programatic"), # line 1008, "programmatic"?
(".git/logs/HEAD", "programatic"), # line 1009, "programmatic"?
(".git/logs/HEAD", "programatic"), # line 1014, "programmatic"?
...
(I'll be elaborating a bit more on the exclude pattern later after
playing with codespell for a bit).
> diff --git a/scripts/check-spelling.py b/scripts/check-spelling.py
> new file mode 100755
> index 00..ce3e7d89f0
> --- /dev/null
> +++ b/scripts/check-spelling.py
> @@ -0,0 +1,135 @@
> +#!/usr/bin/env python3
> +
> +import argparse
> +import re
> +import subprocess
> +import os
> +
> +
> +IGNORE_LIST = [
> +# ignore this script
> +("scripts/check-spelling.py", []),
> +
> +# 3rd-party: keycodemapdb
> +("src/keycodemapdb/", []),
> +
> +# 3rd-party: VirtualBox SDK
> +("src/vbox/vbox_CAPI", []),
> +
> +# 3rd-party: qemu
> +("tests/qemucapabilitiesdata/caps_", []),
> +
> +# other
> +("", ["msdos", "MSDOS", "wan", "WAN", "hda", "HDA", "inout"]),
These can be replaced by --ignore-words-list msdos,wan,hda,inout
> +("NEWS.rst", "crashers"),
IMO this can be reworded.
> +("docs/gitdm/companies/others", "Archiv"),
> +("docs/glib-adoption.rst", "preferrable"),
This too.
> +("docs/js/main.js", "whats"),
Skip all of docs/js?
> +("examples/polkit/libvirt-acl.rules", ["userA", "userB", "userC"]),
> +("src/libvirt-domain.c", "PTD"),
> +("src/libxl/libxl_logger.c", "purposedly"),
> +("src/nwfilter/nwfilter_dhcpsnoop.c", "ether"),
> +("src/nwfilter/nwfilter_ebiptables_driver.c", "parm"),
> +("src/nwfilter/nwfilter_learnipaddr.c", "ether"),
> +("src/qemu/qemu_agent.c", "crypted"),
> +("src/qemu/qemu_agent.h", "crypted"),
> +("src/qemu/qemu_process.c", "wee"),
This too.
> +("src/security/apparmor/libvirt-lxc", "devic"),
> +("src/security/apparmor/libvirt-qemu", "readby"),
> +("src/storage_file/storage_file_probe.c", "conectix"),
> +("src/util/virnetdevmacvlan.c", "calld"),
> +("src/util/virtpm.c", "parm"),
> +("tests/qemuagenttest.c", "IST"),
> +("tests/storagepoolxml2xml", "cant"),
> +("tests/sysinfodata/", "sie"),
This is technically 3rd party.
> +("tests/testutils.c", "nIn"),
> +("tests/vircgroupdata/ovirt-node-6.6.mounts", "hald"),
This is also 3rd party
`
> +("tests/virhostcpudata/", "sie"),
This too.
> +("tools/virt-host-validate-common.c", "sie"),
> +]
> +
> +
> +def ignore(filename, linenumber, word, suggestion):
> +if len(word) <= 2:
> +return True
> +
> +for f, w in IGNORE_LIST:
> +if not filename.startswith(f):
> +continue
> +if word in w or not w:
> +
Re: [libvirt PATCH v5 0/7] cgroup and thread management in ch driver
Ping.. If this patch set is ready to be merged, I'd like to get started on next set. Thank you, Praveen K Paladugu On 1/25/2022 10:19 AM, Praveen K Paladugu wrote: This patchset adds support for cgroup management of ch threads. This version correctly manages cgroups for vcpu and emulator threads created by ch. cgroup management for iothreads is not yet supported. Along with cgroup management, this patchset also enables support for pinning vcpu and emulator threads to selected host cpus. v5: * bumped the verion of callbacks in ch driver to 8.1.0 v4: * addressed all open comments in v3 * dropped all the merged commits v3: * addrressed all the formatting comments in v2 patch set * dropped indentation patches are they do not adhere to libvirt coding style * fixed build issue in qemu driver that was introduced in v2 Praveen K Paladugu (3): qemu,hypervisor: refactor some cgroup mgmt methods ch_process: Setup emulator and iothread settings ch_driver: emulator threadinfo & pinning callbacks Vineeth Pillai (4): ch: methods for cgroup mgmt in ch driver ch_driver,ch_domain: vcpupin callback in ch driver ch_driver: enable typed param string for numatune ch_driver: add numatune callbacks for CH driver src/ch/ch_conf.c | 2 + src/ch/ch_conf.h | 4 +- src/ch/ch_domain.c | 64 src/ch/ch_domain.h | 18 +- src/ch/ch_driver.c | 590 + src/ch/ch_monitor.c| 156 + src/ch/ch_monitor.h| 56 +++- src/ch/ch_process.c| 385 - src/ch/ch_process.h| 3 + src/hypervisor/domain_cgroup.c | 457 - src/hypervisor/domain_cgroup.h | 72 src/libvirt_private.syms | 14 +- src/qemu/qemu_cgroup.c | 413 +-- src/qemu/qemu_cgroup.h | 11 - src/qemu/qemu_driver.c | 14 +- src/qemu/qemu_hotplug.c| 7 +- src/qemu/qemu_process.c| 24 +- 17 files changed, 1835 insertions(+), 455 deletions(-) -- Regards, Praveen K Paladugu
[PATCH] domain_cgroup: Don't put semicolon at the end of VIR_GET_LIMIT_PARAMETER macro
In domain_cgroup.c there's VIR_GET_LIMIT_PARAMETER macro which has a semicolon at the end of its declaration. Well, remove it so that the places where macro is used have to put the semicolon explicitly. This helps with automatic reformatting (at least in vim). Signed-off-by: Michal Privoznik --- src/hypervisor/domain_cgroup.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/hypervisor/domain_cgroup.c b/src/hypervisor/domain_cgroup.c index 61b54f071c..aea0817a7f 100644 --- a/src/hypervisor/domain_cgroup.c +++ b/src/hypervisor/domain_cgroup.c @@ -211,11 +211,11 @@ virDomainCgroupSetMemoryLimitParameters(virCgroup *cgroup, return -1; \ \ if (rc == 1) \ -set_ ## VALUE = true; +set_ ## VALUE = true -VIR_GET_LIMIT_PARAMETER(VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT, swap_hard_limit) -VIR_GET_LIMIT_PARAMETER(VIR_DOMAIN_MEMORY_HARD_LIMIT, hard_limit) -VIR_GET_LIMIT_PARAMETER(VIR_DOMAIN_MEMORY_SOFT_LIMIT, soft_limit) +VIR_GET_LIMIT_PARAMETER(VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT, swap_hard_limit); +VIR_GET_LIMIT_PARAMETER(VIR_DOMAIN_MEMORY_HARD_LIMIT, hard_limit); +VIR_GET_LIMIT_PARAMETER(VIR_DOMAIN_MEMORY_SOFT_LIMIT, soft_limit); #undef VIR_GET_LIMIT_PARAMETER -- 2.34.1
Re: [libvirt PATCH v4 3/9] virthread: Introduce VIR_WITH_MUTEX_LOCK_GUARD
On Fri, Jan 28, 2022 at 10:59:16AM +0100, Tim Wiederhake wrote:
> Modeled after "WITH_QEMU_LOCK_GUARD" (see qemu's include/qemu/lockable.h).
> Uses "__LINE__" instead of "__COUNTER__", as the latter is a GNU extension.
GNU extensions are fine to use, as we explicitly only support GCC
or CLang which is compatible with GCC, so I'd stick with __COUNTER__.
>
> See comment for typical usage.
>
> Signed-off-by: Tim Wiederhake
> ---
> src/util/virthread.h | 20
> 1 file changed, 20 insertions(+)
>
> diff --git a/src/util/virthread.h b/src/util/virthread.h
> index 6cdaf2820e..da17780908 100644
> --- a/src/util/virthread.h
> +++ b/src/util/virthread.h
> @@ -209,3 +209,23 @@ int virThreadLocalSet(virThreadLocal *l, void*)
> G_GNUC_WARN_UNUSED_RESULT;
> return 0; \
> } \
> struct classname ## EatSemicolon
> +
> +/**
> + * VIR_WITH_MUTEX_LOCK_GUARD:
> + *
> + * This macro defines a lock scope such that entering the scope takes the
> lock
> + * and leaving the scope releases the lock. Return statements are allowed
> + * within the scope and release the lock. Break and continue statements leave
> + * the scope early and release the lock.
> + *
> + * virMutex *mutex = ...;
> + *
> + * VIR_WITH_MUTEX_LOCK_GUARD(mutex) {
> + * // `mutex` is locked, and released automatically on scope exit
> + * ...
> + * }
> + */
> +#define VIR_WITH_MUTEX_LOCK_GUARD(m) \
> +for (g_auto(virLockGuard) CONCAT(var, __LINE__) = virLockGuardLock(m); \
> + CONCAT(var, __LINE__).mutex; \
> + CONCAT(var, __LINE__).mutex = (virLockGuardUnlock(& CONCAT(var,
> __LINE__)), NULL))
> --
> 2.31.1
>
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[libvirt PATCH v4 2/9] virthread: Introduce virLockGuard
Locks a virMutex on creation and unlocks it in its destructor.
The VIR_LOCK_GUARD macro is used instead of "g_auto(virLockGuard)" to
work around a clang issue (see https://bugs.llvm.org/show_bug.cgi?id=3888
and https://bugs.llvm.org/show_bug.cgi?id=43482).
Typical usage:
void function(virMutex *m)
{
VIR_LOCK_GUARD lock = virLockGuardLock(m);
/* `m` is locked, and released automatically on scope exit */
...
while (expression) {
VIR_LOCK_GUARD lock2 = virLockGuardLock(...);
/* similar */
}
}
Signed-off-by: Tim Wiederhake
---
src/libvirt_private.syms | 2 ++
src/util/virthread.c | 15 +++
src/util/virthread.h | 10 ++
3 files changed, 27 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ba3462d849..4cfe6c17eb 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3376,6 +3376,8 @@ virCondInit;
virCondSignal;
virCondWait;
virCondWaitUntil;
+virLockGuardLock;
+virLockGuardUnlock;
virMutexDestroy;
virMutexInit;
virMutexInitRecursive;
diff --git a/src/util/virthread.c b/src/util/virthread.c
index e89c1a09fb..5422bb74fd 100644
--- a/src/util/virthread.c
+++ b/src/util/virthread.c
@@ -96,6 +96,21 @@ void virMutexUnlock(virMutex *m)
pthread_mutex_unlock(>lock);
}
+virLockGuard virLockGuardLock(virMutex *m)
+{
+virLockGuard l = { m };
+virMutexLock(m);
+return l;
+}
+
+void virLockGuardUnlock(virLockGuard *l)
+{
+if (!l || !l->mutex)
+return;
+
+virMutexUnlock(g_steal_pointer(>mutex));
+}
+
int virRWLockInit(virRWLock *m)
{
diff --git a/src/util/virthread.h b/src/util/virthread.h
index 55c8263ae6..6cdaf2820e 100644
--- a/src/util/virthread.h
+++ b/src/util/virthread.h
@@ -31,6 +31,11 @@ struct virMutex {
pthread_mutex_t lock;
};
+typedef struct virLockGuard virLockGuard;
+struct virLockGuard {
+virMutex *mutex;
+};
+
typedef struct virRWLock virRWLock;
struct virRWLock {
pthread_rwlock_t lock;
@@ -121,6 +126,11 @@ void virMutexLock(virMutex *m);
void virMutexUnlock(virMutex *m);
+virLockGuard virLockGuardLock(virMutex *m);
+void virLockGuardUnlock(virLockGuard *l);
+G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(virLockGuard, virLockGuardUnlock);
+#define VIR_LOCK_GUARD g_auto(virLockGuard) G_GNUC_UNUSED
+
int virRWLockInit(virRWLock *m) G_GNUC_WARN_UNUSED_RESULT;
void virRWLockDestroy(virRWLock *m);
--
2.31.1
[libvirt PATCH v4 7/9] virChrdevFree: Use VIR_WITH_MUTEX_LOCK
Signed-off-by: Tim Wiederhake
---
src/conf/virchrdev.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/conf/virchrdev.c b/src/conf/virchrdev.c
index c9b2134e3b..8610f0ac5c 100644
--- a/src/conf/virchrdev.c
+++ b/src/conf/virchrdev.c
@@ -291,10 +291,10 @@ void virChrdevFree(virChrdevs *devs)
if (!devs)
return;
-virMutexLock(>lock);
-virHashForEachSafe(devs->hash, virChrdevFreeClearCallbacks, NULL);
-g_clear_pointer(>hash, g_hash_table_unref);
-virMutexUnlock(>lock);
+VIR_WITH_MUTEX_LOCK_GUARD(>lock) {
+virHashForEachSafe(devs->hash, virChrdevFreeClearCallbacks, NULL);
+g_clear_pointer(>hash, g_hash_table_unref);
+}
virMutexDestroy(>lock);
g_free(devs);
--
2.31.1
[libvirt PATCH v4 5/9] virobject: Introduce VIR_WITH_OBJECT_LOCK_GUARD
Modeled after "WITH_QEMU_LOCK_GUARD" (see qemu's include/qemu/lockable.h).
Uses "__LINE__" instead of "__COUNTER__", as the latter is a GNU extension.
See comment for typical usage.
Signed-off-by: Tim Wiederhake
---
src/util/virobject.h | 20
1 file changed, 20 insertions(+)
diff --git a/src/util/virobject.h b/src/util/virobject.h
index dc1ce66a4f..595076bda7 100644
--- a/src/util/virobject.h
+++ b/src/util/virobject.h
@@ -148,3 +148,23 @@ virObjectListFree(void *list);
void
virObjectListFreeCount(void *list,
size_t count);
+
+/**
+ * VIR_WITH_OBJECT_LOCK_GUARD:
+ *
+ * This macro defines a lock scope such that entering the scope takes the lock
+ * and leaving the scope releases the lock. Return statements are allowed
+ * within the scope and release the lock. Break and continue statements leave
+ * the scope early and release the lock.
+ *
+ * virObjectLockable *lockable = ...;
+ *
+ * VIR_WITH_OBJECT_LOCK_GUARD(lockable) {
+ * // `lockable` is locked, and released automatically on scope exit
+ * ...
+ * }
+ */
+#define VIR_WITH_OBJECT_LOCK_GUARD(o) \
+for (g_auto(virLockGuard) CONCAT(var, __LINE__) = virObjectLockGuard(o); \
+ CONCAT(var, __LINE__).mutex; \
+ CONCAT(var, __LINE__).mutex = (virLockGuardUnlock(& CONCAT(var,
__LINE__)), NULL))
--
2.31.1
[libvirt PATCH v4 8/9] bhyveAutostartDomain: Use virObjectLockGuard
Signed-off-by: Tim Wiederhake
---
src/bhyve/bhyve_driver.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index f291f12e50..47ee98e650 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -87,7 +87,8 @@ bhyveAutostartDomain(virDomainObj *vm, void *opaque)
{
const struct bhyveAutostartData *data = opaque;
int ret = 0;
-virObjectLock(vm);
+VIR_LOCK_GUARD lock = virObjectLockGuard(vm);
+
if (vm->autostart && !virDomainObjIsActive(vm)) {
virResetLastError();
ret = virBhyveProcessStart(data->conn, vm,
@@ -98,7 +99,6 @@ bhyveAutostartDomain(virDomainObj *vm, void *opaque)
vm->def->name, virGetLastErrorMessage());
}
}
-virObjectUnlock(vm);
return ret;
}
--
2.31.1
[libvirt PATCH v4 4/9] virobject: Introduce virObjectLockGuard
Typical usage:
void foobar(virObjectLockable *obj)
{
VIR_LOCK_GUARD lock = virObjectLockGuard(obj);
/* `obj` is locked, and released automatically on scope exit */
...
}
Signed-off-by: Tim Wiederhake
---
src/libvirt_private.syms | 1 +
src/util/virobject.c | 16
src/util/virobject.h | 4
3 files changed, 21 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 4cfe6c17eb..463bb39de3 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2944,6 +2944,7 @@ virObjectListFree;
virObjectListFreeCount;
virObjectLock;
virObjectLockableNew;
+virObjectLockGuard;
virObjectNew;
virObjectRef;
virObjectRWLockableNew;
diff --git a/src/util/virobject.c b/src/util/virobject.c
index 588b41802c..74cc84205e 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -426,6 +426,22 @@ virObjectGetRWLockableObj(void *anyobj)
}
+/**
+ * virObjectLockGuard:
+ * @anyobj: any instance of virObjectLockable
+ *
+ * Acquire a lock on @anyobj that will be managed by the virLockGuard object
+ * returned to the caller.
+ */
+virLockGuard
+virObjectLockGuard(void *anyobj)
+{
+virObjectLockable *obj = virObjectGetLockableObj(anyobj);
+
+return virLockGuardLock(>lock);
+}
+
+
/**
* virObjectLock:
* @anyobj: any instance of virObjectLockable
diff --git a/src/util/virobject.h b/src/util/virobject.h
index 1e34c77744..dc1ce66a4f 100644
--- a/src/util/virobject.h
+++ b/src/util/virobject.h
@@ -118,6 +118,10 @@ void *
virObjectRWLockableNew(virClass *klass)
ATTRIBUTE_NONNULL(1);
+virLockGuard
+virObjectLockGuard(void *lockableobj)
+ATTRIBUTE_NONNULL(1);
+
void
virObjectLock(void *lockableobj)
ATTRIBUTE_NONNULL(1);
--
2.31.1
[libvirt PATCH v4 9/9] lxcDomainDetachDeviceHostdevUSBLive: Use VIR_WITH_OBJECT_LOCK_GUARD
Signed-off-by: Tim Wiederhake
---
src/lxc/lxc_driver.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 7bc39120ee..42053de9c3 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -4045,9 +4045,9 @@ lxcDomainDetachDeviceHostdevUSBLive(virLXCDriver *driver,
VIR_WARN("cannot deny device %s for domain %s: %s",
dst, vm->def->name, virGetLastErrorMessage());
-virObjectLock(hostdev_mgr->activeUSBHostdevs);
-virUSBDeviceListDel(hostdev_mgr->activeUSBHostdevs, usb);
-virObjectUnlock(hostdev_mgr->activeUSBHostdevs);
+VIR_WITH_OBJECT_LOCK_GUARD(hostdev_mgr->activeUSBHostdevs) {
+virUSBDeviceListDel(hostdev_mgr->activeUSBHostdevs, usb);
+}
virDomainHostdevRemove(vm->def, idx);
virDomainHostdevDefFree(def);
--
2.31.1
[libvirt PATCH v4 6/9] virChrdevFDStreamCloseCb: Use virLockGuardLock
Signed-off-by: Tim Wiederhake
---
src/conf/virchrdev.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/conf/virchrdev.c b/src/conf/virchrdev.c
index b5477b03d5..c9b2134e3b 100644
--- a/src/conf/virchrdev.c
+++ b/src/conf/virchrdev.c
@@ -237,12 +237,10 @@ static void virChrdevFDStreamCloseCb(virStreamPtr st
G_GNUC_UNUSED,
void *opaque)
{
virChrdevStreamInfo *priv = opaque;
-virMutexLock(>devs->lock);
+VIR_LOCK_GUARD lock = virLockGuardLock(>devs->lock);
/* remove entry from hash */
virHashRemoveEntry(priv->devs->hash, priv->path);
-
-virMutexUnlock(>devs->lock);
}
/**
--
2.31.1
[libvirt PATCH v4 3/9] virthread: Introduce VIR_WITH_MUTEX_LOCK_GUARD
Modeled after "WITH_QEMU_LOCK_GUARD" (see qemu's include/qemu/lockable.h).
Uses "__LINE__" instead of "__COUNTER__", as the latter is a GNU extension.
See comment for typical usage.
Signed-off-by: Tim Wiederhake
---
src/util/virthread.h | 20
1 file changed, 20 insertions(+)
diff --git a/src/util/virthread.h b/src/util/virthread.h
index 6cdaf2820e..da17780908 100644
--- a/src/util/virthread.h
+++ b/src/util/virthread.h
@@ -209,3 +209,23 @@ int virThreadLocalSet(virThreadLocal *l, void*)
G_GNUC_WARN_UNUSED_RESULT;
return 0; \
} \
struct classname ## EatSemicolon
+
+/**
+ * VIR_WITH_MUTEX_LOCK_GUARD:
+ *
+ * This macro defines a lock scope such that entering the scope takes the lock
+ * and leaving the scope releases the lock. Return statements are allowed
+ * within the scope and release the lock. Break and continue statements leave
+ * the scope early and release the lock.
+ *
+ * virMutex *mutex = ...;
+ *
+ * VIR_WITH_MUTEX_LOCK_GUARD(mutex) {
+ * // `mutex` is locked, and released automatically on scope exit
+ * ...
+ * }
+ */
+#define VIR_WITH_MUTEX_LOCK_GUARD(m) \
+for (g_auto(virLockGuard) CONCAT(var, __LINE__) = virLockGuardLock(m); \
+ CONCAT(var, __LINE__).mutex; \
+ CONCAT(var, __LINE__).mutex = (virLockGuardUnlock(& CONCAT(var,
__LINE__)), NULL))
--
2.31.1
[libvirt PATCH v4 1/9] internal: Add CONCAT macro
Using the two-step idiom to force resolution of other macros, e.g.: #define bar BAR CONCAT_(foo, bar) // foobar CONCAT(foo, bar) // fooBAR Signed-off-by: Tim Wiederhake --- src/internal.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/internal.h b/src/internal.h index b6e4332542..4cfb022b41 100644 --- a/src/internal.h +++ b/src/internal.h @@ -100,6 +100,9 @@ #define STREQ_NULLABLE(a, b) (g_strcmp0(a, b) == 0) #define STRNEQ_NULLABLE(a, b) (g_strcmp0(a, b) != 0) +#define CONCAT_(a, b) a ## b +#define CONCAT(a, b) CONCAT_(a, b) + #ifdef WIN32 # ifndef O_CLOEXEC # define O_CLOEXEC _O_NOINHERIT -- 2.31.1
[libvirt PATCH v4 0/9] Automatic mutex management
V1: https://listman.redhat.com/archives/libvir-list/2021-August/msg00823.html V2: https://listman.redhat.com/archives/libvir-list/2021-September/msg00249.html V3: https://listman.redhat.com/archives/libvir-list/2021-September/msg00964.html Changes since V3: * Remove not strictly necessary heap allocations from virLockGuard Regards, Tim Tim Wiederhake (9): internal: Add CONCAT macro virthread: Introduce virLockGuard virthread: Introduce VIR_WITH_MUTEX_LOCK_GUARD virobject: Introduce virObjectLockGuard virobject: Introduce VIR_WITH_OBJECT_LOCK_GUARD virChrdevFDStreamCloseCb: Use virLockGuardLock virChrdevFree: Use VIR_WITH_MUTEX_LOCK bhyveAutostartDomain: Use virObjectLockGuard lxcDomainDetachDeviceHostdevUSBLive: Use VIR_WITH_OBJECT_LOCK_GUARD src/bhyve/bhyve_driver.c | 4 ++-- src/conf/virchrdev.c | 12 +--- src/internal.h | 3 +++ src/libvirt_private.syms | 3 +++ src/lxc/lxc_driver.c | 6 +++--- src/util/virobject.c | 16 src/util/virobject.h | 24 src/util/virthread.c | 15 +++ src/util/virthread.h | 30 ++ 9 files changed, 101 insertions(+), 12 deletions(-) -- 2.31.1
Re: [libvirt PATCHv2 0/2] virsh: domsetlaunchsecstate: report error if no options are passed
On 1/27/22 19:44, Ján Tomko wrote: > Use a different approach that is hopefully more future-proof and also > add a check to the qemu driver, as suggested by Michal. > > Ján Tomko (2): > virsh: domsetlaunchsecstate: report error if no options are passed > qemu: qemuDomainSetLaunchSecurityState: check for params presence > > src/qemu/qemu_driver.c | 33 +++-- > tools/virsh-domain.c | 4 +++- > 2 files changed, 22 insertions(+), 15 deletions(-) > Reviewed-by: Michal Privoznik Michal
Re: [libvirt PATCH] cpu-data.py: Query hyperv enlightenments
On 1/27/22 14:51, Tim Wiederhake wrote: > Reporting hv-* properties properly requires hv to be enabled, > see qemu commit 071ce4b03b. > > Signed-off-by: Tim Wiederhake > --- > tests/cputestdata/cpu-data.py | 7 +++ > 1 file changed, 7 insertions(+) > Reviewed-by: Michal Privoznik Michal
Re: [PATCH v2] virProcessGetStatInfo: add a comment describing why we can not report error
Pinging again in case there is any interest ..
On Tue, Jan 25, 2022 at 4:34 PM Ani Sinha wrote:
>
> ping ...
>
> On Fri, 21 Jan 2022, Ani Sinha wrote:
>
> > virProcessGetStatInfo() currently is unable to report error conditions
> > because
> > that breaks libvirt's public best effort APIs. We add a comment in the
> > function
> > to indicate this. Adding comment here prevents others from going down the
> > path
> > of reporting error conditions in this functions in the future. It also
> > reminds
> > us that at some point in the future we need to fix the code so that this
> > limitations no longer exists.
> >
> > Please also see commit
> > 105dace22cc7 ("Revert "report error when virProcessGetStatInfo() is unable
> > to parse data"")
> >
> > Signed-off-by: Ani Sinha
> > ---
> > src/util/virprocess.c | 6 ++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/src/util/virprocess.c b/src/util/virprocess.c
> > index b559a4257e..9422829b8b 100644
> > --- a/src/util/virprocess.c
> > +++ b/src/util/virprocess.c
> > @@ -1784,6 +1784,12 @@ virProcessGetStatInfo(unsigned long long *cpuTime,
> > virStrToLong_ullp(proc_stat[VIR_PROCESS_STAT_STIME], NULL, 10,
> > ) < 0 ||
> > virStrToLong_l(proc_stat[VIR_PROCESS_STAT_RSS], NULL, 10, ) <
> > 0 ||
> > virStrToLong_i(proc_stat[VIR_PROCESS_STAT_PROCESSOR], NULL, 10,
> > ) < 0) {
> > +/* This function can not report error at present. Reporting error
> > here
> > + * causes some of libvirt's best effort public APIs to fail. This
> > + * resuts in external API behavior change. Until we can fix this in
> > + * a way so that public API behavior remains unchanged, we can only
> > + * write a warning log here.
> > + */
> > VIR_WARN("cannot parse process status data");
> > }
> >
> > --
> > 2.25.1
> >
> >
