Re: [PATCH 00/11] conf: Refactor virSysinfoParseXML and remove pointless XML node validation
On a Tuesday in 2022, Peter Krempa wrote: Peter Krempa (11): conf: domain: Refactor cleanup in virSysinfoBIOSParseXML conf: domain: Remove pointless XML node name validation in virSysinfoBIOSParseXML conf: domain: Reformat XPath queries in virSysinfoSystemParseXML conf: domain: Refactor cleanup in virSysinfoSystemParseXML conf: domain: Remove pointless XML node name validation in virSysinfoSystemParseXML conf: domain: Reformat XPath queries in virSysinfoChassisParseXML conf: domain: Refactor cleanup in virSysinfoChassisParseXML conf: domain: Remove pointless XML node name validation in virSysinfoChassisParseXML conf: domain: Parse 'type' attribute via virXMLPropEnum in virSysinfoParseXML conf: domain: Refactor cleanup in virSysinfoParseXML conf: domain: Remove pointless XML node name validation in virSysinfoParseXML src/conf/domain_conf.c | 139 ++--- src/util/virsysinfo.h | 3 + 2 files changed, 36 insertions(+), 106 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 10/12] qemublocktest: Mark 'network-ssh-qcow2' input XML as invalid
On a Tuesday in 2022, Peter Krempa wrote:
The XML does not conform to the RNG schema as we don't yet expose the
'ssh' protocol officially. Mark the XML as invalid by renaming it.
Signed-off-by: Peter Krempa
---
tests/qemublocktest.c | 2 +-
.../{network-ssh-qcow2.json => network-ssh-qcow2-invalid.json} | 0
.../{network-ssh-qcow2.xml => network-ssh-qcow2-invalid.xml}| 0
3 files changed, 1 insertion(+), 1 deletion(-)
rename tests/qemublocktestdata/imagecreate/{network-ssh-qcow2.json =>
network-ssh-qcow2-invalid.json} (100%)
rename tests/qemublocktestdata/imagecreate/{network-ssh-qcow2.xml =>
network-ssh-qcow2-invalid.xml} (100%)
Reviewed-by: Ján Tomko
Jano
Re: [PATCH 11/12] schema: Rename definition of disk 'target' element to 'diskTarget'
On a Tuesday in 2022, Peter Krempa wrote: Rename 'diskTarget' to 'diskTargetDev' and then 'target' to 'diskTarget'. This will make it less confusing when overriding the definition. Signed-off-by: Peter Krempa --- src/conf/schemas/domainbackup.rng | 4 ++-- src/conf/schemas/domaincheckpoint.rng | 2 +- src/conf/schemas/domaincommon.rng | 9 + src/conf/schemas/domainsnapshot.rng | 2 +- 4 files changed, 9 insertions(+), 8 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 09/12] tests: qemublocktestdata/imagecreate: Remove bogus 'name="vda"' attribute from
On a Tuesday in 2022, Peter Krempa wrote: Signed-off-by: Peter Krempa --- tests/qemublocktestdata/imagecreate/luks-encopts.xml | 2 +- tests/qemublocktestdata/imagecreate/luks-noopts.xml | 2 +- tests/qemublocktestdata/imagecreate/network-gluster-qcow2.xml | 2 +- tests/qemublocktestdata/imagecreate/network-rbd-qcow2.xml | 2 +- tests/qemublocktestdata/imagecreate/network-ssh-qcow2.xml | 2 +- tests/qemublocktestdata/imagecreate/qcow2-luks-encopts.xml| 2 +- tests/qemublocktestdata/imagecreate/qcow2-luks-noopts.xml | 2 +- tests/qemublocktestdata/imagecreate/qcow2-slice.xml | 2 +- tests/qemublocktestdata/imagecreate/qcow2.xml | 2 +- tests/qemublocktestdata/imagecreate/raw-nbd.xml | 2 +- tests/qemublocktestdata/imagecreate/raw-slice.xml | 2 +- tests/qemublocktestdata/imagecreate/raw.xml | 2 +- 12 files changed, 12 insertions(+), 12 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 08/12] qemublocktestdata: Fix 'block-raw-reservations' case
On a Tuesday in 2022, Peter Krempa wrote: The 'reservations' element doesn't have an 'enabled' attribute according to our schema, remove it. Signed-off-by: Peter Krempa --- tests/qemublocktestdata/xml2json/block-raw-reservations.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 05/12] virschematest: Construct path to the schema in the SCHEMAS_PATH
On a Tuesday in 2022, Jonathon Jongsma wrote: On 11/1/22 9:01 AM, Peter Krempa wrote: 'abs_top_srcdir' can be prepended to the schema in the macro. Apart from removing one needles string copy it will also allow pointing to schema needless ^^^ files in the builddir which will come handy in upcoming patches. Signed-off-by: Peter Krempa --- tests/virschematest.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 04/12] util: xml: Refactor cleanup in virXMLValidateAgainstSchema
On a Tuesday in 2022, Peter Krempa wrote: Use automatic freeing of the validator context to remove 'ret'/'cleanup:'. Signed-off-by: Peter Krempa --- src/util/virxml.c | 10 +++--- 1 file changed, 3 insertions(+), 7 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 03/12] util: xml: Refactor cleanup path in virXMLValidatorInit
On a Tuesday in 2022, Peter Krempa wrote: Automatically free 'validator' on errors. Signed-off-by: Peter Krempa --- src/util/virxml.c | 13 + 1 file changed, 5 insertions(+), 8 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 01/12] virschematest: Test 'nodedevxml2xmlout' directory
On a Tuesday in 2022, Peter Krempa wrote: Test the output files against the RNG schema. Signed-off-by: Peter Krempa --- tests/virschematest.c | 1 + 1 file changed, 1 insertion(+) Reviewed-by: Ján Tomko Jano
Re: [PATCH 02/12] qemustatusxml2xml: Remove obsolete 'json' attribute
On a Tuesday in 2022, Peter Krempa wrote: We no longer support HMP-only qemus. Remove the leftover attribute from the test files. Signed-off-by: Peter Krempa --- tests/qemustatusxml2xmldata/migration-out-nbd-in.xml | 2 +- tests/qemustatusxml2xmldata/migration-out-nbd-tls-in.xml | 2 +- tests/qemustatusxml2xmldata/upgrade-in.xml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 42/43] cpu_arm: Avoid use of 'unsigned long'
On a Monday in 2022, Peter Krempa wrote: Covert all use of 'unsigned long' to 'unsigned long long'. Signed-off-by: Peter Krempa --- src/cpu/cpu_arm.c | 37 +++-- src/cpu/cpu_arm_data.h | 4 ++-- 2 files changed, 17 insertions(+), 24 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 43/43] util: xml: Remove unused virXPathULong*
On a Monday in 2022, Peter Krempa wrote: Remove the now-unused functions for parsing 'unsigned long' values via XPath. Signed-off-by: Peter Krempa --- src/libvirt_private.syms | 2 -- src/util/virxml.c| 75 src/util/virxml.h| 8 - 3 files changed, 85 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 41/43] virDomainJobObj: Use 'unsigned int' instead of 'unsigned long' for 'apiFlags' field
On a Monday in 2022, Peter Krempa wrote: The callers store only an 'unsigned int' in the field. Convert it to the proper type including parser/formatter. Signed-off-by: Peter Krempa --- src/conf/virdomainjob.c | 4 ++-- src/conf/virdomainjob.h | 4 ++-- src/qemu/qemu_domainjob.c| 7 +++ src/qemu/qemu_migration.c| 2 +- src/qemu/qemu_migration_params.c | 8 src/qemu/qemu_migration_params.h | 4 ++-- src/qemu/qemu_process.c | 2 +- src/qemu/qemu_process.h | 2 +- 8 files changed, 16 insertions(+), 17 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 40/43] qemuDomainObjPrivateXMLParseBlockjobData: Use virXMLPropUInt instead of virXPathULongHex
On a Monday in 2022, Peter Krempa wrote: Use the function for the proper type. Signed-off-by: Peter Krempa --- src/qemu/qemu_domain.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 39/43] ppc64ModelParse: Switch to virXMLPropUInt from virXPathULongHex
On a Monday in 2022, Peter Krempa wrote: We don't need to do the extra XPath lookups and we can use the proper type right away. Signed-off-by: Peter Krempa --- src/cpu/cpu_ppc64.c | 19 --- 1 file changed, 4 insertions(+), 15 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 38/43] virDomainSEVDefParseXML: Use virXPathUIntBase instead of virXPathULongHex
On a Monday in 2022, Peter Krempa wrote: Use the proper function for an unsigned int. Signed-off-by: Peter Krempa --- src/conf/domain_conf.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 37/43] virDomainNetDef: Change type of 'tune.sndbuf'
On a Monday in 2022, Peter Krempa wrote: Use 'unsigned long long' instead of 'unsigned long' and fix the parser and formatter. Signed-off-by: Peter Krempa --- src/conf/domain_conf.c | 4 ++-- src/conf/domain_conf.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 36/43] virDomainTimerDefParseXML: Use virXMLProp instead of virXPath
On a Monday in 2022, Peter Krempa wrote: Parse the 'frequency' field without an extra XPath. Signed-off-by: Peter Krempa --- src/conf/domain_conf.c | 10 +- 1 file changed, 1 insertion(+), 9 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 35/43] virDomainTimerDef: Convert 'mode' field to proper enum type
On a Monday in 2022, Peter Krempa wrote: Adjust the parser and switch statements to go with it. Note that the XEN/libxl drivers had a 'default:' case for few of the swtich statements so this patch blindly expands it to what it would be in those cases. Signed-off-by: Peter Krempa --- src/conf/domain_conf.c | 12 +++- src/conf/domain_conf.h | 2 +- src/libxl/libxl_conf.c | 6 +- src/libxl/xen_common.c | 6 +- 4 files changed, 14 insertions(+), 12 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 34/43] virDomainTimerDef: Convert 'track' field to proper enum type
On a Monday in 2022, Peter Krempa wrote: Adjust the parser and add missing switch cases to make the complier happy. Signed-off-by: Peter Krempa --- src/conf/domain_conf.c | 13 - src/conf/domain_conf.h | 2 +- src/qemu/qemu_command.c | 2 ++ src/qemu/qemu_validate.c | 2 ++ 4 files changed, 9 insertions(+), 10 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 33/43] virDomainTimerDef: Convert 'tickpolicy' field to proper enum type
On a Monday in 2022, Peter Krempa wrote: Convert the field, adjust the XML parser to use virXMLPropEnum and add the VIR_DOMAIN_TIMER_TICKPOLICY_LAST enum case to all appropriate 'switch' statements. Signed-off-by: Peter Krempa --- src/conf/domain_conf.c | 12 +++- src/conf/domain_conf.h | 2 +- src/qemu/qemu_command.c | 5 + src/qemu/qemu_validate.c | 6 ++ 4 files changed, 15 insertions(+), 10 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 32/43] virDomainTimerDef: Convert 'name' field to proper enum type
On a Monday in 2022, Peter Krempa wrote: Adjust the type and the corresponding parser to use virXMLPropEnum. Signed-off-by: Peter Krempa --- src/conf/domain_conf.c | 13 ++--- src/conf/domain_conf.h | 2 +- 2 files changed, 3 insertions(+), 12 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 29/43] virNetDevVlanParse: Use virXMLProp* helpers instead of XPath lookups
On a Monday in 2022, Peter Krempa wrote: The loop inside virNetDevVlanParse fetches multiple attributes from the element. Convert it to use the virXMLProp* helpers, which also simplifies error reporting. Signed-off-by: Peter Krempa --- src/conf/netdev_vlan_conf.c | 39 - 1 file changed, 17 insertions(+), 22 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 31/43] virDomainTimerDefParseXML: Refactor cleanup
On a Monday in 2022, Peter Krempa wrote: Automatically free the 'def' variable and remove the 'cleanup' label. Signed-off-by: Peter Krempa --- src/conf/domain_conf.c | 30 -- 1 file changed, 12 insertions(+), 18 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 30/43] virDomainTimerCatchupDef: Change members to 'unsigned long long'
On a Monday in 2022, Peter Krempa wrote: The struct used 'unsigned long' variables which we try to avoid due to being different size on different architectures. Convert the stuct and use virXMLPropULongLong instead of virXPathULong *struct when parsing the XML. Signed-off-by: Peter Krempa --- src/conf/domain_conf.c | 34 +- src/conf/domain_conf.h | 6 +++--- 2 files changed, 12 insertions(+), 28 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 28/43] virInterfaceDefParseMtu: Use virXPathUInt instead of virXPathULong
On a Monday in 2022, Peter Krempa wrote: Use the proper convertor function and refactor error reporting. Signed-off-by: Peter Krempa --- src/conf/interface_conf.c | 14 ++ 1 file changed, 6 insertions(+), 8 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 27/43] virNetworkIPDefParseXML: Use virXMLPropUInt instead of virXPathULong
On a Monday in 2022, Peter Krempa wrote: Parse the 'prefix' field directly and adjust the the error message format strings. Signed-off-by: Peter Krempa --- src/conf/network_conf.c | 21 + 1 file changed, 5 insertions(+), 16 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 26/43] virNetDevIPRouteParseXML: Refactor to use 'virXMLProp*' instead of XPath
On a Monday in 2022, Peter Krempa wrote: The function extracts multiple attributes form a single element. Modify the function to stop using multiple XPath lookups. Signed-off-by: Peter Krempa --- src/conf/domain_conf.c| 2 +- src/conf/network_conf.c | 4 +-- src/conf/networkcommon_conf.c | 61 +-- src/conf/networkcommon_conf.h | 3 +- 4 files changed, 19 insertions(+), 51 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 25/43] virQEMUCapsLoadCache: Use 'virXMLPropUInt' instead of 'virXPathULong'
On a Monday in 2022, Peter Krempa wrote: The libvirt version is stored in an 'unsigned int' use the proper XPath query function for the type and remove the temporary variable. Signed-off-by: Peter Krempa --- src/qemu/qemu_capabilities.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 24/43] util: xml: Remove virXPathLong
On a Monday in 2022, Peter Krempa wrote: The function is now unused and we no longer want to promote use of the 'long' type. Signed-off-by: Peter Krempa --- src/libvirt_private.syms | 1 - src/util/virxml.c| 53 src/util/virxml.h| 4 --- 3 files changed, 58 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 23/43] conf: domain: Convert from virXPathLong
On a Monday in 2022, Peter Krempa wrote: Convert the two uses of virXPathLong to proper virXMLPropInt/virXMLPropLongLong so that virXPathLong can be removed in an upcoming patch. Signed-off-by: Peter Krempa --- src/conf/domain_conf.c | 23 --- 1 file changed, 12 insertions(+), 11 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 22/43] testParseNodeInfo: Rewrite to virXPathU(Int|LongLong)
On a Monday in 2022, Peter Krempa wrote: Use the function for appropriate types and simplify the error logic. Signed-off-by: Peter Krempa --- src/test/test_driver.c | 51 +- 1 file changed, 20 insertions(+), 31 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 21/43] util: xml: Disallow aliasing of negative numbers in virXPathUInt
On a Monday in 2022, Peter Krempa wrote: Passing negative number as an alias for the max value is an anti-feature we unfortunately allowed in virsh, but luckily never encouraged in the XML. Refuse numbers with negative sign when parsing unsigned int from XPaths. Signed-off-by: Peter Krempa --- src/util/virxml.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 20/43] virNodeDevCapsDefParseHexId: Use 'virXPathUIntBase'
On a Monday in 2022, Peter Krempa wrote: Switch to the proper function for parsing integer variant of a hex number via XPath and spell out properly that the argument is 'unsigned int'. Signed-off-by: Peter Krempa --- src/conf/node_device_conf.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 19/43] util: xml: Introduce virXPathU(Int|LongLong)Base
On a Monday in 2022, Peter Krempa wrote: In an effort to remove the 'Long' variants of XPath number fetching functions we need a way to replace the hex number parsing capability. The new helpers are created from the originals by adding a 'base' argument and keeping the original function as a wrapper to pass 10. Signed-off-by: Peter Krempa --- src/libvirt_private.syms | 2 ++ src/util/virxml.c| 42 ++-- src/util/virxml.h| 10 ++ 3 files changed, 44 insertions(+), 10 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 18/43] util: xml: Reimplement virXPath(U)Int via virXPathEvalString
On a Monday in 2022, Peter Krempa wrote: Similarly to the refactor of virXPath(U)LongLong drop the ability to convert from the internal double value forcing the use of the 'string()' conversion. In case of 32 bit integers there's no problem with overflows, but we can implement the code identically to what we have in the other helpers. Signed-off-by: Peter Krempa --- src/util/virxml.c | 38 -- 1 file changed, 20 insertions(+), 18 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 17/43] conf: cpu: Extract and refactor parsing of cache from virCPUDefParseXML
On a Monday in 2022, Peter Krempa wrote: Move the parser into a helper function named 'virCPUDefParseXMLCache' and use the virXMLProp* helpers instead of multiple XPath lookups. Signed-off-by: Peter Krempa --- src/conf/cpu_conf.c | 58 ++--- 1 file changed, 34 insertions(+), 24 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 16/43] conf: numa: Don't fetch XML node count in virDomainNumatuneParseXML
On a Monday in 2022, Peter Krempa wrote:
The code only wants to refuse cases where more than one 'numatune'
element is present which can be achieved by using 'virXPathBoolean'.
Signed-off-by: Peter Krempa
---
src/conf/numa_conf.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index 390ef49b84..688aa7b409 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -226,18 +226,13 @@ virDomainNumatuneParseXML(virDomainNuma *numa,
{
g_autofree char *modestr = NULL;
int mode = -1;
-int n = 0;
g_autofree char *placementstr = NULL;
int placement = -1;
g_autofree char *nodesetstr = NULL;
g_autoptr(virBitmap) nodeset = NULL;
xmlNodePtr node = NULL;
-if (virXPathInt("count(./numatune)", ctxt, &n) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("cannot extract numatune nodes"));
-return -1;
-} else if (n > 1) {
+if (virXPathBoolean("count(./numatune) > 1", ctxt) == 1) {
Alternatively, "boolean(./numatune[1])"
Reviewed-by: Ján Tomko
Jano
Re: [PATCH 15/43] conf: node_device: Use 'string()' in XPath expressions for virNodeDevCapsDefParseIntOptional
On a Monday in 2022, Peter Krempa wrote: Upcoming patches will require that the XML XPath query returns a string for conversion in virXPathInt. Convert all the XPaths used with virNodeDevCapsDefParseIntOptional which uses virXPathInt internally. Signed-off-by: Peter Krempa --- src/conf/node_device_conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 05/12] virschematest: Construct path to the schema in the SCHEMAS_PATH
On 11/1/22 9:01 AM, Peter Krempa wrote:
'abs_top_srcdir' can be prepended to the schema in the macro. Apart from
removing one needles string copy it will also allow pointing to schema
needless
files in the builddir which will come handy in upcoming patches.
Signed-off-by: Peter Krempa
---
tests/virschematest.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/tests/virschematest.c b/tests/virschematest.c
index 3c91bd37bb..2a89f6a1c0 100644
--- a/tests/virschematest.c
+++ b/tests/virschematest.c
@@ -151,13 +151,10 @@ testSchemaGrammarReport(const void *opaque)
static virXMLValidator *
testSchemaGrammarLoad(const char *schema)
{
-g_autofree char *schema_path = NULL;
g_autofree char *testname = NULL;
virXMLValidator *ret;
-schema_path = g_strdup_printf("%s/%s", abs_top_srcdir, schema);
-
-ret = virXMLValidatorInit(schema_path);
+ret = virXMLValidatorInit(schema);
testname = g_strdup_printf("test schema grammar file: '%s'", schema);
@@ -335,7 +332,7 @@ mymain(void)
{
int ret = 0;
-#define SCHEMAS_PATH "src/conf/schemas/"
+#define SCHEMAS_PATH abs_top_srcdir "/src/conf/schemas/"
#define DO_TEST(sch, ent) \
if (testSchemaEntries((sch), (ent), G_N_ELEMENTS(ent)) < 0) \
Re: [PATCH 2/6] nodedev: Add VIR_NODE_DEVICE_(CREATE|DEFINE)_XML_VALIDATE flags
On Tue, Nov 01, 2022 at 15:43:40 +0100, Michal Prívozník wrote:
> On 10/20/22 16:37, Peter Krempa wrote:
> > The node device APIs which get XML from the user don't yet support XML
> > validation flags. Introduce virNodeDeviceCreateXMLFlags and
> > virNodeDeviceDefineXMLFlags with the appropriate flags and add virsh
> > support for the new flags.
> >
> > Signed-off-by: Peter Krempa
> > ---
> > docs/manpages/virsh.rst | 10 --
> > include/libvirt/libvirt-nodedev.h | 19 +++
> > src/libvirt-nodedev.c | 4 ++--
> > tools/virsh-nodedev.c | 20 ++--
> > 4 files changed, 47 insertions(+), 6 deletions(-)
>
> > +
> > +/**
> > + * virNodeDeviceDefineXMLFlags:
> > + *
> > + * Since: 8.10.0
> > + */
> > +typedef enum {
> > +VIR_NODE_DEVICE_DEFINE_XML_VALIDATE = 1 << 0, /* Validate the XML
> > document against schema (Since: 8.10.0) */
> > +} virNodeDeviceDefineXMLFlags;
> > +
>
> I know you already pushed these, but I am just wondering whether we
> ought to drop the _XML_ infix as it diverges from the rest of _VALIDATE
> flags.
>
> libvirt.git $ git grep VALIDATE -- include/ | grep _XML_
> include/libvirt/libvirt-nodedev.h:VIR_NODE_DEVICE_CREATE_XML_VALIDATE = 1
> << 0, /* Validate the XML document against schema (Since: 8.10.0) */
> include/libvirt/libvirt-nodedev.h:VIR_NODE_DEVICE_DEFINE_XML_VALIDATE = 1
> << 0, /* Validate the XML document against schema (Since: 8.10.0) */
The infix was based on the full API name the flags bind to:
* 'virNodeDeviceCreateXMLFlags' are for 'virNodeDeviceCreateXML'
There is 'virNodeDeviceCreate' which does not take an XML thus in such
case I think having the proper full name of the API in the name of the
flag should be done to differentiate diverging flags.
* 'virNodeDeviceDefineXMLFlags' are for 'virNodeDeviceDefineXML'
Now here is no other 'define' API so the XML suffix doesn't
differentiate it, but for symmetry IMO the full function name should
be the prefix for the flag.
[PATCH 04/11] conf: domain: Refactor cleanup in virSysinfoSystemParseXML
Register automatic cleanup for virSysinfoSystemDef and use it to
refactor the cleanup code paths in virSysinfoSystemParseXML.
Signed-off-by: Peter Krempa
---
src/conf/domain_conf.c | 24
src/util/virsysinfo.h | 1 +
2 files changed, 9 insertions(+), 16 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b0a978cc62..8e9f415070 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12184,8 +12184,7 @@ virSysinfoSystemParseXML(xmlNodePtr node,
bool uuid_generated)
{
VIR_XPATH_NODE_AUTORESTORE(ctxt)
-int ret = -1;
-virSysinfoSystemDef *def;
+g_autoptr(virSysinfoSystemDef) def = g_new0(virSysinfoSystemDef, 1);
g_autofree char *tmpUUID = NULL;
ctxt->node = node;
@@ -12193,11 +12192,9 @@ virSysinfoSystemParseXML(xmlNodePtr node,
if (!virXMLNodeNameEqual(node, "system")) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("XML does not contain expected 'system' element"));
-return ret;
+return -1;
}
-def = g_new0(virSysinfoSystemDef, 1);
-
def->manufacturer = virXPathString("string(entry[@name='manufacturer'])",
ctxt);
def->product = virXPathString("string(entry[@name='product'])", ctxt);
def->version = virXPathString("string(entry[@name='version'])", ctxt);
@@ -12209,15 +12206,14 @@ virSysinfoSystemParseXML(xmlNodePtr node,
if (virUUIDParse(tmpUUID, uuidbuf) < 0) {
virReportError(VIR_ERR_XML_DETAIL,
"%s", _("malformed uuid element"));
-goto cleanup;
+return -1;
}
if (uuid_generated) {
memcpy(domUUID, uuidbuf, VIR_UUID_BUFLEN);
} else if (memcmp(domUUID, uuidbuf, VIR_UUID_BUFLEN) != 0) {
virReportError(VIR_ERR_XML_DETAIL, "%s",
- _("UUID mismatch between and "
- ""));
-goto cleanup;
+ _("UUID mismatch between and "));
+return -1;
}
/* Although we've validated the UUID as good, virUUIDParse() is
* lax with respect to allowing extraneous "-" and " ", but the
@@ -12232,15 +12228,11 @@ virSysinfoSystemParseXML(xmlNodePtr node,
def->family = virXPathString("string(entry[@name='family'])", ctxt);
if (!def->manufacturer && !def->product && !def->version &&
-!def->serial && !def->uuid && !def->sku && !def->family) {
-g_clear_pointer(&def, virSysinfoSystemDefFree);
-}
+!def->serial && !def->uuid && !def->sku && !def->family)
+return 0;
*sysdef = g_steal_pointer(&def);
-ret = 0;
- cleanup:
-virSysinfoSystemDefFree(def);
-return ret;
+return 0;
}
static int
diff --git a/src/util/virsysinfo.h b/src/util/virsysinfo.h
index 899193dc81..d9f15b06e2 100644
--- a/src/util/virsysinfo.h
+++ b/src/util/virsysinfo.h
@@ -144,6 +144,7 @@ virSysinfoDef *virSysinfoRead(void);
void virSysinfoBIOSDefFree(virSysinfoBIOSDef *def);
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virSysinfoBIOSDef, virSysinfoBIOSDefFree);
void virSysinfoSystemDefFree(virSysinfoSystemDef *def);
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virSysinfoSystemDef, virSysinfoSystemDefFree);
void virSysinfoBaseBoardDefClear(virSysinfoBaseBoardDef *def);
void virSysinfoChassisDefFree(virSysinfoChassisDef *def);
void virSysinfoOEMStringsDefFree(virSysinfoOEMStringsDef *def);
--
2.37.3
[PATCH 07/11] conf: domain: Refactor cleanup in virSysinfoChassisParseXML
Register automatic cleanup for virSysinfoChassisDef and use it to
refactor the cleanup code paths in virSysinfoChassisParseXML.
Signed-off-by: Peter Krempa
---
src/conf/domain_conf.c | 16 +---
src/util/virsysinfo.h | 1 +
2 files changed, 6 insertions(+), 11 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 90478f2528..e10b4d72f8 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12323,19 +12323,16 @@ virSysinfoChassisParseXML(xmlNodePtr node,
virSysinfoChassisDef **chassisdef)
{
VIR_XPATH_NODE_AUTORESTORE(ctxt)
-int ret = -1;
-virSysinfoChassisDef *def;
+g_autoptr(virSysinfoChassisDef) def = g_new0(virSysinfoChassisDef, 1);
ctxt->node = node;
if (!xmlStrEqual(node->name, BAD_CAST "chassis")) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("XML does not contain expected 'chassis' element"));
-return ret;
+return -1;
}
-def = g_new0(virSysinfoChassisDef, 1);
-
def->manufacturer = virXPathString("string(entry[@name='manufacturer'])",
ctxt);
def->version = virXPathString("string(entry[@name='version'])", ctxt);
def->serial = virXPathString("string(entry[@name='serial'])", ctxt);
@@ -12343,14 +12340,11 @@ virSysinfoChassisParseXML(xmlNodePtr node,
def->sku = virXPathString("string(entry[@name='sku'])", ctxt);
if (!def->manufacturer && !def->version &&
-!def->serial && !def->asset && !def->sku) {
-g_clear_pointer(&def, virSysinfoChassisDefFree);
-}
+!def->serial && !def->asset && !def->sku)
+return 0;
*chassisdef = g_steal_pointer(&def);
-ret = 0;
-virSysinfoChassisDefFree(def);
-return ret;
+return 0;
}
diff --git a/src/util/virsysinfo.h b/src/util/virsysinfo.h
index d9f15b06e2..5fa91d9611 100644
--- a/src/util/virsysinfo.h
+++ b/src/util/virsysinfo.h
@@ -147,6 +147,7 @@ void virSysinfoSystemDefFree(virSysinfoSystemDef *def);
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virSysinfoSystemDef, virSysinfoSystemDefFree);
void virSysinfoBaseBoardDefClear(virSysinfoBaseBoardDef *def);
void virSysinfoChassisDefFree(virSysinfoChassisDef *def);
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virSysinfoChassisDef, virSysinfoChassisDefFree);
void virSysinfoOEMStringsDefFree(virSysinfoOEMStringsDef *def);
void virSysinfoDefFree(virSysinfoDef *def);
--
2.37.3
[PATCH 10/11] conf: domain: Refactor cleanup in virSysinfoParseXML
Use automatic pointer freeing to remove the 'error' label.
Signed-off-by: Peter Krempa
---
src/conf/domain_conf.c | 16 +---
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b9494500e2..71997e586a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12450,7 +12450,7 @@ virSysinfoParseXML(xmlNodePtr node,
bool uuid_generated)
{
VIR_XPATH_NODE_AUTORESTORE(ctxt)
-virSysinfoDef *def;
+g_autoptr(virSysinfoDef) def = g_new0(virSysinfoDef, 1);
ctxt->node = node;
@@ -12460,32 +12460,26 @@ virSysinfoParseXML(xmlNodePtr node,
return NULL;
}
-def = g_new0(virSysinfoDef, 1);
-
if (virXMLPropEnum(node, "type", virSysinfoTypeFromString,
VIR_XML_PROP_REQUIRED, &def->type) < 0)
-goto error;
+return NULL;
switch (def->type) {
case VIR_SYSINFO_SMBIOS:
if (virSysinfoParseSMBIOSDef(def, ctxt, domUUID, uuid_generated) < 0)
-goto error;
+return NULL;
break;
case VIR_SYSINFO_FWCFG:
if (virSysinfoParseFWCfgDef(def, node, ctxt) < 0)
-goto error;
+return NULL;
break;
case VIR_SYSINFO_LAST:
break;
}
-return def;
-
- error:
-virSysinfoDefFree(def);
-return NULL;
+return g_steal_pointer(&def);
}
unsigned int
--
2.37.3
[PATCH 06/11] conf: domain: Reformat XPath queries in virSysinfoChassisParseXML
Remove the unneeded linebreaks after assignment operator. Only one line
exceeds 80 colums and just by 4 characters.
Signed-off-by: Peter Krempa
---
src/conf/domain_conf.c | 15 +--
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d9b77eddb4..90478f2528 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12336,16 +12336,11 @@ virSysinfoChassisParseXML(xmlNodePtr node,
def = g_new0(virSysinfoChassisDef, 1);
-def->manufacturer =
-virXPathString("string(entry[@name='manufacturer'])", ctxt);
-def->version =
-virXPathString("string(entry[@name='version'])", ctxt);
-def->serial =
-virXPathString("string(entry[@name='serial'])", ctxt);
-def->asset =
-virXPathString("string(entry[@name='asset'])", ctxt);
-def->sku =
-virXPathString("string(entry[@name='sku'])", ctxt);
+def->manufacturer = virXPathString("string(entry[@name='manufacturer'])",
ctxt);
+def->version = virXPathString("string(entry[@name='version'])", ctxt);
+def->serial = virXPathString("string(entry[@name='serial'])", ctxt);
+def->asset = virXPathString("string(entry[@name='asset'])", ctxt);
+def->sku = virXPathString("string(entry[@name='sku'])", ctxt);
if (!def->manufacturer && !def->version &&
!def->serial && !def->asset && !def->sku) {
--
2.37.3
[PATCH 11/11] conf: domain: Remove pointless XML node name validation in virSysinfoParseXML
The only caller passes 'node' argument originating from an XPath lookup
for the 'sysinfo' element, so there's no point in checking it once more.
Signed-off-by: Peter Krempa
---
src/conf/domain_conf.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 71997e586a..65f8ad20c9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12454,12 +12454,6 @@ virSysinfoParseXML(xmlNodePtr node,
ctxt->node = node;
-if (!virXMLNodeNameEqual(node, "sysinfo")) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("XML does not contain expected 'sysinfo' element"));
-return NULL;
-}
-
if (virXMLPropEnum(node, "type", virSysinfoTypeFromString,
VIR_XML_PROP_REQUIRED, &def->type) < 0)
return NULL;
--
2.37.3
[PATCH 08/11] conf: domain: Remove pointless XML node name validation in virSysinfoChassisParseXML
The only caller passes 'node' argument originating from an XPath lookup
for the 'chassis' element, so there's no point in checking it once more.
Signed-off-by: Peter Krempa
---
src/conf/domain_conf.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e10b4d72f8..6379cfda7c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12327,12 +12327,6 @@ virSysinfoChassisParseXML(xmlNodePtr node,
ctxt->node = node;
-if (!xmlStrEqual(node->name, BAD_CAST "chassis")) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("XML does not contain expected 'chassis' element"));
-return -1;
-}
-
def->manufacturer = virXPathString("string(entry[@name='manufacturer'])",
ctxt);
def->version = virXPathString("string(entry[@name='version'])", ctxt);
def->serial = virXPathString("string(entry[@name='serial'])", ctxt);
--
2.37.3
[PATCH 09/11] conf: domain: Parse 'type' attribute via virXMLPropEnum in virSysinfoParseXML
Rewrite the code to use the simple helper.
Signed-off-by: Peter Krempa
---
src/conf/domain_conf.c | 15 ++-
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6379cfda7c..b9494500e2 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12451,8 +12451,6 @@ virSysinfoParseXML(xmlNodePtr node,
{
VIR_XPATH_NODE_AUTORESTORE(ctxt)
virSysinfoDef *def;
-g_autofree char *typeStr = NULL;
-int type;
ctxt->node = node;
@@ -12464,18 +12462,9 @@ virSysinfoParseXML(xmlNodePtr node,
def = g_new0(virSysinfoDef, 1);
-typeStr = virXMLPropString(node, "type");
-if (typeStr == NULL) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("sysinfo must contain a type attribute"));
-goto error;
-}
-if ((type = virSysinfoTypeFromString(typeStr)) < 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown sysinfo type '%s'"), typeStr);
+if (virXMLPropEnum(node, "type", virSysinfoTypeFromString,
+ VIR_XML_PROP_REQUIRED, &def->type) < 0)
goto error;
-}
-def->type = type;
switch (def->type) {
case VIR_SYSINFO_SMBIOS:
--
2.37.3
[PATCH 01/11] conf: domain: Refactor cleanup in virSysinfoBIOSParseXML
Register automatic cleanup for virSysinfoBIOSDef and use it to refactor
the cleanup code paths in virSysinfoBIOSParseXML.
Signed-off-by: Peter Krempa
---
src/conf/domain_conf.c | 19 ++-
src/util/virsysinfo.h | 1 +
2 files changed, 7 insertions(+), 13 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7984a15c46..15db12876e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12136,19 +12136,16 @@ virSysinfoBIOSParseXML(xmlNodePtr node,
virSysinfoBIOSDef **bios)
{
VIR_XPATH_NODE_AUTORESTORE(ctxt)
-int ret = -1;
-virSysinfoBIOSDef *def;
+g_autoptr(virSysinfoBIOSDef) def = g_new0(virSysinfoBIOSDef, 1);
ctxt->node = node;
if (!virXMLNodeNameEqual(node, "bios")) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("XML does not contain expected 'bios' element"));
-return ret;
+return -1;
}
-def = g_new0(virSysinfoBIOSDef, 1);
-
def->vendor = virXPathString("string(entry[@name='vendor'])", ctxt);
def->version = virXPathString("string(entry[@name='version'])", ctxt);
def->date = virXPathString("string(entry[@name='date'])", ctxt);
@@ -12173,20 +12170,16 @@ virSysinfoBIOSParseXML(xmlNodePtr node,
(year < 0 || (year >= 100 && year < 1900))) {
virReportError(VIR_ERR_XML_DETAIL, "%s",
_("Invalid BIOS 'date' format"));
-goto cleanup;
+return -1;
}
}
if (!def->vendor && !def->version &&
-!def->date && !def->release) {
-g_clear_pointer(&def, virSysinfoBIOSDefFree);
-}
+!def->date && !def->release)
+return 0;
*bios = g_steal_pointer(&def);
-ret = 0;
- cleanup:
-virSysinfoBIOSDefFree(def);
-return ret;
+return 0;
}
static int
diff --git a/src/util/virsysinfo.h b/src/util/virsysinfo.h
index 97e0e18ddf..899193dc81 100644
--- a/src/util/virsysinfo.h
+++ b/src/util/virsysinfo.h
@@ -142,6 +142,7 @@ struct _virSysinfoDef {
virSysinfoDef *virSysinfoRead(void);
void virSysinfoBIOSDefFree(virSysinfoBIOSDef *def);
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virSysinfoBIOSDef, virSysinfoBIOSDefFree);
void virSysinfoSystemDefFree(virSysinfoSystemDef *def);
void virSysinfoBaseBoardDefClear(virSysinfoBaseBoardDef *def);
void virSysinfoChassisDefFree(virSysinfoChassisDef *def);
--
2.37.3
[PATCH 05/11] conf: domain: Remove pointless XML node name validation in virSysinfoSystemParseXML
The only caller passes 'node' argument originating from an XPath lookup
for the 'system' element, so there's no point in checking it once more.
Signed-off-by: Peter Krempa
---
src/conf/domain_conf.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8e9f415070..d9b77eddb4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12189,12 +12189,6 @@ virSysinfoSystemParseXML(xmlNodePtr node,
ctxt->node = node;
-if (!virXMLNodeNameEqual(node, "system")) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("XML does not contain expected 'system' element"));
-return -1;
-}
-
def->manufacturer = virXPathString("string(entry[@name='manufacturer'])",
ctxt);
def->product = virXPathString("string(entry[@name='product'])", ctxt);
def->version = virXPathString("string(entry[@name='version'])", ctxt);
--
2.37.3
[PATCH 02/11] conf: domain: Remove pointless XML node name validation in virSysinfoBIOSParseXML
The only caller passes 'node' argument originating from an XPath lookup
for the 'bios' element, so there's no point in checking it once more.
Signed-off-by: Peter Krempa
---
src/conf/domain_conf.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 15db12876e..eab2e98792 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12140,12 +12140,6 @@ virSysinfoBIOSParseXML(xmlNodePtr node,
ctxt->node = node;
-if (!virXMLNodeNameEqual(node, "bios")) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("XML does not contain expected 'bios' element"));
-return -1;
-}
-
def->vendor = virXPathString("string(entry[@name='vendor'])", ctxt);
def->version = virXPathString("string(entry[@name='version'])", ctxt);
def->date = virXPathString("string(entry[@name='date'])", ctxt);
--
2.37.3
[PATCH 03/11] conf: domain: Reformat XPath queries in virSysinfoSystemParseXML
Remove the unneeded linebreaks after assignment operator. Only one line
exceeds 80 colums and just by 4 characters.
Signed-off-by: Peter Krempa
---
src/conf/domain_conf.c | 18 ++
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index eab2e98792..b0a978cc62 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12198,14 +12198,10 @@ virSysinfoSystemParseXML(xmlNodePtr node,
def = g_new0(virSysinfoSystemDef, 1);
-def->manufacturer =
-virXPathString("string(entry[@name='manufacturer'])", ctxt);
-def->product =
-virXPathString("string(entry[@name='product'])", ctxt);
-def->version =
-virXPathString("string(entry[@name='version'])", ctxt);
-def->serial =
-virXPathString("string(entry[@name='serial'])", ctxt);
+def->manufacturer = virXPathString("string(entry[@name='manufacturer'])",
ctxt);
+def->product = virXPathString("string(entry[@name='product'])", ctxt);
+def->version = virXPathString("string(entry[@name='version'])", ctxt);
+def->serial = virXPathString("string(entry[@name='serial'])", ctxt);
tmpUUID = virXPathString("string(entry[@name='uuid'])", ctxt);
if (tmpUUID) {
unsigned char uuidbuf[VIR_UUID_BUFLEN];
@@ -12232,10 +12228,8 @@ virSysinfoSystemParseXML(xmlNodePtr node,
virUUIDFormat(uuidbuf, uuidstr);
def->uuid = g_strdup(uuidstr);
}
-def->sku =
-virXPathString("string(entry[@name='sku'])", ctxt);
-def->family =
-virXPathString("string(entry[@name='family'])", ctxt);
+def->sku = virXPathString("string(entry[@name='sku'])", ctxt);
+def->family = virXPathString("string(entry[@name='family'])", ctxt);
if (!def->manufacturer && !def->product && !def->version &&
!def->serial && !def->uuid && !def->sku && !def->family) {
--
2.37.3
[PATCH 00/11] conf: Refactor virSysinfoParseXML and remove pointless XML node validation
Peter Krempa (11): conf: domain: Refactor cleanup in virSysinfoBIOSParseXML conf: domain: Remove pointless XML node name validation in virSysinfoBIOSParseXML conf: domain: Reformat XPath queries in virSysinfoSystemParseXML conf: domain: Refactor cleanup in virSysinfoSystemParseXML conf: domain: Remove pointless XML node name validation in virSysinfoSystemParseXML conf: domain: Reformat XPath queries in virSysinfoChassisParseXML conf: domain: Refactor cleanup in virSysinfoChassisParseXML conf: domain: Remove pointless XML node name validation in virSysinfoChassisParseXML conf: domain: Parse 'type' attribute via virXMLPropEnum in virSysinfoParseXML conf: domain: Refactor cleanup in virSysinfoParseXML conf: domain: Remove pointless XML node name validation in virSysinfoParseXML src/conf/domain_conf.c | 139 ++--- src/util/virsysinfo.h | 3 + 2 files changed, 36 insertions(+), 106 deletions(-) -- 2.37.3
Re: [PATCH] tests: qemucapabilities: Add data for the qemu-7.2 dev cycle
On Tue, Nov 01, 2022 at 15:31:37 +0100, Michal Prívozník wrote: > On 10/17/22 16:13, Peter Krempa wrote: > > Add data based on the v7.1.0-998-g5c2439a92c qemu commit. > > > > Notable changes: > > > > - New machine types and corresponding objects: > > - pc-i440fx-7.2, pc-i440fx-7.2-machine, pc-q35-7.2, pc-q35-7.2-machine > > - New disk frontend properties: > > - account-failed, account-invalid > > - New CPU flags and some CPU features become migratable > > (corresponding 'cpu-host-model' test changed output) > > - New unstable commands for debugging virtio: > > x-query-virtio, x-query-virtio-status, x-query-virtio-queue-status, > > x-query-virtio-vhost-queue-status, x-query-virtio-queue-element > > > > Signed-off-by: Peter Krempa > > --- > > .../domaincapsdata/qemu_7.2.0-q35.x86_64.xml | 242 + > > .../domaincapsdata/qemu_7.2.0-tcg.x86_64.xml | 252 + > > tests/domaincapsdata/qemu_7.2.0.x86_64.xml| 242 + > > .../caps_7.2.0.x86_64.replies | 38928 > > .../caps_7.2.0.x86_64.xml | 3548 ++ > > .../cpu-host-model.x86_64-latest.args | 2 +- > > 6 files changed, 43213 insertions(+), 1 deletion(-) > > create mode 100644 tests/domaincapsdata/qemu_7.2.0-q35.x86_64.xml > > create mode 100644 tests/domaincapsdata/qemu_7.2.0-tcg.x86_64.xml > > create mode 100644 tests/domaincapsdata/qemu_7.2.0.x86_64.xml > > create mode 100644 tests/qemucapabilitiesdata/caps_7.2.0.x86_64.replies > > create mode 100644 tests/qemucapabilitiesdata/caps_7.2.0.x86_64.xml > > > > Hopefuly, I'm not late for the party. Since you're refreshing this, can > you please do this for qemu v7.1.0-1429-g7208429223 or newer? It has > something I have patches for (thread-context object). I was actually updating my system so that I could also get a snapshot of the libblkio stuff that was added recently. I'll push an updated version soon.
Re: [PATCH 2/6] nodedev: Add VIR_NODE_DEVICE_(CREATE|DEFINE)_XML_VALIDATE flags
On 10/20/22 16:37, Peter Krempa wrote:
> The node device APIs which get XML from the user don't yet support XML
> validation flags. Introduce virNodeDeviceCreateXMLFlags and
> virNodeDeviceDefineXMLFlags with the appropriate flags and add virsh
> support for the new flags.
>
> Signed-off-by: Peter Krempa
> ---
> docs/manpages/virsh.rst | 10 --
> include/libvirt/libvirt-nodedev.h | 19 +++
> src/libvirt-nodedev.c | 4 ++--
> tools/virsh-nodedev.c | 20 ++--
> 4 files changed, 47 insertions(+), 6 deletions(-)
> +
> +/**
> + * virNodeDeviceDefineXMLFlags:
> + *
> + * Since: 8.10.0
> + */
> +typedef enum {
> +VIR_NODE_DEVICE_DEFINE_XML_VALIDATE = 1 << 0, /* Validate the XML
> document against schema (Since: 8.10.0) */
> +} virNodeDeviceDefineXMLFlags;
> +
I know you already pushed these, but I am just wondering whether we
ought to drop the _XML_ infix as it diverges from the rest of _VALIDATE
flags.
libvirt.git $ git grep VALIDATE -- include/ | grep _XML_
include/libvirt/libvirt-nodedev.h:VIR_NODE_DEVICE_CREATE_XML_VALIDATE = 1
<< 0, /* Validate the XML document against schema (Since: 8.10.0) */
include/libvirt/libvirt-nodedev.h:VIR_NODE_DEVICE_DEFINE_XML_VALIDATE = 1
<< 0, /* Validate the XML document against schema (Since: 8.10.0) */
Michal
Re: [PATCH] tests: qemucapabilities: Add data for the qemu-7.2 dev cycle
On 10/17/22 16:13, Peter Krempa wrote: > Add data based on the v7.1.0-998-g5c2439a92c qemu commit. > > Notable changes: > > - New machine types and corresponding objects: > - pc-i440fx-7.2, pc-i440fx-7.2-machine, pc-q35-7.2, pc-q35-7.2-machine > - New disk frontend properties: > - account-failed, account-invalid > - New CPU flags and some CPU features become migratable > (corresponding 'cpu-host-model' test changed output) > - New unstable commands for debugging virtio: > x-query-virtio, x-query-virtio-status, x-query-virtio-queue-status, > x-query-virtio-vhost-queue-status, x-query-virtio-queue-element > > Signed-off-by: Peter Krempa > --- > .../domaincapsdata/qemu_7.2.0-q35.x86_64.xml | 242 + > .../domaincapsdata/qemu_7.2.0-tcg.x86_64.xml | 252 + > tests/domaincapsdata/qemu_7.2.0.x86_64.xml| 242 + > .../caps_7.2.0.x86_64.replies | 38928 > .../caps_7.2.0.x86_64.xml | 3548 ++ > .../cpu-host-model.x86_64-latest.args | 2 +- > 6 files changed, 43213 insertions(+), 1 deletion(-) > create mode 100644 tests/domaincapsdata/qemu_7.2.0-q35.x86_64.xml > create mode 100644 tests/domaincapsdata/qemu_7.2.0-tcg.x86_64.xml > create mode 100644 tests/domaincapsdata/qemu_7.2.0.x86_64.xml > create mode 100644 tests/qemucapabilitiesdata/caps_7.2.0.x86_64.replies > create mode 100644 tests/qemucapabilitiesdata/caps_7.2.0.x86_64.xml > Hopefuly, I'm not late for the party. Since you're refreshing this, can you please do this for qemu v7.1.0-1429-g7208429223 or newer? It has something I have patches for (thread-context object). Thanks, Michal
Re: [PATCH 14/43] conf: node_device: Convert rest of virXPathUInt XPath expressions to number
On a Monday in 2022, Peter Krempa wrote: Convert the rest of the XPath expressions used with virXPathUInt directly to convert via string(). This will become mandatory in upcoming patches. Signed-off-by: Peter Krempa --- src/conf/node_device_conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 12/43] util: xml: Disallow aliasing of negative numbers in virXPathULongLong
On a Monday in 2022, Peter Krempa wrote: Passing negative number as an alias for the max value is an anti-feature we unfortunately allowed in virsh, but luckily never encouraged in the XML. Refuse numbers with negative sign when parsing unsigned long long from XPaths. Signed-off-by: Peter Krempa --- src/util/virxml.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 13/43] conf: node_device: Use 'string()' in XPath expressions for virNodeDevCapsDefParseUInt
On a Monday in 2022, Peter Krempa wrote: Upcoming patches will require that the XML XPath query returns a string for conversion in virXPathUInt. Convert all the XPaths used with virNodeDevCapsDefParseUInt which uses virXPathUInt internally. Signed-off-by: Peter Krempa --- src/conf/node_device_conf.c | 30 +++--- 1 file changed, 15 insertions(+), 15 deletions(-) Reviewed-by: Ján Tomko Jano
[PATCH 08/12] qemublocktestdata: Fix 'block-raw-reservations' case
The 'reservations' element doesn't have an 'enabled' attribute according to our schema, remove it. Signed-off-by: Peter Krempa --- tests/qemublocktestdata/xml2json/block-raw-reservations.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemublocktestdata/xml2json/block-raw-reservations.xml b/tests/qemublocktestdata/xml2json/block-raw-reservations.xml index 3ebfe61186..bf59137fd4 100644 --- a/tests/qemublocktestdata/xml2json/block-raw-reservations.xml +++ b/tests/qemublocktestdata/xml2json/block-raw-reservations.xml @@ -1,7 +1,7 @@ - + -- 2.38.1
[PATCH 05/12] virschematest: Construct path to the schema in the SCHEMAS_PATH
'abs_top_srcdir' can be prepended to the schema in the macro. Apart from
removing one needles string copy it will also allow pointing to schema
files in the builddir which will come handy in upcoming patches.
Signed-off-by: Peter Krempa
---
tests/virschematest.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/tests/virschematest.c b/tests/virschematest.c
index 3c91bd37bb..2a89f6a1c0 100644
--- a/tests/virschematest.c
+++ b/tests/virschematest.c
@@ -151,13 +151,10 @@ testSchemaGrammarReport(const void *opaque)
static virXMLValidator *
testSchemaGrammarLoad(const char *schema)
{
-g_autofree char *schema_path = NULL;
g_autofree char *testname = NULL;
virXMLValidator *ret;
-schema_path = g_strdup_printf("%s/%s", abs_top_srcdir, schema);
-
-ret = virXMLValidatorInit(schema_path);
+ret = virXMLValidatorInit(schema);
testname = g_strdup_printf("test schema grammar file: '%s'", schema);
@@ -335,7 +332,7 @@ mymain(void)
{
int ret = 0;
-#define SCHEMAS_PATH "src/conf/schemas/"
+#define SCHEMAS_PATH abs_top_srcdir "/src/conf/schemas/"
#define DO_TEST(sch, ent) \
if (testSchemaEntries((sch), (ent), G_N_ELEMENTS(ent)) < 0) \
--
2.38.1
[PATCH 03/12] util: xml: Refactor cleanup path in virXMLValidatorInit
Automatically free 'validator' on errors.
Signed-off-by: Peter Krempa
---
src/util/virxml.c | 13 +
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/src/util/virxml.c b/src/util/virxml.c
index 35c0340779..e9595da97d 100644
--- a/src/util/virxml.c
+++ b/src/util/virxml.c
@@ -1608,7 +1608,7 @@ virXMLValidatorRNGErrorIgnore(void *ctx G_GNUC_UNUSED,
virXMLValidator *
virXMLValidatorInit(const char *schemafile)
{
-virXMLValidator *validator = NULL;
+g_autoptr(virXMLValidator) validator = NULL;
validator = g_new0(virXMLValidator, 1);
@@ -1619,7 +1619,7 @@ virXMLValidatorInit(const char *schemafile)
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to create RNG parser for %s"),
validator->schemafile);
-goto error;
+return NULL;
}
xmlRelaxNGSetParserErrors(validator->rngParser,
@@ -1632,25 +1632,22 @@ virXMLValidatorInit(const char *schemafile)
_("Unable to parse RNG %s: %s"),
validator->schemafile,
virBufferCurrentContent(&validator->buf));
-goto error;
+return NULL;
}
if (!(validator->rngValid = xmlRelaxNGNewValidCtxt(validator->rng))) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to create RNG validation context %s"),
validator->schemafile);
-goto error;
+return NULL;
}
xmlRelaxNGSetValidErrors(validator->rngValid,
virXMLValidatorRNGErrorCatch,
virXMLValidatorRNGErrorIgnore,
&validator->buf);
-return validator;
- error:
-virXMLValidatorFree(validator);
-return NULL;
+return g_steal_pointer(&validator);
}
--
2.38.1
[PATCH 12/12] virschematest: Add infrastructure for testing single devices
Introduce an internal schema for a single device and use it to test the
various files in tests/qemuhotplugtestdevices and
tests/qemublocktestdata directories.
This also requires us to implement schema for (some) privateData bits
for the disk source.
Signed-off-by: Peter Krempa
---
tests/schemas/device.rng.in | 51
tests/schemas/meson.build| 2 +
tests/schemas/privatedata.rng.in | 68
tests/virschematest.c| 7
4 files changed, 128 insertions(+)
create mode 100644 tests/schemas/device.rng.in
create mode 100644 tests/schemas/privatedata.rng.in
diff --git a/tests/schemas/device.rng.in b/tests/schemas/device.rng.in
new file mode 100644
index 00..b322b5275e
--- /dev/null
+++ b/tests/schemas/device.rng.in
@@ -0,0 +1,51 @@
+
+http://relaxng.org/ns/structure/1.0";
datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes";>
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/schemas/meson.build b/tests/schemas/meson.build
index 33c16bb2fb..851355d4e9 100644
--- a/tests/schemas/meson.build
+++ b/tests/schemas/meson.build
@@ -5,6 +5,8 @@ virschematest_test_schemas_conf = configuration_data({
virschematest_schemas = [
'cpu-baseline.rng.in',
+ 'device.rng.in',
+ 'privatedata.rng.in',
]
foreach file : virschematest_schemas
diff --git a/tests/schemas/privatedata.rng.in b/tests/schemas/privatedata.rng.in
new file mode 100644
index 00..945f7a06b3
--- /dev/null
+++ b/tests/schemas/privatedata.rng.in
@@ -0,0 +1,68 @@
+
+http://relaxng.org/ns/structure/1.0";
datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes";>
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ storage
+ format
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/virschematest.c b/tests/virschematest.c
index 29a1d59134..fcf3838630 100644
--- a/tests/virschematest.c
+++ b/tests/virschematest.c
@@ -322,6 +322,12 @@ static const struct testSchemaEntry testsCpuBaseline[] = {
{ . dir = "tests/cputestdata" },
};
+static const struct testSchemaEntry testDevice[] = {
+{ .dir = "tests/qemuhotplugtestdevices" },
+{ .dir = "tests/qemublocktestdata/imagecreate" },
+{ .dir = "tests/qemublocktestdata/xml2json" },
+};
+
static int
mymain(void)
{
@@ -352,6 +358,7 @@ mymain(void)
DO_TEST(SCHEMAS_PATH "storagevol.rng", schemaStorageVol);
DO_TEST(INTERNAL_SCHEMAS_PATH "cpu-baseline.rng", testsCpuBaseline);
+DO_TEST(INTERNAL_SCHEMAS_PATH "device.rng", testDevice);
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
--
2.38.1
[PATCH 10/12] qemublocktest: Mark 'network-ssh-qcow2' input XML as invalid
The XML does not conform to the RNG schema as we don't yet expose the
'ssh' protocol officially. Mark the XML as invalid by renaming it.
Signed-off-by: Peter Krempa
---
tests/qemublocktest.c | 2 +-
.../{network-ssh-qcow2.json => network-ssh-qcow2-invalid.json} | 0
.../{network-ssh-qcow2.xml => network-ssh-qcow2-invalid.xml}| 0
3 files changed, 1 insertion(+), 1 deletion(-)
rename tests/qemublocktestdata/imagecreate/{network-ssh-qcow2.json =>
network-ssh-qcow2-invalid.json} (100%)
rename tests/qemublocktestdata/imagecreate/{network-ssh-qcow2.xml =>
network-ssh-qcow2-invalid.xml} (100%)
diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 1c1013d4d9..010b52f4b3 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -1211,7 +1211,7 @@ mymain(void)
TEST_IMAGE_CREATE("network-gluster-qcow2", NULL);
TEST_IMAGE_CREATE("network-rbd-qcow2", NULL);
-TEST_IMAGE_CREATE("network-ssh-qcow2", NULL);
+TEST_IMAGE_CREATE("network-ssh-qcow2-invalid", NULL);
#define TEST_BITMAP_DETECT(testname) \
do { \
diff --git a/tests/qemublocktestdata/imagecreate/network-ssh-qcow2.json
b/tests/qemublocktestdata/imagecreate/network-ssh-qcow2-invalid.json
similarity index 100%
rename from tests/qemublocktestdata/imagecreate/network-ssh-qcow2.json
rename to tests/qemublocktestdata/imagecreate/network-ssh-qcow2-invalid.json
diff --git a/tests/qemublocktestdata/imagecreate/network-ssh-qcow2.xml
b/tests/qemublocktestdata/imagecreate/network-ssh-qcow2-invalid.xml
similarity index 100%
rename from tests/qemublocktestdata/imagecreate/network-ssh-qcow2.xml
rename to tests/qemublocktestdata/imagecreate/network-ssh-qcow2-invalid.xml
--
2.38.1
[PATCH 06/12] virschematest: Improve testing schemas in 'tests/cputestdata'
The 'cputestdata' directory has a collection of XML files with very
complicated naming schemes for various input and output XML files.
Rather than trying to write complex regexes for selecting specific files
which diverged already multiple times we can introduce an internal
schema file which will cover all of the 3 top level elements used in the
XML files.
Schema for is taken from our main RNG schema, is just a
collection of elements, and finally is a simple enough
to describe inline.
To keep the validator happy we have to generate the schema file to
place full paths for the included documents.
Signed-off-by: Peter Krempa
---
tests/meson.build | 1 +
tests/schemas/cpu-baseline.rng.in | 84 +++
tests/schemas/meson.build | 16 ++
tests/virschematest.c | 13 ++---
4 files changed, 106 insertions(+), 8 deletions(-)
create mode 100644 tests/schemas/cpu-baseline.rng.in
create mode 100644 tests/schemas/meson.build
diff --git a/tests/meson.build b/tests/meson.build
index 1149211756..3365dce307 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -208,6 +208,7 @@ tests_deps += executable(
],
)
+subdir('schemas')
# build and define libvirt tests
diff --git a/tests/schemas/cpu-baseline.rng.in
b/tests/schemas/cpu-baseline.rng.in
new file mode 100644
index 00..1a3b19a45b
--- /dev/null
+++ b/tests/schemas/cpu-baseline.rng.in
@@ -0,0 +1,84 @@
+
+http://relaxng.org/ns/structure/1.0";
datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes";>
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ x86
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/schemas/meson.build b/tests/schemas/meson.build
new file mode 100644
index 00..33c16bb2fb
--- /dev/null
+++ b/tests/schemas/meson.build
@@ -0,0 +1,16 @@
+# we need to replace proper paths to our schemas in the test schemas
+virschematest_test_schemas_conf = configuration_data({
+ 'SCHEMADIR': meson.project_source_root() / 'src' / 'conf' / 'schemas',
+})
+
+virschematest_schemas = [
+ 'cpu-baseline.rng.in',
+]
+
+foreach file : virschematest_schemas
+ configure_file(
+input: file,
+output: '@BASENAME@',
+configuration: virschematest_test_schemas_conf,
+ )
+endforeach
diff --git a/tests/virschematest.c b/tests/virschematest.c
index 2a89f6a1c0..29a1d59134 100644
--- a/tests/virschematest.c
+++ b/tests/virschematest.c
@@ -318,13 +318,8 @@ static const struct testSchemaEntry schemaStorageVol[] = {
{ .file = "examples/xml/test/testvol.xml" },
};
-static const struct testSchemaEntry schemaCpu[] = {
-{ . dir = "tests/cputestdata",
- . dirRegex = "^[^-]+-cpuid-.*(-host|-guest|-json)\\.xml$" },
-{ . dir = "tests/cputestdata",
- . dirRegex = "^[^-]+-baseline-.*-result\\.xml$" },
-{ . dir = "tests/cputestdata",
- . dirRegex = "^[^-]+-(?!cpuid|baseline).*$" },
+static const struct testSchemaEntry testsCpuBaseline[] = {
+{ . dir = "tests/cputestdata" },
};
static int
@@ -333,6 +328,7 @@ mymain(void)
int ret = 0;
#define SCHEMAS_PATH abs_top_srcdir "/src/conf/schemas/"
+#define INTERNAL_SCHEMAS_PATH abs_builddir "/schemas/"
#define DO_TEST(sch, ent) \
if (testSchemaEntries((sch), (ent), G_N_ELEMENTS(ent)) < 0) \
@@ -354,7 +350,8 @@ mymain(void)
DO_TEST(SCHEMAS_PATH "storagepoolcaps.rng", schemaStoragepoolcaps);
DO_TEST(SCHEMAS_PATH "storagepool.rng", schemaStoragePool);
DO_TEST(SCHEMAS_PATH "storagevol.rng", schemaStorageVol);
-DO_TEST(SCHEMAS_PATH "cpu.rng", schemaCpu);
+
+DO_TEST(INTERNAL_SCHEMAS_PATH "cpu-baseline.rng", testsCpuBaseline);
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
--
2.38.1
[PATCH 11/12] schema: Rename definition of disk 'target' element to 'diskTarget'
Rename 'diskTarget' to 'diskTargetDev' and then 'target' to 'diskTarget'. This will make it less confusing when overriding the definition. Signed-off-by: Peter Krempa --- src/conf/schemas/domainbackup.rng | 4 ++-- src/conf/schemas/domaincheckpoint.rng | 2 +- src/conf/schemas/domaincommon.rng | 9 + src/conf/schemas/domainsnapshot.rng | 2 +- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/conf/schemas/domainbackup.rng b/src/conf/schemas/domainbackup.rng index 1ac9da62c1..bfc29a6c06 100644 --- a/src/conf/schemas/domainbackup.rng +++ b/src/conf/schemas/domainbackup.rng @@ -157,7 +157,7 @@ - + @@ -227,7 +227,7 @@ - + diff --git a/src/conf/schemas/domaincheckpoint.rng b/src/conf/schemas/domaincheckpoint.rng index a1c8b0bb9c..72c4186235 100644 --- a/src/conf/schemas/domaincheckpoint.rng +++ b/src/conf/schemas/domaincheckpoint.rng @@ -55,7 +55,7 @@ - + diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 85e2e0c57b..ebb39de3ef 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -1512,7 +1512,7 @@ - + @@ -2351,15 +2351,16 @@ - + (ioemu:)?(fd|hd|sd|vd|xvd|ubd)[a-zA-Z0-9_]+ - + + - + diff --git a/src/conf/schemas/domainsnapshot.rng b/src/conf/schemas/domainsnapshot.rng index 3db9f458ba..4048266f1d 100644 --- a/src/conf/schemas/domainsnapshot.rng +++ b/src/conf/schemas/domainsnapshot.rng @@ -127,7 +127,7 @@ - + -- 2.38.1
[PATCH 09/12] tests: qemublocktestdata/imagecreate: Remove bogus 'name="vda"' attribute from
Signed-off-by: Peter Krempa --- tests/qemublocktestdata/imagecreate/luks-encopts.xml | 2 +- tests/qemublocktestdata/imagecreate/luks-noopts.xml | 2 +- tests/qemublocktestdata/imagecreate/network-gluster-qcow2.xml | 2 +- tests/qemublocktestdata/imagecreate/network-rbd-qcow2.xml | 2 +- tests/qemublocktestdata/imagecreate/network-ssh-qcow2.xml | 2 +- tests/qemublocktestdata/imagecreate/qcow2-luks-encopts.xml| 2 +- tests/qemublocktestdata/imagecreate/qcow2-luks-noopts.xml | 2 +- tests/qemublocktestdata/imagecreate/qcow2-slice.xml | 2 +- tests/qemublocktestdata/imagecreate/qcow2.xml | 2 +- tests/qemublocktestdata/imagecreate/raw-nbd.xml | 2 +- tests/qemublocktestdata/imagecreate/raw-slice.xml | 2 +- tests/qemublocktestdata/imagecreate/raw.xml | 2 +- 12 files changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/qemublocktestdata/imagecreate/luks-encopts.xml b/tests/qemublocktestdata/imagecreate/luks-encopts.xml index bb0ee54adc..15a6038bde 100644 --- a/tests/qemublocktestdata/imagecreate/luks-encopts.xml +++ b/tests/qemublocktestdata/imagecreate/luks-encopts.xml @@ -1,4 +1,4 @@ - + diff --git a/tests/qemublocktestdata/imagecreate/luks-noopts.xml b/tests/qemublocktestdata/imagecreate/luks-noopts.xml index ac224b02de..fc4013afcf 100644 --- a/tests/qemublocktestdata/imagecreate/luks-noopts.xml +++ b/tests/qemublocktestdata/imagecreate/luks-noopts.xml @@ -1,4 +1,4 @@ - + diff --git a/tests/qemublocktestdata/imagecreate/network-gluster-qcow2.xml b/tests/qemublocktestdata/imagecreate/network-gluster-qcow2.xml index f3dbf24180..12d7de0f52 100644 --- a/tests/qemublocktestdata/imagecreate/network-gluster-qcow2.xml +++ b/tests/qemublocktestdata/imagecreate/network-gluster-qcow2.xml @@ -1,4 +1,4 @@ - + diff --git a/tests/qemublocktestdata/imagecreate/network-rbd-qcow2.xml b/tests/qemublocktestdata/imagecreate/network-rbd-qcow2.xml index 0f6c1ddb98..63aeaeb9be 100644 --- a/tests/qemublocktestdata/imagecreate/network-rbd-qcow2.xml +++ b/tests/qemublocktestdata/imagecreate/network-rbd-qcow2.xml @@ -1,4 +1,4 @@ - + diff --git a/tests/qemublocktestdata/imagecreate/network-ssh-qcow2.xml b/tests/qemublocktestdata/imagecreate/network-ssh-qcow2.xml index 4c44f81c81..046f7892db 100644 --- a/tests/qemublocktestdata/imagecreate/network-ssh-qcow2.xml +++ b/tests/qemublocktestdata/imagecreate/network-ssh-qcow2.xml @@ -1,4 +1,4 @@ - + diff --git a/tests/qemublocktestdata/imagecreate/qcow2-luks-encopts.xml b/tests/qemublocktestdata/imagecreate/qcow2-luks-encopts.xml index d6616bd164..0d0315167f 100644 --- a/tests/qemublocktestdata/imagecreate/qcow2-luks-encopts.xml +++ b/tests/qemublocktestdata/imagecreate/qcow2-luks-encopts.xml @@ -1,4 +1,4 @@ - + diff --git a/tests/qemublocktestdata/imagecreate/qcow2-luks-noopts.xml b/tests/qemublocktestdata/imagecreate/qcow2-luks-noopts.xml index e2d1c42424..707d86b459 100644 --- a/tests/qemublocktestdata/imagecreate/qcow2-luks-noopts.xml +++ b/tests/qemublocktestdata/imagecreate/qcow2-luks-noopts.xml @@ -1,4 +1,4 @@ - + diff --git a/tests/qemublocktestdata/imagecreate/qcow2-slice.xml b/tests/qemublocktestdata/imagecreate/qcow2-slice.xml index 6c5ae3107b..06ce282247 100644 --- a/tests/qemublocktestdata/imagecreate/qcow2-slice.xml +++ b/tests/qemublocktestdata/imagecreate/qcow2-slice.xml @@ -1,4 +1,4 @@ - + diff --git a/tests/qemublocktestdata/imagecreate/qcow2.xml b/tests/qemublocktestdata/imagecreate/qcow2.xml index f3c235f82f..6ca08dda7c 100644 --- a/tests/qemublocktestdata/imagecreate/qcow2.xml +++ b/tests/qemublocktestdata/imagecreate/qcow2.xml @@ -1,4 +1,4 @@ - + diff --git a/tests/qemublocktestdata/imagecreate/raw-nbd.xml b/tests/qemublocktestdata/imagecreate/raw-nbd.xml index 256bf23797..9c5a4a5952 100644 --- a/tests/qemublocktestdata/imagecreate/raw-nbd.xml +++ b/tests/qemublocktestdata/imagecreate/raw-nbd.xml @@ -1,4 +1,4 @@ - + diff --git a/tests/qemublocktestdata/imagecreate/raw-slice.xml b/tests/qemublocktestdata/imagecreate/raw-slice.xml index adc7a175ce..f440ba9922 100644 --- a/tests/qemublocktestdata/imagecreate/raw-slice.xml +++ b/tests/qemublocktestdata/imagecreate/raw-slice.xml @@ -1,4 +1,4 @@ - + diff --git a/tests/qemublocktestdata/imagecreate/raw.xml b/tests/qemublocktestdata/imagecreate/raw.xml index 3a91600bd8..316b7abb12 100644 --- a/tests/qemublocktestdata/imagecreate/raw.xml +++ b/tests/qemublocktestdata/imagecreate/raw.xml @@ -1,4 +1,4 @@ - + -- 2.38.1
[PATCH 04/12] util: xml: Refactor cleanup in virXMLValidateAgainstSchema
Use automatic freeing of the validator context to remove
'ret'/'cleanup:'.
Signed-off-by: Peter Krempa
---
src/util/virxml.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/src/util/virxml.c b/src/util/virxml.c
index e9595da97d..a2fe2e6b6d 100644
--- a/src/util/virxml.c
+++ b/src/util/virxml.c
@@ -1671,19 +1671,15 @@ int
virXMLValidateAgainstSchema(const char *schemafile,
xmlDocPtr doc)
{
-virXMLValidator *validator = NULL;
-int ret = -1;
+g_autoptr(virXMLValidator) validator = NULL;
if (!(validator = virXMLValidatorInit(schemafile)))
return -1;
if (virXMLValidatorValidate(validator, doc) < 0)
-goto cleanup;
+return -1;
-ret = 0;
- cleanup:
-virXMLValidatorFree(validator);
-return ret;
+return 0;
}
--
2.38.1
[PATCH 02/12] qemustatusxml2xml: Remove obsolete 'json' attribute
We no longer support HMP-only qemus. Remove the leftover attribute from the test files. Signed-off-by: Peter Krempa --- tests/qemustatusxml2xmldata/migration-out-nbd-in.xml | 2 +- tests/qemustatusxml2xmldata/migration-out-nbd-tls-in.xml | 2 +- tests/qemustatusxml2xmldata/upgrade-in.xml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/qemustatusxml2xmldata/migration-out-nbd-in.xml b/tests/qemustatusxml2xmldata/migration-out-nbd-in.xml index 636accf054..95eca028e9 100644 --- a/tests/qemustatusxml2xmldata/migration-out-nbd-in.xml +++ b/tests/qemustatusxml2xmldata/migration-out-nbd-in.xml @@ -1,6 +1,6 @@ - + diff --git a/tests/qemustatusxml2xmldata/migration-out-nbd-tls-in.xml b/tests/qemustatusxml2xmldata/migration-out-nbd-tls-in.xml index 2cd6c9a5e9..2e37184715 100644 --- a/tests/qemustatusxml2xmldata/migration-out-nbd-tls-in.xml +++ b/tests/qemustatusxml2xmldata/migration-out-nbd-tls-in.xml @@ -1,6 +1,6 @@ - + diff --git a/tests/qemustatusxml2xmldata/upgrade-in.xml b/tests/qemustatusxml2xmldata/upgrade-in.xml index 7fa56429f4..6e00bb5ddf 100644 --- a/tests/qemustatusxml2xmldata/upgrade-in.xml +++ b/tests/qemustatusxml2xmldata/upgrade-in.xml @@ -1,5 +1,5 @@ - + -- 2.38.1
[PATCH 07/12] schema: Introduce scaffolding for schema for elements
Libvirt internally (e.g. in the status XML) stores additional data for various objects described by the XML. The data is usually stored in or similar sub-elements. This patch adds possibility for internal schema files to describe the elements by schema while still disallowing them for the public schema. This patch adds definitions for private data of and the corresponding storage source of a disk. Signed-off-by: Peter Krempa --- src/conf/schemas/domaincommon.rng | 3 +++ src/conf/schemas/privatedata.rng | 30 ++ 2 files changed, 33 insertions(+) create mode 100644 src/conf/schemas/privatedata.rng diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index ebdf21fe99..85e2e0c57b 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -6,6 +6,7 @@ + + + + + + + + + + + + + + + + + + + -- 2.38.1
[PATCH 01/12] virschematest: Test 'nodedevxml2xmlout' directory
Test the output files against the RNG schema.
Signed-off-by: Peter Krempa
---
tests/virschematest.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tests/virschematest.c b/tests/virschematest.c
index cae502a338..3c91bd37bb 100644
--- a/tests/virschematest.c
+++ b/tests/virschematest.c
@@ -280,6 +280,7 @@ static const struct testSchemaEntry schemaNetworkport[] = {
static const struct testSchemaEntry schemaNodedev[] = {
{ .dir = "tests/nodedevschemadata" },
+{ .dir = "tests/nodedevxml2xmlout" },
{ .file = "examples/xml/test/testdev.xml" },
};
--
2.38.1
[PATCH 00/12] tests: Schema testing improvements
This series:
- cleans up the XML validator code a bit
- adds testing of nodedevxml2xmlout directory
- cleans up some mistakes in un-tested XML files
- adds internal schema and improves testing of
- cputestdata
- simplifies file selection by using a custom schema
- device schemas (e.g. qemuhotplugtest)
After this series the outstanding list of XML files which are not
validated is:
./examples/xml/test/testnodeinline.xml
./examples/xml/test/testnode.xml
./tests/virsh-auth.xml
All of the above should be tested, but there's no schema for them yet.
./src/cpu_map/*.xml
These are internal input-only data files distributed by libvirt.
Creating a schema for them is possible, but not very justified and
neither simple.
./tests/qemumigparamsdata/*.xml
./tests/qemumigrationcookiexmldata/*.xml
Internal input and output files. Writing a schema could theoretically
make sense as they are used for migration but currently doesn't seem to
be worth it.
./tests/qemucapabilitiesdata/*.xml
Internal input and output, for caching only, thus any incompatibility
allows us to simply drop them. Not worth writing schema.
./tests/qemustatusxml2xmldata/*.xml
Internal status XML, input/output. Writing schema might make sense to
avoid regressions and have a scaffold for future.
./tests/networkxml2xmlupdatein/.xml
Snippets of xml, hard to write custom schema for.
./tests/nodedevmdevctldata/*.xml
./tests/virstorageutildata/*.xml
Non-libvirt test data.
Peter Krempa (12):
virschematest: Test 'nodedevxml2xmlout' directory
qemustatusxml2xml: Remove obsolete 'json' attribute
util: xml: Refactor cleanup path in virXMLValidatorInit
util: xml: Refactor cleanup in virXMLValidateAgainstSchema
virschematest: Construct path to the schema in the SCHEMAS_PATH
virschematest: Improve testing schemas in 'tests/cputestdata'
schema: Introduce scaffolding for schema for elements
qemublocktestdata: Fix 'block-raw-reservations' case
tests: qemublocktestdata/imagecreate: Remove bogus 'name="vda"'
attribute from
qemublocktest: Mark 'network-ssh-qcow2' input XML as invalid
schema: Rename definition of disk 'target' element to 'diskTarget'
virschematest: Add infrastructure for testing single devices
src/conf/schemas/domainbackup.rng | 4 +-
src/conf/schemas/domaincheckpoint.rng | 2 +-
src/conf/schemas/domaincommon.rng | 12 ++-
src/conf/schemas/domainsnapshot.rng | 2 +-
src/conf/schemas/privatedata.rng | 30 +++
src/util/virxml.c | 23 ++---
tests/meson.build | 1 +
tests/qemublocktest.c | 2 +-
.../imagecreate/luks-encopts.xml | 2 +-
.../imagecreate/luks-noopts.xml | 2 +-
.../imagecreate/network-gluster-qcow2.xml | 2 +-
.../imagecreate/network-rbd-qcow2.xml | 2 +-
...w2.json => network-ssh-qcow2-invalid.json} | 0
...cow2.xml => network-ssh-qcow2-invalid.xml} | 2 +-
.../imagecreate/qcow2-luks-encopts.xml| 2 +-
.../imagecreate/qcow2-luks-noopts.xml | 2 +-
.../imagecreate/qcow2-slice.xml | 2 +-
tests/qemublocktestdata/imagecreate/qcow2.xml | 2 +-
.../qemublocktestdata/imagecreate/raw-nbd.xml | 2 +-
.../imagecreate/raw-slice.xml | 2 +-
tests/qemublocktestdata/imagecreate/raw.xml | 2 +-
.../xml2json/block-raw-reservations.xml | 2 +-
.../migration-out-nbd-in.xml | 2 +-
.../migration-out-nbd-tls-in.xml | 2 +-
tests/qemustatusxml2xmldata/upgrade-in.xml| 2 +-
tests/schemas/cpu-baseline.rng.in | 84 +++
tests/schemas/device.rng.in | 51 +++
tests/schemas/meson.build | 18
tests/schemas/privatedata.rng.in | 68 +++
tests/virschematest.c | 28 ---
30 files changed, 304 insertions(+), 53 deletions(-)
create mode 100644 src/conf/schemas/privatedata.rng
rename tests/qemublocktestdata/imagecreate/{network-ssh-qcow2.json =>
network-ssh-qcow2-invalid.json} (100%)
rename tests/qemublocktestdata/imagecreate/{network-ssh-qcow2.xml =>
network-ssh-qcow2-invalid.xml} (88%)
create mode 100644 tests/schemas/cpu-baseline.rng.in
create mode 100644 tests/schemas/device.rng.in
create mode 100644 tests/schemas/meson.build
create mode 100644 tests/schemas/privatedata.rng.in
--
2.38.1
Release of libvirt-8.9.0
The 8.9.0 release of both libvirt and libvirt-python is tagged and signed tarballs and source RPMs are available at https://libvirt.org/sources/ https://libvirt.org/sources/python/ Thanks everybody who helped with this release by sending patches, reviewing, testing, or providing feedback. Your work is greatly appreciated. * New features * Add ``virt-qemu-qmp-proxy`` for emulating a QMP socket for libvirt managed VMs ``virt-qemu-qmp-proxy`` tool provides a way to expose an emulated QMP server socket for a VM managed by libvirt. This allows existing QMP-only clients to work with libvirt managed VMs. Note: libvirt is not interpreting the communication between the tool using the proxy and qemu itself, so any state-changing commands may desynchronize libvirt. Use at your own risk. * qemu: Core Scheduling support To avoid side channel attacks, the Linux kernel allows creating groups of processes that trust each other and thus can be scheduled to run on hyperthreads of a CPU core at the same time. This is now implemented for QEMU domains too (see ``sched_core`` knob in qemu.conf), although not enabled by default, just yet. * Improvements * qemu: Add hypervisor-specific statistics to ``virConnectGetAllDomainStats`` The new stats group ``VIR_DOMAIN_STATS_VM`` of ``virConnectGetAllDomainStats``, also exposed as ``virsh domstats --vm``, returns hypervisor-specific stats fields for given VM. * Add ``vendor`` attribute for CPU models in domain capabilities Users can now see the vendor of each CPU model in domain capabilities and use it, e.g., for filtering usable CPU models based on host CPU vendor. * virsh: Add ``--model`` option for ``hypervisor-cpu-baseline`` This is a shortcut for calling ``hypervisor-cpu-baseline`` with a single CPU model and no additional features. It can be used for determining which features block a particular CPU model from being usable. * Improved documentation of CPU ``usable`` attribute in domain capabilities * Report ``channel`` and ``redirdev`` devices in domain capabilities The channel and redirect devices supported by the hypervisor are now reported in domain capabilities. * meson: Bump minimal required meson version Newer meson versions deprecate some functions used. These were replaced with their newer counterparts and the minimal required mesion version was bumped to 0.56.0. * qemu: Add flags to keep or remove TPM state for ``virDomainUndefineFlags`` ``VIR_DOMAIN_UNDEFINE_TPM`` and ``VIR_DOMAIN_UNDEFINE_KEEP_TPM`` specify accordingly to delete or keep a TPM's persistent state directory structure and files when undefining a domain. In virsh the flags are exposed as ``--tpm`` and ``--keep-tpm`` for the sub-command ``undefine``. * Bug fixes * qemu: Disable all blocker features in CPU baseline Three years ago QEMU renamed some CPU features (mostly those containing an underscore). When such renamed feature was reported by QEMU as blocking usability of a CPU model, we would fail to explicitly disable it when creating a baseline CPU definition using this model. This bug did not have any functional impact when the default ``check='partial'`` attribute was used for guest CPU definition in domain XML, but it could have caused failures to start a domain with ``check='full'`` in some cases. * qemu: Do not crash after restart with active migration In 8.8.0 release libvirt daemon would crash after it was restarted during an active outgoing migration. * qemu: Refresh state after restore from a save image When a domain is restored from a saved image, libvirt now queries QEMU for those parts of runtime information that were not part of the save image. For instance: MAC address of a macvtap NICs, tray state of CD-ROMs, allocated size of virtio-mem, and others. Enjoy. Jirka
Re: [PATCH for 8.9.0] Document caveats of 'VIR_DOMAIN_STATS_VM' group of statistics
On Tue, Nov 01, 2022 at 11:28:11 +0100, Peter Krempa wrote: > The original patches adding the functionality neglected to add any form > of documentation for the stats fields returned for this group. > > The stats are directly converted from qemu's 'query-stats(-schema)' QMP > command without any further interpretation. The 'query-stats-schema' has > the following disclaimer: > > Note: runtime-collected statistics and their names fall outside QEMU's usual >deprecation policies. QEMU will try to keep the set of available data >stable, together with their names, but will not guarantee stability >at all costs; the same is true of providers that source statistics >externally, e.g. from Linux. For example, if the same value is being >tracked with different names on different architectures or by different >providers, one of them might be renamed. A statistic might go away if >an algorithm is changed or some code is removed; changing a default >might cause previously useful statistics to always report 0. Such >changes, however, are expected to be rare. > > Since libvirt is not doing any form of conversion of the stats we can't > meaningfully document any of the returned fields. At the same time we > can't even meaningfully provide any form of API stability for the filed > names. > > Modify the documentation for the 'VIR_DOMAIN_STATS_VM' group both in the > API docs and in the virsh man page to reflect that and disclaim any form > of stability guarantees we provide normally. > > Fixes: 8c9e3dae142 > Signed-off-by: Peter Krempa > --- > docs/manpages/virsh.rst | 36 ++-- > src/libvirt-domain.c | 26 +- > tools/virsh-domain-monitor.c | 2 +- > 3 files changed, 60 insertions(+), 4 deletions(-) > > diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst > index 61fcb2e9ca..cb2dbaec18 100644 > --- a/docs/manpages/virsh.rst > +++ b/docs/manpages/virsh.rst > @@ -2297,7 +2297,7 @@ domstats > > domstats [--raw] [--enforce] [--backing] [--nowait] [--state] >[--cpu-total] [--balloon] [--vcpu] [--interface] > - [--block] [--perf] [--iothread] [--memory] [--dirtyrate] > + [--block] [--perf] [--iothread] [--memory] [--dirtyrate] [--vm] >[[--list-active] [--list-inactive] > [--list-persistent] [--list-transient] [--list-running]y > [--list-paused] [--list-shutoff] [--list-other]] | [domain ...] > @@ -2317,7 +2317,7 @@ The individual statistics groups are selectable via > specific flags. By > default all supported statistics groups are returned. Supported > statistics groups flags are: *--state*, *--cpu-total*, *--balloon*, > *--vcpu*, *--interface*, *--block*, *--perf*, *--iothread*, *--memory*, > -*--dirtyrate*. > +*--dirtyrate*, *--vm*. > > Note that - depending on the hypervisor type and version or the domain state > - not all of the following statistics may be returned. > @@ -2533,6 +2533,38 @@ not available for statistical purposes. > * ``dirtyrate.vcpu..megabytes_per_second`` - the calculated memory dirty >rate for a virtual cpu in MiB/s > > +*--vm* returns: > + > +The *--vm* option enables reporting of hypervisor-specific statistics. Naming > +and meaning of the fields is entirely hypervisor dependant. s/dependant/dependent/ :-) twice, as the second copy in libvirt_domain.c contains the same typo > + > +The statistics in this group have following naming scheme: s/following/the following/ (twice) > + > + ``vm.$NAME.$TYPE`` > + > + ``$NAME`` > + name of the statistics field provided by the hypervisor > + > + ``$TYPE`` > + Type of the value. Following types are returned: s/Following/The following/ (twice) > + > + ``cur`` > + current instant value > + ``sum`` > + aggregate value > + ``max`` > + peak value > + > + The returned value may be either an unsigned long long or a boolean. > + > + **WARNING**: The stats reported in this group are runtime-collected and > + hypervisor originated, thus fall outside of the usual stable API > + policies of libvirt. > + > + Libvirt can't guarantee that the statistics reported from the outside > + source will be present in further versions of the hypervisor, or that > + naming or meaning will stay consistent. Changes to existing fields, > + however, are expected to be rare. > > Selecting a specific statistics groups doesn't guarantee that the > daemon supports the selected group of stats. Flag *--enforce* > diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c > index 52fa136186..4728ddd6ff 100644 > --- a/src/libvirt-domain.c > +++ b/src/libvirt-domain.c The three corrections mentioned above need to be applied here as well. Reviewed-by: Jiri Denemark
Re: [PATCH for 8.9.0] Document caveats of 'VIR_DOMAIN_STATS_VM' group of statistics
On a Tuesday in 2022, Peter Krempa wrote: The original patches adding the functionality neglected to add any form of documentation for the stats fields returned for this group. The stats are directly converted from qemu's 'query-stats(-schema)' QMP command without any further interpretation. The 'query-stats-schema' has the following disclaimer: Note: runtime-collected statistics and their names fall outside QEMU's usual deprecation policies. QEMU will try to keep the set of available data stable, together with their names, but will not guarantee stability at all costs; the same is true of providers that source statistics externally, e.g. from Linux. For example, if the same value is being tracked with different names on different architectures or by different providers, one of them might be renamed. A statistic might go away if an algorithm is changed or some code is removed; changing a default might cause previously useful statistics to always report 0. Such changes, however, are expected to be rare. Since libvirt is not doing any form of conversion of the stats we can't meaningfully document any of the returned fields. At the same time we can't even meaningfully provide any form of API stability for the filed *field names. Modify the documentation for the 'VIR_DOMAIN_STATS_VM' group both in the API docs and in the virsh man page to reflect that and disclaim any form of stability guarantees we provide normally. Fixes: 8c9e3dae142 Signed-off-by: Peter Krempa --- docs/manpages/virsh.rst | 36 ++-- src/libvirt-domain.c | 26 +- tools/virsh-domain-monitor.c | 2 +- 3 files changed, 60 insertions(+), 4 deletions(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 2/2] qemu_namespace: Make qemuDomainGetPreservedMounts() more robust wrt running VMs
On a Monday in 2022, Michal Privoznik wrote: The aim of qemuDomainGetPreservedMounts() is to get a list of filesystems mounted under /dev and optionally generate a path for each one where they are moved temporarily when building the namespace. And if given domain is also running it looks into its mount table rather than at the host one. But if it did look at the domain's private mount table, it find /dev mounted twice: the first time by udev, the second time the tmpfs mounted by us. Now, later in the function there's a "sorting" algorithm that tries to reduce number of mount points needing preservation, by identifying nested mount points. And if we keep the second occurrence of /dev on the list, well, after the "sorting" we are left with nothing but "/dev" because all other mount points are nested. Fixes: 46b03819ae8d833b11c2aaccb2c2a0361727f51b Signed-off-by: Michal Privoznik --- src/qemu/qemu_namespace.c | 11 +++ 1 file changed, 11 insertions(+) Reviewed-by: Ján Tomko Jano
Re: [PATCH 1/2] qemu_namespace: Don't leak memory in qemuDomainGetPreservedMounts()
On a Monday in 2022, Michal Privoznik wrote: The aim of qemuDomainGetPreservedMounts() is to get a list of filesystems mounted under /dev and optionally generate a path for each one where they are moved temporarily when building the namespace. And the function tries to be a bit clever about it. For instance, if /dev/shm mount point exists, there's no need to consider /dev/shm/a nor /dev/shm/b as preserving just 'top level' /dev/shm gives the same result. To achieve this, the function iterates over the list of filesystem as returned by virFileGetMountSubtree() and removes the nested ones. However, it does so in a bit clumsy way: plain VIR_DELETE_ELEMENT() is used without freeing the string itself. Therefore, if all three aforementioned example paths appeared on the list, /dev/shm/a and /dev/shm/b strings would be leaked. And when I think about it more, there's no real need to shrink the array down (realloc()). It's going to be free()-d when returning from the function. Switch to VIR_DELETE_ELEMENT_INPLACE() then. Fixes: cdd9205dfffa3aaed935446a41f0d2dd1357c268 Signed-off-by: Michal Privoznik --- src/qemu/qemu_namespace.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Reviewed-by: Ján Tomko Jano
Re: [PATCH 1/2] qemu_namespace: Don't leak memory in qemuDomainGetPreservedMounts()
On Mon, Oct 31, 2022 at 16:52:59 +0100, Michal Privoznik wrote: > The aim of qemuDomainGetPreservedMounts() is to get a list of > filesystems mounted under /dev and optionally generate a path for > each one where they are moved temporarily when building the > namespace. And the function tries to be a bit clever about it. > For instance, if /dev/shm mount point exists, there's no need to > consider /dev/shm/a nor /dev/shm/b as preserving just 'top level' > /dev/shm gives the same result. To achieve this, the function > iterates over the list of filesystem as returned by > virFileGetMountSubtree() and removes the nested ones. However, it > does so in a bit clumsy way: plain VIR_DELETE_ELEMENT() is used > without freeing the string itself. Therefore, if all three > aforementioned example paths appeared on the list, /dev/shm/a and > /dev/shm/b strings would be leaked. > > And when I think about it more, there's no real need to shrink > the array down (realloc()). It's going to be free()-d when > returning from the function. Switch to > VIR_DELETE_ELEMENT_INPLACE() then. > > Fixes: cdd9205dfffa3aaed935446a41f0d2dd1357c268 > Signed-off-by: Michal Privoznik > --- > src/qemu/qemu_namespace.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Reviewed-by: Peter Krempa
Re: [PATCH 3/3] NEWS: Add the stats VIR_DOMAIN_STATS_VM of virConnectGetAllDomainStats
On Tue, Nov 01, 2022 at 13:49:12 +0800, Han Han wrote: > Signed-off-by: Han Han > --- > NEWS.rst | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/NEWS.rst b/NEWS.rst > index 7b856f4d3f..06fd4fa84f 100644 > --- a/NEWS.rst > +++ b/NEWS.rst > @@ -41,6 +41,12 @@ v8.9.0 (unreleased) > > * **Improvements** > > + * qemu: Add the stats of VM for virConnectGetAllDomainStats > + > +The stats ``VIR_DOMAIN_STATS_VM`` of virConnectGetAllDomainStats is to > +get the fd-based KVM statistics for the target VM. It could be used by > +the command line ``virsh domstats --vm`` > + I'll word this as: + * qemu: Add hypervisor-specific statistics to ``virConnectGetAllDomainStats`` + +The new stats group ``VIR_DOMAIN_STATS_VM`` of +``virConnectGetAllDomainStats``, also exposed as ``virsh domstats --vm``, +returns hypervisor-specific stats fields for given VM. + once my patch adding the docs for the new group is applied.
[PATCH for 8.9.0] Document caveats of 'VIR_DOMAIN_STATS_VM' group of statistics
The original patches adding the functionality neglected to add any form of documentation for the stats fields returned for this group. The stats are directly converted from qemu's 'query-stats(-schema)' QMP command without any further interpretation. The 'query-stats-schema' has the following disclaimer: Note: runtime-collected statistics and their names fall outside QEMU's usual deprecation policies. QEMU will try to keep the set of available data stable, together with their names, but will not guarantee stability at all costs; the same is true of providers that source statistics externally, e.g. from Linux. For example, if the same value is being tracked with different names on different architectures or by different providers, one of them might be renamed. A statistic might go away if an algorithm is changed or some code is removed; changing a default might cause previously useful statistics to always report 0. Such changes, however, are expected to be rare. Since libvirt is not doing any form of conversion of the stats we can't meaningfully document any of the returned fields. At the same time we can't even meaningfully provide any form of API stability for the filed names. Modify the documentation for the 'VIR_DOMAIN_STATS_VM' group both in the API docs and in the virsh man page to reflect that and disclaim any form of stability guarantees we provide normally. Fixes: 8c9e3dae142 Signed-off-by: Peter Krempa --- docs/manpages/virsh.rst | 36 ++-- src/libvirt-domain.c | 26 +- tools/virsh-domain-monitor.c | 2 +- 3 files changed, 60 insertions(+), 4 deletions(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 61fcb2e9ca..cb2dbaec18 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -2297,7 +2297,7 @@ domstats domstats [--raw] [--enforce] [--backing] [--nowait] [--state] [--cpu-total] [--balloon] [--vcpu] [--interface] - [--block] [--perf] [--iothread] [--memory] [--dirtyrate] + [--block] [--perf] [--iothread] [--memory] [--dirtyrate] [--vm] [[--list-active] [--list-inactive] [--list-persistent] [--list-transient] [--list-running]y [--list-paused] [--list-shutoff] [--list-other]] | [domain ...] @@ -2317,7 +2317,7 @@ The individual statistics groups are selectable via specific flags. By default all supported statistics groups are returned. Supported statistics groups flags are: *--state*, *--cpu-total*, *--balloon*, *--vcpu*, *--interface*, *--block*, *--perf*, *--iothread*, *--memory*, -*--dirtyrate*. +*--dirtyrate*, *--vm*. Note that - depending on the hypervisor type and version or the domain state - not all of the following statistics may be returned. @@ -2533,6 +2533,38 @@ not available for statistical purposes. * ``dirtyrate.vcpu..megabytes_per_second`` - the calculated memory dirty rate for a virtual cpu in MiB/s +*--vm* returns: + +The *--vm* option enables reporting of hypervisor-specific statistics. Naming +and meaning of the fields is entirely hypervisor dependant. + +The statistics in this group have following naming scheme: + + ``vm.$NAME.$TYPE`` + + ``$NAME`` + name of the statistics field provided by the hypervisor + + ``$TYPE`` + Type of the value. Following types are returned: + + ``cur`` + current instant value + ``sum`` + aggregate value + ``max`` + peak value + + The returned value may be either an unsigned long long or a boolean. + + **WARNING**: The stats reported in this group are runtime-collected and + hypervisor originated, thus fall outside of the usual stable API + policies of libvirt. + + Libvirt can't guarantee that the statistics reported from the outside + source will be present in further versions of the hypervisor, or that + naming or meaning will stay consistent. Changes to existing fields, + however, are expected to be rare. Selecting a specific statistics groups doesn't guarantee that the daemon supports the selected group of stats. Flag *--enforce* diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 52fa136186..4728ddd6ff 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -12482,7 +12482,31 @@ virConnectGetDomainCapabilities(virConnectPtr conn, * unsigned long long. * * VIR_DOMAIN_STATS_VM: - * Return fd-based KVM statistics for the target VM + * Return hypervisor-specific statistics. Note that the naming and meaning + * of the fields is entirely hypervisor dependant. + * + * The statistics in this group have following naming scheme: + * + * "vm.$NAME.$TYPE" + * + * $NAME - name of the statistics field provided by the hypervisor + * + * $TYPE - Type of the value. Following types are returned: + * 'cur' - current instant value + * 'sum' - aggregate value + * 'max' - peak
Re: [PATCH 3/3] NEWS: Add the stats VIR_DOMAIN_STATS_VM of virConnectGetAllDomainStats
On Tue, Nov 01, 2022 at 13:49:12 +0800, Han Han wrote: > Signed-off-by: Han Han > --- > NEWS.rst | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/NEWS.rst b/NEWS.rst > index 7b856f4d3f..06fd4fa84f 100644 > --- a/NEWS.rst > +++ b/NEWS.rst > @@ -41,6 +41,12 @@ v8.9.0 (unreleased) > > * **Improvements** > > + * qemu: Add the stats of VM for virConnectGetAllDomainStats > + > +The stats ``VIR_DOMAIN_STATS_VM`` of virConnectGetAllDomainStats is to > +get the fd-based KVM statistics for the target VM. It could be used by > +the command line ``virsh domstats --vm`` The stats reported in this group are totally undocumented. It doesn't make much sense until that is addressed to advertise it's presence as users simply won't be able to use them properly. I'll see whether adding docs before the release is feasible or not. In the meanwhile I'll push the 1/3 and 2/3
Re: [PATCH 2/3] NEWS: Mention the tool virt-qemu-qmp-proxy
On Tue, Nov 01, 2022 at 13:49:11 +0800, Han Han wrote: > Signed-off-by: Han Han > --- > NEWS.rst | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/NEWS.rst b/NEWS.rst > index 1d4fb62d5e..7b856f4d3f 100644 > --- a/NEWS.rst > +++ b/NEWS.rst > @@ -17,6 +17,13 @@ v8.9.0 (unreleased) > > * **New features** > > + * tools: Add the tool for proxying QMP via libvirt QEMU guests > + > +This tool ``virt-qemu-qmp-proxy`` provides a way to expose a QMP proxy > server > +that communicates with a QEMU guest managed by libvirt. This enables > standard > +QMP client tools to interact with libvirt managed guests. I'll reword this and add a disclaimer that use of this tool may de-sync the state and thus should be used at own risk: * Add ``virt-qemu-qmp-proxy`` for emulating a QMP socket for libvirt managed VMs ``virt-qemu-qmp-proxy`` tool provides a way to expose an emulated QMP server socket for a VM managed by libvirt. This allows existing QMP-only clients to work with libvirt managed VMs. **Note:** libvirt is not interpreting the communication between the tool using the proxy and qemu itself, so any state-changing commands may desynchronize libvirt. Use at your own risk.
Re: [PATCH 1/3] NEWS: Mention UNDEFINE_TPM and UNDEFINE_KEEP_TPM flags
On Tue, Nov 01, 2022 at 13:49:10 +0800, Han Han wrote: > Signed-off-by: Han Han > --- > NEWS.rst | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/NEWS.rst b/NEWS.rst > index 893ad6f370..1d4fb62d5e 100644 > --- a/NEWS.rst > +++ b/NEWS.rst > @@ -17,6 +17,13 @@ v8.9.0 (unreleased) > > * **New features** > > + * qemu: Add flags to keep or remove TPM state for the > virDomainUndefineFlags > + > +``VIR_DOMAIN_UNDEFINE_TPM`` and ``VIR_DOMAIN_UNDEFINE_KEEP_TPM`` specify > +accordingly to delete or keep a TPM's persistent state directory > structure > +and files when undefine a domain. In virsh, they are the ``--tpm`` and > +``--keep-tpm`` for the sub-command undefine. > + I'll reword this as: * qemu: Add flags to keep or remove TPM state for ``virDomainUndefineFlags`` ``VIR_DOMAIN_UNDEFINE_TPM`` and ``VIR_DOMAIN_UNDEFINE_KEEP_TPM`` specify accordingly to delete or keep a TPM's persistent state directory structure and files when undefining a domain. In virsh the flags are exposed as ``--tpm`` and ``--keep-tpm`` for the sub-command ``undefine``. and I'll put it under the 'Improvements' section.
Re: [PATCH] include: Fix the introduced version for VIR_DOMAIN_STATS_VM
On Tue, Nov 01, 2022 at 10:45:08AM +0800, Han Han wrote: > Fixes: 8c9e3dae14 > > Signed-off-by: Han Han > --- Reviewed-by: Erik Skultety
