RE: Question about skipping virDomainDiskDefAssignAddress

2021-07-15 Thread Motohiro Kawahito
> From: "Daniel P. Berrangé" 
> To: Motohiro Kawahito 
> Cc: libvir-list@redhat.com
> Date: 2021/07/16 00:42
> Subject: [EXTERNAL] Re: Question about skipping 
virDomainDiskDefAssignAddress
> 

> With the exception of paravirtualized Xen guests, this field in
> libvirt XML is *completely* independant of the guest assigned
> device name.
> 
> eg the XML might say /dev/vda, but the guest might decde to
> call it /dev/sda, or /dev/whatever or really absolutely
> anything.

Thank you very much for this information! I understand it. 
However, I don't understand how we can pass the target device information 
(e.g. 0A80) without an error. Do you know how to do it?

My question was not good, so I opened another thread (How do we specify 
disk device names for non-Linux VMs in XML?).




[RFC PATCH v2 6/8] qemu: force special parameters enabled for TDX guest

2021-07-15 Thread Zhenzhong Duan
TDX guest requires some special parameters to boot, They are:

 "-machine q35-*"
 "pic=no"
 "kernel_irqchip=split"

Signed-off-by: Zhenzhong Duan 
---
 src/qemu/qemu_command.c  |  2 +-
 src/qemu/qemu_validate.c | 11 +++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2bc8173d58..c53b0e237d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6980,7 +6980,7 @@ qemuBuildMachineCommandLine(virCommand *cmd,
 virBufferAddLit(, ",confidential-guest-support=lsec0");
 break;
 case VIR_DOMAIN_LAUNCH_SECURITY_TDX:
-virBufferAddLit(, 
",confidential-guest-support=lsec0,kvm-type=tdx");
+virBufferAddLit(, 
",confidential-guest-support=lsec0,kvm-type=tdx,pic=no");
 break;
 case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
 break;
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 309d48e62f..2cb05dc5b2 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -1243,6 +1243,17 @@ qemuValidateDomainDef(const virDomainDef *def,
  "this QEMU binary"));
 return -1;
 }
+if (!qemuDomainIsQ35(def)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("Intel TDX is supported with q35 machine 
types only"));
+return -1;
+}
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_KERNEL_IRQCHIP) ||
+ def->features[VIR_DOMAIN_FEATURE_IOAPIC] != 
VIR_DOMAIN_IOAPIC_QEMU) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("INTEL TDX launch security needs split kernel 
irqchip"));
+return -1;
+}
 break;
 case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
 break;
-- 
2.25.1



[RFC PATCH v2 4/8] conf: add tdx as launch security type

2021-07-15 Thread Zhenzhong Duan
When 'tdx' is used, the VM will launched with Intel TDX feature enabled.
TDX feature supports running encrypted VM (Trust Domain, TD) under the
control of KVM. A TD runs in a CPU model which protects the
confidentiality of its memory and its CPU state from other software

There is a child element 'policy' and three optional element for tdx type.
In 'policy', bit 0 is used to enable TDX debug, other bits are reserved
currently. mrconfigid, mrowner and mrownerconfig are hex string of 48 * 2
length each.

For example:

 
   0x0001
   xxx...xxx
   xxx...xxx
   xxx...xxx
 

Signed-off-by: Zhenzhong Duan 
---
 docs/schemas/domaincommon.rng | 16 
 src/conf/domain_conf.c| 47 +++
 src/conf/domain_conf.h|  9 +++
 src/conf/virconftypes.h   |  2 ++
 4 files changed, 74 insertions(+)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index b81c51728d..fd77601886 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -486,6 +486,7 @@
 
   sev
   s390-pv
+  tdx
 
   
   
@@ -519,6 +520,21 @@
 
   
 
+
+  
+
+  
+
+
+  
+
+  
+
+
+  
+
+  
+
   
 
   
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 92ab22d3fd..9510aa7b1f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1402,6 +1402,7 @@ VIR_ENUM_IMPL(virDomainLaunchSecurity,
   "",
   "sev",
   "s390-pv",
+  "tdx",
 );
 
 static virClass *virDomainObjClass;
@@ -3502,6 +3503,10 @@ virDomainSecDefFree(virDomainSecDef *def)
 g_free(def->data.sev.dh_cert);
 g_free(def->data.sev.session);
 break;
+case VIR_DOMAIN_LAUNCH_SECURITY_TDX:
+g_free(def->data.tdx.mrconfigid);
+g_free(def->data.tdx.mrowner);
+g_free(def->data.tdx.mrownerconfig);
 case VIR_DOMAIN_LAUNCH_SECURITY_PV:
 case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
 case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
@@ -14773,6 +14778,29 @@ virDomainSEVDefParseXML(virDomainSEVDef *def,
 }
 
 
+static int
+virDomainTDXDefParseXML(virDomainTDXDef *def,
+xmlXPathContextPtr ctxt)
+{
+VIR_XPATH_NODE_AUTORESTORE(ctxt)
+unsigned long policy;
+
+if (virXPathULongHex("string(./policy)", ctxt, ) < 0) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("failed to get launch security policy for "
+ "launch security type TDX"));
+return -1;
+}
+
+def->policy = policy;
+def->mrconfigid = virXPathString("string(./mrconfigid)", ctxt);
+def->mrowner = virXPathString("string(./mrowner)", ctxt);
+def->mrownerconfig = virXPathString("string(./mrownerconfig)", ctxt);
+
+return 0;
+}
+
+
 static virDomainSecDef *
 virDomainSecDefParseXML(xmlNodePtr lsecNode,
 xmlXPathContextPtr ctxt)
@@ -14792,6 +14820,10 @@ virDomainSecDefParseXML(xmlNodePtr lsecNode,
 if (virDomainSEVDefParseXML(>data.sev, lsecNode, ctxt) < 0)
 return NULL;
 break;
+case VIR_DOMAIN_LAUNCH_SECURITY_TDX:
+if (virDomainTDXDefParseXML(>data.tdx, ctxt) < 0)
+return NULL;
+break;
 case VIR_DOMAIN_LAUNCH_SECURITY_PV:
 if ((n = virXPathNodeSet("./*", ctxt, NULL)) < 0)
 return NULL;
@@ -26932,6 +26964,21 @@ virDomainSecDefFormat(virBuffer *buf, virDomainSecDef 
*sec)
 break;
 }
 
+case VIR_DOMAIN_LAUNCH_SECURITY_TDX: {
+virDomainTDXDef *tdx = >data.tdx;
+
+virBufferAsprintf(, "0x%04x\n", tdx->policy);
+
+if (tdx->mrconfigid)
+virBufferEscapeString(, "%s\n", 
tdx->mrconfigid);
+if (tdx->mrowner)
+virBufferEscapeString(, "%s\n", 
tdx->mrowner);
+if (tdx->mrownerconfig)
+virBufferEscapeString(, 
"%s\n", tdx->mrownerconfig);
+
+break;
+}
+
 case VIR_DOMAIN_LAUNCH_SECURITY_PV:
 break;
 
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 5c22f252d0..b29045d0c4 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2646,6 +2646,7 @@ typedef enum {
 VIR_DOMAIN_LAUNCH_SECURITY_NONE,
 VIR_DOMAIN_LAUNCH_SECURITY_SEV,
 VIR_DOMAIN_LAUNCH_SECURITY_PV,
+VIR_DOMAIN_LAUNCH_SECURITY_TDX,
 
 VIR_DOMAIN_LAUNCH_SECURITY_LAST,
 } virDomainLaunchSecurity;
@@ -2661,10 +2662,18 @@ struct _virDomainSEVDef {
 unsigned int reduced_phys_bits;
 };
 
+struct _virDomainTDXDef {
+unsigned int policy;
+char *mrconfigid;
+char *mrowner;
+char *mrownerconfig;
+};
+
 struct _virDomainSecDef {
 virDomainLaunchSecurity sectype;
 union {
 virDomainSEVDef sev;
+virDomainTDXDef tdx;
 } data;
 };
 
diff --git a/src/conf/virconftypes.h 

[RFC PATCH v2 2/8] qemu: Add TDX capability

2021-07-15 Thread Zhenzhong Duan
QEMU_CAPS_TDX_GUEST set means TDX supported with this qemu.

Signed-off-by: Chenyi Qiang 
Signed-off-by: Zhenzhong Duan 
---
 src/qemu/qemu_capabilities.c | 2 ++
 src/qemu/qemu_capabilities.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 9085c0b875..6a29ec607a 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -637,6 +637,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
   "confidential-guest-support",
   "query-display-options",
   "s390-pv-guest",
+  "tdx-guest",
 );
 
 
@@ -1356,6 +1357,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
 { "virtio-gpu-gl-pci", QEMU_CAPS_VIRTIO_GPU_GL_PCI },
 { "virtio-vga-gl", QEMU_CAPS_VIRTIO_VGA_GL },
 { "s390-pv-guest", QEMU_CAPS_S390_PV_GUEST },
+{ "tdx-guest", QEMU_CAPS_TDX_GUEST},
 };
 
 
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index f99bb211e0..2aa38f55e9 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -617,6 +617,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT, /* -machine 
confidential-guest-support */
 QEMU_CAPS_QUERY_DISPLAY_OPTIONS, /* 'query-display-options' qmp command 
present */
 QEMU_CAPS_S390_PV_GUEST, /* -object s390-pv-guest,... */
+QEMU_CAPS_TDX_GUEST, /* -object tdx-guest,... */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
-- 
2.25.1



[RFC PATCH v2 1/8] qemu: Check if INTEL Trust Domain Extention support is enabled

2021-07-15 Thread Zhenzhong Duan
Implement TDX check in order to generate domain feature capability
correctly in case the availability of the feature changed.

For INTEL TDX the verification is:
 - checking if /sys/firmware/tdx_seam/vendor_id contains the
   value "0x8086": meaning TDX is enabled in the host kernel.

Signed-off-by: Zhenzhong Duan 
---
 src/qemu/qemu_capabilities.c | 21 -
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 0d93cc2052..9085c0b875 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4748,6 +4748,24 @@ virQEMUCapsKVMSupportsSecureGuestAMD(void)
 }
 
 
