Use automatic memory freeing and modern XML parsers to simplify the
function.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Format the rule attributes in two passes, first for positive 'match' and
second pass for negative. This removes the crazy logic for switching
between match modes inside the formatter.
The refactor makes it also more clear in which cases we actually do
format something.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use virXMLNodeGetSubelementList to get the elements to process.
The new approach documents the complexity of the parser, which is
designed to ignore unknown attributes and parse only a single kind of
them after finding the first valid one.
Note that the XML schema doesn't actually allow having multiple
sub-elements, but I'm not sure how that translates to actual configs
present.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use modern parsing. Invalid numbers are now rejected. Semantis for
numbers out of range is preserved.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Convert the fields to the proper types and use virXMLPropEnum for
parsing.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use virXMLFormatElement to simplify the formatter. Drop return value of
virNWFilterRuleDefFormat as there are no errors to report.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use virXMLNodeGetSubelement(List) instead of the looped parser and
simplify the code.
Note that handling of the 'bootp' element now conforms to the schema
where we allow just one and the 'file' attribute is mandatory.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The new helper is similar to virXPathNodeSet list but for cases where we
want to get subelements directly rather than using XPath.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There's nothing to clean up in the 'host' local variable on error as
the function which fills it makes sure to fill it only on success. In
such case it's also directly assigned to the array thus the 'host'
variable is cleared.
Remove the 'cleanup' label and 'ret' variable as we can now directly
return -1 on error.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Extract the 'inbound'/'outbound' subelements using
virXMLNodeGetSubelement to simplify the code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Remove the unnecessary check for valid arguments and use
virXMLPropULongLong instead of hand-written property parsers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Every caller will pass 'qdevid' as it's populated in the data
mandatorily with qemu-4.2 and onwards due to mandatory -blockdev use.
Thus we can drop compatibility with the old way of matching the disk via
alias.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Every caller will pass 'qdevid' as it's populated in the data
mandatorily with qemu-4.2 and onwards due to mandatory -blockdev use.
Thus we can drop compatibility with the old way of matching the disk via
alias.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Historically this didn't work with any supported qemu version as we
don't set the alias of the device, and thus qemu uses a different alias
resulting in a failure to startup the VM:
internal error: unable to execute QEMU command 'block_set_io_throttle': Device 'drive-sd-disk0' not found
Refuse setting throttling as this is unlikely to be needed and proper
fix requires using -device instead of -drive if=sd.
Note that this was broken when I moved the setup of throttling as a
command at startup for blockdev integration quite a while ago. Until
then throttling was passed as arguments for -drive.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function doesn't modify it. Fix the argument declaration so that the
function can be used in a context where we have a 'const' disk
definition.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When a user requests debug logging by setting the environment variable:
LIBVIRT_DEBUG=1
we should log any errors regardless of the setting of e.g.
'LIBVIRT_LOG_OUTPUTS' as the code will log every 'debug' and 'info'
level message to stderr but will skip 'error' level messages.
This obviously makes debugging things very complicated as you can get to
a situation when the error itself is missing.
This can happen e.g. in tests.
Fix the issue by probing the default log level and calling the logger if
it's set for VIR_LOG_DEBUG.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When starting a domain, it's done so in two steps (actually more,
but lets focus on just the following two):
1) qemuProcessPrepareDomain(), followed by
2) qemuProcessPrepareHost().
Now, in the first step (PrepareDomain()), PCI backends for all
hostdevs is set (qemuProcessPrepareDomain() ->
qemuProcessPrepareDomainHostdevs() -> qemuDomainPrepareHostdev()
-> qemuDomainPrepareHostdevPCI()). Perfect.
But then, additional hostdevs may appear, because in the host
prepare phase we may insert some hostdevs into domain definition
(qemuProcessPrepareHost() -> qemuProcessNetworkPrepareDevices()).
Now, these additional hostdevs don't undergo the same prepare as
hostdevs that were already present in the domain definition (i.e.
in qemuProcessPrepareDomain() phase). Therefore, we have to call
corresponding prepare function explicitly.
NB, the interface hotplug code (qemuDomainAttachNetDevice()) does
not suffer from this problem, because it calls top level
qemuDomainAttachHostDevice() which is used to hotplug regular
hostdevs too and as such calls qemuDomainPrepareHostdev().
Fixes: 3b87709c768480e085556e06bd8d08f62270d42d
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2209853
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
It's almost like we've anticipated this. Our XML parser and
formatter handles @address and @dev attributes of <portForward/>
element completely independent of each other. And as of commit
2023_03_29.b10b983~3 passt allows handling these two separately
too. All that's left is generate the cmd line according to this
new fact.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2210287
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We allow (some) domain devices to have a different <seclabel/>
than the top level domain one (this is mostly to allow access to
a resource for multiple domains). Now, we do couple of sanity
checks for such <seclabel/>, e.g. when the <label/> is specified,
but '@relabel' is set to no. But what we are missing is the
opposite: when '@relabel' is set, but no <label/> was provided.
Our schema already denies such combination. Make our parser
behave the same.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2160356
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In virNodeDeviceGetSCSIHostCaps, there is a pattern of reusing
a tmp value and stealing the pointer.
But in two case it is not stolen. Use separate variables for them
to avoid mixing autofree with manual free() calls.
Fixes: 8a0cb5f73ade3900546718eabe70cb064c6bd22c
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This is fairly trivial. Just set .memaddr attribute if a value
was set in the XML.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2180679
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
After a QEMU domain is started, among other thing we query memory
device information. And while memory address is returned by QEMU
for all models, we store it only for DIMMs and NVDIMMs. Do store
it for VIRTIO_MEM and VIRTIO_PMEM too.
This effectively reports the address the virtio-mem/virtio-pmem
is mapped to in live XML.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Both virtio-mem and virtio-pmem devices have '.memaddr' attribute
which controls the address where they are mapped in the guest
memory. Ideally, users do not need to specify this as QEMU does
the right thing and computes addresses automatically on startup.
But soon, we will need to record this address as it is part of
guest ABI. And also, there might be some users that want to
control this value. Now, we are in a bit of a pickle, because
both these device types already have a PCI address, therefore we
can't just use <address/> blindly. But what we can do, is
introduce <address/> under the <target/> element. This is also
more conceptual, as knobs under <target/> control guest visible
config of memory device (and .memaddr surely falls into that
category).
NB, SgxEPCDeviceInfo struct in QMP definition also has .memaddr
attribute, but because of the way we build cmd line there's no
(easy) way to set the attribute. So ignore that for now.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Due to missed break; statement the virDomainInputDefPostParse()
is called not only for VIR_DOMAIN_DEVICE_INPUT but also
VIR_DOMAIN_DEVICE_LEASE and VIR_DOMAIN_DEVICE_NET, which can lead
to all sort of unpredictable results.
Fixes: c4bc4d3b82fbe22e03c986ca896090f481df5c10
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This brings the tool's list of features in sync with qemu
commit 886c0453cbf10eebd42a9ccf89c3e46eb389c357.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Introduced in QEMU by commit v8.0.0-7eb061b06e.
Signed-off-by: Lin Yang <lin.a.yang@intel.com>
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
Instead of updating defined mdevs only add another update for active
devices as well to cover transient mdev devices as well.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2143158
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Again, this fixes the same problem as one of previous commits,
but this time for memory hotplug. Long story short, if there's a
domain running and the emulator thread is restricted to a subset
of host NUMA nodes, but the memory that's about to be hotplugged
requires memory from a host NUMA node that's not in the set we
need to allow emulator thread to access the node, temporarily.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Consider a domain with two guest NUMA nodes and the following
<numatune/> setting :
<numatune>
<memory mode="strict" nodeset="0"/>
<memnode cellid="0" mode="strict" nodeset="1"/>
</numatune>
What this means is the emulator thread is pinned onto host NUMA
node #0 (by setting corresponding cpuset.mems to "0"), and two
memory-backend-* objects are created:
-object '{"qom-type":"memory-backend-ram","id":"ram-node0", .., "host-nodes":[1],"policy":"bind"}' \
-numa node,nodeid=0,cpus=0-1,memdev=ram-node0 \
-object '{"qom-type":"memory-backend-ram","id":"ram-node1", .., "host-nodes":[0],"policy":"bind"}' \
-numa node,nodeid=1,cpus=2-3,memdev=ram-node1 \
Note, the emulator thread is pinned well before QEMU is even
exec()-ed.
Now, the way memory allocation works in QEMU is: the emulator
thread calls mmap() followed by mbind() (which is sane, that's
how everybody should do it). BUT, because the thread is already
restricted by CGroups to just NUMA node #0, calling:
mbind(host-nodes:[1]); /* made up syntax (TM) */
fails. This is expected though. Kernel was instructed to place
the memory at NUMA node "0" and yet, process is trying to place
it elsewhere.
We used to solve this by not restricting emulator thread at all
initially, and only after it's done initializing (i.e. we got the
QMP greeting) we placed it onto desired nodes. But this had its
own problems (e.g. QEMU might have locked pieces of its memory
which were then unable to migrate onto different NUMA nodes).
Therefore, in v5.1.0-rc1~282 we've changed this and set cgroups
upfront (even before exec()-ing QEMU). And this used to work, but
something has changed (I can't really put my finger on it).
Therefore, for the initialization start the thread with union of
all configured host NUMA nodes ("0-1" in our example) and fix the
placement only after QEMU is started.
NB, the memory hotplug suffers the same problem, but that will
be fixed in the next commit.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2138150
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Inside of qemuProcessSetupPid() there's @numatune variable which
is set to vm->def->numa, but it lives only in one block. In the
rest of places the expanded form (vm->def->numa) is used instead.
Move the variable declaration at the beginning of the function
and use it instead of the expanded form.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
We cannot use host-nodes attribute for it, but there is no reason for us
to skip the preallocation optimisation using thread-context in such
case. Thankfully returning the proper nodemask from
qemuBuildMemoryBackendProps is enough to trigger this.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The QEMU interface is still in a state of flux, and KVM support
has been pulled shortly after having been merged. Let's not
commit to a stable interface in libvirt just yet.
Reverts: 720e8f13ff71377580cd37b118cee8a1f982d1d8
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
