Re: [PATCHv2 1/5] qemu: Add support for NVMe namespace disk bus type
On Sun, Apr 27, 2025 at 19:48:03 +0800, honglei.w...@smartx.com wrote:
> From: ray
>
> This patch extends the disk bus support by introducing a new nvme-ns bus type.
>
> The nvme-ns bus disk needs to be attached to nvme controller. A controller
> can contain multiple nvme-ns disk devices.
>
> Signed-off-by: ray
> ---
> src/conf/domain_conf.c | 39 +++
> src/conf/domain_conf.h | 7 +++
> src/conf/domain_postparse.c| 2 ++
> src/conf/domain_validate.c | 4 +++-
> src/conf/virconftypes.h| 2 ++
> src/hyperv/hyperv_driver.c | 2 ++
> src/qemu/qemu_alias.c | 1 +
> src/qemu/qemu_command.c| 26 ++
> src/qemu/qemu_domain_address.c | 5 +
> src/qemu/qemu_hotplug.c| 14 --
> src/qemu/qemu_postparse.c | 1 +
> src/qemu/qemu_validate.c | 12
> src/test/test_driver.c | 2 ++
> src/util/virutil.c | 2 +-
> src/vbox/vbox_common.c | 2 ++
> src/vmx/vmx.c | 1 +
> 16 files changed, 118 insertions(+), 4 deletions(-)
Reviewing this would likely be simpler if addition of the controller
was split from the addition of the disk.
[...]
> @@ -2563,6 +2565,7 @@ virDomainControllerDefNew(virDomainControllerType type)
> case VIR_DOMAIN_CONTROLLER_TYPE_SATA:
> case VIR_DOMAIN_CONTROLLER_TYPE_CCID:
> case VIR_DOMAIN_CONTROLLER_TYPE_ISA:
> +case VIR_DOMAIN_CONTROLLER_TYPE_NVME:
> case VIR_DOMAIN_CONTROLLER_TYPE_LAST:
> break;
> }
[]
> @@ -8832,6 +8845,8 @@ virDomainControllerDefParseXML(virDomainXMLOption
> *xmlopt,
> int ntargetNodes = 0;
> g_autofree xmlNodePtr *modelNodes = NULL;
> int nmodelNodes = 0;
> +g_autofree xmlNodePtr *serialNodes = NULL;
> +int nserialNodes = 0;
> int numaNode = -1;
> int ports;
> VIR_XPATH_NODE_AUTORESTORE(ctxt)
> @@ -8969,6 +8984,18 @@ virDomainControllerDefParseXML(virDomainXMLOption
> *xmlopt,
> if (virXMLPropInt(node, "ports", 10, VIR_XML_PROP_NONNEGATIVE, &ports,
> -1) < 0)
> return NULL;
>
> +if ((nserialNodes = virXPathNodeSet("./serial", ctxt, &serialNodes)) >
> 1) {
> +virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("Multiple elements in controller
> definition not allowed"));
We ususally defer validation of minor infractions such as this to the
schema rather than having code for it.
> +return NULL;
> +}
> +
> +if (nserialNodes == 1) {
> +if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_NVME) {
You have a switch statement checking type right below ...
> + def->opts.nvmeopts.serial =
> virXMLNodeContentString(serialNodes[0]);
> +}
> +}
> +
> switch (def->type) {
> case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: {
> if (virXMLPropInt(node, "vectors", 10, VIR_XML_PROP_NONNEGATIVE,
> @@ -9054,6 +9081,7 @@ virDomainControllerDefParseXML(virDomainXMLOption
> *xmlopt,
> case VIR_DOMAIN_CONTROLLER_TYPE_SATA:
> case VIR_DOMAIN_CONTROLLER_TYPE_CCID:
> case VIR_DOMAIN_CONTROLLER_TYPE_ISA:
> +case VIR_DOMAIN_CONTROLLER_TYPE_NVME:
... so remove all of the above for:
def->opts.nvmeopts.serial = virXPathString("string(./serial)", ctxt);
> case VIR_DOMAIN_CONTROLLER_TYPE_LAST:
> default:
> break;
[...]
> @@ -24028,6 +24060,12 @@ virDomainControllerDefFormat(virBuffer *buf,
> }
> break;
>
> +case VIR_DOMAIN_CONTROLLER_TYPE_NVME:
> +if (def->opts.nvmeopts.serial != NULL) {
> +virBufferAsprintf(&childBuf, "%s\n",
> def->opts.nvmeopts.serial);
User provided strings must be formatted using 'virBufferEscapeString'
to ensure proper XML entity escaping. That function also skips the
formatting if the string argument is NULL so no check will be needed.
> +}
> +break;
> +
> case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
> if (virDomainControllerDefFormatPCI(&childBuf, def, flags) < 0)
> return -1;
[...]
> @@ -766,6 +768,10 @@ struct _virDomainXenbusControllerOpts {
> int maxEventChannels; /* -1 == undef */
> };
>
> +struct _virDomainNVMeControllerOpts {
> +char *serial;
I don't see a corresponding change to free this field so it will likely
be leaked.
> +};
> +
> /* Stores the virtual disk controller configuration */
> struct _virDomainControllerDef {
> virDomainControllerType type;
[...]
> diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
> index d0d4bc0bf4..1ad8350117 100644
> --- a/src/conf/domain_validate.c
> +++ b/src/conf/domain_validate.c
> @@ -267,6 +267,7 @@ virDomainDiskAddressDiskBusCompatibility(virDomainDiskBus
> bus,
> case VIR_DOMAIN_DISK_BUS_UML:
> case VIR_DOMAIN_DISK_BUS_SD:
> case VIR_DOMAIN_DISK_BUS_NONE:
> +case VIR_DOMAIN_DISK_BUS_NVME_NS:
Since you are using a 'drive' address type here I t
Re: [PATCHv2 2/5] qemu_capabilities: Add support for nvme-ns bus capabilities
On Sun, Apr 27, 2025 at 19:48:04 +0800, honglei.w...@smartx.com wrote: > From: ray > > Signed-off-by: ray > --- Generally patches adding a capability flag should only add the capability flag. Thus you'll need to split this patch. 1) capability addition - move it ahead to the beginning of the series (doing that will allow you to do capability checks when adding the code instead when adding the capability - note the full flag name (QEMU_CAPS...) in the summary line of the commit message 2) domain caps addition - that ought to be done after the code addition is compelete, thus after you add the XML schema bits and implementation > src/qemu/qemu_capabilities.c | 5 + > src/qemu/qemu_capabilities.h | 1 + > src/qemu/qemu_validate.c | 6 ++ > tests/domaincapsdata/qemu_10.0.0-q35.x86_64+amdsev.xml | 1 + > tests/domaincapsdata/qemu_10.0.0-q35.x86_64.xml | 1 + > tests/domaincapsdata/qemu_10.0.0-tcg.x86_64+amdsev.xml | 1 + > tests/domaincapsdata/qemu_10.0.0-tcg.x86_64.xml | 1 + > tests/domaincapsdata/qemu_10.0.0.s390x.xml | 1 + > tests/domaincapsdata/qemu_10.0.0.x86_64+amdsev.xml | 1 + > tests/domaincapsdata/qemu_10.0.0.x86_64.xml | 1 + > tests/domaincapsdata/qemu_4.2.0-virt.aarch64.xml | 1 + > tests/domaincapsdata/qemu_4.2.0.aarch64.xml | 1 + > tests/domaincapsdata/qemu_4.2.0.ppc64.xml| 1 + > tests/domaincapsdata/qemu_5.0.0-tcg-virt.riscv64.xml | 1 + > tests/domaincapsdata/qemu_5.0.0-virt.aarch64.xml | 1 + > tests/domaincapsdata/qemu_5.0.0-virt.riscv64.xml | 1 + > tests/domaincapsdata/qemu_5.0.0.aarch64.xml | 1 + > tests/domaincapsdata/qemu_5.0.0.ppc64.xml| 1 + > tests/domaincapsdata/qemu_5.1.0.sparc.xml| 1 + All of the above is suspicious. The test files were forgotten after we dropped support for the qemu versions mentioned in them. How did you modify these? Interestingly there are more of the files for unsupported versions but you didnt modify all of them. Anyways I'll send a patch deleting those. The rest of the patch looks good but most of the code additions will need to be moved to corresponding patches.
Re: [RFC PATCH 0/3] single-binary: make QAPI generated files common
On 4/25/25 08:38, Markus Armbruster wrote:
Pierrick Bouvier writes:
Note: This RFC was posted to trigger a discussion around this topic, and it's
not expected to merge it as it is.
Context
===
Linaro is working towards heterogeneous emulation, mixing several architectures
in a single QEMU process. The first prerequisite is to be able to build such a
binary, which we commonly name "single-binary" in our various series.
An (incomplete) list of series is available here:
https://patchew.org/search?q=project%3AQEMU+single-binary
We don't expect to change existing command line interface or any observable
behaviour, it should be identical to existing binaries. If anyone notices a
difference, it will be a bug.
Define "notice a difference" :) More on that below.
Given a single-binary *named* exactly like an existing qemu-system-X
binary, any user or QEMU management layer should not be able to
distinguish it from the real qemu-system-X binary.
The new potential things will be:
- introduction of an (optional) -target option, which allows to
override/disambiguate default target detected.
- potentially more boards/cpus/devices visible, once we start developing
heterogeneous emulation. See it as a new CONFIG_{new_board} present.
Out of that, once the current target is identified, based on argv[0],
there should be absolutely no difference, whether in the behaviour, UI,
command line, or the monitor interfaces.
Maybe (i.e. probably) one day people will be interested to create a new
shiny command line for heteregenous scenarios, but for now, this is
*not* the goal we pursue. We just want to be able to manually define a
board mixing two different cpu architectures, without reinventing all
the wheels coming with that. Once everything is ready (and not before),
it will be a good time to revisit the command line interface to reflect
this. Definitely a small task compared to all we have left to do now.
Finally, even if we decide to do those changes, I think they should be
reflected on both existing binaries and the new single-binary. It would
be a mistake to create "yet another" way to use QEMU, just because we
have N architectures available instead of one.
The first objective we target is to combine qemu-system-arm and
qemu-system-aarch64 in a single binary, showing that we can build and link such
a thing. While being useless from a feature point of view, it allows us to make
good progress towards the goal, and unify two "distinct" architectures, and gain
experience on problems met.
Makes sense to me.
Our current approach is to remove compilation units duplication to be able to
link all object files together. One of the concerned subsystem is QAPI.
QAPI
QAPI generated files contain conditional clauses to define various structures,
enums, and commands only for specific targets. This forces files to be
compiled for every target.
To be precise: conditionals that use macros restricted to
target-specific code, i.e. the ones poisoned by exec/poison.h. Let's
call them target-specific QAPI conditionals.
The QAPI generator is blissfully unaware of all this.
Indeed, the only thing QAPI generaor is aware of is that it's a compile
time definition, since it implements those with #if, compared to a
runtime check.
The build system treats QAPI modules qapi/*-target.json as
target-specific. The .c files generated for them are compiled per
target. See qapi/meson.build.
Only such target-specific modules can can use target-specific QAPI
conditionals. Use in target-independent modules will generate C that
won't compile.
Poisoned macros used in qapi/*-target.json:
CONFIG_KVM
TARGET_ARM
TARGET_I386
TARGET_LOONGARCH64
TARGET_MIPS
TARGET_PPC
TARGET_RISCV
TARGET_S390X
What we try to do here is to build them only once
instead.
You're trying to eliminate target-specific QAPI conditionals. Correct?
Yes, but without impacting the list of commands exposed. Thus, it would
be needed to select at runtime to expose/register commands.
In the past, we identied that the best approach to solve this is to expose code
for all targets (thus removing all #if clauses), and stub missing
symbols for concerned targets.
This affects QAPI/QMP introspection, i.e. the value of query-qmp-schema.
Management applications can no longer use introspection to find out
whether target-specific things are available.
As asked on my previous email answering Daniel, would that be possible
to build the schema dynamically, so we can decide what to expose or not
introspection wise?
For instance, query-cpu-definitions is implemented for targets arm,
i386, loongarch, mips, ppc, riscv, and s390x. It initially was for
fewer targets, and more targets followed one by one. Still more may
follow in the future. Right now, management applications can use
introspection to find out whether it is available. That stops working
when you make i
Re: [RFC PATCH 0/3] single-binary: make QAPI generated files common
On 4/25/25 14:07, Pierrick Bouvier wrote: QAPI/QMP introspection has all commands and events, and all types reachable from them. query-qmp-schema returns an array, where each array element describes one command, event, or type. When a command, event, or type is conditional in the schema, the element is wrapped in the #if generated for the condition. After reading and answering to your valuable email, I definitely think the introspection schema we expose should be adapted, independently of how we build QAPI code (i.e. using #ifdef TARGET or not). Is it something technically hard to achieve? From existing json format, we could imagine to change semantic of "if" field to mean "expose in the schema, and register associated commands", instead "introduce ifdefs around this". QAPI generator would not have to know about what is inside the ifs, simply calling expression passed as value, and condition command registering and schema exposition with that.
[PATCH] po/zh_CN.po: Fix some translation issues
From: QiangWei Zhang Swap the order of the two objects to ensure that the logic of the two objects translated into Chinese is correct. Signed-off-by: QiangWei Zhang --- po/zh_CN.po | 28 ++-- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/po/zh_CN.po b/po/zh_CN.po index bf04782f88..73f7aa0549 100644 --- a/po/zh_CN.po +++ b/po/zh_CN.po @@ -5802,11 +5802,11 @@ msgstr "??? '%1$s' ???" #, c-format msgid "Domain '%1$s' created from %2$s\n" -msgstr "??? %1$s '%2$s'\n" +msgstr "??? %2$s '%1$s'\n" #, c-format msgid "Domain '%1$s' defined from %2$s\n" -msgstr "??? %1$s '%2$s'\n" +msgstr "??? %2$s '%1$s'\n" #, c-format msgid "Domain '%1$s' destroyed\n" @@ -8770,7 +8770,7 @@ msgstr " '%1$s' ??? 0 " #, c-format msgid "Failed to unbind PCI device '%1$s' from %2$s" -msgstr "??? %1$s ??? PCI ?? '%2$s' ??" +msgstr "??? %2$s ??? PCI ?? '%1$s' ??" #, c-format msgid "Failed to undefine bridge interface %1$s" @@ -9798,7 +9798,7 @@ msgstr "? %1$s XML ?\n" #, c-format msgid "Interface %1$s defined from %2$s\n" -msgstr "?? %1$s ??? %2$s\n" +msgstr "?? %2$s ??? %1$s\n" #, c-format msgid "Interface %1$s destroyed\n" @@ -12240,11 +12240,11 @@ msgstr "??? %1$s XML ??\n" #, c-format msgid "Network %1$s created from %2$s\n" -msgstr "???%1$s%2$s\n" +msgstr "???%2$s%1$s\n" #, c-format msgid "Network %1$s defined from %2$s\n" -msgstr "??? %1$s%2$s\n" +msgstr "??? %2$s%1$s\n" #, c-format msgid "Network %1$s destroyed\n" @@ -12325,7 +12325,7 @@ msgstr "??? %1$s XML ??\n" #, c-format msgid "Network filter %1$s defined from %2$s\n" -msgstr "?? %1$s %2$s\n" +msgstr "?? %2$s %1$s\n" #, c-format msgid "Network filter %1$s undefined\n" @@ -12340,7 +12340,7 @@ msgstr "?%1$s" #, c-format msgid "Network filter binding on %1$s created from %2$s\n" -msgstr "???%1$s ? %2$s ???\n" +msgstr "???%2$s ? %1$s ???\n" #, c-format msgid "Network filter binding on %1$s deleted\n" @@ -12376,7 +12376,7 @@ msgstr "???: %1$s" #, c-format msgid "Network port %1$s created from %2$s\n" -msgstr "??? %1$s ? %2$s\n" +msgstr "??? %2$s ? %1$s\n" #, c-format msgid "Network port %1$s deleted\n" @@ -12805,7 +12805,7 @@ msgstr "??" #, c-format msgid "Node device %1$s created from %2$s\n" -msgstr "?? %1$s ? %2$s\n" +msgstr "?? %2$s ? %1$s\n" #, c-format msgid "Node device '%1$s' defined from '%2$s'\n" @@ -13609,7 +13609,7 @@ msgstr "? %1$s\n" #, c-format msgid "Pool %1$s created from %2$s\n" -msgstr "??? %1$s %2$s\n" +msgstr "??? %2$s %1$s\n" #, c-format msgid "Pool %1$s defined\n" @@ -13617,7 +13617,7 @@ msgstr "? %1$s\n" #, c-format msgid "Pool %1$s defined from %2$s\n" -msgstr "??? %1$s %2$s\n" +msgstr "??? %2$s %1$s\n" #, c-format msgid "Pool %1$s deleted\n" @@ -20571,7 +20571,7 @@ msgstr "???" #, c-format msgid "Vol %1$s cloned from %2$s\n" -msgstr "??? %1$s %2$s\n" +msgstr "??? %2$s %1$s\n" #, c-format msgid "Vol %1$s created\n" @@ -20579,7 +20579,7 @@ msgstr "??? %1$s ?\n" #, c-format msgid "Vol %1$s created from %2$s\n" -msgstr "??? %1$s %2$s\n" +msgstr "??? %2$s %1$s\n" #, c-format msgid "Vol %1$s created from input vol %2$s\n" -- 2.27.0
Re: [RFC PATCH 0/3] single-binary: make QAPI generated files common
On 4/25/25 11:21 PM, Markus Armbruster wrote:
Trouble is some uses of the second kind are in QAPI conditionals. I can
see three options:
(1) Drop these conditionals.
(2) Replace them by run-time checks.
(3) Have target-specific QAPI-generated code for multiple targets
coexist in the single binary.
As far as I can tell, your RFC series is an incomplete attempt at (2).
I gather you considered (3), but you dislike it for its bloat and
possibly other reasons. I sympathize; the QAPI-generated code is plenty
bloated as it is, in good part to early design decisions (not mine).
Your "no noticeable differences" goal precludes (1).
Back to (2). In C, replacing compile-time conditionals by run-time
checks means replacing #if FOO by if (foo). Such a transformation isn't
possible in the QAPI schema. To make it possible, we need to evolve the
QAPI schema language.
docs/devel/qapi-code-gen.rst describes what we have:
COND = STRING
| { 'all: [ COND, ... ] }
| { 'any: [ COND, ... ] }
| { 'not': COND }
[]
The C code generated for the definition will then be guarded by an #if
preprocessing directive with an operand generated from that condition:
* STRING will generate defined(STRING)
* { 'all': [COND, ...] } will generate (COND && ...)
* { 'any': [COND, ...] } will generate (COND || ...)
* { 'not': COND } will generate !COND
So, conditions are expression trees where the leaves are preprocessor
symbols and the inner nodes are operators.
It's not quite obvious to me how to best evolve this to support run-time
checks.
After looking at the introspection code, I don't see any major blocker.
We need to keep some of existing "if", as they are based on config-host,
and should apply.
We can introduce a new "available_if" (or any other name), which
generates a runtime check when building the schema, or when serializing
a struct.
This way, by modifying the .json with:
- if: 'TARGET_I386'
+ available_if: 'target_i386()'
This way, we keep the possibility to have ifdef, and we can expose at
runtime based on available_if. So we can keep the exact same schema we
have today per target.
Whatever we choose should support generating Rust and Go as well. Why?
Rust usage in QEMU is growing, and we'll likely need to generate some
Rust from the QAPI schema. Victor Toso has been working on Go bindings
for use in Go QMP client software.
I don't see any blocker with that. If you mention generating Rust and Go
from qapi json definitions, it's already dependent on C preprocessor
because of ifdef constant. So it will have to be adapted anyway.
Having the same function (target_i386()) name through different
languages is not something hard to achieve.
The build system treats QAPI modules qapi/*-target.json as
target-specific. The .c files generated for them are compiled per
target. See qapi/meson.build.
Only such target-specific modules can can use target-specific QAPI
conditionals. Use in target-independent modules will generate C that
won't compile.
Poisoned macros used in qapi/*-target.json:
CONFIG_KVM
TARGET_ARM
TARGET_I386
TARGET_LOONGARCH64
TARGET_MIPS
TARGET_PPC
TARGET_RISCV
TARGET_S390X
What we try to do here is to build them only once
instead.
You're trying to eliminate target-specific QAPI conditionals. Correct?
Yes, but without impacting the list of commands exposed. Thus, it would
be needed to select at runtime to expose/register commands.
Conditionals affect more than just commands.
Thus, the proposal above to do the same for concerned struct members.
In the past, we identied that the best approach to solve this is to expose code
for all targets (thus removing all #if clauses), and stub missing
symbols for concerned targets.
This affects QAPI/QMP introspection, i.e. the value of query-qmp-schema.
Management applications can no longer use introspection to find out
whether target-specific things are available.
As asked on my previous email answering Daniel, would that be possible
to build the schema dynamically, so we can decide what to expose or not
introspection wise?
QAPI was designed to be compile-time static. Revising such fundamental
design assumptions is always fraught. I can't give you a confident
assessment now. All I can offer you is my willingness to explore
solutions. See "really fancy" below.
Fun fact: we used to generate the value of query-qmp-schema as a single
string. We switched to the current, more bloated representation to
support conditionals (commit 7d0f982bfbb).
It's nice to have this, and this is what would allow us to
conditionnally include or not various definitions/commands/fields. I was
a bit worried we would have a "static string", but was glad to find a
static list instead.
For instance, query-cpu-definitions is implemented for targets arm,
i386, loongarch
Re: [PATCH V1 0/6] fast qom tree get
On 4/28/2025 4:04 AM, Markus Armbruster wrote: Steven Sistare writes: On 4/9/2025 3:39 AM, Markus Armbruster wrote: Hi Steve, I apologize for the slow response. Steve Sistare writes: Using qom-list and qom-get to get all the nodes and property values in a QOM tree can take multiple seconds because it requires 1000's of individual QOM requests. Some managers fetch the entire tree or a large subset of it when starting a new VM, and this cost is a substantial fraction of start up time. "Some managers"... could you name one? My personal experience is with Oracle's OCI, but likely others could benefit. Elsewhere in this thread, we examined libvirt's use qom-get. Its use of qom-get is also noticably slow, and your work could speed it up. However, most of its use is for working around QMP interface shortcomings around probing CPU flags. Addressing these would help it even more. This makes me wonder what questions Oracle's OCI answers with the help of qom-get. Can you briefly describe them? Even if OCI would likewise be helped more by better QMP queries, your fast qom tree get work might still be useful. We already optimized our queries as a first step, but what remains is still significant, which is why I submitted this RFE. - Steve
Entering freeze for libvirt-11.3.0
I have just tagged v11.3.0-rc1 in the repository and pushed signed tarballs to https://download.libvirt.org/ Please give the release candidate some testing and in case you find a serious issue which should have a fix in the upcoming release, feel free to reply to this thread to make sure the issue is more visible. If you have not done so yet, please update NEWS.rst to document any significant change you made since the last release. Thanks, Jirka
Re: [RFC PATCH 0/3] single-binary: make QAPI generated files common
On Fri, Apr 25, 2025 at 17:38:44 +0200, Markus Armbruster via Devel wrote:
> Pierrick Bouvier writes:
[...]
> To be precise: conditionals that use macros restricted to
> target-specific code, i.e. the ones poisoned by exec/poison.h. Let's
> call them target-specific QAPI conditionals.
>
> The QAPI generator is blissfully unaware of all this.
>
> The build system treats QAPI modules qapi/*-target.json as
> target-specific. The .c files generated for them are compiled per
> target. See qapi/meson.build.
>
> Only such target-specific modules can can use target-specific QAPI
> conditionals. Use in target-independent modules will generate C that
> won't compile.
>
> Poisoned macros used in qapi/*-target.json:
>
> CONFIG_KVM
> TARGET_ARM
> TARGET_I386
> TARGET_LOONGARCH64
> TARGET_MIPS
> TARGET_PPC
> TARGET_RISCV
> TARGET_S390X
I've had a look at what bits of the QMP schema are depending on the
above defines which libvirt uses.
In many cases libvirt could restrict the use of given command/property
to only supported architectures. We decided to simply probe the presence
of the command because it's convenient to not have to filter them any
more
- query-gic-capabilities
- libvirt already calls this only for ARM guests based on the
definition
- query-sev and friends
- libvirt uses presence of 'query-sev' to decide if the binary
supports it; patching in a platofrm check is possible although
inconvenient
- query-sgx and friends
- similar to sev
-query-cpu-definitions and friends
- see below
>
> >What we try to do here is to build them only once
> instead.
>
> You're trying to eliminate target-specific QAPI conditionals. Correct?
>
> > In the past, we identied that the best approach to solve this is to expose
> > code
> > for all targets (thus removing all #if clauses), and stub missing
> > symbols for concerned targets.
>
> This affects QAPI/QMP introspection, i.e. the value of query-qmp-schema.
>
> Management applications can no longer use introspection to find out
> whether target-specific things are available.
Indeed and libvirt already uses this in few cases as noded above.
>
> For instance, query-cpu-definitions is implemented for targets arm,
> i386, loongarch, mips, ppc, riscv, and s390x. It initially was for
> fewer targets, and more targets followed one by one. Still more may
> follow in the future. Right now, management applications can use
> introspection to find out whether it is available. That stops working
> when you make it available for all targets, stubbed out for the ones
> that don't (yet) implement it.
>
> Management applications may have to be adjusted for this.
>
> This is not an attempt to shoot down your approach. I'm merely
> demonstrating limitations of your promise "if anyone notices a
> difference, it will be a bug."
>
> Now, we could get really fancy and try to keep introspection the same by
> applying conditionals dynamically somehow. I.e. have the single binary
> return different introspection values depending on the actual guest's
> target.
I wonder how this will work if libvirt is probing a binary. Libvirt does
not look at the filename. It can't because it can be a
user-specified/compiled binary, override script, or a distro that chose
to rename the binary.
The second thing that libvirt does after 'query-version' is
'query-target'.
So what should libvirt do once multiple targets are supported?
How do we query CPUs for each of the supported targets?
Will the result be the same if we query them one at a time or all at
once?
> This requires fixing the target before introspection. Unless this is
> somehow completely transparent (wrapper scripts, or awful hacks based on
> the binary's filename, perhaps), management applications may have to be
> adjusted to actually do that.
As noted filename will not work. Users can specify any filename and
create override scripts or rename the binary.
>
> Applies not just to introspection. Consider query-cpu-definitions
> again. It currently returns CPU definitions for *the* target. What
> would a single binary's query-cpu-definitions return? The CPU
> definitions for *all* its targets? Management applications then receive
> CPUs that won't work, which may upset them. To avoid noticable
> difference, we again have to fix the target before we look.
Ah I see you had a similar question :D
>
> Of course, "fixing the target" stops making sense once we move to
> heterogeneous machines with multiple targets.
>
> > This series build QAPI generated code once, by removing all TARGET_{arch}
> > and
> > CONFIG_KVM clauses. What it does *not* at the moment is:
> > - prevent target specific commands to be visible for all targets
> > (see TODO comment on patch 2 explaining how to address this)
> > - nothing was done to hide all this from generated documentation
Re: [PATCH] domaincapstest: Remove XMLs for already dropped qemu versions (4.2.0 - 5.1.0)
On a Monday in 2025, Peter Krempa via Devel wrote: From: Peter Krempa The files were forgotten after the previous bump to use qemu-5.2 as minimum. The data for qemu-5.2, qemu-6.0, and qemu-6.1 was already removed when bumping to qemu-6.2. Signed-off-by: Peter Krempa --- .../domaincapsdata/qemu_4.2.0-q35.x86_64.xml | 329 - .../domaincapsdata/qemu_4.2.0-tcg.x86_64.xml | 274 --- .../qemu_4.2.0-virt.aarch64.xml | 206 --- tests/domaincapsdata/qemu_4.2.0.aarch64.xml | 206 --- tests/domaincapsdata/qemu_4.2.0.ppc64.xml | 174 - tests/domaincapsdata/qemu_4.2.0.s390x.xml | 280 --- tests/domaincapsdata/qemu_4.2.0.x86_64.xml| 329 - .../domaincapsdata/qemu_5.0.0-q35.x86_64.xml | 331 -- .../qemu_5.0.0-tcg-virt.riscv64.xml | 159 - .../domaincapsdata/qemu_5.0.0-tcg.x86_64.xml | 276 --- .../qemu_5.0.0-virt.aarch64.xml | 219 .../qemu_5.0.0-virt.riscv64.xml | 162 - tests/domaincapsdata/qemu_5.0.0.aarch64.xml | 219 tests/domaincapsdata/qemu_5.0.0.ppc64.xml | 181 -- tests/domaincapsdata/qemu_5.0.0.x86_64.xml| 331 -- .../domaincapsdata/qemu_5.1.0-q35.x86_64.xml | 263 -- .../domaincapsdata/qemu_5.1.0-tcg.x86_64.xml | 276 --- tests/domaincapsdata/qemu_5.1.0.sparc.xml | 145 tests/domaincapsdata/qemu_5.1.0.x86_64.xml| 263 -- 19 files changed, 4623 deletions(-) delete mode 100644 tests/domaincapsdata/qemu_4.2.0-q35.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_4.2.0-tcg.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_4.2.0-virt.aarch64.xml delete mode 100644 tests/domaincapsdata/qemu_4.2.0.aarch64.xml delete mode 100644 tests/domaincapsdata/qemu_4.2.0.ppc64.xml delete mode 100644 tests/domaincapsdata/qemu_4.2.0.s390x.xml delete mode 100644 tests/domaincapsdata/qemu_4.2.0.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_5.0.0-q35.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_5.0.0-tcg-virt.riscv64.xml delete mode 100644 tests/domaincapsdata/qemu_5.0.0-tcg.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_5.0.0-virt.aarch64.xml delete mode 100644 tests/domaincapsdata/qemu_5.0.0-virt.riscv64.xml delete mode 100644 tests/domaincapsdata/qemu_5.0.0.aarch64.xml delete mode 100644 tests/domaincapsdata/qemu_5.0.0.ppc64.xml delete mode 100644 tests/domaincapsdata/qemu_5.0.0.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_5.1.0-q35.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_5.1.0-tcg.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_5.1.0.sparc.xml delete mode 100644 tests/domaincapsdata/qemu_5.1.0.x86_64.xml Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
[PATCH 1/3] internal: Introduce ATTRIBUTE_NOIPA
From: Michal Privoznik
Currently, if we want to mock a function the noinline attribute
is appended after the function (via G_NO_INLINE macro). This used
to work for non pure functions. But there are some trivial
functions (for instance virQEMUCapsProbeHVF()) that are pure,
i.e. have no side effect, and while their call from other parts
of the code is not optimized out, their call from within the same
compilation unit (qemu_capabilities.c) is optimized out.
This is because inlining and semantic interposition are two
different things. Even GCC's documentation for noinline attribute
[1] states that clearly:
This function attribute prevents a function from being
considered for inlining. It also disables some other
interprocedural optimizations; it’s preferable to use the more
comprehensive noipa attribute instead if that is your goal.
Even if a function is declared with the noinline attribute,
there are optimizations other than inlining that can cause
calls to be optimized away if it does not have side effects,
although the function call is live.
Unfortunately, despite attempts [2] Clang still does not support
the attribute and thus we have to rely on noinline +
-fsemantic-interposition combo.
1:
https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-noinline-function-attribute
2: https://reviews.llvm.org/D101011
Signed-off-by: Michal Privoznik
---
docs/coding-style.rst | 2 +-
scripts/cocci-macro-file.h | 1 +
src/internal.h | 21 +
3 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/docs/coding-style.rst b/docs/coding-style.rst
index fe5fe9a906..81382f11d4 100644
--- a/docs/coding-style.rst
+++ b/docs/coding-style.rst
@@ -633,7 +633,7 @@ analysis tools understand the code better:
``G_GNUC_FALLTHROUGH``
allow code reuse by multiple switch cases
-``G_NO_INLINE``
+``ATTRIBUTE_NOIPA``
the function is mocked in the test suite
``G_GNUC_NORETURN``
diff --git a/scripts/cocci-macro-file.h b/scripts/cocci-macro-file.h
index c3112663d1..b26018c20b 100644
--- a/scripts/cocci-macro-file.h
+++ b/scripts/cocci-macro-file.h
@@ -21,6 +21,7 @@
#pragma once
+#define ATTRIBUTE_NOIPA
#define ATTRIBUTE_NONNULL(x)
#define ATTRIBUTE_PACKED
diff --git a/src/internal.h b/src/internal.h
index 20aa9b1d41..b96661249e 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -177,6 +177,27 @@
# endif
#endif
+/**
+ *
+ * ATTRIBUTE_NOIPA
+ *
+ * Force compiler to disable interprocedural optimizations between the
+ * function with this attribute and its callers. This implies noinline
+ * attribute and some others and allows us to mock functions even if
+ * they are pure.
+ *
+ * WARNING: on compilers which don't support the attribute (clang) this
+ * is silently declared as noinline which in combination with
+ * -fsemantic-interposition option does roughly the same.
+ */
+#ifndef ATTRIBUTE_NOIPA
+# if defined(__has_attribute) && __has_attribute(noipa)
+# define ATTRIBUTE_NOIPA __attribute__((noipa))
+# else
+# define ATTRIBUTE_NOIPA G_NO_INLINE
+# endif
+#endif
+
#define VIR_WARNINGS_NO_CAST_ALIGN \
_Pragma ("GCC diagnostic push") \
_Pragma ("GCC diagnostic ignored \"-Wcast-align\"")
--
2.49.0
[PATCH 3/3] scripts: Adapt mock-noinline.py to ATTRIBUTE_NOIPA
From: Michal Privoznik
The script is renamed to mock-noipa.py and adjusted to check for
the new attribute.
Signed-off-by: Michal Privoznik
---
build-aux/syntax-check.mk | 4 ++--
scripts/meson.build | 2 +-
scripts/{mock-noinline.py => mock-noipa.py} | 6 +++---
3 files changed, 6 insertions(+), 6 deletions(-)
rename scripts/{mock-noinline.py => mock-noipa.py} (93%)
diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index d414e033df..16a28241db 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -1326,9 +1326,9 @@ sc_spacing-check:
$(PERL) $(top_srcdir)/build-aux/check-spacing.pl || \
{ echo 'incorrect formatting' 1>&2; exit 1; }
-sc_mock-noinline:
+sc_mock-noipa:
$(AM_V_GEN)$(VC_LIST_EXCEPT) | $(GREP) '\.[ch]$$' | $(RUNUTF8) \
- $(PYTHON) $(top_srcdir)/scripts/mock-noinline.py
+ $(PYTHON) $(top_srcdir)/scripts/mock-noipa.py
sc_header-ifdef:
$(AM_V_GEN)$(VC_LIST_EXCEPT) | $(GREP) '\.[h]$$' | $(RUNUTF8) xargs \
diff --git a/scripts/meson.build b/scripts/meson.build
index 2798e302ab..bc38277dbf 100644
--- a/scripts/meson.build
+++ b/scripts/meson.build
@@ -29,7 +29,7 @@ scripts = [
'meson-install-web.py',
'meson-python.sh',
'meson-timestamp.py',
- 'mock-noinline.py',
+ 'mock-noipa.py',
'prohibit-duplicate-header.py',
'qemu-replies-tool.py',
]
diff --git a/scripts/mock-noinline.py b/scripts/mock-noipa.py
similarity index 93%
rename from scripts/mock-noinline.py
rename to scripts/mock-noipa.py
index 77a5ca23e2..66a5f38bdc 100755
--- a/scripts/mock-noinline.py
+++ b/scripts/mock-noipa.py
@@ -22,7 +22,7 @@ import sys
noninlined = {}
mocked = {}
-# Functions in public header don't get the noinline annotation
+# Functions in public header don't get the noipa annotation
noninlined["virEventAddTimeout"] = True
# This one confuses the script as its defined in the mock file
# but is actually just a local helper
@@ -43,7 +43,7 @@ def scan_annotations(filename):
elif line.isspace():
func = None
-if "G_NO_INLINE" in line:
+if "ATTRIBUTE_NOIPA" in line:
if func is not None:
noninlined[func] = True
@@ -74,7 +74,7 @@ warned = False
for func in mocked.keys():
if func not in noninlined:
warned = True
-print("%s is mocked at %s but missing 'G_NO_INLINE' annotation" %
+print("%s is mocked at %s but missing 'ATTRIBUTE_NOIPA' annotation" %
(func, mocked[func]), file=sys.stderr)
if warned:
--
2.49.0
[PATCH 0/3] Preper noipa attribute over noinline
Somewhat green-ish pipeline:
https://gitlab.com/MichalPrivoznik/libvirt/-/pipelines/1790043585
Jobs that failed are unrelated.
Thing is, our upstream pipeline started failing on Rawhide + gcc because
of IPA. The unfortunate part is, noinline does not guarantee the
function is mockable. The fortunate part is, GCC has noipa attribute
which is even advocated for in its documentation.
Given we are after the freeze and this is potentially hazardous, I'm
okay if this is merged after the release.
Michal Prívozník (3):
internal: Introduce ATTRIBUTE_NOIPA
src: s/G_NO_INLINE/ATTRIBUTE_NOIPA/
scripts: Adapt mock-noinline.py to ATTRIBUTE_NOIPA
build-aux/syntax-check.mk | 4 ++--
docs/coding-style.rst | 2 +-
scripts/cocci-macro-file.h | 1 +
scripts/meson.build | 2 +-
scripts/{mock-noinline.py => mock-noipa.py} | 6 +++---
src/cpu/cpu.h | 2 +-
src/hypervisor/domain_interface.h | 2 +-
src/internal.h | 21 +
src/libxl/libxl_capabilities.h | 2 +-
src/qemu/qemu_capabilities.h| 4 ++--
src/qemu/qemu_capspriv.h| 2 +-
src/qemu/qemu_command.h | 6 +++---
src/qemu/qemu_hotplug.c | 2 +-
src/qemu/qemu_hotplug.h | 2 +-
src/qemu/qemu_interface.h | 2 +-
src/qemu/qemu_monitor.h | 2 +-
src/qemu/qemu_monitor_json.h| 2 +-
src/qemu/qemu_monitor_priv.h| 2 +-
src/qemu/qemu_process.h | 6 +++---
src/rpc/virnetsocket.h | 4 ++--
src/util/vircgroupv2devices.h | 2 +-
src/util/vircommand.h | 2 +-
src/util/virdevmapper.h | 2 +-
src/util/virfile.h | 20 ++--
src/util/virfirewalld.h | 2 +-
src/util/virhashcode.h | 2 +-
src/util/virhostcpu.h | 8
src/util/virhostmem.h | 2 +-
src/util/virhostuptime.h| 2 +-
src/util/viridentitypriv.h | 2 +-
src/util/virmacaddr.h | 2 +-
src/util/virnetdev.h| 12 ++--
src/util/virnetdevbandwidth.h | 2 +-
src/util/virnetdevip.h | 2 +-
src/util/virnetdevmacvlan.h | 2 +-
src/util/virnetdevopenvswitch.h | 2 +-
src/util/virnetdevtap.h | 6 +++---
src/util/virnuma.h | 18 +-
src/util/virpci.h | 2 +-
src/util/virprocess.h | 6 +++---
src/util/virrandom.h| 4 ++--
src/util/virscsi.h | 2 +-
src/util/virscsivhost.h | 2 +-
src/util/virtpm.h | 2 +-
src/util/virutil.h | 16
src/util/viruuid.h | 4 ++--
46 files changed, 113 insertions(+), 91 deletions(-)
rename scripts/{mock-noinline.py => mock-noipa.py} (93%)
--
2.49.0
[PATCH 2/3] src: s/G_NO_INLINE/ATTRIBUTE_NOIPA/
From: Michal Privoznik Per change in coding style done in previous commit, ATTRIBUTE_NOIPA should be used instead of G_NO_INLINE for functions that are mocked in our test suite. Do the change. Signed-off-by: Michal Privoznik --- src/cpu/cpu.h | 2 +- src/hypervisor/domain_interface.h | 2 +- src/libxl/libxl_capabilities.h| 2 +- src/qemu/qemu_capabilities.h | 4 ++-- src/qemu/qemu_capspriv.h | 2 +- src/qemu/qemu_command.h | 6 +++--- src/qemu/qemu_hotplug.c | 2 +- src/qemu/qemu_hotplug.h | 2 +- src/qemu/qemu_interface.h | 2 +- src/qemu/qemu_monitor.h | 2 +- src/qemu/qemu_monitor_json.h | 2 +- src/qemu/qemu_monitor_priv.h | 2 +- src/qemu/qemu_process.h | 6 +++--- src/rpc/virnetsocket.h| 4 ++-- src/util/vircgroupv2devices.h | 2 +- src/util/vircommand.h | 2 +- src/util/virdevmapper.h | 2 +- src/util/virfile.h| 20 ++-- src/util/virfirewalld.h | 2 +- src/util/virhashcode.h| 2 +- src/util/virhostcpu.h | 8 src/util/virhostmem.h | 2 +- src/util/virhostuptime.h | 2 +- src/util/viridentitypriv.h| 2 +- src/util/virmacaddr.h | 2 +- src/util/virnetdev.h | 12 ++-- src/util/virnetdevbandwidth.h | 2 +- src/util/virnetdevip.h| 2 +- src/util/virnetdevmacvlan.h | 2 +- src/util/virnetdevopenvswitch.h | 2 +- src/util/virnetdevtap.h | 6 +++--- src/util/virnuma.h| 18 +- src/util/virpci.h | 2 +- src/util/virprocess.h | 6 +++--- src/util/virrandom.h | 4 ++-- src/util/virscsi.h| 2 +- src/util/virscsivhost.h | 2 +- src/util/virtpm.h | 2 +- src/util/virutil.h| 16 src/util/viruuid.h| 4 ++-- 40 files changed, 84 insertions(+), 84 deletions(-) diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index fc6a812eaa..5c281b11b8 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -233,7 +233,7 @@ virCPUGetHost(virArch arch, virDomainCapsCPUModels *models); virCPUDef * -virCPUProbeHost(virArch arch) G_NO_INLINE; +virCPUProbeHost(virArch arch) ATTRIBUTE_NOIPA; virCPUDef * virCPUBaseline(virArch arch, diff --git a/src/hypervisor/domain_interface.h b/src/hypervisor/domain_interface.h index b399085f81..2dd5d7165b 100644 --- a/src/hypervisor/domain_interface.h +++ b/src/hypervisor/domain_interface.h @@ -57,4 +57,4 @@ int virDomainInterfaceBridgeConnect(virDomainDef *def, ebtablesContext *ebtables, bool macFilter, const char *bridgeHelperName) -ATTRIBUTE_NONNULL(2) G_NO_INLINE; +ATTRIBUTE_NONNULL(2) ATTRIBUTE_NOIPA; diff --git a/src/libxl/libxl_capabilities.h b/src/libxl/libxl_capabilities.h index fd6332b63e..a0074faf8f 100644 --- a/src/libxl/libxl_capabilities.h +++ b/src/libxl/libxl_capabilities.h @@ -47,4 +47,4 @@ libxlMakeDomainCapabilities(virDomainCaps *domCaps, int libxlDomainGetEmulatorType(const virDomainDef *def) -G_NO_INLINE; +ATTRIBUTE_NOIPA; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index ea7c14daa9..2fb3c9ae93 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -940,10 +940,10 @@ virSGXCapability * virQEMUCapsGetSGXCapabilities(virQEMUCaps *qemuCaps); bool -virQEMUCapsGetKVMSupportsSecureGuest(virQEMUCaps *qemuCaps) G_NO_INLINE; +virQEMUCapsGetKVMSupportsSecureGuest(virQEMUCaps *qemuCaps) ATTRIBUTE_NOIPA; bool -virQEMUCapsProbeHVF(virQEMUCaps *qemuCaps) G_NO_INLINE; +virQEMUCapsProbeHVF(virQEMUCaps *qemuCaps) ATTRIBUTE_NOIPA; virArch virQEMUCapsArchFromString(const char *arch); const char *virQEMUCapsArchToString(virArch arch); diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h index 06b36d2eb8..96964bbfa0 100644 --- a/src/qemu/qemu_capspriv.h +++ b/src/qemu/qemu_capspriv.h @@ -82,7 +82,7 @@ virQEMUCapsGetCPUModelX86Data(virQEMUCaps *qemuCaps, virCPUDef * virQEMUCapsProbeHostCPU(virArch hostArch, -virDomainCapsCPUModels *models) G_NO_INLINE; +virDomainCapsCPUModels *models) ATTRIBUTE_NOIPA; void virQEMUCapsSetGICCapabilities(virQEMUCaps *qemuCaps, diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 79d4f47690..b45986881a 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -75,7 +75,7 @@ qemuBuildTLSx509BackendProps(const char *tlspath, /* Open a UNIX socket for chardev FD passing */ int qemuOpenChrChardevUNIXSocket(const virDomainChrSourceDef *dev) -G_NO_INLINE; +ATTRIBUTE_NOIPA; virJSONValue * qemuBuildChrDeviceProps(const virDomainDef *vmdef, @@ -270,7 +270,7 @@
Re: [PATCH 1/3] internal: Introduce ATTRIBUTE_NOIPA
On Mon, Apr 28, 2025 at 03:20:56PM +0200, Michal Privoznik via Devel wrote: > From: Michal Privoznik > > Currently, if we want to mock a function the noinline attribute > is appended after the function (via G_NO_INLINE macro). This used > to work for non pure functions. But there are some trivial > functions (for instance virQEMUCapsProbeHVF()) that are pure, > i.e. have no side effect, and while their call from other parts > of the code is not optimized out, their call from within the same > compilation unit (qemu_capabilities.c) is optimized out. > > This is because inlining and semantic interposition are two > different things. Even GCC's documentation for noinline attribute > [1] states that clearly: > > This function attribute prevents a function from being > considered for inlining. It also disables some other > interprocedural optimizations; it’s preferable to use the more > comprehensive noipa attribute instead if that is your goal. > > Even if a function is declared with the noinline attribute, > there are optimizations other than inlining that can cause > calls to be optimized away if it does not have side effects, > although the function call is live. > > Unfortunately, despite attempts [2] Clang still does not support > the attribute and thus we have to rely on noinline + > -fsemantic-interposition combo. > > 1: > https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-noinline-function-attribute > 2: https://reviews.llvm.org/D101011 > > Signed-off-by: Michal Privoznik > --- > docs/coding-style.rst | 2 +- > scripts/cocci-macro-file.h | 1 + > src/internal.h | 21 + > 3 files changed, 23 insertions(+), 1 deletion(-) > > diff --git a/docs/coding-style.rst b/docs/coding-style.rst > index fe5fe9a906..81382f11d4 100644 > --- a/docs/coding-style.rst > +++ b/docs/coding-style.rst > @@ -633,7 +633,7 @@ analysis tools understand the code better: > ``G_GNUC_FALLTHROUGH`` > allow code reuse by multiple switch cases > > -``G_NO_INLINE`` > +``ATTRIBUTE_NOIPA`` > the function is mocked in the test suite > > ``G_GNUC_NORETURN`` > diff --git a/scripts/cocci-macro-file.h b/scripts/cocci-macro-file.h > index c3112663d1..b26018c20b 100644 > --- a/scripts/cocci-macro-file.h > +++ b/scripts/cocci-macro-file.h > @@ -21,6 +21,7 @@ > > #pragma once > > +#define ATTRIBUTE_NOIPA > #define ATTRIBUTE_NONNULL(x) > #define ATTRIBUTE_PACKED > > diff --git a/src/internal.h b/src/internal.h > index 20aa9b1d41..b96661249e 100644 > --- a/src/internal.h > +++ b/src/internal.h > @@ -177,6 +177,27 @@ > # endif > #endif > > +/** > + * > + * ATTRIBUTE_NOIPA > + * > + * Force compiler to disable interprocedural optimizations between the > + * function with this attribute and its callers. This implies noinline > + * attribute and some others and allows us to mock functions even if > + * they are pure. > + * > + * WARNING: on compilers which don't support the attribute (clang) this > + * is silently declared as noinline which in combination with > + * -fsemantic-interposition option does roughly the same. > + */ > +#ifndef ATTRIBUTE_NOIPA > +# if defined(__has_attribute) && __has_attribute(noipa) > +# define ATTRIBUTE_NOIPA __attribute__((noipa)) > +# else > +# define ATTRIBUTE_NOIPA G_NO_INLINE Hmm, that's kinda misleading. How about we detach our naming from the impl choice ? ATTRIBUTE_MOCKABLE ? With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH V1 0/6] fast qom tree get
Steven Sistare writes: > On 4/9/2025 3:39 AM, Markus Armbruster wrote: >> Hi Steve, I apologize for the slow response. >> >> Steve Sistare writes: >> >>> Using qom-list and qom-get to get all the nodes and property values in a >>> QOM tree can take multiple seconds because it requires 1000's of individual >>> QOM requests. Some managers fetch the entire tree or a large subset >>> of it when starting a new VM, and this cost is a substantial fraction of >>> start up time. >> >> "Some managers"... could you name one? > > My personal experience is with Oracle's OCI, but likely others could benefit. Elsewhere in this thread, we examined libvirt's use qom-get. Its use of qom-get is also noticably slow, and your work could speed it up. However, most of its use is for working around QMP interface shortcomings around probing CPU flags. Addressing these would help it even more. This makes me wonder what questions Oracle's OCI answers with the help of qom-get. Can you briefly describe them? Even if OCI would likewise be helped more by better QMP queries, your fast qom tree get work might still be useful.
Re: [PATCHv2 3/5] schema: Add nvme controller and nvme-ns bus defination
On Sun, Apr 27, 2025 at 19:48:05 +0800, honglei.w...@smartx.com wrote: > From: ray > > Signed-off-by: ray > --- > src/conf/schemas/domaincommon.rng | 11 ++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/src/conf/schemas/domaincommon.rng > b/src/conf/schemas/domaincommon.rng > index 5597d5a66b..7bdad3c793 100644 > --- a/src/conf/schemas/domaincommon.rng > +++ b/src/conf/schemas/domaincommon.rng > @@ -2518,7 +2518,7 @@ > > > > - name="pattern">(ioemu:)?(fd|hd|sd|vd|xvd|ubd)[a-zA-Z0-9_]+ > + name="pattern">(ioemu:)?(fd|hd|sd|vd|xvd|ubd|nvmens)[a-zA-Z0-9_]+ > > > > @@ -2539,6 +2539,7 @@ > uml > sata > sd > +nvme-ns > > > > @@ -3044,6 +3045,14 @@ > > > > + > + > + nvme > + > + > + > + So here the 'serial' is declared as mandatory. It's optional in the XML parser/formatter and mandatory in the commandline formatter. With other devices it's optional so it's most likely going to need an block. This series is also completely lacking documentation (docs/formatdomain.rst) documenting bot the new controller and disk type.
Re: [PATCH] po/zh_CN.po: Fix some translation issues
On Sun, Apr 27, 2025 at 10:44:20 +0800, liu.son...@zte.com.cn wrote: > From: QiangWei Zhang > > Swap the order of the two objects to ensure that the logic of the > two objects translated into Chinese is correct. Hi, we use Fedora Weblate for translations: https://translate.fedoraproject.org/projects/libvirt/libvirt/ Could you please make the changes there? Thanks, Jirka
Re: [PATCHv2 5/5] NEWS: Document qemu nvme disk emulation feature
On Sun, Apr 27, 2025 at 19:48:07 +0800, honglei.w...@smartx.com wrote: > From: ray > > Signed-off-by: ray > --- > NEWS.rst | 17 + > 1 file changed, 17 insertions(+) > > diff --git a/NEWS.rst b/NEWS.rst > index 3c13a84a1b..938c9ba559 100644 > --- a/NEWS.rst > +++ b/NEWS.rst > @@ -38,6 +38,23 @@ v11.3.0 (unreleased) > At the moment it doesn't provide any new features compared to > , but allows a more flexible configuration. > > + * qemu: Support emulated NVMe disks with other storage backends > + > +Domain XMLs can now include emulated NVMe disks backed by other storage > +backends such as file. > +They are configured with:: > + > + > + > + > + > + > + > + > +nvme-controller-serial-value > + function='0x0'/> > + > + Don't forget to move this to the 11.4 section. the 11.3 release is already in freeze.
Re: [PATCHv2 4/5] tests: Add test case for nvme-ns device configuration
On Sun, Apr 27, 2025 at 19:48:06 +0800, honglei.w...@smartx.com wrote:
> From: ray
>
> Signed-off-by: ray
> ---
> .../disk-nvme-ns-device.x86_64-latest.args | 36 +++
> .../disk-nvme-ns-device.x86_64-latest.xml | 42
> ++
> tests/qemuxmlconfdata/disk-nvme-ns-device.xml | 41 +
> tests/qemuxmlconftest.c| 1 +
> 4 files changed, 120 insertions(+)
> create mode 100644
> tests/qemuxmlconfdata/disk-nvme-ns-device.x86_64-latest.args
> create mode 100644
> tests/qemuxmlconfdata/disk-nvme-ns-device.x86_64-latest.xml
> create mode 100644 tests/qemuxmlconfdata/disk-nvme-ns-device.xml
>
> diff --git a/tests/qemuxmlconfdata/disk-nvme-ns-device.x86_64-latest.args
> b/tests/qemuxmlconfdata/disk-nvme-ns-device.x86_64-latest.args
> new file mode 100644
> index 00..d5971a4407
> --- /dev/null
> +++ b/tests/qemuxmlconfdata/disk-nvme-ns-device.x86_64-latest.args
> @@ -0,0 +1,36 @@
[...]
> +-device
> '{"driver":"nvme","id":"nvme0","serial":"nvme-controller-abcdef","bus":"pci.0","addr":"0x5"}'
> \
> +-blockdev
> '{"driver":"file","filename":"/tmp/data-1.img","node-name":"libvirt-1-storage","read-only":false}'
> \
> +-device
> '{"driver":"nvme-ns","bus":"nvme0","drive":"libvirt-1-storage","id":"nvme-ns0-0-0","bootindex":1}'
> \
Hmm, does bootindex even work here? Shouldn't the bootindex apply to the
controller instead?
[...]
> diff --git a/tests/qemuxmlconfdata/disk-nvme-ns-device.xml
> b/tests/qemuxmlconfdata/disk-nvme-ns-device.xml
> new file mode 100644
> index 00..88bb5956e5
> --- /dev/null
> +++ b/tests/qemuxmlconfdata/disk-nvme-ns-device.xml
> @@ -0,0 +1,41 @@
> +
> + QEMUGuest1
> + c7a5fdbd-edaf-9455-926a-d65c16db1809
> + 219136
> + 219136
> + 1
> +
> +hvm
> +
> +
> +
> +qemu64
> +
> +
> + destroy
> + restart
> + destroy
> +
> +/usr/bin/qemu-system-x86_64
> +
> +
> +
So since the controller type is 'nvme' the value of 'bus' ought to be
'nvme' as well instead of 'nvme-ns'. Same way as we have with 'scsi'.
I also thing that the prefix of dev should be just 'nvme'. Note that the
dev prefix is a libvirt identifier which may (and in this case will not)
be same as in the guest.
Also since the controller supports multiple namespaces please add an
example which does so.
> +
> +
> +
> + function='0x2'/>
> +
> +
> +
> + function='0x0'/>
> +
> +
> + nvme-controller-abcdef
[1]
Indentation of the XML is incorrect here.
> + function='0x0'/>
> +
Also since multiple nvme controllers are possible please add an example
without serial. As noted above at least one of the examples ought to
have multiple namespaces to show the setup.
> +
> +
> +
> +
> +
> +
Re: [PATCH] po/zh_CN.po: Fix some translation issues
On Mon, Apr 28, 2025 at 09:57:50 +0200, Jiri Denemark via Devel wrote: > On Sun, Apr 27, 2025 at 10:44:20 +0800, liu.son...@zte.com.cn wrote: > > From: QiangWei Zhang > > > > Swap the order of the two objects to ensure that the logic of the > > two objects translated into Chinese is correct. > > Hi, we use Fedora Weblate for translations: > > https://translate.fedoraproject.org/projects/libvirt/libvirt/ > > Could you please make the changes there? Specifically, libvirt pulls the translations from weblate unconditionally. Any changes done to the files in the libvirt repo would be overwritten on the next pull. That also means that there is no need to do anything else once updating the translations there.
[PATCH 2/2] scripts: Fix reading list of files in mock-noinline.py
From: Michal Privoznik
The mock-noinline.py script is fed list of files through its
stdin, each file on its own line. Unfortunately, the way the
script is written does nothing as the trailing newline character
prevents any .endswith() match. Strip each line of white spaces.
Signed-off-by: Michal Privoznik
---
scripts/mock-noinline.py | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/scripts/mock-noinline.py b/scripts/mock-noinline.py
index ec617bbc2b..77a5ca23e2 100755
--- a/scripts/mock-noinline.py
+++ b/scripts/mock-noinline.py
@@ -63,7 +63,8 @@ def scan_overrides(filename):
mocked[name] = "%s:%d" % (filename, lineno)
-for filename in sys.stdin.readlines():
+for filename in sys.stdin:
+filename = filename.rstrip()
if filename.endswith(".h"):
scan_annotations(filename)
elif filename.endswith("mock.c"):
--
2.49.0
[PATCH 1/2] util: Add missing G_NO_INLINE annotation
From: Michal Privoznik
There are two functions that are mocked, but are missing required
G_NO_INLINE attribute: virFirewallDIsRegistered() and
virHostCPUGetPhysAddrSize(). Add it.
Signed-off-by: Michal Privoznik
---
src/util/virfirewalld.h | 2 +-
src/util/virhostcpu.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/util/virfirewalld.h b/src/util/virfirewalld.h
index 43803ee89a..2c7d3e22cd 100644
--- a/src/util/virfirewalld.h
+++ b/src/util/virfirewalld.h
@@ -33,7 +33,7 @@ typedef enum {
int virFirewallDGetVersion(unsigned long long *version);
int virFirewallDGetBackend(void);
-int virFirewallDIsRegistered(void);
+int virFirewallDIsRegistered(void) G_NO_INLINE;
int virFirewallDGetZones(char ***zones, size_t *nzones);
int virFirewallDGetPolicies(char ***policies, size_t *npolicies);
bool virFirewallDZoneExists(const char *match);
diff --git a/src/util/virhostcpu.h b/src/util/virhostcpu.h
index 1f47634c33..92db35232b 100644
--- a/src/util/virhostcpu.h
+++ b/src/util/virhostcpu.h
@@ -90,7 +90,7 @@ virHostCPUTscInfo *virHostCPUGetTscInfo(void);
int virHostCPUGetSignature(char **signature);
int virHostCPUGetPhysAddrSize(const virArch hostArch,
- unsigned int *size);
+ unsigned int *size) G_NO_INLINE;
int virHostCPUGetHaltPollTime(pid_t pid,
unsigned long long *haltPollSuccess,
--
2.49.0
[PATCH 0/2] Make mock-noinline.py work again
*** BLURB HERE *** Michal Prívozník (2): util: Add missing G_NO_INLINE annotation scripts: Fix reading list of files in mock-noinline.py scripts/mock-noinline.py | 3 ++- src/util/virfirewalld.h | 2 +- src/util/virhostcpu.h| 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) -- 2.49.0
Re: [RFC PATCH 0/3] single-binary: make QAPI generated files common
On 28/4/25 13:07, Markus Armbruster wrote: Peter Krempa writes: The second thing that libvirt does after 'query-version' is 'query-target'. So what should libvirt do once multiple targets are supported? How do we query CPUs for each of the supported targets? Will the result be the same if we query them one at a time or all at once? Pierrick's stated goal is to have no noticable differences between the single binary and the qemu-system- it covers. This is obviously impossible if we can interact with the single binary before the target is fixed. My naive impression is "management applications" aims mostly for virtualization (likely 1 single target). Heterogeneous (more than 1 target) setups imply some kind of emulation. Are *current* "management applications" interested in managing heterogeneous machines? If so, we could introduce 'query-targets' (note the 's' suffix for plural). Some users (EDA industry) are interested in using some QEMU transport layer (QMP?) to dynamically create machines (see [*] for example). IIUC only a subset of current QMP commands is needed for that. Maybe we can only adapt and enable these required commands for the heterogeneous binary, and keep the current ones unmodified for the single-target binary (where you can run a single target at a time, identically to current qemu-system-FOO binaries). [*] https://lore.kernel.org/qemu-devel/20220223090706.4888-1-damien.he...@greensocs.com/
[PATCH] qemucapabilitiesdata: Enable GTK graphics for 'caps_10.0.0_x86_64'
From: Peter Krempa
The common x86_64 test output was usually built without GTK as I've had
that in my build script for a long time. Enable it now as GTK UI is
enabled by many distros and upcoming patches plan to add support to
libvirt as well.
Signed-off-by: Peter Krempa
---
tests/qemucapabilitiesdata/caps_10.0.0_x86_64.replies | 8
1 file changed, 8 insertions(+)
diff --git a/tests/qemucapabilitiesdata/caps_10.0.0_x86_64.replies
b/tests/qemucapabilitiesdata/caps_10.0.0_x86_64.replies
index c2fd4f8d3d..6b2a2c2af7 100644
--- a/tests/qemucapabilitiesdata/caps_10.0.0_x86_64.replies
+++ b/tests/qemucapabilitiesdata/caps_10.0.0_x86_64.replies
@@ -5581,6 +5581,10 @@
"name": "142",
"tag": "type",
"variants": [
+{
+ "case": "gtk",
+ "type": "422"
+},
{
"case": "curses",
"type": "424"
@@ -13701,6 +13705,9 @@
{
"name": "none"
},
+{
+ "name": "gtk"
+},
{
"name": "sdl"
},
@@ -13721,6 +13728,7 @@
"values": [
"default",
"none",
+"gtk",
"sdl",
"egl-headless",
"curses",
--
2.49.0
Re: [PATCH] qemu: Add GTK display with OpenGL support
On Mon, Apr 28, 2025 at 12:57:42 +0200, Peter Krempa via Devel wrote: > On Mon, Apr 28, 2025 at 12:33:19 +0200, Peter Krempa via Devel wrote: > > On Mon, Apr 28, 2025 at 12:22:22 +0200, Kirill Shchetiniuk via Devel wrote: [...] > Since the 'gtk' backend in support was introduced predating qemu-2.12. > I'm guessing what's happening is that you tried to use > DO_TEST_CAPS_LATEST which failed. > > For the above the solution is not to ignore the check but rather one of: > > 1) update the 'x86_64' test data with a qemu built with gtk support [...] > Since I'm the one maintaining the capability dumps, maitaining a variant > for this is overkill; adding it to the main build is possible (I can > send the update). Update of the test data adding GTK support: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/5LQ7AYCWWMT5PTETUV4CEKLKA5XCQOLS/
Re: [PATCH 0/2] Make mock-noinline.py work again
On Mon, Apr 28, 2025 at 13:47:48 +0200, Michal Privoznik via Devel wrote: > *** BLURB HERE *** Reviewed-by: Peter Krempa > > Michal Prívozník (2): > util: Add missing G_NO_INLINE annotation > scripts: Fix reading list of files in mock-noinline.py
Re: [PATCH] qemucapabilitiesdata: Enable GTK graphics for 'caps_10.0.0_x86_64'
On Mon, Apr 28, 2025 at 02:24:20PM +0200, Peter Krempa via Devel wrote: > From: Peter Krempa > > The common x86_64 test output was usually built without GTK as I've had > that in my build script for a long time. Enable it now as GTK UI is > enabled by many distros and upcoming patches plan to add support to > libvirt as well. > > Signed-off-by: Peter Krempa > --- > tests/qemucapabilitiesdata/caps_10.0.0_x86_64.replies | 8 > 1 file changed, 8 insertions(+) Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature
Re: [PATCH] qemu: Add GTK display with OpenGL support
On Mon, Apr 28, 2025 at 12:22:22 +0200, Kirill Shchetiniuk via Devel wrote:
> From: Kirill Shchetiniuk
>
> Introduce GTK display support with OpenGL for the QEMU driver.
>
> - Add new XML options for GTK display type.
> - Include capability flags for the QEMU driver.
>
> Note: The `QEMU_CAPS_GTK_GL` flag cannot yet be checked, so device
> definition validation is incomplete. A placeholder is left for
> future implementation, when the GTK along with OpenGL capability
> check would be available in QEMU.
>
> Resolves: https://gitlab.com/libvirt/libvirt/-/issues/570
>
> Signed-off-by: Kirill Shchetiniuk
> ---
[...]
> diff --git a/src/conf/schemas/domaincommon.rng
> b/src/conf/schemas/domaincommon.rng
> index 5597d5a66b..61c1d3ad97 100644
> --- a/src/conf/schemas/domaincommon.rng
> +++ b/src/conf/schemas/domaincommon.rng
> @@ -4593,6 +4593,34 @@
>
>
>
> +
> +
> +gtk
> +
> +
> +
> +
> +
> +
> +
> +
> +
Since this is a path you want to use:
> +
> +
> +
[...]
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index e6d308534f..e97992ff56 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -8483,6 +8483,34 @@ qemuBuildGraphicsDBusCommandLine(virDomainDef *def,
> }
>
>
> +static int
> +qemuBuildGraphicsGTKCommandLine(virQEMUDriverConfig *cfg G_GNUC_UNUSED,
Why are you passing this argument if it's not used?
> +virCommand *cmd,
> +virDomainGraphicsDef *graphics)
> +{
> +g_auto(virBuffer) opt = VIR_BUFFER_INITIALIZER;
> +
> +if (graphics->data.gtk.xauth)
> +virCommandAddEnvPair(cmd, "XAUTHORITY", graphics->data.gtk.xauth);
I also realized that this is passin the path to the '.Xauthority' file
to the qemu process. Note that in the default privileged configuration
where qemu runs as a different process this will not work as the qemu
process will not have access to the cookie inside the file due to
permissions.
Also you can't simply apply security labelling on that file because it
would break other things.
I don't have a suggestion how to fix this, besides perhaps copying the
authority file at startup somewhere for the qemu process to be able to
access it.
Either way in the current state it will require mentioning this caveat.
[PATCH] qemu: Add GTK display with OpenGL support
From: Kirill Shchetiniuk
Introduce GTK display support with OpenGL for the QEMU driver.
- Add new XML options for GTK display type.
- Include capability flags for the QEMU driver.
Note: The `QEMU_CAPS_GTK_GL` flag cannot yet be checked, so device
definition validation is incomplete. A placeholder is left for
future implementation, when the GTK along with OpenGL capability
check would be available in QEMU.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/570
Signed-off-by: Kirill Shchetiniuk
---
src/conf/domain_conf.c| 79 ++-
src/conf/domain_conf.h| 8 ++
src/conf/schemas/domaincommon.rng | 28 +++
src/qemu/qemu_capabilities.c | 5 ++
src/qemu/qemu_capabilities.h | 2 +
src/qemu/qemu_command.c | 39 -
src/qemu/qemu_domain.c| 1 +
src/qemu/qemu_driver.c| 2 +
src/qemu/qemu_extdevice.c | 2 +
src/qemu/qemu_hotplug.c | 1 +
src/qemu/qemu_process.c | 8 ++
src/qemu/qemu_validate.c | 16
src/security/virt-aa-helper.c | 6 ++
src/vmx/vmx.c | 1 +
tests/domaincapsdata/qemu_10.0.0.s390x.xml| 1 +
tests/domaincapsdata/qemu_7.0.0.ppc64.xml | 1 +
tests/domaincapsdata/qemu_7.1.0.ppc64.xml | 1 +
tests/domaincapsdata/qemu_7.2.0.ppc.xml | 1 +
.../qemu_8.2.0-tcg-virt.loongarch64.xml | 1 +
.../qemu_8.2.0-virt.aarch64.xml | 1 +
.../qemu_8.2.0-virt.loongarch64.xml | 1 +
tests/domaincapsdata/qemu_8.2.0.aarch64.xml | 1 +
tests/domaincapsdata/qemu_8.2.0.armv7l.xml| 1 +
tests/domaincapsdata/qemu_9.0.0.sparc.xml | 1 +
tests/domaincapsdata/qemu_9.1.0.s390x.xml | 1 +
tests/domaincapsdata/qemu_9.2.0.s390x.xml | 1 +
.../caps_10.0.0_s390x.xml | 1 +
.../qemucapabilitiesdata/caps_7.0.0_ppc64.xml | 1 +
.../qemucapabilitiesdata/caps_7.1.0_ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml | 1 +
.../caps_8.2.0_aarch64.xml| 1 +
.../caps_8.2.0_armv7l.xml | 1 +
.../caps_8.2.0_loongarch64.xml| 1 +
.../qemucapabilitiesdata/caps_9.0.0_sparc.xml | 1 +
.../qemucapabilitiesdata/caps_9.1.0_s390x.xml | 1 +
.../qemucapabilitiesdata/caps_9.2.0_s390x.xml | 1 +
36 files changed, 218 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 542d6ade91..73550f7fcf 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -960,6 +960,7 @@ VIR_ENUM_IMPL(virDomainGraphics,
"spice",
"egl-headless",
"dbus",
+ "gtk",
);
VIR_ENUM_IMPL(virDomainGraphicsListen,
@@ -2054,6 +2055,12 @@ void virDomainGraphicsDefFree(virDomainGraphicsDef *def)
g_free(def->data.dbus.rendernode);
break;
+case VIR_DOMAIN_GRAPHICS_TYPE_GTK:
+g_free(def->data.gtk.display);
+g_free(def->data.gtk.xauth);
+g_free(def->data.gtk.rendernode);
+break;
+
case VIR_DOMAIN_GRAPHICS_TYPE_LAST:
break;
}
@@ -12199,6 +12206,39 @@ virDomainGraphicsDefParseXMLDBus(virDomainGraphicsDef
*def,
}
+static int
+virDomainGraphicsDefParseXMLGTK(virDomainGraphicsDef *def,
+xmlNodePtr node,
+xmlXPathContextPtr ctxt)
+{
+VIR_XPATH_NODE_AUTORESTORE(ctxt)
+xmlNodePtr glNode;
+virTristateBool fullscreen;
+
+ctxt->node = node;
+
+if (virXMLPropTristateBool(node, "fullscreen", VIR_XML_PROP_NONE,
+ &fullscreen) < 0)
+return -1;
+
+virTristateBoolToBool(fullscreen, &def->data.gtk.fullscreen);
+def->data.gtk.xauth = virXMLPropString(node, "xauth");
+def->data.gtk.display = virXMLPropString(node, "display");
+
+if ((glNode = virXPathNode("./gl", ctxt))) {
+def->data.gtk.rendernode = virXMLPropString(glNode,
+ "rendernode");
+
+if (virXMLPropTristateBool(glNode, "enable",
+ VIR_XML_PROP_REQUIRED,
+ &def->data.gtk.gl) < 0)
+return -1;
+}
+
+return 0;
+}
+
+
virDomainGraphicsDef *
virDomainGraphicsDefNew(virDomainXMLOption *xmlopt)
{
@@ -12275,6 +12315,10 @@ virDomainGraphicsDefParseXML(virDomainXMLOption
*xmlopt,
if (virDomainGraphicsDefParseXMLDBus(def, node, ctxt) < 0)
goto error;
break;
+case VIR_DOMAIN_GRAPHICS_TYPE_GTK:
+if (virDomainGraphicsDefParseXMLGTK(def, node, ctxt) < 0)
+goto error;
+break;
case VIR_DOMAIN_GRAPHICS_TYPE_LAST:
break;
}
@@ -27118,6 +27162,23 @@ virDomainGraphicsDefFormatDBus(virBuffer *attrBuf,
Re: [RFC PATCH 0/3] single-binary: make QAPI generated files common
On Fri, Apr 25, 2025 at 14:07:34 -0700, Pierrick Bouvier wrote:
> On 4/25/25 08:38, Markus Armbruster wrote:
> > Pierrick Bouvier writes:
> >
> > > Note: This RFC was posted to trigger a discussion around this topic, and
> > > it's
> > > not expected to merge it as it is.
> > >
> > > Context
> > > ===
> > >
> > > Linaro is working towards heterogeneous emulation, mixing several
> > > architectures
> > > in a single QEMU process. The first prerequisite is to be able to build
> > > such a
> > > binary, which we commonly name "single-binary" in our various series.
> > > An (incomplete) list of series is available here:
> > > https://patchew.org/search?q=project%3AQEMU+single-binary
> > >
> > > We don't expect to change existing command line interface or any
> > > observable
> > > behaviour, it should be identical to existing binaries. If anyone notices
> > > a
> > > difference, it will be a bug.
> >
> > Define "notice a difference" :) More on that below.
> >
>
> Given a single-binary *named* exactly like an existing qemu-system-X binary,
> any user or QEMU management layer should not be able to distinguish it from
> the real qemu-system-X binary.
>
> The new potential things will be:
> - introduction of an (optional) -target option, which allows to
> override/disambiguate default target detected.
> - potentially more boards/cpus/devices visible, once we start developing
> heterogeneous emulation. See it as a new CONFIG_{new_board} present.
>
> Out of that, once the current target is identified, based on argv[0], there
> should be absolutely no difference, whether in the behaviour, UI, command
> line, or the monitor interfaces.
Okay, so assuming that the correctly named binaries will apply whatever
necessary magic to have them behave identically as they did.
I'll also ignore the distros that rename them assuming they do it in a
way that stays compatible.
The question is how the new unified binary will behave when being
introspected:
- Can the unified binary be introspected without selecting an
architecture?
(by introspection I mean starting with -M none and querying stuff via
QMP)
if no: libvirt will have a chicken&egg problem deciding what to do
- What will be the answer for the platform-specific stuff such as CPU
definitions?
e.g. does/can an architecture need to be instantiated later via QMP?
can it be changed dynamically?
Re: [PATCH] qemu: Add GTK display with OpenGL support
On Mon, Apr 28, 2025 at 12:22:22 +0200, Kirill Shchetiniuk via Devel wrote:
> From: Kirill Shchetiniuk
>
> Introduce GTK display support with OpenGL for the QEMU driver.
>
> - Add new XML options for GTK display type.
> - Include capability flags for the QEMU driver.
>
> Note: The `QEMU_CAPS_GTK_GL` flag cannot yet be checked, so device
> definition validation is incomplete. A placeholder is left for
> future implementation, when the GTK along with OpenGL capability
> check would be available in QEMU.
Can you elaborate? WHy can't it be checked?
> Resolves: https://gitlab.com/libvirt/libvirt/-/issues/570
>
> Signed-off-by: Kirill Shchetiniuk
> ---
You will need to split this patch.
At least the capability flag needs to be a separate commit that only
adds the capability flag.
I'm also missing qemuxmlconftest test cases and docs/formatdomain.rst
documentation changes.
[...]
> 36 files changed, 218 insertions(+), 2 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 542d6ade91..73550f7fcf 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -960,6 +960,7 @@ VIR_ENUM_IMPL(virDomainGraphics,
>"spice",
>"egl-headless",
>"dbus",
> + "gtk",
> );
>
> VIR_ENUM_IMPL(virDomainGraphicsListen,
> @@ -2054,6 +2055,12 @@ void virDomainGraphicsDefFree(virDomainGraphicsDef
> *def)
> g_free(def->data.dbus.rendernode);
> break;
>
> +case VIR_DOMAIN_GRAPHICS_TYPE_GTK:
> +g_free(def->data.gtk.display);
> +g_free(def->data.gtk.xauth);
> +g_free(def->data.gtk.rendernode);
> +break;
> +
> case VIR_DOMAIN_GRAPHICS_TYPE_LAST:
> break;
> }
> @@ -12199,6 +12206,39 @@
> virDomainGraphicsDefParseXMLDBus(virDomainGraphicsDef *def,
> }
>
>
> +static int
> +virDomainGraphicsDefParseXMLGTK(virDomainGraphicsDef *def,
> +xmlNodePtr node,
> +xmlXPathContextPtr ctxt)
> +{
> +VIR_XPATH_NODE_AUTORESTORE(ctxt)
> +xmlNodePtr glNode;
> +virTristateBool fullscreen;
> +
> +ctxt->node = node;
> +
> +if (virXMLPropTristateBool(node, "fullscreen", VIR_XML_PROP_NONE,
> + &fullscreen) < 0)
You parse a tristate ...
> +return -1;
> +
> +virTristateBoolToBool(fullscreen, &def->data.gtk.fullscreen);
... but store it only as boolean?
> +def->data.gtk.xauth = virXMLPropString(node, "xauth");
> +def->data.gtk.display = virXMLPropString(node, "display");
> +
> +if ((glNode = virXPathNode("./gl", ctxt))) {
> +def->data.gtk.rendernode = virXMLPropString(glNode,
> + "rendernode");
> +
> +if (virXMLPropTristateBool(glNode, "enable",
> + VIR_XML_PROP_REQUIRED,
> + &def->data.gtk.gl) < 0)
> +return -1;
> +}
> +
> +return 0;
> +}
> +
> +
> virDomainGraphicsDef *
> virDomainGraphicsDefNew(virDomainXMLOption *xmlopt)
> {
[...]
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index a804335c85..de7ce95499 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -732,6 +732,8 @@ VIR_ENUM_IMPL(virQEMUCaps,
>
>/* 475 */
>"virtio-scsi.iothread-mapping", /*
> QEMU_CAPS_VIRTIO_SCSI_IOTHREAD_MAPPING */
> + "gtk", /* QEMU_CAPS_GTK */
> + "gtk-gl", /* QEMU_CAPS_GTK_GL */
> );
>
>
> @@ -1602,6 +1604,7 @@ static struct virQEMUCapsStringFlags
> virQEMUCapsQMPSchemaQueries[] = {
> { "set-numa-node/arg-type/+hmat-lb", QEMU_CAPS_NUMA_HMAT },
> { "query-cpu-model-expansion/ret-type/deprecated-props",
> QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION_DEPRECATED_PROPS },
> { "migrate-incoming/arg-type/exit-on-error",
> QEMU_CAPS_MIGRATE_INCOMING_EXIT_ON_ERROR },
> +{ "query-display-options/ret-type/type/^gtk", QEMU_CAPS_GTK },
> };
>
> typedef struct _virQEMUCapsObjectTypeProps virQEMUCapsObjectTypeProps;
> @@ -6499,6 +6502,8 @@
> virQEMUCapsFillDomainDeviceGraphicsCaps(virQEMUDriverConfig *cfg,
> VIR_DOMAIN_GRAPHICS_TYPE_RDP);
> }
> }
> +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_GTK))
> +VIR_DOMAIN_CAPS_ENUM_SET(dev->type, VIR_DOMAIN_GRAPHICS_TYPE_GTK);
> }
>
>
[...]
> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index b2c3c9e2f6..be92785d71 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -4523,6 +4523,17 @@ qemuValidateDomainDeviceDefRDPGraphics(const
> virDomainGraphicsDef *graphics)
> }
>
>
> +static int
> +qemuValidateDomainDeviceDefGTKGraphics(const virDomainGraphicsDef *graphics
> G_GNUC_UNUSED,
> + virQEMUCaps *qemuCaps G_GNUC_UNUSED)
> +{
> +/* TODO: OpenGL check */
>
Re: [PATCH] qemu: Add GTK display with OpenGL support
On Mon, Apr 28, 2025 at 12:33:19 +0200, Peter Krempa via Devel wrote: > On Mon, Apr 28, 2025 at 12:22:22 +0200, Kirill Shchetiniuk via Devel wrote: > > From: Kirill Shchetiniuk > > > > Introduce GTK display support with OpenGL for the QEMU driver. > > > > - Add new XML options for GTK display type. > > - Include capability flags for the QEMU driver. > > > > Note: The `QEMU_CAPS_GTK_GL` flag cannot yet be checked, so device > > definition validation is incomplete. A placeholder is left for > > future implementation, when the GTK along with OpenGL capability > > check would be available in QEMU. > > Can you elaborate? WHy can't it be checked? Since the 'gtk' backend in support was introduced predating qemu-2.12. I'm guessing what's happening is that you tried to use DO_TEST_CAPS_LATEST which failed. For the above the solution is not to ignore the check but rather one of: 1) update the 'x86_64' test data with a qemu built with gtk support 2) add a variant of the test data from a qemu built with gtk support 3) use DO_TEST_FULL and ARG_FLAGS to add the capability for the test file 4) base the tesst on one of the other arches which were built wit gtk Since I'm the one maintaining the capability dumps, maitaining a variant for this is overkill; adding it to the main build is possible (I can send the update). I was historically building qemu with '--disable-gtk' as it mirrored what fedora was doing but it seems that GTK is enabled in the current fedora build. Option 3) is acceptable for a niche feature and option 4) can theoretically regress in the future.
Re: [RFC PATCH 0/3] single-binary: make QAPI generated files common
Peter Krempa writes: > On Fri, Apr 25, 2025 at 17:38:44 +0200, Markus Armbruster via Devel wrote: >> Pierrick Bouvier writes: > > [...] > >> To be precise: conditionals that use macros restricted to >> target-specific code, i.e. the ones poisoned by exec/poison.h. Let's >> call them target-specific QAPI conditionals. >> >> The QAPI generator is blissfully unaware of all this. >> >> The build system treats QAPI modules qapi/*-target.json as >> target-specific. The .c files generated for them are compiled per >> target. See qapi/meson.build. >> >> Only such target-specific modules can can use target-specific QAPI >> conditionals. Use in target-independent modules will generate C that >> won't compile. >> >> Poisoned macros used in qapi/*-target.json: >> >> CONFIG_KVM >> TARGET_ARM >> TARGET_I386 >> TARGET_LOONGARCH64 >> TARGET_MIPS >> TARGET_PPC >> TARGET_RISCV >> TARGET_S390X Commands and events: CPU introspection: query-cpu-model-baseline, query-cpu-model-comparison, query-cpu-model-expansion, query-cpu-definitions S390 KVM CPU stuff: set-cpu-topology, CPU_POLARIZATION_CHANGE, query-s390x-cpu-polarization. GIC: query-gic-capabilities SEV: query-sev, query-sev-launch-measure, query-sev-capabilities, sev-inject-launch-secret, query-sev-attestation-report SGX: query-sgx, query-sgx-capabilities Xen: xen-event-list, xen-event-inject An odd duck: rtc-reset-reinjection > I've had a look at what bits of the QMP schema are depending on the > above defines which libvirt uses. > > In many cases libvirt could restrict the use of given command/property > to only supported architectures. We decided to simply probe the presence > of the command because it's convenient to not have to filter them any > more > > - query-gic-capabilities > - libvirt already calls this only for ARM guests based on the > definition > > - query-sev and friends > - libvirt uses presence of 'query-sev' to decide if the binary > supports it; patching in a platofrm check is possible although > inconvenient > > - query-sgx and friends > - similar to sev > > -query-cpu-definitions and friends > - see below Large subset of my list. >> >What we try to do here is to build them only >> > once >> instead. >> >> You're trying to eliminate target-specific QAPI conditionals. Correct? >> >> > In the past, we identied that the best approach to solve this is to expose >> > code >> > for all targets (thus removing all #if clauses), and stub missing >> > symbols for concerned targets. >> >> This affects QAPI/QMP introspection, i.e. the value of query-qmp-schema. >> >> Management applications can no longer use introspection to find out >> whether target-specific things are available. > > Indeed and libvirt already uses this in few cases as noded above. > >> >> For instance, query-cpu-definitions is implemented for targets arm, >> i386, loongarch, mips, ppc, riscv, and s390x. It initially was for >> fewer targets, and more targets followed one by one. Still more may >> follow in the future. Right now, management applications can use >> introspection to find out whether it is available. That stops working >> when you make it available for all targets, stubbed out for the ones >> that don't (yet) implement it. >> >> Management applications may have to be adjusted for this. >> >> This is not an attempt to shoot down your approach. I'm merely >> demonstrating limitations of your promise "if anyone notices a >> difference, it will be a bug." >> >> Now, we could get really fancy and try to keep introspection the same by >> applying conditionals dynamically somehow. I.e. have the single binary >> return different introspection values depending on the actual guest's >> target. > > I wonder how this will work if libvirt is probing a binary. Libvirt does > not look at the filename. It can't because it can be a > user-specified/compiled binary, override script, or a distro that chose > to rename the binary. > > The second thing that libvirt does after 'query-version' is > 'query-target'. > > So what should libvirt do once multiple targets are supported? > > How do we query CPUs for each of the supported targets? > > Will the result be the same if we query them one at a time or all at > once? Pierrick's stated goal is to have no noticable differences between the single binary and the qemu-system- it covers. This is obviously impossible if we can interact with the single binary before the target is fixed. >> This requires fixing the target before introspection. Unless this is >> somehow completely transparent (wrapper scripts, or awful hacks based on >> the binary's filename, perhaps), management applications may have to be >> adjusted to actually do that. > > As noted filename will not work. Users can specify any filename and > create override scripts or rename the binary. True. >> Applies not just to introspection.
Re: [PATCH] qemu: Add GTK display with OpenGL support
On Mon, Apr 28, 2025 at 12:33:19PM +0200, Peter Krempa wrote:
> On Mon, Apr 28, 2025 at 12:22:22 +0200, Kirill Shchetiniuk via Devel wrote:
> > From: Kirill Shchetiniuk
> >
> > Introduce GTK display support with OpenGL for the QEMU driver.
> >
> > - Add new XML options for GTK display type.
> > - Include capability flags for the QEMU driver.
> >
> > Note: The `QEMU_CAPS_GTK_GL` flag cannot yet be checked, so device
> > definition validation is incomplete. A placeholder is left for
> > future implementation, when the GTK along with OpenGL capability
> > check would be available in QEMU.
>
> Can you elaborate? WHy can't it be checked?
So I have discussed this with Michal Privoznik, it's possible to
directly gather if GTK capability is enable, but there is now
straight way to check if OpenGL for GTK is enabled too. Michal
purposed a way to check this with a small hack, check if GTK
capability is presented along with egl-headless, but we decided
better not to check OpenGL for GTK cabability for now, as it's not
straightforward approch, and I decided to leave a placeholer
for the future check to do not forget to add this later.
>
> > Resolves: https://gitlab.com/libvirt/libvirt/-/issues/570
> >
> > Signed-off-by: Kirill Shchetiniuk
> > ---
>
> You will need to split this patch.
>
> At least the capability flag needs to be a separate commit that only
> adds the capability flag.
>
> I'm also missing qemuxmlconftest test cases and docs/formatdomain.rst
> documentation changes.
>
Sure, will this with next series, thanks!
>
> [...]
>
> > 36 files changed, 218 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index 542d6ade91..73550f7fcf 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -960,6 +960,7 @@ VIR_ENUM_IMPL(virDomainGraphics,
> >"spice",
> >"egl-headless",
> >"dbus",
> > + "gtk",
> > );
> >
> > VIR_ENUM_IMPL(virDomainGraphicsListen,
> > @@ -2054,6 +2055,12 @@ void virDomainGraphicsDefFree(virDomainGraphicsDef
> > *def)
> > g_free(def->data.dbus.rendernode);
> > break;
> >
> > +case VIR_DOMAIN_GRAPHICS_TYPE_GTK:
> > +g_free(def->data.gtk.display);
> > +g_free(def->data.gtk.xauth);
> > +g_free(def->data.gtk.rendernode);
> > +break;
> > +
> > case VIR_DOMAIN_GRAPHICS_TYPE_LAST:
> > break;
> > }
> > @@ -12199,6 +12206,39 @@
> > virDomainGraphicsDefParseXMLDBus(virDomainGraphicsDef *def,
> > }
> >
> >
> > +static int
> > +virDomainGraphicsDefParseXMLGTK(virDomainGraphicsDef *def,
> > +xmlNodePtr node,
> > +xmlXPathContextPtr ctxt)
> > +{
> > +VIR_XPATH_NODE_AUTORESTORE(ctxt)
> > +xmlNodePtr glNode;
> > +virTristateBool fullscreen;
> > +
> > +ctxt->node = node;
> > +
> > +if (virXMLPropTristateBool(node, "fullscreen", VIR_XML_PROP_NONE,
> > + &fullscreen) < 0)
>
> You parse a tristate ...
>
> > +return -1;
> > +
> > +virTristateBoolToBool(fullscreen, &def->data.gtk.fullscreen);
>
> ... but store it only as boolean?
So I saw the same approach in virDomainGraphicsDefParseXMLSDL and decided to
use it too. Do you suggest virXPathBoolean instead or something else?
>
> > +def->data.gtk.xauth = virXMLPropString(node, "xauth");
> > +def->data.gtk.display = virXMLPropString(node, "display");
> > +
> > +if ((glNode = virXPathNode("./gl", ctxt))) {
> > +def->data.gtk.rendernode = virXMLPropString(glNode,
> > + "rendernode");
> > +
> > +if (virXMLPropTristateBool(glNode, "enable",
> > + VIR_XML_PROP_REQUIRED,
> > + &def->data.gtk.gl) < 0)
> > +return -1;
> > +}
> > +
> > +return 0;
> > +}
> > +
> > +
> > virDomainGraphicsDef *
> > virDomainGraphicsDefNew(virDomainXMLOption *xmlopt)
> > {
>
> [...]
>
> > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> > index a804335c85..de7ce95499 100644
> > --- a/src/qemu/qemu_capabilities.c
> > +++ b/src/qemu/qemu_capabilities.c
> > @@ -732,6 +732,8 @@ VIR_ENUM_IMPL(virQEMUCaps,
> >
> >/* 475 */
> >"virtio-scsi.iothread-mapping", /*
> > QEMU_CAPS_VIRTIO_SCSI_IOTHREAD_MAPPING */
> > + "gtk", /* QEMU_CAPS_GTK */
> > + "gtk-gl", /* QEMU_CAPS_GTK_GL */
> > );
> >
> >
> > @@ -1602,6 +1604,7 @@ static struct virQEMUCapsStringFlags
> > virQEMUCapsQMPSchemaQueries[] = {
> > { "set-numa-node/arg-type/+hmat-lb", QEMU_CAPS_NUMA_HMAT },
> > { "query-cpu-model-expansion/ret-type/deprecated-props",
> > QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION_DEPRECATED_PROPS },
> > { "migrate-incoming/arg-type/exit-on-error",
> > QEMU_CAPS_MIGRATE_INCOMING_EXIT_ON_ERROR },
> > +{ "query-displ
Re: [PATCH 0/3] Preper noipa attribute over noinline
On Mon, Apr 28, 2025 at 03:20:55PM +0200, Michal Privoznik via Devel wrote: > Somewhat green-ish pipeline: > > https://gitlab.com/MichalPrivoznik/libvirt/-/pipelines/1790043585 > > Jobs that failed are unrelated. > > Thing is, our upstream pipeline started failing on Rawhide + gcc because > of IPA. The unfortunate part is, noinline does not guarantee the > function is mockable. The fortunate part is, GCC has noipa attribute > which is even advocated for in its documentation. Took a while to dig into history of this, as I know we debated noipa in the past. AFAICT, we decided NOT to use noipa solely because it was a GCC only solution, but despite that, we ended up having to use add a CLang-only solution anyway in the form of -fsemantic-interposition. Given the latter, and that we have long known 'noinline' was insufficient, it makes sense to finally use 'noipa' on GCC. > Given we are after the freeze and this is potentially hazardous, I'm > okay if this is merged after the release. Yeah, probably best to wait until next cycle, as we've got a long history of unexpected edge cases with mocking & interactions with compiler optimization. Would be good to have a cycle to debug any possible fallout. With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH] qemu: Add GTK display with OpenGL support
On Mon, Apr 28, 2025 at 02:40:49PM +0200, Peter Krempa wrote:
> On Mon, Apr 28, 2025 at 12:22:22 +0200, Kirill Shchetiniuk via Devel wrote:
> > From: Kirill Shchetiniuk
> >
> > Introduce GTK display support with OpenGL for the QEMU driver.
> >
> > - Add new XML options for GTK display type.
> > - Include capability flags for the QEMU driver.
> >
> > Note: The `QEMU_CAPS_GTK_GL` flag cannot yet be checked, so device
> > definition validation is incomplete. A placeholder is left for
> > future implementation, when the GTK along with OpenGL capability
> > check would be available in QEMU.
> >
> > Resolves: https://gitlab.com/libvirt/libvirt/-/issues/570
> >
> > Signed-off-by: Kirill Shchetiniuk
> > ---
>
> [...]
>
> > diff --git a/src/conf/schemas/domaincommon.rng
> > b/src/conf/schemas/domaincommon.rng
> > index 5597d5a66b..61c1d3ad97 100644
> > --- a/src/conf/schemas/domaincommon.rng
> > +++ b/src/conf/schemas/domaincommon.rng
> > @@ -4593,6 +4593,34 @@
> >
> >
> >
> > +
> > +
> > +gtk
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
>
> Since this is a path you want to use:
>
>
>
Will fix it with next series, also saw the approach for xauth in SDL
device schema, will fix it there too later with separate patch, to maintain
it consistent.
>
> > +
> > +
> > +
>
> [...]
>
> > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> > index e6d308534f..e97992ff56 100644
> > --- a/src/qemu/qemu_command.c
> > +++ b/src/qemu/qemu_command.c
> > @@ -8483,6 +8483,34 @@ qemuBuildGraphicsDBusCommandLine(virDomainDef *def,
> > }
> >
> >
> > +static int
> > +qemuBuildGraphicsGTKCommandLine(virQEMUDriverConfig *cfg G_GNUC_UNUSED,
>
> Why are you passing this argument if it's not used?
>
> > +virCommand *cmd,
> > +virDomainGraphicsDef *graphics)
> > +{
> > +g_auto(virBuffer) opt = VIR_BUFFER_INITIALIZER;
> > +
> > +if (graphics->data.gtk.xauth)
> > +virCommandAddEnvPair(cmd, "XAUTHORITY", graphics->data.gtk.xauth);
>
> I also realized that this is passin the path to the '.Xauthority' file
> to the qemu process. Note that in the default privileged configuration
> where qemu runs as a different process this will not work as the qemu
> process will not have access to the cookie inside the file due to
> permissions.
>
> Also you can't simply apply security labelling on that file because it
> would break other things.
>
> I don't have a suggestion how to fix this, besides perhaps copying the
> authority file at startup somewhere for the qemu process to be able to
> access it.
>
> Either way in the current state it will require mentioning this caveat.
>
>
Yeah, I already noticed that issue with access to xauth file. I generaly
thought to make an `xauth` attribute mandatory to force the user to create
a readable file with cookie for qemu user, but not sure if it's a good
approach.
Re: [PATCH 1/2] util: Add missing G_NO_INLINE annotation
On Mon, Apr 28, 2025 at 01:47:49PM +0200, Michal Privoznik via Devel wrote: > From: Michal Privoznik > > There are two functions that are mocked, but are missing required > G_NO_INLINE attribute: virFirewallDIsRegistered() and > virHostCPUGetPhysAddrSize(). Add it. > > Signed-off-by: Michal Privoznik > --- > src/util/virfirewalld.h | 2 +- > src/util/virhostcpu.h | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Daniel P. Berrangé With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH 2/2] scripts: Fix reading list of files in mock-noinline.py
On Mon, Apr 28, 2025 at 01:47:50PM +0200, Michal Privoznik via Devel wrote: > From: Michal Privoznik > > The mock-noinline.py script is fed list of files through its > stdin, each file on its own line. Unfortunately, the way the > script is written does nothing as the trailing newline character > prevents any .endswith() match. Strip each line of white spaces. > > Signed-off-by: Michal Privoznik > --- > scripts/mock-noinline.py | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Reviewed-by: Daniel P. Berrangé With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH V1 0/6] fast qom tree get
Steven Sistare writes: > On 4/28/2025 4:04 AM, Markus Armbruster wrote: >> Steven Sistare writes: >> >>> On 4/9/2025 3:39 AM, Markus Armbruster wrote: Hi Steve, I apologize for the slow response. Steve Sistare writes: > Using qom-list and qom-get to get all the nodes and property values in a > QOM tree can take multiple seconds because it requires 1000's of > individual > QOM requests. Some managers fetch the entire tree or a large subset > of it when starting a new VM, and this cost is a substantial fraction of > start up time. "Some managers"... could you name one? >>> >>> My personal experience is with Oracle's OCI, but likely others could >>> benefit. >> >> Elsewhere in this thread, we examined libvirt's use qom-get. Its use of >> qom-get is also noticably slow, and your work could speed it up. >> However, most of its use is for working around QMP interface >> shortcomings around probing CPU flags. Addressing these would help it >> even more. >> >> This makes me wonder what questions Oracle's OCI answers with the help >> of qom-get. Can you briefly describe them? >> >> Even if OCI would likewise be helped more by better QMP queries, your >> fast qom tree get work might still be useful. > > We already optimized our queries as a first step, but what remains is still > significant, which is why I submitted this RFE. I understand your motivation. I'd like to learn more on what OCI actually needs from QMP, to be able to better serve it and potentially other management applications.