+/*
+ * Check whether INTEL Trust Domain Extention (x86) is enabled
+ */
+static bool
+virQEMUCapsKVMSupportsSecureGuestINTEL(void)
+{
+g_autofree char *modValue = NULL;
+
+if (virFileReadValueString(, "/sys/firmware/tdx_seam/vendor_id") 
< 0)
+return false;
+
+if (STRNEQ(modValue,"0x8086"))
+return false;
+
+return true;
+}
+
+
 /*
  * Check whether the secure guest functionality is enabled.
  * See the specific architecture function for details on the verifications 
made.
@@ -4761,7 +4779,8 @@ virQEMUCapsKVMSupportsSecureGuest(void)
 return virQEMUCapsKVMSupportsSecureGuestS390();
 
 if (ARCH_IS_X86(arch))
-return virQEMUCapsKVMSupportsSecureGuestAMD();
+return virQEMUCapsKVMSupportsSecureGuestAMD() ||
+   virQEMUCapsKVMSupportsSecureGuestINTEL();
 
 return false;
 }
-- 
2.25.1



[RFC PATCH v2 8/8] qemu: Add firmware descriptor support for TDX

2021-07-15 Thread Zhenzhong Duan
Add a firmware descriptor support for TDVF, then libvirt can
auto match TDVF fimware with td-guest.

Signed-off-by: Zhenzhong Duan 
---
 docs/schemas/domaincommon.rng |   1 +
 src/qemu/qemu_capabilities.c  |   2 +
 src/qemu/qemu_firmware.c  | 101 +-
 src/qemu/qemu_validate.c  |   7 +++
 4 files changed, 108 insertions(+), 3 deletions(-)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 9d0b51ee12..8232025bf7 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -275,6 +275,7 @@
 
   bios
   efi
+  generic
 
   
 
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index d3c30a17e7..a01d4c26db 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -5934,6 +5934,8 @@ virQEMUCapsFillDomainOSCaps(virDomainCapsOS *os,
 VIR_DOMAIN_CAPS_ENUM_SET(os->firmware, 
VIR_DOMAIN_OS_DEF_FIRMWARE_BIOS);
 if (autoFirmwares & (1ULL << VIR_DOMAIN_OS_DEF_FIRMWARE_EFI))
 VIR_DOMAIN_CAPS_ENUM_SET(os->firmware, VIR_DOMAIN_OS_DEF_FIRMWARE_EFI);
+if (autoFirmwares & (1ULL << VIR_DOMAIN_OS_DEF_FIRMWARE_GENERIC))
+VIR_DOMAIN_CAPS_ENUM_SET(os->firmware, 
VIR_DOMAIN_OS_DEF_FIRMWARE_GENERIC);
 
 if (virQEMUCapsFillDomainLoaderCaps(capsLoader, secure,
 firmwaresAlt ? firmwaresAlt : 
firmwares,
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index e144b36f94..28e006eb82 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -84,12 +84,18 @@ struct _qemuFirmwareMappingMemory {
 char *filename;
 };
 
+typedef struct _qemuFirmwareMappingGeneric qemuFirmwareMappingGeneric;
+struct _qemuFirmwareMappingGeneric {
+char *filename;
+};
+
 
 typedef enum {
 QEMU_FIRMWARE_DEVICE_NONE = 0,
 QEMU_FIRMWARE_DEVICE_FLASH,
 QEMU_FIRMWARE_DEVICE_KERNEL,
 QEMU_FIRMWARE_DEVICE_MEMORY,
+QEMU_FIRMWARE_DEVICE_GENERIC,
 
 QEMU_FIRMWARE_DEVICE_LAST
 } qemuFirmwareDevice;
@@ -101,6 +107,7 @@ VIR_ENUM_IMPL(qemuFirmwareDevice,
   "flash",
   "kernel",
   "memory",
+  "generic",
 );
 
 
@@ -112,6 +119,7 @@ struct _qemuFirmwareMapping {
 qemuFirmwareMappingFlash flash;
 qemuFirmwareMappingKernel kernel;
 qemuFirmwareMappingMemory memory;
+qemuFirmwareMappingGeneric generic;
 } data;
 };
 
@@ -135,6 +143,7 @@ typedef enum {
 QEMU_FIRMWARE_FEATURE_SECURE_BOOT,
 QEMU_FIRMWARE_FEATURE_VERBOSE_DYNAMIC,
 QEMU_FIRMWARE_FEATURE_VERBOSE_STATIC,
+QEMU_FIRMWARE_FEATURE_INTEL_TDX,
 
 QEMU_FIRMWARE_FEATURE_LAST
 } qemuFirmwareFeature;
@@ -151,7 +160,8 @@ VIR_ENUM_IMPL(qemuFirmwareFeature,
   "requires-smm",
   "secure-boot",
   "verbose-dynamic",
-  "verbose-static"
+  "verbose-static",
+  "intel-tdx"
 );
 
 
@@ -213,6 +223,13 @@ 
qemuFirmwareMappingMemoryFreeContent(qemuFirmwareMappingMemory *memory)
 }
 
 
+static void
+qemuFirmwareMappingGenericFreeContent(qemuFirmwareMappingGeneric *generic)
+{
+g_free(generic->filename);
+}
+
+
 static void
 qemuFirmwareMappingFreeContent(qemuFirmwareMapping *mapping)
 {
@@ -226,6 +243,9 @@ qemuFirmwareMappingFreeContent(qemuFirmwareMapping *mapping)
 case QEMU_FIRMWARE_DEVICE_MEMORY:
 qemuFirmwareMappingMemoryFreeContent(>data.memory);
 break;
+case QEMU_FIRMWARE_DEVICE_GENERIC:
+qemuFirmwareMappingGenericFreeContent(>data.generic);
+break;
 case QEMU_FIRMWARE_DEVICE_NONE:
 case QEMU_FIRMWARE_DEVICE_LAST:
 break;
@@ -424,6 +444,25 @@ qemuFirmwareMappingMemoryParse(const char *path,
 }
 
 
+static int
+qemuFirmwareMappingGenericParse(const char *path,
+   virJSONValue *doc,
+   qemuFirmwareMappingGeneric *generic)
+{
+const char *filename;
+
+if (!(filename = virJSONValueObjectGetString(doc, "filename"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("missing 'filename' in '%s'"),
+   path);
+}
+
+generic->filename = g_strdup(filename);
+
+return 0;
+}
+
+
 static int
 qemuFirmwareMappingParse(const char *path,
  virJSONValue *doc,
@@ -469,6 +508,10 @@ qemuFirmwareMappingParse(const char *path,
 if (qemuFirmwareMappingMemoryParse(path, mapping, 
>mapping.data.memory) < 0)
 return -1;
 break;
+case QEMU_FIRMWARE_DEVICE_GENERIC:
+if (qemuFirmwareMappingGenericParse(path, mapping, 
>mapping.data.generic) < 0)
+return -1;
+break;
 
 case QEMU_FIRMWARE_DEVICE_NONE:
 case QEMU_FIRMWARE_DEVICE_LAST:
@@ -740,6 +783,19 @@ qemuFirmwareMappingMemoryFormat(virJSONValue *mapping,
 }
 
 
+static int
+qemuFirmwareMappingGenericFormat(virJSONValue 

[RFC PATCH v2 7/8] qemu: Add general loader support

2021-07-15 Thread Zhenzhong Duan
Intel TDX requires a general loader to hold its firmware TDVF.

Add new loader type VIR_DOMAIN_LOADER_TYPE_GENERIC and
VIR_DOMAIN_OS_DEF_FIRMWARE_GENERIC to support this feature.

XML looks like:


  /path/to/TDVF-binary


Qemu command line looks like:

$QEMU ... \
  -device loader,file=/path/to/TDVF-binary

Signed-off-by: Zhenzhong Duan 
---
 docs/schemas/domaincommon.rng | 1 +
 src/conf/domain_conf.c| 2 ++
 src/conf/domain_conf.h| 2 ++
 src/qemu/qemu_capabilities.c  | 3 +++
 src/qemu/qemu_command.c   | 5 +
 src/qemu/qemu_namespace.c | 1 +
 6 files changed, 14 insertions(+)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index fd77601886..9d0b51ee12 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -319,6 +319,7 @@
 
   rom
   pflash
+  generic
 
   
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9510aa7b1f..fbbbe708d4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1311,6 +1311,7 @@ VIR_ENUM_IMPL(virDomainLoader,
   "none",
   "rom",
   "pflash",
+  "generic",
 );
 
 VIR_ENUM_IMPL(virDomainIOAPIC,
@@ -1333,6 +1334,7 @@ VIR_ENUM_IMPL(virDomainOsDefFirmware,
   "none",
   "bios",
   "efi",
+  "generic",
 );
 
 VIR_ENUM_IMPL(virDomainOsDefFirmwareFeature,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index b29045d0c4..99b74683a0 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2163,6 +2163,7 @@ typedef enum {
 VIR_DOMAIN_LOADER_TYPE_NONE = 0,
 VIR_DOMAIN_LOADER_TYPE_ROM,
 VIR_DOMAIN_LOADER_TYPE_PFLASH,
+VIR_DOMAIN_LOADER_TYPE_GENERIC,
 
 VIR_DOMAIN_LOADER_TYPE_LAST
 } virDomainLoader;
@@ -2246,6 +2247,7 @@ typedef enum {
 VIR_DOMAIN_OS_DEF_FIRMWARE_NONE = 0,
 VIR_DOMAIN_OS_DEF_FIRMWARE_BIOS = VIR_DOMAIN_LOADER_TYPE_ROM,
 VIR_DOMAIN_OS_DEF_FIRMWARE_EFI = VIR_DOMAIN_LOADER_TYPE_PFLASH,
+VIR_DOMAIN_OS_DEF_FIRMWARE_GENERIC = VIR_DOMAIN_LOADER_TYPE_GENERIC,
 
 VIR_DOMAIN_OS_DEF_FIRMWARE_LAST
 } virDomainOsDefFirmware;
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index e9906a2f32..d3c30a17e7 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -5888,6 +5888,9 @@ virQEMUCapsFillDomainLoaderCaps(virDomainCapsLoader 
*capsLoader,
 VIR_DOMAIN_CAPS_ENUM_SET(capsLoader->type,
  VIR_DOMAIN_LOADER_TYPE_PFLASH);
 
+VIR_DOMAIN_CAPS_ENUM_SET(capsLoader->type,
+ VIR_DOMAIN_LOADER_TYPE_GENERIC);
+
 
 VIR_DOMAIN_CAPS_ENUM_SET(capsLoader->readonly,
  VIR_TRISTATE_BOOL_YES,
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c53b0e237d..99812e37d8 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9640,6 +9640,11 @@ qemuBuildDomainLoaderCommandLine(virCommand *cmd,
 qemuBuldDomainLoaderPflashCommandLine(cmd, loader, qemuCaps);
 break;
 
+case VIR_DOMAIN_LOADER_TYPE_GENERIC:
+virCommandAddArg(cmd, "-device");
+virCommandAddArgFormat(cmd, "loader,file=%s", loader->path);
+break;
+
 case VIR_DOMAIN_LOADER_TYPE_NONE:
 case VIR_DOMAIN_LOADER_TYPE_LAST:
 /* nada */
diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c
index e902f0eecc..aa635b1375 100644
--- a/src/qemu/qemu_namespace.c
+++ b/src/qemu/qemu_namespace.c
@@ -569,6 +569,7 @@ qemuDomainSetupLoader(virDomainObj *vm,
 if (loader) {
 switch ((virDomainLoader) loader->type) {
 case VIR_DOMAIN_LOADER_TYPE_ROM:
+case VIR_DOMAIN_LOADER_TYPE_GENERIC:
 *paths = g_slist_prepend(*paths, g_strdup(loader->path));
 break;
 
-- 
2.25.1



[RFC PATCH v2 3/8] conf: expose TDX feature in domain capabilities

2021-07-15 Thread Zhenzhong Duan
Extend qemu TDX capability to domain capabilities.

Signed-off-by: Chenyi Qiang 
Signed-off-by: Zhenzhong Duan 
---
 docs/formatdomaincaps.html.in  | 17 +
 docs/schemas/domaincaps.rng|  9 +
 src/conf/domain_capabilities.c |  1 +
 src/conf/domain_capabilities.h |  1 +
 src/qemu/qemu_capabilities.c   | 16 
 5 files changed, 44 insertions(+)

diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in
index 62f1940e6a..3f057af515 100644
--- a/docs/formatdomaincaps.html.in
+++ b/docs/formatdomaincaps.html.in
@@ -570,6 +570,7 @@
   cbitpos47/cbitpos
   reduced-phys-bits1/reduced-phys-bits
 /sev
+tdx supported='yes'/
   /features
 /domainCapabilities
 
@@ -635,6 +636,22 @@
   a look at SEV in domain 
XML
 
 
+TDX capabilities
+
+Trust Domain Extensions(TDX) capabilities are exposed under the
+tdx element.
+TDX is an Intel technology that extends Virtual Machines Extensions (VMX)
+to with a new kind of virtual machine guest called Trust Domain (TD). A TD
+runs in a CPU model which protects the confidentiality of its memory 
contents
+and its CPU state from any other software, including the hosting Virtual 
Machine
+Monitor (VMM), unless explicitly shared by the TD itself.
+
+
+  For more details on the TDX feature, please follow resources in the
+  Intel developer's document. In order to use TDX with libvirt have
+  a look at TDX in domain 
XML
+
+
 
   cbitpos
   When memory encryption is enabled, one of the physical address bits
diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng
index d7ee60dd16..60001b3c43 100644
--- a/docs/schemas/domaincaps.rng
+++ b/docs/schemas/domaincaps.rng
@@ -253,6 +253,9 @@
   
 
   
+  
+
+  
 
   
 
@@ -307,6 +310,12 @@
 
   
 
+  
+
+  
+
+  
+
   
 
   
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index 83d3320980..2380eacde9 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -43,6 +43,7 @@ VIR_ENUM_IMPL(virDomainCapsFeature,
   "backingStoreInput",
   "backup",
   "s390-pv",
+  "tdx",
 );
 
 static virClass *virDomainCapsClass;
diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
index 34b9b8a693..cd3f5be472 100644
--- a/src/conf/domain_capabilities.h
+++ b/src/conf/domain_capabilities.h
@@ -180,6 +180,7 @@ typedef enum {
 VIR_DOMAIN_CAPS_FEATURE_BACKING_STORE_INPUT,
 VIR_DOMAIN_CAPS_FEATURE_BACKUP,
 VIR_DOMAIN_CAPS_FEATURE_S390_PV,
+VIR_DOMAIN_CAPS_FEATURE_TDX,
 
 VIR_DOMAIN_CAPS_FEATURE_LAST
 } virDomainCapsFeature;
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 6a29ec607a..e9906a2f32 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -6351,6 +6351,21 @@ virQEMUCapsFillDomainFeatureS390PVCaps(virQEMUCaps 
*qemuCaps,
 }
 
 
+static void
+virQEMUCapsFillDomainFeatureTDXCaps(virQEMUCaps *qemuCaps,
+virDomainCaps *domCaps)
+{
+if (ARCH_IS_X86(qemuCaps->arch)) {
+if (virQEMUCapsGet(qemuCaps, 
QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT) &&
+virQEMUCapsGet(qemuCaps, QEMU_CAPS_TDX_GUEST) &&
+virQEMUCapsGetKVMSupportsSecureGuest(qemuCaps))
+domCaps->features[VIR_DOMAIN_CAPS_FEATURE_TDX] = 
VIR_TRISTATE_BOOL_YES;
+else
+domCaps->features[VIR_DOMAIN_CAPS_FEATURE_TDX] = 
VIR_TRISTATE_BOOL_NO;
+}
+}
+
+
 int
 virQEMUCapsFillDomainCaps(virQEMUCaps *qemuCaps,
   virArch hostarch,
@@ -6398,6 +6413,7 @@ virQEMUCapsFillDomainCaps(virQEMUCaps *qemuCaps,
 virQEMUCapsFillDomainFeatureGICCaps(qemuCaps, domCaps);
 virQEMUCapsFillDomainFeatureSEVCaps(qemuCaps, domCaps);
 virQEMUCapsFillDomainFeatureS390PVCaps(qemuCaps, domCaps);
+virQEMUCapsFillDomainFeatureTDXCaps(qemuCaps, domCaps);
 
 return 0;
 }
-- 
2.25.1



[RFC PATCH v2 5/8] qemu: Add command line and validation for TDX type

2021-07-15 Thread Zhenzhong Duan
QEMU will provides 'tdx-guest' object which is used to launch encrypted
VMs on Intel platform using TDX feature. A typical TDX guest launch
command line looks like:

$QEMU ... \
  -object tdx-guest,id=tdx0,debug=on \
  -machine q35,confidential-guest-support=tdx0,kvm-type=tdx

Signed-off-by: Zhenzhong Duan 
---
 src/qemu/qemu_command.c   | 33 +
 src/qemu/qemu_firmware.c  |  1 +
 src/qemu/qemu_namespace.c |  1 +
 src/qemu/qemu_process.c   |  1 +
 src/qemu/qemu_validate.c  | 10 ++
 5 files changed, 46 insertions(+)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index db78deb122..2bc8173d58 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6979,6 +6979,9 @@ qemuBuildMachineCommandLine(virCommand *cmd,
 case VIR_DOMAIN_LAUNCH_SECURITY_PV:
 virBufferAddLit(, ",confidential-guest-support=lsec0");
 break;
+case VIR_DOMAIN_LAUNCH_SECURITY_TDX:
+virBufferAddLit(, 
",confidential-guest-support=lsec0,kvm-type=tdx");
+break;
 case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
 break;
 case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
@@ -9897,6 +9900,33 @@ qemuBuildPVCommandLine(virDomainObj *vm, virCommand *cmd)
 }
 
 
+static int
+qemuBuildTDXCommandLine(virDomainObj *vm, virCommand *cmd,
+virDomainTDXDef *tdx)
+{
+g_autoptr(virJSONValue) props = NULL;
+g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
+qemuDomainObjPrivate *priv = vm->privateData;
+
+VIR_DEBUG("policy=0x%x", tdx->policy);
+
+if (qemuMonitorCreateObjectProps(, "tdx-guest", "lsec0",
+ "B:debug", !!(tdx->policy & 1),
+ "S:mrconfigid", tdx->mrconfigid,
+ "S:mrowner", tdx->mrowner,
+ "S:mrownerconfig", tdx->mrownerconfig,
+ NULL) < 0)
+return -1;
+
+if (qemuBuildObjectCommandlineFromJSON(, props, priv->qemuCaps) < 0)
+return -1;
+
+virCommandAddArg(cmd, "-object");
+virCommandAddArgBuffer(cmd, );
+return 0;
+}
+
+
 static int
 qemuBuildSecCommandLine(virDomainObj *vm, virCommand *cmd,
 virDomainSecDef *sec)
@@ -9911,6 +9941,9 @@ qemuBuildSecCommandLine(virDomainObj *vm, virCommand *cmd,
 case VIR_DOMAIN_LAUNCH_SECURITY_PV:
 return qemuBuildPVCommandLine(vm, cmd);
 break;
+case VIR_DOMAIN_LAUNCH_SECURITY_TDX:
+return qemuBuildTDXCommandLine(vm, cmd, >data.tdx);
+break;
 case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
 break;
 case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index 77c452746f..e144b36f94 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -1070,6 +1070,7 @@ qemuFirmwareMatchDomain(const virDomainDef *def,
 }
 break;
 case VIR_DOMAIN_LAUNCH_SECURITY_PV:
+case VIR_DOMAIN_LAUNCH_SECURITY_TDX:
 case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
 break;
 case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c
index 42865a6497..e902f0eecc 100644
--- a/src/qemu/qemu_namespace.c
+++ b/src/qemu/qemu_namespace.c
@@ -608,6 +608,7 @@ qemuDomainSetupLaunchSecurity(virDomainObj *vm,
 VIR_DEBUG("Set up launch security for SEV");
 break;
 case VIR_DOMAIN_LAUNCH_SECURITY_PV:
+case VIR_DOMAIN_LAUNCH_SECURITY_TDX:
 case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
 break;
 case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index f2a523e4f7..b5324c85a1 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6706,6 +6706,7 @@ qemuProcessPrepareLaunchSecurityGuestInput(virDomainObj 
*vm)
 case VIR_DOMAIN_LAUNCH_SECURITY_SEV:
 return qemuProcessPrepareSEVGuestInput(vm);
 case VIR_DOMAIN_LAUNCH_SECURITY_PV:
+case VIR_DOMAIN_LAUNCH_SECURITY_TDX:
 case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
 break;
 case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 7482bedee6..309d48e62f 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -1234,6 +1234,16 @@ qemuValidateDomainDef(const virDomainDef *def,
 return -1;
 }
 break;
+ case VIR_DOMAIN_LAUNCH_SECURITY_TDX:
+if (!virQEMUCapsGet(qemuCaps, 
QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT) ||
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_TDX_GUEST) ||
+!virQEMUCapsGetKVMSupportsSecureGuest(qemuCaps)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("INTEL TDX launch security is not supported 
with "
+ "this QEMU binary"));
+ 

[RFC PATCH v2 0/8] LIBVIRT: X86: TDX support

2021-07-15 Thread Zhenzhong Duan
Thanks Peter, Pavel and Daniel's comments on v1 version, now the v2 comes.

* What's TDX?
TDX stands for Trust Domain Extensions which isolates VMs from
the virtual-machine manager (VMM)/hypervisor and any other software on
the platform.

To support TDX, multiple software components, not only KVM but also QEMU,
guest Linux and virtual bios, need to be updated. For more details, please
check link[1], there are TDX spec links and public repository link at github
for each software component.

This patchset is another software component to extend libvirt to support TDX,
with which one can start a VM from high level rather than running qemu directly.


* The goal of this RFC patch
The purpose of this post is to get feedback early on high level design issue of
libvirt enhancement for TDX. Referenced much on AMD SEV and S390 PV implemention
at link[2][3]. This 2nd version is rebased on upstream + s390 v4 version as
shown in [3] to utilize the common launchsecurity framework code.


* Patch organization
- patch 1-3: Support query of TDX capabilities.
- patch 4-6: Add TDX type to launchsecurity framework.
- patch   7: Add general loader support for TDX.
- patch   8: Add firmware descriptor support for TDX.


* Misc
Just let you know we have released v2 version of TDX qemu in [1], and the API
for libvirt is keeping stable. Using these patches we have succesfully booted
and tested a guest both with and without TDX enabled.


* Diff to v1:
- give up using qmp cmd and check TDX directly on host for TDX capabilities.
- use launchsecurity framework to support TDX
- use . for general loader
- add auto firmware match feature for TDX

A example TDVF fimware description file 70-edk2-x86_64-tdx.json:
{
"description": "UEFI firmware for x86_64, supporting Intel TDX",
"interface-types": [
"uefi"
],
"mapping": {
"device": "generic",
"filename": "/usr/share/OVMF/OVMF_CODE-tdx.fd"
},
"targets": [
{
"architecture": "x86_64",
"machines": [
"pc-q35-*"
]
}
],
"features": [
"intel-tdx",
"verbose-dynamic"
],
"tags": [

]
}


Links:
[1] https://lists.nongnu.org/archive/html/qemu-devel/2021-07/msg01682.html
[2] https://github.com/codomania/libvirt/commits/v9
[3] https://www.mail-archive.com/libvir-list@redhat.com/msg219144.html

Zhenzhong Duan (8):
  qemu: Check if INTEL Trust Domain Extention support is enabled
  qemu: Add TDX capability
  conf: expose TDX feature in domain capabilities
  conf: add tdx as launch security type
  qemu: Add command line and validation for TDX type
  qemu: force special parameters enabled for TDX guest
  qemu: Add general loader support
  qemu: Add firmware descriptor support for TDX

 docs/formatdomaincaps.html.in  |  17 ++
 docs/schemas/domaincaps.rng|   9 +++
 docs/schemas/domaincommon.rng  |  18 ++
 src/conf/domain_capabilities.c |   1 +
 src/conf/domain_capabilities.h |   1 +
 src/conf/domain_conf.c |  49 
 src/conf/domain_conf.h |  11 
 src/conf/virconftypes.h|   2 +
 src/qemu/qemu_capabilities.c   |  44 ++-
 src/qemu/qemu_capabilities.h   |   1 +
 src/qemu/qemu_command.c|  38 +
 src/qemu/qemu_firmware.c   | 100 -
 src/qemu/qemu_namespace.c  |   2 +
 src/qemu/qemu_process.c|   1 +
 src/qemu/qemu_validate.c   |  28 +
 15 files changed, 319 insertions(+), 3 deletions(-)

-- 
2.25.1



How do we specify disk device names for non-Linux VMs in XML?

2021-07-15 Thread Motohiro Kawahito
The XML parser (or verifier) in libvirt seems to support only Linux device 
names for a disk, such as hda, sda and vda.
Can anyone let us know how libvirt supports disk device names for 
non-Linux VMs?
We are developing a new hypervisor driver for a non-Linux VM, which uses a 
different device naming convention from Linux, something like 0A80. 0A81, 
0A82.
Thank you so much. 

Motohiro Kawahito, Commercial Systems, IBM Research - Tokyo
19-21 Nihonbashi, Hakozaki-cho Chuo-ku, Tokyo 103-8510, Japan 




RE: [libvirt][PATCH v4 0/4] Support query and use SGX

2021-07-15 Thread Huang, Haibin



> -Original Message-
> From: Pavel Hrdina 
> Sent: Wednesday, July 7, 2021 5:48 PM
> To: Huang, Haibin 
> Cc: libvir-list@redhat.com; Ding, Jian-feng ; Yang,
> Lin A ; Lu, Lianhao 
> Subject: Re: [libvirt][PATCH v4 0/4] Support query and use SGX
> 
> On Thu, Jul 01, 2021 at 08:10:25PM +0800, Haibin Huang wrote:
> > This patch series provides support for enabling Intel's Software Guard
> Extensions (SGX) feature in guest VM.
> >
> > Giving the SGX support in QEMU is still pending for reviewing, this
> > patch series is not submmited for code review, but only describe the
> > SGX enabling solution design that contains changes to
> virConnectGetDomainCapabilities API response and domain definition. All
> comments/suggestions would be highly appreciated.
> >
> > Intel Software Guard Extensions (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.
> >
> > 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 should contain
> >
> > 
> >   N
> > 
> 
> I don't think that Intel SGX belongs into  in libvirt.
> Similar feature to AMD SEV is Intel TDX which would be implement using
>  as it offers isolation between host and VM.
> 
> Looking at the patches this doesn't even use confidential-guest-support 
> machine
> option, it adds a new memory backend and enables CPU features only if libvirt
> uses  so it would not work with any other CPU mode.
> 
> To me this sounds like we should split the feature into two components where
> one would add support for the new memory backend into correct XML part [1]
> and the other component would be support for CPU features related to Intel
> SGX [2].
[Haibin] ok, those specific CPU features we added have been deleted and let 
user to specify it in [2].
Do we need to add new element in memory backend for SGX EPC memory?
> 
> Pavel
> 
> [1] 
> [2] 
> 
> > Haibin Huang (1):
> >   Support to query SGX capability
> >
> > Lin Yang (3):
> >   conf: Introduce SGX related element into domain xml
> >   qemu: Add command-line to generate SGX EPC memory backend
> >   qemu: Add command-line to enable SGX
> >
> >  src/conf/domain_capabilities.c|  29 
> >  src/conf/domain_capabilities.h|  13 ++
> >  src/conf/domain_conf.c| 106 +
> >  src/conf/domain_conf.h|  10 ++
> >  src/conf/virconftypes.h   |   3 +
> >  src/libvirt_private.syms  |   2 +-
> >  src/qemu/qemu_capabilities.c  | 146 ++
> >  src/qemu/qemu_capabilities.h  |   6 +
> >  src/qemu/qemu_command.c   |  30 
> >  src/qemu/qemu_monitor.c   |  10 ++
> >  src/qemu/qemu_monitor.h   |   3 +
> >  src/qemu/qemu_monitor_json.c  |  91 +++
> >  src/qemu/qemu_monitor_json.h  |   3 +
> >  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_1.5.3-q35.x86_64.xml  |   1 +
> >  .../domaincapsdata/qemu_1.5.3-tcg.x86_64.xml  |   1 +
> >  tests/domaincapsdata/qemu_1.5.3.x86_64.xml|   1 +
> >  .../domaincapsdata/qemu_1.6.0-q35.x86_64.xml  |   1 +
> >  .../domaincapsdata/qemu_1.6.0-tcg.x86_64.xml  |   1 +
> >  tests/domaincapsdata/qemu_1.6.0.x86_64.xml|   1 +
> >  .../domaincapsdata/qemu_1.7.0-q35.x86_64.xml  |   1 +
> >  .../domaincapsdata/qemu_1.7.0-tcg.x86_64.xml  |   1 +
> >  tests/domaincapsdata/qemu_1.7.0.x86_64.xml|   1 +
> >  .../domaincapsdata/qemu_2.1.1-q35.x86_64.xml  |   1 +
> >  .../domaincapsdata/qemu_2.1.1-tcg.x86_64.xml  |   1 +
> >  tests/domaincapsdata/qemu_2.1.1.x86_64.xml|   1 +
> >  .../domaincapsdata/qemu_2.10.0-q35.x86_64.xml |   1 +
> >  .../domaincapsdata/qemu_2.10.0-tcg.x86_64.xml |   1 +
> >  .../qemu_2.10.0-virt.aarch64.xml  |   1 +
> >  tests/domaincapsdata/qemu_2.10.0.aarch64.xml  |   1 +
> >  tests/domaincapsdata/qemu_2.10.0.ppc64.xml|   1 +
> >  tests/domaincapsdata/qemu_2.10.0.s390x.xml|   1 +
> >  tests/domaincapsdata/qemu_2.10.0.x86_64.xml   |   1 +
> >  

Re:[PATCH] qemu: Fix error returned value in qemuGetProcessInfo when fscanf execute failed

2021-07-15 Thread wang.yi59
Hi Michal,

Thanks for your reply.

> On 7/15/21 8:18 AM, Yi Wang wrote:
> > From: Long YunJian 
> >
> > If fscanf execute failed, qemuGetProcessInfo shuld return -1,
> > but it return 0 at the end. Zero means success for the caller,
> > so we shuld return -1 in the case of failure.
> >
...
> > /* cutime -> endcode */
> > @@ -1455,6 +1457,8 @@ qemuGetProcessInfo(unsigned long long *cpuTime, int 
> > *lastCpu, long *vm_rss,
> > "%*u %*u %*u %*u %*u %*u %*u %*u %*u %*u %*d %d",
> > , , , ) != 4) {
> >  VIR_WARN("cannot parse process status data");
> > +VIR_FORCE_FCLOSE(pidinfo);
> > +return -1;
>
> There's another VIR_FORCE_FCLOSE() at the end of the function. It should
> be removed.

VIR_FORCE_FCLOSE() at the end of the function means in the
situation of success, but VIR_FORCE_FCLOSE before return -1 is
failure situation. if removed, it will result in not close pidinfo.

>
> But is there an actual error you're seeing? Has /proc/NNN/stat format
> changed? Also, please use git send-email if possible - it allows
> reviewers to use git am (which doesn't work with multipart e-mails).

I haven't meet fscanf return error,  but in some abnormal situation,
such as interrupted by a signal or out of memory, it may return failure.

Actually I used git send-email to send this patch, if there's something
wrong, please let me know and I will forward that to our IT department :)

>
> Michal

[PATCH v1] virtqemud: remove sysconfig file

2021-07-15 Thread Olaf Hering
sysconfig files are owned by the admin of the host. He has the liberty
to put anything he wants into these files. This makes it difficult to
provide different defaults.

Remove the sysconfig file and place the current desired default into
the service file.

Local customizations can now go either into /etc/sysconfig/virtqemud
or /etc/systemd/system/virtqemud.service.d/my-knobs.conf

Signed-off-by: Olaf Hering 
---

Untested, just to demonstrate how it could be done for every sysconfig file.

 libvirt.spec.in   |  1 -
 src/qemu/meson.build  |  5 -
 src/qemu/virtqemud.service.in |  1 +
 src/qemu/virtqemud.sysconf| 12 
 4 files changed, 1 insertion(+), 18 deletions(-)
 delete mode 100644 src/qemu/virtqemud.sysconf

diff --git a/libvirt.spec.in b/libvirt.spec.in
index cb48dd0be0..2cc3e61e3c 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1689,7 +1689,6 @@ exit 0
 
 %if %{with_qemu}
 %files daemon-driver-qemu
-%config(noreplace) %{_sysconfdir}/sysconfig/virtqemud
 %config(noreplace) %{_sysconfdir}/libvirt/virtqemud.conf
 %{_datadir}/augeas/lenses/virtqemud.aug
 %{_datadir}/augeas/lenses/tests/test_virtqemud.aug
diff --git a/src/qemu/meson.build b/src/qemu/meson.build
index 3898d23877..6b4dd58309 100644
--- a/src/qemu/meson.build
+++ b/src/qemu/meson.build
@@ -165,11 +165,6 @@ if conf.has('WITH_QEMU')
 'in_file': files('virtqemud.init.in'),
   }
 
-  sysconf_files += {
-'name': 'virtqemud',
-'file': files('virtqemud.sysconf'),
-  }
-
   virt_install_dirs += [
 localstatedir / 'lib' / 'libvirt' / 'qemu',
 runstatedir / 'libvirt' / 'qemu',
diff --git a/src/qemu/virtqemud.service.in b/src/qemu/virtqemud.service.in
index 8abc9d3a7f..a5cb145668 100644
--- a/src/qemu/virtqemud.service.in
+++ b/src/qemu/virtqemud.service.in
@@ -18,6 +18,7 @@ Documentation=https://libvirt.org
 
 [Service]
 Type=notify
+Environment=VIRTQEMUD_ARGS="--timeout 120"
 EnvironmentFile=-@sysconfdir@/sysconfig/virtqemud
 ExecStart=@sbindir@/virtqemud $VIRTQEMUD_ARGS
 ExecReload=/bin/kill -HUP $MAINPID
diff --git a/src/qemu/virtqemud.sysconf b/src/qemu/virtqemud.sysconf
deleted file mode 100644
index 87b626e3ed..00
--- a/src/qemu/virtqemud.sysconf
+++ /dev/null
@@ -1,12 +0,0 @@
-# Customizations for the virtqemud.service systemd unit
-
-VIRTQEMUD_ARGS="--timeout 120"
-
-# Override the QEMU/SDL default audio driver probing when
-# starting virtual machines using SDL graphics
-#
-# NB these have no effect for VMs using VNC, unless vnc_allow_host_audio
-# is enabled in /etc/libvirt/qemu.conf
-#QEMU_AUDIO_DRV=sdl
-#
-#SDL_AUDIODRIVER=pulse



Re: Question about skipping virDomainDiskDefAssignAddress

2021-07-15 Thread Daniel P . Berrangé
On Fri, Jul 16, 2021 at 12:28:42AM +0900, Motohiro Kawahito wrote:
> Hi, is there any existing way for skipping virDomainDiskDefAssignAddress 
> for disk configuration?
> 
> I want to send the following XML to libvirt, but I got the error 
> "virDomainDiskDefAssignAddress:7642 : XML error: Unknown disk name '0A80' 
> and no address specified".
> 
> 
> 
> 
> 
> 
> According to the source code, target device must be one of {"fd", "hd", 
> "vd", "sd", "xvd", "ubd"} + alphabet (+ number). Our guest OS is not 
> Linux, so I'd like to skip this limitation. Actually, I want to specify 
> 4-digits hexadecimal number in target device.

Ah, you're mis-understanding the impact of this device name field.

With the exception of paravirtualized Xen guests, this field in
libvirt XML is *completely* independant of the guest assigned
device name.

eg the XML might say /dev/vda, but the guest might decde to
call it /dev/sda, or /dev/whatever or really absolutely
anything.

The only thing that the target dev=xx does in the libvirt
XML is to establish a device ordering relationship between
disks in the XML. ie libvirt will configure /dev/vda before
/dev/vdb.  The guest will probably then probe in the this
same order, but that's entirely guest dependant - some may
probe devices in paralell.

IOW, just use /dev/NNN prefix that matches the bus type,
regardless of what your guest OS is. Your guest OS will
just do its own thing as needed.

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 :|



Question about skipping virDomainDiskDefAssignAddress

2021-07-15 Thread Motohiro Kawahito
Hi, is there any existing way for skipping virDomainDiskDefAssignAddress 
for disk configuration?

I want to send the following XML to libvirt, but I got the error 
"virDomainDiskDefAssignAddress:7642 : XML error: Unknown disk name '0A80' 
and no address specified".






According to the source code, target device must be one of {"fd", "hd", 
"vd", "sd", "xvd", "ubd"} + alphabet (+ number). Our guest OS is not 
Linux, so I'd like to skip this limitation. Actually, I want to specify 
4-digits hexadecimal number in target device.

Is there any hypervisor driver for Windows? I guess that it also has the 
same problem.

Motohiro Kawahito, Commercial Systems, IBM Research - Tokyo
19-21 Nihonbashi, Hakozaki-cho Chuo-ku, Tokyo 103-8510, Japan 




Re: [libvirt PATCH 2/2] virsh: allow nodedev-list --all --tree

2021-07-15 Thread Shalini Chellathurai Saroja



On 7/7/21 11:29 PM, Jonathon Jongsma wrote:

Allow the tree view with --all so that we can see all inactive mdevs in
a tree structure nested under their parent devices.

Signed-off-by: Jonathon Jongsma

Reviewed-by: Shalini Chellathurai Saroja 


--
Kind regards
Shalini Chellathurai Saroja
Linux on Z and Virtualization Development
Vorsitzende des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294




Re: [libvirt PATCH 1/2] nodedev: fix parent device of inactive mdevs

2021-07-15 Thread Shalini Chellathurai Saroja



On 7/7/21 11:29 PM, Jonathon Jongsma wrote:

Inactive mdevs were simply formatting their parent name as the value
received from mdevctl rather than looking up the libvirt nodedev name of
the parent device. This resulted in a parent value of e.g.
':5b:00.0' instead of 'pci__5b_00_0'. This prevented defining a
new mdev device from the output of nodedev-dumpxml.

Unfortunately, it's not simple to fix this comprehensively due to the
fact that mdevctl supports defining (inactive) mdevs for parent devices
that do not actually exist on the host (yet). So for those persistent
mdev definitions that do not have a valid parent in the device list, the
parent device will be set to the root "computer" device.

Unfortunately, because the value of the 'parent' field now depends on
the configuration of the host, the mdevctl parsing test will output
'computer' for all test devices. Fixing this would require a more
extensive mock test environment.

Fixes:https://bugzilla.redhat.com/show_bug.cgi?id=1979761

Signed-off-by: Jonathon Jongsma


Hello Jonathon,

I also tested the patch with vfio_ap devices. It works fine. Thank you.

Reviewed-by: Shalini Chellathurai Saroja 


--
Kind regards
Shalini Chellathurai Saroja
Linux on Z and Virtualization Development
Vorsitzende des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294




Re: [PATCH v2 2/3] util: Add virGetCpuHaltPollTime and virGetDebugFsKvmValue

2021-07-15 Thread Yang Fei



On 2021/7/14 22:17, Michal Prívozník wrote:
> On 7/14/21 2:28 PM, Yang Fei wrote:
>> Add helper function virGetCpuHaltPollTime and virGetDebugFsKvmValue
>> to obtain the halt polling time. If system mount debugfs, and the
>> kernel support halt polling time statistic, it will work.
>>
>> Signed-off-by: Yang Fei 
>> ---
>>  src/libvirt_private.syms |  2 ++
>>  src/util/virutil.c   | 78 
>>  src/util/virutil.h   |  9 +
>>  3 files changed, 89 insertions(+)
>>
>> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
>> index 68e4b6aab8..f92213b8c2 100644
>> --- a/src/libvirt_private.syms
>> +++ b/src/libvirt_private.syms
>> @@ -3479,6 +3479,8 @@ virDoesUserExist;
>>  virDoubleToStr;
>>  virFormatIntDecimal;
>>  virFormatIntPretty;
>> +virGetCpuHaltPollTime;
>> +virGetDebugFsKvmValue;
>>  virGetDeviceID;
>>  virGetDeviceUnprivSGIO;
>>  virGetGroupID;
>> diff --git a/src/util/virutil.c b/src/util/virutil.c
>> index c0d25fe247..7e4531b4b4 100644
>> --- a/src/util/virutil.c
>> +++ b/src/util/virutil.c
>> @@ -1959,3 +1959,81 @@ virGetValueRaw(const char *path,
>>  
>>  return 0;
>>  }
>> +
>> +int
>> +virGetDebugFsKvmValue(struct dirent *ent,
>> +const char *path,
>> +const char *filename,
>> +unsigned long long *value)
>> +{
>> +g_autofree char *valToStr = NULL;
>> +g_autofree char *valPath = NULL;
>> +
>> +valPath = g_strdup_printf("%s/%s/%s", path, ent->d_name, filename);
>> +
>> +if (virGetValueRaw(valPath, ) < 0) {
>> +return -1;
>> +}
>> +
>> +/* 10 is a Cardinality, must be between 2 and 36 inclusive,
>> + * or special value 0. Used in fuction strtoull()
>> + */
>> +if (virStrToLong_ull(valToStr, NULL, 10, value) < 0) {
>> +virReportError(VIR_ERR_INTERNAL_ERROR,
>> +   _("Unable to parse '%s' as an integer"), valToStr);
>> +return -1;
>> +}
>> +
>> +return 0;
>> +}
> 
> This looks very close to what virFileReadValueUllong() does.
> 

I will directly use the function virFileReadValueUllong() in the next version.

>> +
>> +int
>> +virGetCpuHaltPollTime(pid_t pid,
>> +  unsigned long long *haltPollSuccess,
>> +  unsigned long long *haltPollFail)
>> +{
>> +g_autofree char *pidToStr = NULL;
>> +g_autofree char *debugFsPath = NULL;
>> +g_autofree char *completePath = NULL;
>> +struct dirent *ent = NULL;
>> +DIR *dir = NULL;
>> +int ret = -1;
>> +int flag = 0;
>> +
>> +if (!(debugFsPath = virFileFindMountPoint("debugfs"))) {
>> +virReportSystemError(errno, "%s",
>> + _("unable to find debugfs mountpoint"));
>> +return ret;
>> +}
> 
> Is this something worth polluting logs with error? What if we run under
> kernel that doesn't have debugfs?
> 

If kernel doesn't have debugfs, halt polling time will not be presented in 
domstats,
and return what it can obtain.

Could I use VIR_WARN() or VIR_INFO() to give the fail messege?

>> +
>> +completePath = g_strdup_printf("%s/%s", debugFsPath, "kvm");
>> +if (virDirOpen(, completePath) < 0) {
>> +virReportError(VIR_ERR_INTERNAL_ERROR,
>> +   "%s %s", "Can not open directory", completePath);
>> +return ret;
>> +}
> 
> virDirOpen() reports an error on failure. No need to rewrite it. And in
> the light of previous comment maybe virDirOpenIfExists() instead?
> 

Yes, virDirOpenIfExists() is indeed more appropriate. I will fix it next 
version.

>> +
>> +pidToStr = g_strdup_printf("%lld%c", (unsigned long long)pid, '-');
>> +while (virDirRead(dir, , NULL) > 0) {
>> +if (STRPREFIX(ent->d_name, pidToStr)) {
>> +flag = 1;
>> +break;
>> +}
>> +}
>> +
>> +if (flag == 0) {
>> +virReportError(VIR_ERR_INTERNAL_ERROR,
>> +   _("Could not find VM(Pid %s) in '%s'"), pidToStr, 
>> completePath);
>> +goto cleanup;
>> +}
> 
> Again, not big fan. What it my domain runs TCG? Also, please rename
> 'flag' to something more specific, e.g. "found".
> 

If domain runs TCG, the pid-fd directory will not be generated in the path. 
Domstats just return the
data it can obtain. That's really not appropriate to report a error here.

And I'll give the variable a more appropriate name in next version.

>> +
>> +if (virGetDebugFsKvmValue(ent, completePath, "halt_poll_success_ns", 
>> haltPollSuccess) < 0 ||
>> +virGetDebugFsKvmValue(ent, completePath, "halt_poll_fail_ns", 
>> haltPollFail) < 0) {
>> +goto cleanup;
>> +}
>> +
>> +ret = 0;
>> +cleanup:
>> +closedir(dir);
> 
> Again, please make sure 'ninja test' passes. We have a syntax-check rule
> that forbids direct call of closedir(). The proper way is to define dir
> variable like this:
> 
> g_autoptr(DIR) dir = NULL;
> 

I'll pay more attention about 

Re: [PATCH v2 3/3] qemu: Introduce qemuDomainGetStatsCpuHaltPollTime

2021-07-15 Thread Yang Fei



On 2021/7/14 22:17, Michal Prívozník wrote:
> On 7/14/21 2:28 PM, Yang Fei wrote:
>> This function is used to obtain the halt polling time. The kernel
>> provides statistics about haltpollsuccess.time and
>> haltpollfail.time. We add it in domstats, so that we can use
>> command 'virsh domstats VM' to get it if system support.
>>
>> Signed-off-by: Yang Fei 
>> ---
>>  src/qemu/qemu_driver.c | 28 
>>  1 file changed, 28 insertions(+)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 235f575901..6163037ec3 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -17839,6 +17839,31 @@ qemuDomainGetStatsCpuCgroup(virDomainObj *dom,
>>  return 0;
>>  }
>>  
>> +#ifdef __linux__
>> +static int
>> +qemuDomainGetStatsCpuHaltPollTime(virDomainObj *dom,
>> + virTypedParamList *params)
>> +{
>> +unsigned long long haltPollSuccess = 0;
>> +unsigned long long haltPollFail = 0;
>> +pid_t pid = dom->pid;
>> +int err = 0;
>> +
>> +err = virGetCpuHaltPollTime(pid, , );
>> +if (!err && virTypedParamListAddULLong(params, haltPollSuccess, 
>> "haltpollsuccess.time") < 0)
>> +return -1;
>> +if (!err && virTypedParamListAddULLong(params, haltPollFail, 
>> "haltpollfail.time") < 0)
>> +return -1;
> 
> I know you copied this pattern from qemuDomainGetStatsCpuCgroup() but
> it's not as appealing is it could be. How about:
> 
> if (virGetCpuHaltPollTime() < 0)
>   return 0;
> 
> if (virTypedParamListAddULLong() < 0 ||
> virTypedParamListAddULLong() < 0)
> return -1;
> 
> return 0;
> 
> 
> Also, please document these new parameters in src/libvirt-domain.c just
> like every other parameter is (look around line 11626).
> 

OK, I'll add it in next version.

>> +return 0;
>> +}
>> +#else
>> +static int
>> +qemuDomainGetStatsCpuHaltPollTime(virDomainObj *dom,
>> + virTypedParamList *params)
>> +{
>> +return 0;
>> +}
>> +#endif
> 
> Once my suggestions from previous patches are worked in, this non-Linux
> variant won't be necessary.
> 
>>  
>>  static int
>>  qemuDomainGetStatsCpu(virQEMUDriver *driver,
>> @@ -17852,6 +17877,9 @@ qemuDomainGetStatsCpu(virQEMUDriver *driver,
>>  if (qemuDomainGetStatsCpuCache(driver, dom, params) < 0)
>>  return -1;
>>  
>> +if (qemuDomainGetStatsCpuHaltPollTime(dom, params) < 0)
>> +return -1;
>> +
>>  return 0;
>>  }
>>  
>>
> 
> Michal
> 
> .
> 




Re: [PATCH v2 1/3] util: Move virGetCgroupValueRaw to vircgroup.c and rename it virGetValueRaw

2021-07-15 Thread Yang Fei



On 2021/7/14 22:17, Michal Prívozník wrote:
> On 7/14/21 2:28 PM, Yang Fei wrote:
>> Move virGetCgroupValueRaw from vircgroup.c, so that we can call
>> it more appropriately at any where. And change it to a more
>> generic name virGetValueRaw.
>> Replace virGetCgroupValueRaw by virGetValueRaw in cgroup data obtain.
>>
>> Signed-off-by: Yang Fei 
>> ---
>>  src/util/vircgroup.c | 29 ++---
>>  src/util/vircgrouppriv.h |  3 ---
>>  src/util/vircgroupv1.c   |  5 +++--
>>  src/util/vircgroupv2.c   |  5 +++--
>>  src/util/virutil.c   | 23 +++
>>  src/util/virutil.h   |  3 +++
>>  6 files changed, 34 insertions(+), 34 deletions(-)
> 
> I don't think this is necessary. We have whole family of functions that
> read a file and translate its contents into an integer. For instance
> virFileReadValueUllong().
> 
> BTW: make sure that 'ninja test' passes after each commit.
> 
> Michal
> 
> .
> 

Yes, virFileReadValueUllong() is also meets the requirement. I will drop this 
patch in next version.

Thanks for reminding me and I will be more careful afterwards.




Re: [libvirt PATCH 0/2] Tiny misc clean-ups

2021-07-15 Thread Boris Fiuczynski

On 7/15/21 2:04 PM, Martin Kletzander wrote:

Some whitespaces were wrong and one debug message was confusing to newcomers.

Martin Kletzander (2):
   util: Make one debug message nicer
   whitespace clean-ups

  docs/schemas/capability.rng| 10 +-
  src/conf/capabilities.c|  2 +-
  src/util/virresctrl.c  |  4 ++--
  .../vircaps-x86_64-resctrl-fake-feature.xml|  4 ++--
  tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml   |  4 ++--
  5 files changed, 12 insertions(+), 12 deletions(-)



Looks good to me.
Reviewed-by: Boris Fiuczynski 

--
Mit freundlichen Grüßen/Kind regards
   Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294




Re: [PATCH v2] nodedev: fix internal error when no defined mdevs exist

2021-07-15 Thread Boris Fiuczynski

On 7/15/21 2:16 PM, Martin Kletzander wrote:

On Wed, Jul 14, 2021 at 06:40:57PM +0200, Boris Fiuczynski wrote:

Commit e9b534905f4 introduced an error when parsing an empty list
returned from mdevctl.

This occurs e.g. if nodedev-undefine is used to undefine the last
defined mdev which cuases the following error messages



causes


libvirtd[33143]: internal error: Unexpected format for mdevctl response
libvirtd[33143]: internal error: failed to query mdevs from mdevctl:
libvirtd[33143]: mdevctl failed to updated mediated devices

Signed-off-by: Boris Fiuczynski 
---
src/node_device/node_device_driver.c    | 5 +
tests/nodedevmdevctldata/mdevctl-list-empty.json    | 1 +
tests/nodedevmdevctldata/mdevctl-list-empty.out.xml | 0
tests/nodedevmdevctltest.c  | 1 +
4 files changed, 7 insertions(+)
create mode 100644 tests/nodedevmdevctldata/mdevctl-list-empty.json
create mode 100644 tests/nodedevmdevctldata/mdevctl-list-empty.out.xml

diff --git a/src/node_device/node_device_driver.c 
b/src/node_device/node_device_driver.c

index b4dd57e5f4..b16b05f9b2 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -1120,6 +1120,11 @@ nodeDeviceParseMdevctlJSON(const char *jsonstring,
    goto error;
    }

+    if (virJSONValueArraySize(json_devicelist) == 0) {
+    *devs = outdevs;
+    return 0;


It would be more clear if you just did:

   *devs = NULL;


Yeah, sure.



And I, personally, would also add a VIR_DEBUG here.


I considered the scenario that mdevctl has no mediated device 
definitions stored a very normal case and not worth a debug message as 
the rest of the code does not report debug data if mediated device 
definitions exist.




Other than that

Reviewed-by: Martin Kletzander 


Thanks for reviewing.
Do you want me to send a v3 with the changes?

--
Mit freundlichen Grüßen/Kind regards
   Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294




Re: [PATCH v2] nodedev: fix internal error when no defined mdevs exist

2021-07-15 Thread Shalini Chellathurai Saroja



On 7/14/21 6:40 PM, Boris Fiuczynski wrote:

Commit e9b534905f4 introduced an error when parsing an empty list
returned from mdevctl.

This occurs e.g. if nodedev-undefine is used to undefine the last
defined mdev which cuases the following error messages

  libvirtd[33143]: internal error: Unexpected format for mdevctl response
  libvirtd[33143]: internal error: failed to query mdevs from mdevctl:
  libvirtd[33143]: mdevctl failed to updated mediated devices

Signed-off-by: Boris Fiuczynski


Hello Boris,

I tested the patch, it works fine.

Reviewed-by: Shalini Chellathurai Saroja 

--
Kind regards
Shalini Chellathurai Saroja
Linux on Z and Virtualization Development
Vorsitzende des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294




Re: [PATCH 0/2] domcaps: Add support for 'memoryBacking' element

2021-07-15 Thread Michal Prívozník
On 7/15/21 2:18 PM, Kristina Hanicova wrote:
> virt-manager needs to know if memfd memory source type is supported in
> order to use virtiofs.
> 
> Regarding:
> https://listman.redhat.com/archives/virt-tools-list/2021-July/msg0.html
> 
> Kristina Hanicova (2):
>   conf: domcaps: Report 
>   qemu: capabilities: fill in domcaps 
> 

>  67 files changed, 508 insertions(+)
> 

Reviewed-by: Michal Privoznik 

and pushed.

Michal



Re: [libvirt PATCH 0/2] Tiny misc clean-ups

2021-07-15 Thread Andrea Bolognani
On Thu, Jul 15, 2021 at 02:04:22PM +0200, Martin Kletzander wrote:
> Some whitespaces were wrong and one debug message was confusing to newcomers.
>
> Martin Kletzander (2):
>   util: Make one debug message nicer
>   whitespace clean-ups

Reviewed-by: Andrea Bolognani 

-- 
Andrea Bolognani / Red Hat / Virtualization



[PATCH 1/2] conf: domcaps: Report

2021-07-15 Thread Kristina Hanicova
We need to report via domcapabilities if specifying shared memory
is supported without hugepages or numa config in order to find
out if domain has suitable setup to make virtiofs work.
The solution is to report source types of memory backing to
determine if memfd is a valid option.

Signed-off-by: Kristina Hanicova 
---
 docs/formatdomaincaps.html.in  | 28 
 docs/schemas/domaincaps.rng| 10 ++
 src/conf/domain_capabilities.c | 14 ++
 src/conf/domain_capabilities.h |  8 
 4 files changed, 60 insertions(+)

diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in
index 62f1940e6a..10d23f4c0b 100644
--- a/docs/formatdomaincaps.html.in
+++ b/docs/formatdomaincaps.html.in
@@ -290,6 +290,34 @@
 domainCapabilities
 
 
+Memory Backing
+
+
+  The memory backing element indicates whether or not
+  memory backing
+  is supported.
+
+
+
+domainCapabilities
+  ...
+  memoryBacking supported='yes'
+enum name='sourceType'
+  valueanonymous/value
+  valuefile/value
+  valuememfd/value
+/enum
+  /memoryBacking
+  ...
+domainCapabilities
+
+
+
+  sourceType
+  Options for the type attribute of the
+  memoryBackingsource element.
+
+
 Devices
 
 
diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng
index fc668e0c78..69d7824e7c 100644
--- a/docs/schemas/domaincaps.rng
+++ b/docs/schemas/domaincaps.rng
@@ -37,6 +37,9 @@
 
   
 
+
+  
+
 
   
 
@@ -165,6 +168,13 @@
 
   
 
+  
+
+  
+  
+
+  
+
   
 
   
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index cb90ae0176..73139d0ec6 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -457,6 +457,18 @@ virDomainCapsCPUFormat(virBuffer *buf,
 virBufferAddLit(buf, "\n");
 }
 
+static void
+virDomainCapsMemoryBackingFormat(virBuffer *buf,
+ const virDomainCapsMemoryBacking 
*memoryBacking)
+{
+FORMAT_PROLOGUE(memoryBacking);
+
+ENUM_PROCESS(memoryBacking, sourceType, virDomainMemorySourceTypeToString);
+
+FORMAT_EPILOGUE(memoryBacking);
+}
+
+
 static void
 virDomainCapsDeviceDiskFormat(virBuffer *buf,
   const virDomainCapsDeviceDisk *disk)
@@ -632,6 +644,8 @@ virDomainCapsFormat(const virDomainCaps *caps)
 virDomainCapsOSFormat(, >os);
 virDomainCapsCPUFormat(, >cpu);
 
+virDomainCapsMemoryBackingFormat(, >memoryBacking);
+
 virBufferAddLit(, "\n");
 virBufferAdjustIndent(, 2);
 
diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
index b6433b20c9..a3765832c1 100644
--- a/src/conf/domain_capabilities.h
+++ b/src/conf/domain_capabilities.h
@@ -62,6 +62,13 @@ struct _virDomainCapsOS {
 virDomainCapsLoader loader; /* Info about virDomainLoaderDef */
 };
 
+STATIC_ASSERT_ENUM(VIR_DOMAIN_MEMORY_SOURCE_LAST);
+typedef struct _virDomainCapsMemoryBacking virDomainCapsMemoryBacking;
+struct _virDomainCapsMemoryBacking {
+virTristateBool supported;
+virDomainCapsEnum sourceType; /* virDomainMemorySource */
+};
+
 STATIC_ASSERT_ENUM(VIR_DOMAIN_DISK_DEVICE_LAST);
 STATIC_ASSERT_ENUM(VIR_DOMAIN_DISK_BUS_LAST);
 STATIC_ASSERT_ENUM(VIR_DOMAIN_DISK_MODEL_LAST);
@@ -196,6 +203,7 @@ struct _virDomainCaps {
 
 virDomainCapsOS os;
 virDomainCapsCPU cpu;
+virDomainCapsMemoryBacking memoryBacking;
 virDomainCapsDeviceDisk disk;
 virDomainCapsDeviceGraphics graphics;
 virDomainCapsDeviceVideo video;
-- 
2.31.1



[PATCH 2/2] qemu: capabilities: fill in domcaps

2021-07-15 Thread Kristina Hanicova
Signed-off-by: Kristina Hanicova 
---
 src/qemu/qemu_capabilities.c  | 22 +++
 src/qemu/qemu_capabilities.h  |  3 +++
 .../domaincapsdata/qemu_2.11.0-q35.x86_64.xml |  6 +
 .../domaincapsdata/qemu_2.11.0-tcg.x86_64.xml |  6 +
 tests/domaincapsdata/qemu_2.11.0.s390x.xml|  6 +
 tests/domaincapsdata/qemu_2.11.0.x86_64.xml   |  6 +
 .../domaincapsdata/qemu_2.12.0-q35.x86_64.xml |  7 ++
 .../domaincapsdata/qemu_2.12.0-tcg.x86_64.xml |  7 ++
 .../qemu_2.12.0-virt.aarch64.xml  |  7 ++
 tests/domaincapsdata/qemu_2.12.0.aarch64.xml  |  7 ++
 tests/domaincapsdata/qemu_2.12.0.ppc64.xml|  7 ++
 tests/domaincapsdata/qemu_2.12.0.s390x.xml|  7 ++
 tests/domaincapsdata/qemu_2.12.0.x86_64.xml   |  7 ++
 .../domaincapsdata/qemu_3.0.0-q35.x86_64.xml  |  7 ++
 .../domaincapsdata/qemu_3.0.0-tcg.x86_64.xml  |  7 ++
 tests/domaincapsdata/qemu_3.0.0.ppc64.xml |  7 ++
 tests/domaincapsdata/qemu_3.0.0.s390x.xml |  7 ++
 tests/domaincapsdata/qemu_3.0.0.x86_64.xml|  7 ++
 .../domaincapsdata/qemu_3.1.0-q35.x86_64.xml  |  7 ++
 .../domaincapsdata/qemu_3.1.0-tcg.x86_64.xml  |  7 ++
 tests/domaincapsdata/qemu_3.1.0.ppc64.xml |  7 ++
 tests/domaincapsdata/qemu_3.1.0.x86_64.xml|  7 ++
 .../domaincapsdata/qemu_4.0.0-q35.x86_64.xml  |  7 ++
 .../domaincapsdata/qemu_4.0.0-tcg.x86_64.xml  |  7 ++
 .../qemu_4.0.0-virt.aarch64.xml   |  7 ++
 tests/domaincapsdata/qemu_4.0.0.aarch64.xml   |  7 ++
 tests/domaincapsdata/qemu_4.0.0.ppc64.xml |  7 ++
 tests/domaincapsdata/qemu_4.0.0.s390x.xml |  7 ++
 tests/domaincapsdata/qemu_4.0.0.x86_64.xml|  7 ++
 .../domaincapsdata/qemu_4.1.0-q35.x86_64.xml  |  7 ++
 .../domaincapsdata/qemu_4.1.0-tcg.x86_64.xml  |  7 ++
 tests/domaincapsdata/qemu_4.1.0.x86_64.xml|  7 ++
 .../domaincapsdata/qemu_4.2.0-q35.x86_64.xml  |  7 ++
 .../domaincapsdata/qemu_4.2.0-tcg.x86_64.xml  |  7 ++
 .../qemu_4.2.0-virt.aarch64.xml   |  7 ++
 tests/domaincapsdata/qemu_4.2.0.aarch64.xml   |  7 ++
 tests/domaincapsdata/qemu_4.2.0.ppc64.xml |  7 ++
 tests/domaincapsdata/qemu_4.2.0.s390x.xml |  7 ++
 tests/domaincapsdata/qemu_4.2.0.x86_64.xml|  7 ++
 .../domaincapsdata/qemu_5.0.0-q35.x86_64.xml  |  7 ++
 .../domaincapsdata/qemu_5.0.0-tcg.x86_64.xml  |  7 ++
 .../qemu_5.0.0-virt.aarch64.xml   |  7 ++
 tests/domaincapsdata/qemu_5.0.0.aarch64.xml   |  7 ++
 tests/domaincapsdata/qemu_5.0.0.ppc64.xml |  7 ++
 tests/domaincapsdata/qemu_5.0.0.x86_64.xml|  7 ++
 .../domaincapsdata/qemu_5.1.0-q35.x86_64.xml  |  7 ++
 .../domaincapsdata/qemu_5.1.0-tcg.x86_64.xml  |  7 ++
 tests/domaincapsdata/qemu_5.1.0.sparc.xml |  7 ++
 tests/domaincapsdata/qemu_5.1.0.x86_64.xml|  7 ++
 .../domaincapsdata/qemu_5.2.0-q35.x86_64.xml  |  7 ++
 .../domaincapsdata/qemu_5.2.0-tcg.x86_64.xml  |  7 ++
 .../qemu_5.2.0-virt.aarch64.xml   |  7 ++
 tests/domaincapsdata/qemu_5.2.0.aarch64.xml   |  7 ++
 tests/domaincapsdata/qemu_5.2.0.ppc64.xml |  7 ++
 tests/domaincapsdata/qemu_5.2.0.s390x.xml |  7 ++
 tests/domaincapsdata/qemu_5.2.0.x86_64.xml|  7 ++
 .../domaincapsdata/qemu_6.0.0-q35.x86_64.xml  |  7 ++
 .../domaincapsdata/qemu_6.0.0-tcg.x86_64.xml  |  7 ++
 tests/domaincapsdata/qemu_6.0.0.s390x.xml |  7 ++
 tests/domaincapsdata/qemu_6.0.0.x86_64.xml|  7 ++
 .../domaincapsdata/qemu_6.1.0-q35.x86_64.xml  |  7 ++
 .../domaincapsdata/qemu_6.1.0-tcg.x86_64.xml  |  7 ++
 tests/domaincapsdata/qemu_6.1.0.x86_64.xml|  7 ++
 63 files changed, 448 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index d1cd8f11ac..e0c3a07568 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -6005,6 +6005,26 @@ virQEMUCapsFillDomainFeaturesFromQEMUCaps(virQEMUCaps 
*qemuCaps,
 }
 
 
+void
+virQEMUCapsFillDomainMemoryBackingCaps(virQEMUCaps *qemuCaps,
+  virDomainCapsMemoryBacking *memoryBacking)
+{
+memoryBacking->supported = VIR_TRISTATE_BOOL_YES;
+memoryBacking->sourceType.report = true;
+
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_MEMFD))
+VIR_DOMAIN_CAPS_ENUM_SET(memoryBacking->sourceType,
+ VIR_DOMAIN_MEMORY_SOURCE_MEMFD);
+
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE))
+VIR_DOMAIN_CAPS_ENUM_SET(memoryBacking->sourceType,
+ VIR_DOMAIN_MEMORY_SOURCE_FILE);
+
+VIR_DOMAIN_CAPS_ENUM_SET(memoryBacking->sourceType,
+ VIR_DOMAIN_MEMORY_SOURCE_ANONYMOUS);
+}
+
+
 static void
 virQEMUCapsFillDomainDeviceDiskCaps(virQEMUCaps *qemuCaps,
 const char *machine,
@@ -6322,6 +6342,7 

[PATCH 0/2] domcaps: Add support for 'memoryBacking' element

2021-07-15 Thread Kristina Hanicova
virt-manager needs to know if memfd memory source type is supported in
order to use virtiofs.

Regarding:
https://listman.redhat.com/archives/virt-tools-list/2021-July/msg0.html

Kristina Hanicova (2):
  conf: domcaps: Report 
  qemu: capabilities: fill in domcaps 

 docs/formatdomaincaps.html.in | 28 +++
 docs/schemas/domaincaps.rng   | 10 +++
 src/conf/domain_capabilities.c| 14 ++
 src/conf/domain_capabilities.h|  8 ++
 src/qemu/qemu_capabilities.c  | 22 +++
 src/qemu/qemu_capabilities.h  |  3 ++
 .../domaincapsdata/qemu_2.11.0-q35.x86_64.xml |  6 
 .../domaincapsdata/qemu_2.11.0-tcg.x86_64.xml |  6 
 tests/domaincapsdata/qemu_2.11.0.s390x.xml|  6 
 tests/domaincapsdata/qemu_2.11.0.x86_64.xml   |  6 
 .../domaincapsdata/qemu_2.12.0-q35.x86_64.xml |  7 +
 .../domaincapsdata/qemu_2.12.0-tcg.x86_64.xml |  7 +
 .../qemu_2.12.0-virt.aarch64.xml  |  7 +
 tests/domaincapsdata/qemu_2.12.0.aarch64.xml  |  7 +
 tests/domaincapsdata/qemu_2.12.0.ppc64.xml|  7 +
 tests/domaincapsdata/qemu_2.12.0.s390x.xml|  7 +
 tests/domaincapsdata/qemu_2.12.0.x86_64.xml   |  7 +
 .../domaincapsdata/qemu_3.0.0-q35.x86_64.xml  |  7 +
 .../domaincapsdata/qemu_3.0.0-tcg.x86_64.xml  |  7 +
 tests/domaincapsdata/qemu_3.0.0.ppc64.xml |  7 +
 tests/domaincapsdata/qemu_3.0.0.s390x.xml |  7 +
 tests/domaincapsdata/qemu_3.0.0.x86_64.xml|  7 +
 .../domaincapsdata/qemu_3.1.0-q35.x86_64.xml  |  7 +
 .../domaincapsdata/qemu_3.1.0-tcg.x86_64.xml  |  7 +
 tests/domaincapsdata/qemu_3.1.0.ppc64.xml |  7 +
 tests/domaincapsdata/qemu_3.1.0.x86_64.xml|  7 +
 .../domaincapsdata/qemu_4.0.0-q35.x86_64.xml  |  7 +
 .../domaincapsdata/qemu_4.0.0-tcg.x86_64.xml  |  7 +
 .../qemu_4.0.0-virt.aarch64.xml   |  7 +
 tests/domaincapsdata/qemu_4.0.0.aarch64.xml   |  7 +
 tests/domaincapsdata/qemu_4.0.0.ppc64.xml |  7 +
 tests/domaincapsdata/qemu_4.0.0.s390x.xml |  7 +
 tests/domaincapsdata/qemu_4.0.0.x86_64.xml|  7 +
 .../domaincapsdata/qemu_4.1.0-q35.x86_64.xml  |  7 +
 .../domaincapsdata/qemu_4.1.0-tcg.x86_64.xml  |  7 +
 tests/domaincapsdata/qemu_4.1.0.x86_64.xml|  7 +
 .../domaincapsdata/qemu_4.2.0-q35.x86_64.xml  |  7 +
 .../domaincapsdata/qemu_4.2.0-tcg.x86_64.xml  |  7 +
 .../qemu_4.2.0-virt.aarch64.xml   |  7 +
 tests/domaincapsdata/qemu_4.2.0.aarch64.xml   |  7 +
 tests/domaincapsdata/qemu_4.2.0.ppc64.xml |  7 +
 tests/domaincapsdata/qemu_4.2.0.s390x.xml |  7 +
 tests/domaincapsdata/qemu_4.2.0.x86_64.xml|  7 +
 .../domaincapsdata/qemu_5.0.0-q35.x86_64.xml  |  7 +
 .../domaincapsdata/qemu_5.0.0-tcg.x86_64.xml  |  7 +
 .../qemu_5.0.0-virt.aarch64.xml   |  7 +
 tests/domaincapsdata/qemu_5.0.0.aarch64.xml   |  7 +
 tests/domaincapsdata/qemu_5.0.0.ppc64.xml |  7 +
 tests/domaincapsdata/qemu_5.0.0.x86_64.xml|  7 +
 .../domaincapsdata/qemu_5.1.0-q35.x86_64.xml  |  7 +
 .../domaincapsdata/qemu_5.1.0-tcg.x86_64.xml  |  7 +
 tests/domaincapsdata/qemu_5.1.0.sparc.xml |  7 +
 tests/domaincapsdata/qemu_5.1.0.x86_64.xml|  7 +
 .../domaincapsdata/qemu_5.2.0-q35.x86_64.xml  |  7 +
 .../domaincapsdata/qemu_5.2.0-tcg.x86_64.xml  |  7 +
 .../qemu_5.2.0-virt.aarch64.xml   |  7 +
 tests/domaincapsdata/qemu_5.2.0.aarch64.xml   |  7 +
 tests/domaincapsdata/qemu_5.2.0.ppc64.xml |  7 +
 tests/domaincapsdata/qemu_5.2.0.s390x.xml |  7 +
 tests/domaincapsdata/qemu_5.2.0.x86_64.xml|  7 +
 .../domaincapsdata/qemu_6.0.0-q35.x86_64.xml  |  7 +
 .../domaincapsdata/qemu_6.0.0-tcg.x86_64.xml  |  7 +
 tests/domaincapsdata/qemu_6.0.0.s390x.xml |  7 +
 tests/domaincapsdata/qemu_6.0.0.x86_64.xml|  7 +
 .../domaincapsdata/qemu_6.1.0-q35.x86_64.xml  |  7 +
 .../domaincapsdata/qemu_6.1.0-tcg.x86_64.xml  |  7 +
 tests/domaincapsdata/qemu_6.1.0.x86_64.xml|  7 +
 67 files changed, 508 insertions(+)

-- 
2.31.1



Re: [PATCH v2] nodedev: fix internal error when no defined mdevs exist

2021-07-15 Thread Martin Kletzander

On Wed, Jul 14, 2021 at 06:40:57PM +0200, Boris Fiuczynski wrote:

Commit e9b534905f4 introduced an error when parsing an empty list
returned from mdevctl.

This occurs e.g. if nodedev-undefine is used to undefine the last
defined mdev which cuases the following error messages



causes


libvirtd[33143]: internal error: Unexpected format for mdevctl response
libvirtd[33143]: internal error: failed to query mdevs from mdevctl:
libvirtd[33143]: mdevctl failed to updated mediated devices

Signed-off-by: Boris Fiuczynski 
---
src/node_device/node_device_driver.c| 5 +
tests/nodedevmdevctldata/mdevctl-list-empty.json| 1 +
tests/nodedevmdevctldata/mdevctl-list-empty.out.xml | 0
tests/nodedevmdevctltest.c  | 1 +
4 files changed, 7 insertions(+)
create mode 100644 tests/nodedevmdevctldata/mdevctl-list-empty.json
create mode 100644 tests/nodedevmdevctldata/mdevctl-list-empty.out.xml

diff --git a/src/node_device/node_device_driver.c 
b/src/node_device/node_device_driver.c
index b4dd57e5f4..b16b05f9b2 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -1120,6 +1120,11 @@ nodeDeviceParseMdevctlJSON(const char *jsonstring,
goto error;
}

+if (virJSONValueArraySize(json_devicelist) == 0) {
+*devs = outdevs;
+return 0;


It would be more clear if you just did:

  *devs = NULL;

And I, personally, would also add a VIR_DEBUG here.

Other than that

Reviewed-by: Martin Kletzander 


signature.asc
Description: PGP signature


[libvirt PATCH 1/2] util: Make one debug message nicer

2021-07-15 Thread Martin Kletzander
This was bothering someone as the debug message looked like there was an issue
despite it being just a debug message.  Change it to what is actually happening
and why the name is being skipped.

Signed-off-by: Martin Kletzander 
---
 src/util/virresctrl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
index a7d36f492cd2..2a3bcda6f59b 100644
--- a/src/util/virresctrl.c
+++ b/src/util/virresctrl.c
@@ -542,7 +542,7 @@ virResctrlGetCacheInfo(virResctrlInfo *resctrl,
 
 type = virResctrlTypeFromString(endptr);
 if (type < 0) {
-VIR_DEBUG("Cannot parse resctrl cache info type '%s'", endptr);
+VIR_DEBUG("Ignoring resctrl cache info with suffix '%s'", endptr);
 continue;
 }
 
-- 
2.32.0



[libvirt PATCH 0/2] Tiny misc clean-ups

2021-07-15 Thread Martin Kletzander
Some whitespaces were wrong and one debug message was confusing to newcomers.

Martin Kletzander (2):
  util: Make one debug message nicer
  whitespace clean-ups

 docs/schemas/capability.rng| 10 +-
 src/conf/capabilities.c|  2 +-
 src/util/virresctrl.c  |  4 ++--
 .../vircaps-x86_64-resctrl-fake-feature.xml|  4 ++--
 tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml   |  4 ++--
 5 files changed, 12 insertions(+), 12 deletions(-)

-- 
2.32.0





[libvirt PATCH 2/2] whitespace clean-ups

2021-07-15 Thread Martin Kletzander
Signed-off-by: Martin Kletzander 
---
 docs/schemas/capability.rng| 10 +-
 src/conf/capabilities.c|  2 +-
 src/util/virresctrl.c  |  2 +-
 .../vircaps-x86_64-resctrl-fake-feature.xml|  4 ++--
 tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml   |  4 ++--
 5 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng
index 66dba829a826..7747a0155854 100644
--- a/docs/schemas/capability.rng
+++ b/docs/schemas/capability.rng
@@ -283,11 +283,11 @@
   
 
   
-
-  
-
-  
-
+  
+
+  
+
+  
   
 
   
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index f311377469df..15c76d791a4b 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -1056,7 +1056,7 @@ virCapabilitiesFormatMemoryBandwidth(virBuffer *buf,
   node->id, cpus_str);
 
 virBufferAsprintf(,
-  "\n",
   control->granularity, control->min,
   control->max_allocation);
diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
index 2a3bcda6f59b..1c41d1d35603 100644
--- a/src/util/virresctrl.c
+++ b/src/util/virresctrl.c
@@ -662,7 +662,7 @@ virResctrlGetMemoryBandwidthInfo(virResctrlInfo *resctrl)
 rv = virFileReadValueUint(_membw->max_allocation,
   SYSFS_RESCTRL_PATH "/info/MB/num_closids");
 if (rv == -2) {
- /* Similar reasoning to min_bandwidth above. */
+/* Similar reasoning to min_bandwidth above. */
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot get max allocation from resctrl memory 
info"));
 }
diff --git a/tests/vircaps2xmldata/vircaps-x86_64-resctrl-fake-feature.xml 
b/tests/vircaps2xmldata/vircaps-x86_64-resctrl-fake-feature.xml
index 5f3678e0726b..1327d85c9815 100644
--- a/tests/vircaps2xmldata/vircaps-x86_64-resctrl-fake-feature.xml
+++ b/tests/vircaps2xmldata/vircaps-x86_64-resctrl-fake-feature.xml
@@ -56,10 +56,10 @@
 
 
   
-
+
   
   
-
+
   
   
 
diff --git a/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml 
b/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
index c386edd4b0ca..b638bbd1c96d 100644
--- a/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
+++ b/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
@@ -54,10 +54,10 @@
 
 
   
-
+
   
   
-
+
   
   
 
-- 
2.32.0



Re: [PATCH] qemu: Fix error returned value in qemuGetProcessInfo when fscanf execute failed

2021-07-15 Thread Michal Prívozník
On 7/15/21 8:18 AM, Yi Wang wrote:
> From: Long YunJian 
> 
> If fscanf execute failed, qemuGetProcessInfo shuld return -1,
> but it return 0 at the end. Zero means success for the caller,
> so we shuld return -1 in the case of failure.
> 
> Signed-off-by: Long YunJian 
> Signed-off-by: Yi Wang 
> ---
>  src/qemu/qemu_driver.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index df44c3fbd0..4c3785fa36 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -1442,11 +1442,13 @@ qemuGetProcessInfo(unsigned long long *cpuTime, int 
> *lastCpu, long *vm_rss,
>  return -1;
>  
>  pidinfo = fopen(proc, "r");
> +if (!pidinfo) {
> +return -1;
> +}
>  
>  /* See 'man proc' for information about what all these fields are. We're
>   * only interested in a very few of them */
> -if (!pidinfo ||
> -fscanf(pidinfo,
> +if (fscanf(pidinfo,
> /* pid -> stime */
> "%*d (%*[^)]) %*c %*d %*d %*d %*d %*d %*u %*u %*u %*u %*u 
> %llu %llu"
> 
> /* cutime -> endcode */
> @@ -1455,6 +1457,8 @@ qemuGetProcessInfo(unsigned long long *cpuTime, int 
> *lastCpu, long *vm_rss,
> "%*u %*u %*u %*u %*u %*u %*u %*u %*u %*u %*d %d",
> , , , ) != 4) {
>  VIR_WARN("cannot parse process status data");
> +VIR_FORCE_FCLOSE(pidinfo);
> +return -1;

There's another VIR_FORCE_FCLOSE() at the end of the function. It should
be removed.

But is there an actual error you're seeing? Has /proc/NNN/stat format
changed? Also, please use git send-email if possible - it allows
reviewers to use git am (which doesn't work with multipart e-mails).

Michal



Re: [PATCH] ci: refresh cirrus variables for FreeBSD python rename

2021-07-15 Thread Andrea Bolognani
On Wed, Jul 14, 2021 at 05:30:26PM +0100, Daniel P. Berrangé wrote:
> All the python packages got renamed from py37- to py38-
>
> Signed-off-by: Daniel P. Berrangé 
> ---
>  ci/cirrus/freebsd-12.vars  | 4 ++--
>  ci/cirrus/freebsd-13.vars  | 4 ++--
>  ci/cirrus/freebsd-current.vars | 4 ++--
>  ci/cirrus/macos-11.vars| 2 +-
>  4 files changed, 7 insertions(+), 7 deletions(-)

Reviewed-by: Andrea Bolognani 

-- 
Andrea Bolognani / Red Hat / Virtualization




Re: [PATCH v2 0/3] Do not erase duplicate devices from namespace

2021-07-15 Thread Michal Prívozník
On 7/14/21 4:46 PM, Kristina Hanicova wrote:
> This is v2 of:
> https://listman.redhat.com/archives/libvir-list/2021-July/msg00129.html
> 
> Changes since v1 (suggested by Michal and Jano):
> * change of int variable to one-word bool
> * isolate changes outside of qemu to separate patch
> * rewritten assignment of return value
> * slight change of documentation
> * extension of use to all devices, not just input
> * @created variable is set in one core function instead of setting it in
>   each device function separately
> 
> Kristina Hanicova (3):
>   qemu: Check for existing file in namespace
>   virprocess: Return retval of the child on success, not 0
>   qemu: Do not erase duplicate devices from namespace if error occurs
> 
>  src/qemu/qemu_domain.c|  4 +--
>  src/qemu/qemu_hotplug.c   | 27 +--
>  src/qemu/qemu_namespace.c | 71 +--
>  src/qemu/qemu_namespace.h | 18 ++
>  src/qemu/qemu_process.c   |  2 +-
>  src/util/virprocess.c | 12 ---
>  6 files changed, 75 insertions(+), 59 deletions(-)
> 

Reviewed-by: Michal Privoznik 

and pushed.

Michal



Re: [PATCH] ci: refresh cirrus variables for FreeBSD python rename

2021-07-15 Thread Michal Prívozník
On 7/14/21 6:30 PM, Daniel P. Berrangé wrote:
> All the python packages got renamed from py37- to py38-
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  ci/cirrus/freebsd-12.vars  | 4 ++--
>  ci/cirrus/freebsd-13.vars  | 4 ++--
>  ci/cirrus/freebsd-current.vars | 4 ++--
>  ci/cirrus/macos-11.vars| 2 +-
>  4 files changed, 7 insertions(+), 7 deletions(-)

Reviewed-by: Michal Privoznik 

Michal



Re: [PATCH v2 3/3] qemu: Do not erase duplicate devices from namespace if error occurs

2021-07-15 Thread Michal Prívozník
On 7/14/21 4:46 PM, Kristina Hanicova wrote:
> If the attempt to attach a device failed, we erased the
> unattached device from the namespace. This resulted in erasing an
> already attached device in case of a duplicate. We need to check
> for existing file in the namespace in order to determine erasing
> it in case of a failure.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1780508
> 
> Signed-off-by: Kristina Hanicova 
> ---
>  src/qemu/qemu_domain.c|  4 +--
>  src/qemu/qemu_hotplug.c   | 27 +++
>  src/qemu/qemu_namespace.c | 55 +++
>  src/qemu/qemu_namespace.h | 18 -
>  src/qemu/qemu_process.c   |  2 +-
>  5 files changed, 55 insertions(+), 51 deletions(-)
> 

> diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c
> index eb048a2faa..46ee95b8c8 100644
> --- a/src/qemu/qemu_namespace.c
> +++ b/src/qemu/qemu_namespace.c

> @@ -1235,7 +1236,8 @@ qemuNamespacePrepareOneItem(qemuNamespaceMknodData 
> *data,
>  
>  static int
>  qemuNamespaceMknodPaths(virDomainObj *vm,
> -GSList *paths)
> +GSList *paths,
> +bool *created)
>  {
>  qemuDomainObjPrivate *priv = vm->privateData;
>  virQEMUDriver *driver = priv->driver;
> @@ -1280,15 +1282,13 @@ qemuNamespaceMknodPaths(virDomainObj *vm,
>  if (qemuSecurityPreFork(driver->securityManager) < 0)
>  goto cleanup;
>  
> -if (virProcessRunInMountNamespace(vm->pid,
> -  qemuNamespaceMknodHelper,
> -  ) < 0) {
> -qemuSecurityPostFork(driver->securityManager);
> -goto cleanup;
> -}
> +ret = virProcessRunInMountNamespace(vm->pid, qemuNamespaceMknodHelper,
> +);
> +if (ret == 0 && created != NULL)
> +*created = true;
> +
>  qemuSecurityPostFork(driver->securityManager);
>  

Here it's better if qemuSecurityPostFork() is called before the if().
The reason we have PreFork() and PostFork() calls is to fight
async-signal unsafe functions; Anyway - PreFork() locks
driver->securityManager() and only PostFork() unlocks it. And it's just
virProcessRunInMountNamespace() that's in critical section, not if().
Micro optimization, I know.

Michal



Re: [PATCH v2 1/3] qemu: Check for existing file in namespace

2021-07-15 Thread Michal Prívozník
On 7/14/21 4:46 PM, Kristina Hanicova wrote:
> Signed-off-by: Kristina Hanicova 
> ---
>  src/qemu/qemu_namespace.c | 16 +---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c
> index 98495e8ef8..eb048a2faa 100644
> --- a/src/qemu/qemu_namespace.c
> +++ b/src/qemu/qemu_namespace.c
> @@ -929,6 +929,10 @@ qemuNamespaceMknodOne(qemuNamespaceMknodItem *data)
>  bool isDev = S_ISCHR(data->sb.st_mode) || S_ISBLK(data->sb.st_mode);
>  bool isReg = S_ISREG(data->sb.st_mode) || S_ISFIFO(data->sb.st_mode) || 
> S_ISSOCK(data->sb.st_mode);
>  bool isDir = S_ISDIR(data->sb.st_mode);
> +bool exists = false;
> +
> +if (virFileExists(data->file))
> +exists = true;
>  
>  if (virFileMakeParentPath(data->file) < 0) {
>  virReportSystemError(errno,
> @@ -1039,7 +1043,7 @@ qemuNamespaceMknodOne(qemuNamespaceMknodItem *data)
>  virFileMoveMount(data->target, data->file) < 0)
>  goto cleanup;
>  
> -ret = 0;
> +ret = exists ? 1 : 0;

This ternary operator feels redundant. Plain 'ret = exists' does the
same thing.

>   cleanup:
>  if (ret < 0 && delDevice) {
>  if (isDir)
> @@ -1069,15 +1073,21 @@ qemuNamespaceMknodHelper(pid_t pid G_GNUC_UNUSED,
>  qemuNamespaceMknodData *data = opaque;
>  size_t i;
>  int ret = -1;
> +bool exists = false;
>  
>  qemuSecurityPostFork(data->driver->securityManager);
>  
>  for (i = 0; i < data->nitems; i++) {
> -if (qemuNamespaceMknodOne(>items[i]) < 0)
> +int rc = 0;
> +
> +if ((rc = qemuNamespaceMknodOne(>items[i])) < 0)
>  goto cleanup;
> +
> +if (rc > 0)
> +exists = true;
>  }
>  
> -ret = 0;
> +ret = exists ? 1 : 0;

Same here.

>   cleanup:
>  qemuNamespaceMknodDataClear(data);
>  return ret;
> 

Michal



[libvirt][PATCH v5 1/6] conf: Introduce SGX related element into domain xml

2021-07-15 Thread Haibin Huang
From: Lin Yang 

  
    1024
  
---
 docs/schemas/domaincommon.rng |  62 +---
 src/conf/domain_conf.c| 128 ++
 src/conf/domain_conf.h|  10 +++
 src/conf/virconftypes.h   |   3 +
 4 files changed, 149 insertions(+), 54 deletions(-)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 0d0dcbc5ce..24fa8b030c 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -460,35 +460,45 @@
 
   
 
-  
-sev
-  
-  
-
-  
-
-
-  
-
-
-  
-
-
-  
-
+  
+
+  
+sev
+  
+  
+
   
-
-
-  
-
+  
+
   
-
-
-  
-
+  
+
   
-
-  
+  
+
+  
+
+  
+  
+
+  
+
+  
+  
+
+  
+
+  
+
+
+  
+sgx
+  
+  
+
+  
+
+  
 
   
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ef67efa1da..22ee02a540 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1336,6 +1336,7 @@ VIR_ENUM_IMPL(virDomainLaunchSecurity,
   VIR_DOMAIN_LAUNCH_SECURITY_LAST,
   "",
   "sev",
+  "sgx",
 );
 
 static virClassPtr virDomainObjClass;
@@ -3409,6 +3410,16 @@ virDomainSEVDefFree(virDomainSEVDefPtr def)
 }
 
 
+static void
+virDomainSGXDefFree(virDomainSGXDefPtr def)
+{
+if (!def)
+return;
+
+VIR_FREE(def);
+}
+
+
 void virDomainDefFree(virDomainDefPtr def)
 {
 size_t i;
@@ -3597,6 +3608,7 @@ void virDomainDefFree(virDomainDefPtr def)
 (def->ns.free)(def->namespaceData);
 
 virDomainSEVDefFree(def->sev);
+virDomainSGXDefFree(def->sgx);
 
 xmlFreeNode(def->metadata);
 
@@ -16700,39 +16712,17 @@ virDomainMemoryTargetDefParseXML(xmlNodePtr node,
 return 0;
 }
 
-
 static virDomainSEVDefPtr
-virDomainSEVDefParseXML(xmlNodePtr sevNode,
-xmlXPathContextPtr ctxt)
+virDomainSEVDefParseXML(xmlXPathContextPtr ctxt)
 {
 VIR_XPATH_NODE_AUTORESTORE(ctxt);
 virDomainSEVDefPtr def;
 unsigned long policy;
-g_autofree char *type = NULL;
 
 if (VIR_ALLOC(def) < 0)
 return NULL;
 
-ctxt->node = sevNode;
-
-if (!(type = virXMLPropString(sevNode, "type"))) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
-   _("missing launch security type"));
-goto error;
-}
-
-def->sectype = virDomainLaunchSecurityTypeFromString(type);
-switch ((virDomainLaunchSecurity) def->sectype) {
-case VIR_DOMAIN_LAUNCH_SECURITY_SEV:
-break;
-case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
-case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
-default:
-virReportError(VIR_ERR_XML_ERROR,
-   _("unsupported launch security type '%s'"),
-   type);
-goto error;
-}
+def->sectype = VIR_DOMAIN_LAUNCH_SECURITY_SEV;
 
 if (virXPathUInt("string(./cbitpos)", ctxt, >cbitpos) < 0) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -16764,6 +16754,66 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode,
 return NULL;
 }
 
+static virDomainSGXDefPtr
+virDomainSGXDefParseXML(xmlXPathContextPtr ctxt)
+{
+virDomainSGXDefPtr def;
+
+if (VIR_ALLOC(def) < 0)
+return NULL;
+
+def->sectype = VIR_DOMAIN_LAUNCH_SECURITY_SGX;
+
+if (virDomainParseMemory("./epc_size", "./epc_size/@unit", ctxt,
+ >epc_size, false, false) < 0)
+goto error;
+
+return def;
+
+ error:
+virDomainSGXDefFree(def);
+return NULL;
+}
+
+static int
+virDomainLaunchSecurityDefParseXML(xmlNodePtr launchSecurityNode,
+   xmlXPathContextPtr ctxt,
+   virDomainDefPtr def)
+{
+VIR_XPATH_NODE_AUTORESTORE(ctxt);
+g_autofree char *type = NULL;
+
+ctxt->node = launchSecurityNode;
+
+if (!(type = virXMLPropString(launchSecurityNode, "type"))) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("missing launch security type"));
+return -1;
+}
+
+switch ((virDomainLaunchSecurity) 
virDomainLaunchSecurityTypeFromString(type)) {
+case VIR_DOMAIN_LAUNCH_SECURITY_SEV:
+def->sev = virDomainSEVDefParseXML(ctxt);
+if (def->sev == NULL)
+return -1;
+break;
+case VIR_DOMAIN_LAUNCH_SECURITY_SGX:
+def->sgx = virDomainSGXDefParseXML(ctxt);
+if (def->sgx == NULL)
+return -1;
+break;
+case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
+case 

[libvirt][PATCH v5 3/6] Support to query SGX capability

2021-07-15 Thread Haibin Huang
1.Add SGX feature in domain capabilities
2.Get sgx capabilities by query-sgx-capabilities
3.Transfer the B to KB for epc_size
4.Delete sgx1 and sgx2

Signed-off-by: Haibin Huang 
---
 docs/schemas/domaincaps.rng|  20 +
 src/conf/domain_capabilities.c |  29 +++
 src/conf/domain_capabilities.h |  13 +++
 src/libvirt_private.syms   |   1 +
 src/qemu/qemu_capabilities.c   | 139 +
 src/qemu/qemu_capabilities.h   |   6 ++
 src/qemu/qemu_monitor.c|  10 +++
 src/qemu/qemu_monitor.h|   3 +
 src/qemu/qemu_monitor_json.c   |  87 +
 src/qemu/qemu_monitor_json.h   |   3 +
 10 files changed, 311 insertions(+)

diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng
index 325581476d..0dc97b29bc 100644
--- a/docs/schemas/domaincaps.rng
+++ b/docs/schemas/domaincaps.rng
@@ -219,6 +219,9 @@
   
 
   
+  
+
+  
 
   
 
@@ -267,6 +270,23 @@
 
   
 
+  
+
+  
+  
+
+  
+yes
+no
+  
+
+
+  
+
+  
+
+  
+
   
 
   
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index d61108e125..8024200951 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -91,6 +91,16 @@ virSEVCapabilitiesFree(virSEVCapability *cap)
 }
 
 
+void
+virSGXCapabilitiesFree(virSGXCapability *cap)
+{
+if (!cap)
+return;
+
+VIR_FREE(cap);
+}
+
+
 static void
 virDomainCapsDispose(void *obj)
 {
@@ -101,6 +111,7 @@ virDomainCapsDispose(void *obj)
 virObjectUnref(caps->cpu.custom);
 virCPUDefFree(caps->cpu.hostModel);
 virSEVCapabilitiesFree(caps->sev);
+virSGXCapabilitiesFree(caps->sgx);
 
 virDomainCapsStringValuesFree(>os.loader.values);
 }
@@ -564,6 +575,23 @@ virDomainCapsFeatureSEVFormat(virBufferPtr buf,
 return;
 }
 
+static void
+virDomainCapsFeatureSGXFormat(virBufferPtr buf,
+  virSGXCapabilityPtr const sgx)
+{
+if (!sgx) {
+virBufferAddLit(buf, "\n");
+} else {
+virBufferAddLit(buf, "\n");
+virBufferAdjustIndent(buf, 2);
+virBufferAsprintf(buf, "%s\n", sgx->flc ? "yes" : "no");
+virBufferAsprintf(buf, "%llu\n", 
sgx->epc_size);
+virBufferAdjustIndent(buf, -2);
+virBufferAddLit(buf, "\n");
+}
+
+return;
+}
 
 static void
 virDomainCapsFormatFeatures(const virDomainCaps *caps,
@@ -584,6 +612,7 @@ virDomainCapsFormatFeatures(const virDomainCaps *caps,
 }
 
 virDomainCapsFeatureSEVFormat(, caps->sev);
+virDomainCapsFeatureSGXFormat(, caps->sgx);
 
 virXMLFormatElement(buf, "features", NULL, );
 }
diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
index 685d5e2a44..0b2447e81f 100644
--- a/src/conf/domain_capabilities.h
+++ b/src/conf/domain_capabilities.h
@@ -150,6 +150,13 @@ struct _virDomainCapsCPU {
 virDomainCapsCPUModelsPtr custom;
 };
 
+typedef struct _virSGXCapability virSGXCapability;
+typedef virSGXCapability *virSGXCapabilityPtr;
+struct _virSGXCapability {
+bool flc;
+unsigned long long epc_size;
+};
+
 typedef struct _virSEVCapability virSEVCapability;
 typedef virSEVCapability *virSEVCapabilityPtr;
 struct _virSEVCapability {
@@ -191,6 +198,7 @@ struct _virDomainCaps {
 
 virDomainCapsFeatureGIC gic;
 virSEVCapabilityPtr sev;
+virSGXCapabilityPtr sgx;
 /* add new domain features here */
 
 virTristateBool features[VIR_DOMAIN_CAPS_FEATURE_LAST];
@@ -239,4 +247,9 @@ int virDomainCapsDeviceDefValidate(const virDomainCaps 
*caps,
 void
 virSEVCapabilitiesFree(virSEVCapability *capabilities);
 
+void
+virSGXCapabilitiesFree(virSGXCapability *capabilities);
+
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(virSEVCapability, virSEVCapabilitiesFree);
+
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virSGXCapability, virSGXCapabilitiesFree);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 01c2e710cd..7b464f9592 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -215,6 +215,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 ff6ba8c9e9..e2103c7975 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -597,6 +597,9 @@ VIR_ENUM_IMPL(virQEMUCaps,
   "spapr-tpm-proxy",
   "numa.hmat",
   "blockdev-hostdev-scsi",
+
+  /* 380 */
+  "sgx-epc",
 );
 
 
@@ -698,11 +701,14 @@ struct _virQEMUCaps {
 
 virSEVCapability *sevCapabilities;
 
+virSGXCapability *sgxCapabilities;
+
 /* Capabilities which may differ depending on the accelerator. */
 virQEMUCapsAccel kvm;
 virQEMUCapsAccel tcg;
 };
 
+
 struct virQEMUCapsSearchData {
 virArch arch;
 const char 

[libvirt][PATCH v5 5/6] Add create guest unit test

2021-07-15 Thread Haibin Huang
Signed-off-by: Haibin Huang 
---
 .../launch-security-sgx.xml   | 20 ++
 tests/genericxml2xmltest.c|  1 +
 .../launch-security-sgx.x86_64-5.1.0.args | 40 +++
 .../qemuxml2argvdata/launch-security-sgx.xml  | 34 
 tests/qemuxml2argvtest.c  |  1 +
 5 files changed, 96 insertions(+)
 create mode 100644 tests/genericxml2xmlindata/launch-security-sgx.xml
 create mode 100644 tests/qemuxml2argvdata/launch-security-sgx.x86_64-5.1.0.args
 create mode 100644 tests/qemuxml2argvdata/launch-security-sgx.xml

diff --git a/tests/genericxml2xmlindata/launch-security-sgx.xml 
b/tests/genericxml2xmlindata/launch-security-sgx.xml
new file mode 100644
index 00..f5fdfaf134
--- /dev/null
+++ b/tests/genericxml2xmlindata/launch-security-sgx.xml
@@ -0,0 +1,20 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219100
+  219100
+  1
+  
+hvm
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+  
+  
+1024
+  
+
diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c
index 102abfdec2..ba0740807c 100644
--- a/tests/genericxml2xmltest.c
+++ b/tests/genericxml2xmltest.c
@@ -231,6 +231,7 @@ mymain(void)
 DO_TEST("tseg");
 
 DO_TEST("launch-security-sev");
+DO_TEST("launch-security-sgx");
 
 DO_TEST_DIFFERENT("cputune");
 
diff --git a/tests/qemuxml2argvdata/launch-security-sgx.x86_64-5.1.0.args 
b/tests/qemuxml2argvdata/launch-security-sgx.x86_64-5.1.0.args
new file mode 100644
index 00..feae83fa8e
--- /dev/null
+++ b/tests/qemuxml2argvdata/launch-security-sgx.x86_64-5.1.0.args
@@ -0,0 +1,40 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-QEMUGuest1 \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-x86_64 \
+-name guest=QEMUGuest1,debug-threads=on \
+-S \
+-object secret,id=masterKey0,format=raw,\
+file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
+-machine pc-1.0,accel=kvm,usb=off,dump-guest-core=off \
+-cpu host,migratable=on \
+-m 214 \
+-overcommit mem-lock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,fd=1729,server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-boot strict=on \
+-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
+-blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1",\
+"node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \
+-blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw",\
+"file":"libvirt-1-storage"}' \
+-device ide-hd,bus=ide.0,unit=0,drive=libvirt-1-format,id=ide0-0-0,bootindex=1 
\
+-object memory-backend-epc,id=mem1,size=1024K,prealloc \
+-sgx-epc id=epc1,memdev=mem1 \
+-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
+resourcecontrol=deny \
+-msg timestamp=on
diff --git a/tests/qemuxml2argvdata/launch-security-sgx.xml 
b/tests/qemuxml2argvdata/launch-security-sgx.xml
new file mode 100644
index 00..89b7f1d942
--- /dev/null
+++ b/tests/qemuxml2argvdata/launch-security-sgx.xml
@@ -0,0 +1,34 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219100
+  219100
+  1
+  
+  
+hvm
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu-system-x86_64
+
+  
+  
+  
+  
+
+
+
+
+
+
+
+  
+  
+1024
+  
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 01839cb88c..52cc9a3897 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -3291,6 +3291,7 @@ mymain(void)
 DO_TEST_CAPS_ARCH_LATEST("vhost-vsock-ccw-auto", "s390x");
 
 DO_TEST_CAPS_VER("launch-security-sev", "2.12.0");
+DO_TEST_CAPS_VER("launch-security-sgx", "5.1.0");
 
 DO_TEST_CAPS_LATEST("vhost-user-fs-fd-memory");
 DO_TEST_CAPS_LATEST("vhost-user-fs-hugepages");
-- 
2.17.1



[libvirt][PATCH v5 4/6] Add guest use sgx document

2021-07-15 Thread Haibin Huang
Signed-off-by: Haibin Huang 
---
 docs/formatdomain.rst | 28 +++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 218f0c1718..d7319133ac 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -7377,7 +7377,7 @@ Note: DEA/TDEA is synonymous with DES/TDES.
 
 Launch Security
 ---
-
+The Security includes sev and sgx.
 The contents of the  element is used to provide
 the guest owners input used for creating an encrypted VM using the AMD SEV
 feature (Secure Encrypted Virtualization). SEV is an extension to the AMD-V
@@ -7448,6 +7448,32 @@ spec 
`__
session blob defined in the SEV API spec. See SEV spec LAUNCH_START section
for the session blob format.
 
+The contents of the  element is used to provide
+the guest owners input used for creating an encrypted VM using the INTEL SGX
+feature (Software Guard Extensions). Intel SGX is a technology that was 
developed
+to meet the needs of the Trusted Computing industry. It allows user-land code
+to create private memory regions, called enclaves, that are isolated from other
+process running at the same or higher privilege levels. The code running inside
+an enclave is effectively isolated from other applications, the operating 
system,
+the hyper-visor, et cetera. For more information see the `SGX
+developer Guide 
`__
+
+::
+
+   
+ ...
+ 
+   1024
+ 
+ ...
+   
+
+``epc_size``
+ The required ``epc_size`` element are limited developers should endeavor to
+ keep their applications small.enclave size. The value of ``epc_size`` is
+ hypervisor dependent and can be obtained through the ``sgx`` element from
+ the domain capabilities.
+
 :anchor:``
 
 Example configs
-- 
2.17.1



[libvirt][PATCH v5 0/6] Support query and use SGX

2021-07-15 Thread Haibin Huang
This patch series provides support for enabling Intel's Software Guard 
Extensions (SGX) feature in guest VM.
 
Giving the SGX support in QEMU is still pending for reviewing, this 
patch series is not submmited for code review, but only describe the 
SGX enabling solution design that contains changes to 
virConnectGetDomainCapabilities API response and domain definition. All 
comments/suggestions would be highly appreciated.
  
Intel Software Guard Extensions (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.
  
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 should contain
  
 
   N
 

Haibin Huang (4):
  Support to query SGX capability
  Add guest use sgx document
  Add create guest unit test
  Add get qemu and domain capabilities unit test

Lin Yang (2):
  conf: Introduce SGX related element into domain xml
  qemu: Add command-line to generate SGX EPC memory backend

 docs/formatdomain.rst |28 +-
 docs/schemas/domaincaps.rng   |20 +
 docs/schemas/domaincommon.rng |62 +-
 src/conf/domain_capabilities.c|29 +
 src/conf/domain_capabilities.h|13 +
 src/conf/domain_conf.c|   128 +-
 src/conf/domain_conf.h|10 +
 src/conf/virconftypes.h   | 3 +
 src/libvirt_private.syms  | 1 +
 src/qemu/qemu_capabilities.c  |   139 +
 src/qemu/qemu_capabilities.h  | 6 +
 src/qemu/qemu_command.c   |23 +
 src/qemu/qemu_monitor.c   |10 +
 src/qemu/qemu_monitor.h   | 3 +
 src/qemu/qemu_monitor_json.c  |87 +
 src/qemu/qemu_monitor_json.h  | 3 +
 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_1.5.3-q35.x86_64.xml  | 1 +
 .../domaincapsdata/qemu_1.5.3-tcg.x86_64.xml  | 1 +
 tests/domaincapsdata/qemu_1.5.3.x86_64.xml| 1 +
 .../domaincapsdata/qemu_1.6.0-q35.x86_64.xml  | 1 +
 .../domaincapsdata/qemu_1.6.0-tcg.x86_64.xml  | 1 +
 tests/domaincapsdata/qemu_1.6.0.x86_64.xml| 1 +
 .../domaincapsdata/qemu_1.7.0-q35.x86_64.xml  | 1 +
 .../domaincapsdata/qemu_1.7.0-tcg.x86_64.xml  | 1 +
 tests/domaincapsdata/qemu_1.7.0.x86_64.xml| 1 +
 .../domaincapsdata/qemu_2.1.1-q35.x86_64.xml  | 1 +
 .../domaincapsdata/qemu_2.1.1-tcg.x86_64.xml  | 1 +
 tests/domaincapsdata/qemu_2.1.1.x86_64.xml| 1 +
 .../domaincapsdata/qemu_2.10.0-q35.x86_64.xml | 1 +
 .../domaincapsdata/qemu_2.10.0-tcg.x86_64.xml | 1 +
 .../qemu_2.10.0-virt.aarch64.xml  | 1 +
 tests/domaincapsdata/qemu_2.10.0.aarch64.xml  | 1 +
 tests/domaincapsdata/qemu_2.10.0.ppc64.xml| 1 +
 tests/domaincapsdata/qemu_2.10.0.s390x.xml| 1 +
 tests/domaincapsdata/qemu_2.10.0.x86_64.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

[libvirt][PATCH v5 2/6] qemu: Add command-line to generate SGX EPC memory backend

2021-07-15 Thread Haibin Huang
From: Lin Yang 

According to the result parsing from xml, add the argument of
SGX EPC memory backend into QEMU command line:

-object memory-backend-epc,id=mem1,size=K,prealloc \
-sgx-epc id=epc1,memdev=mem1
---
 src/qemu/qemu_command.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 01812cd39b..3993f95988 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9424,6 +9424,27 @@ qemuBuildSEVCommandLine(virDomainObjPtr vm, 
virCommandPtr cmd,
 return 0;
 }
 
+
+static int
+qemuBuildSGXCommandLine(virCommandPtr cmd, virDomainSGXDefPtr sgx)
+{
+if (!sgx)
+return 0;
+
+VIR_DEBUG("sgx->epc_size=%lluKiB", sgx->epc_size);
+
+virCommandAddArg(cmd, "-object");
+virCommandAddArgFormat(cmd,
+   "memory-backend-epc,id=mem1,size=%lluK,prealloc",
+   sgx->epc_size);
+
+virCommandAddArg(cmd, "-sgx-epc");
+virCommandAddArg(cmd, "id=epc1,memdev=mem1");
+
+return 0;
+}
+
+
 static int
 qemuBuildVMCoreInfoCommandLine(virCommandPtr cmd,
const virDomainDef *def)
@@ -10114,6 +10135,8 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
 
 if (qemuBuildSEVCommandLine(vm, cmd, def->sev) < 0)
 return NULL;
+if (qemuBuildSGXCommandLine(cmd, def->sgx) < 0)
+return NULL;
 
 if (snapshot)
 virCommandAddArgList(cmd, "-loadvm", snapshot->def->name, NULL);
-- 
2.17.1



[PATCH] qemu: Fix error returned value in qemuGetProcessInfo when fscanf execute failed

2021-07-15 Thread Yi Wang
From: Long YunJian 

If fscanf execute failed, qemuGetProcessInfo shuld return -1,
but it return 0 at the end. Zero means success for the caller,
so we shuld return -1 in the case of failure.

Signed-off-by: Long YunJian 
Signed-off-by: Yi Wang 
---
 src/qemu/qemu_driver.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index df44c3fbd0..4c3785fa36 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1442,11 +1442,13 @@ qemuGetProcessInfo(unsigned long long *cpuTime, int 
*lastCpu, long *vm_rss,
 return -1;
 
 pidinfo = fopen(proc, "r");
+if (!pidinfo) {
+return -1;
+}
 
 /* See 'man proc' for information about what all these fields are. We're
  * only interested in a very few of them */
-if (!pidinfo ||
-fscanf(pidinfo,
+if (fscanf(pidinfo,
/* pid -> stime */
"%*d (%*[^)]) %*c %*d %*d %*d %*d %*d %*u %*u %*u %*u %*u %llu 
%llu"
/* cutime -> endcode */
@@ -1455,6 +1457,8 @@ qemuGetProcessInfo(unsigned long long *cpuTime, int 
*lastCpu, long *vm_rss,
"%*u %*u %*u %*u %*u %*u %*u %*u %*u %*u %*d %d",
, , , ) != 4) {
 VIR_WARN("cannot parse process status data");
+VIR_FORCE_FCLOSE(pidinfo);
+return -1;
 }
 
 /* We got jiffies
-- 
2.18.1