Since Xen 4.5 libxl allows to set affinities during domain creation.
This enables Xen to allocate the domain memory on NUMA systems close to
the specified pcpus.
Libvirt can now handle <domain/cputune/vcpupin> in domU.xml correctly.
Without this change, Xen will create the domU and assign NUMA memory and
vcpu affinities on its own. Later libvirt will adjust the affinity,
which may move the vcpus away from the assigned NUMA node.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
When fetching the value of a private secret, we need to use an elevated
identity otherwise the secret driver will deny access.
When using the modular daemons, the elevated identity needs to be active
before the secret driver connection is opened, and it will apply to all
APIs calls made on that conncetion.
When using the monolithic daemon, the identity at time of opening the
connection is ignored, and the elevated identity needs to be active
precisely at the time the virSecretGetValue API call is made.
After acquiring the secret value, the elevated identity should be
cleared.
This sounds complex, but is fairly straightfoward with the automatic
cleanup callbacks.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
What this function really does it takes ownership of all pointers
passed (well, except for the first one - caps - to which it
registers new NUMA node). But since all info is passed as a
single pointer it's hard to tell (and use g_auto*). Let's use
double pointers to make the ownership transfer obvious.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
xc_get_max_cpus from Xen version 4.3 may return 0 in case xc_physinfo
fails. This has been fixed in Xen 4.4. Remove the obsolete result check
from libvirt. Just convert libxl error codes to plain -1.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
In Xen 4.2 struct libxl_event_hooks had a member which was erroneously
declared const. Since libvirt requires at least Xen 4.6, remove the dead
code.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
The g_path_is_absolute() considers more situations
than just a simply "path[0] == '/'".
Related issue: https://gitlab.com/libvirt/libvirt/-/issues/12
Signed-off-by: Luke Yue <lukedyue@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use the appropriate type for the variable and refactor the XML parser to
parse it correctly using virXMLPropEnum.
Changes to other places using switch statements were required.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Over several years of debugging reports related to VM shutdown, destruction,
and cleanup, I've found that logging of all events received from libxl and
logging the entry of libxlDomainCleanup has proven useful. Add the these
debug messages upstream to aid in future debugging.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Historically, we declared pointer type to our types:
typedef struct _virXXX virXXX;
typedef virXXX *virXXXPtr;
But usefulness of such declaration is questionable, at best.
Unfortunately, we can't drop every such declaration - we have to
carry some over, because they are part of public API (e.g.
virDomainPtr). But for internal types - we can do drop them and
use what every other C project uses 'virXXX *'.
This change was generated by a very ugly shell script that
generated sed script which was then called over each file in the
repository. For the shell script refer to the cover letter:
https://listman.redhat.com/archives/libvir-list/2021-March/msg00537.html
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Upcoming changes will use different LIBXL_API_VERSION variants.
Prepare libxl_set_memory_target, which changed the storage size of
parameter "target_memkb" in Xen 4.8.
No functional change intended.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Upcoming changes will use different LIBXL_API_VERSION variants.
Prepare libxl_send_trigger, which got a new parameter
"ao_how" in Xen 4.12. libvirt does not use this parameter.
No functional change intended.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Upcoming changes will use different LIBXL_API_VERSION variants.
Prepare libxl_set_vcpuonline, which got a new parameter
"ao_how" in Xen 4.12. libvirt does not use this parameter.
No functional change intended.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Upcoming changes will use different LIBXL_API_VERSION variants.
Prepare libxl_get_free_memory, which changed storage size of parameter
"memkb" in Xen 4.8.
No functional change intended.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Upcoming changes will use different LIBXL_API_VERSION variants.
Prepare libxl_domain_need_memory, which changed the storage size of
"need_memkb" in Xen 4.8. With Xen 4.12 the libxl_domain_config
parameter was changed
No functional change intended.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Upcoming changes will use different LIBXL_API_VERSION variants.
Prepare libxl_domain_unpause, which got a new parameter
"ao_how" in Xen 4.12. libvirt does not use this parameter.
No functional change intended.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Upcoming changes will use different LIBXL_API_VERSION variants.
Prepare libxl_domain_pause, which got a new parameter
"ao_how" in Xen 4.12. libvirt does not use this parameter.
No functional change intended.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Upcoming changes will use different LIBXL_API_VERSION variants.
Prepare libxl_domain_reboot, which got a new parameter
"ao_how" in Xen 4.12. libvirt does not use this parameter.
No functional change intended.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Upcoming changes will use different LIBXL_API_VERSION variants.
Prepare libxl_domain_shutdown, which got a new parameter
"ao_how" in Xen 4.12. libvirt does not use this parameter.
No functional change intended.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Upcoming changes will use different LIBXL_API_VERSION variants.
Prepare libxl_retrieve_domain_configuration, which got a new parameter
"libxl_asyncop_how" in Xen 4.12. libvirt does not use this parameter.
No functional change intended.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Upcoming changes will use different LIBXL_API_VERSION variants.
Prepare libxl_domain_create_restore, which got a new parameter
"send_back_fd" in Xen 4.7. libvirt does not use this parameter.
No functional change intended.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Recently, a few commits back I've switched bunch of code to
g_steal_pointer() using coccinelle. Problem was that the semantic
patch used was slightly off:
@@
expression a, b;
@@
+ b = g_steal_pointer(&a);
- b = a;
... when != a
- a = NULL;
Problem is that, "... when != a" is supposed to jump over those
lines, which don't contain expression a. My idea was to replace
the following pattern too:
ptrX = ptrY;
if (something(ptrZ) < 0) goto error;
ptrY = NULL;
But what I missed is that the following pattern is also matched
and replaced:
ptrX = ptrY;
if (something(ptrX) < 0) goto error;
ptrY = NULL;
This is not necessarily correct - as demonstrated by our hotplug
code. The real problem is ambiguous memory ownership transfer
(functions which add device to domain def take ownership only on
success), but to not tackle the real issue let's revert those
parts.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The order in which virNetworkUpdate() accepts @section and
@command arguments is not the same as in which it passes them
onto networkUpdate() callback. Until recently, it did not really
matter, because calling the API on client side meant arguments
were encoded in reversed order (compared to the public API), but
then on the server it was fixed again - because the server
decoded RPC (still swapped), called public API (still swapped)
and in turn called the network driver callback (with reversing
the order - so magically fixing the order).
Long story short, if the public API is called even number of
times those swaps cancel each other out. The problem is when the
API is called an odd numbed of times - which happens with split
daemons and the right URI. There's one call in the client (e.g.
virsh net-update), the other in a hypervisor daemon (say
virtqemud) which ends up calling the API in the virnetworkd.
The fix is obvious - fix the order in which arguments are passed
to the callback.
But, to maintain compatibility with older, yet unfixed, daemons
new connection feature is introduced. The feature is detected
just before calling the callback and allows client to pass
arguments in correct order (talking to fixed daemon) or in
reversed order (talking to older daemon).
Unfortunately, older client talking to newer daemon can't be
fixed. Let's hope that it's less frequent scenario.
Fixes: 574b9bc66b
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1870552
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Generated by the following spatch:
@@
expression a, b;
@@
+ b = g_steal_pointer(&a);
- b = a;
... when != a
- a = NULL;
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
In short, virXXXPtr type is going away. With big bang. And to
help us rewrite the code with a sed script, it's better if each
variable is declared on its own line.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use g_strsplit to split the string and avoid use of stack'd strings.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_strndup with a freed buffer instead of the more complex approach
using virStrncpy.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Make the temporary string an autofree-ing pointer and copy the contents.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Copy the input string so that we don't have to use a static buffer and
virStrncpy.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Generated using the following spatch:
@@
expression path;
@@
- virFileMakePath(path)
+ g_mkdir_with_parents(path, 0777)
However, 14 occurrences were not replaced, e.g. in
virHostdevManagerNew(). I don't really understand why.
Fixed by hand afterwards.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Commit 887dd0d331 caused a small regression in NodeDeviceDetach in the libxl
driver when the 'driver' parameter is not specified. E.g.
# virsh nodedev-detach pci_0000_0a_10_0
error: Failed to detach device pci_0000_0a_10_0
error: An error occurred, but the cause is unknown
If the driver name is not specified, NULL is passed to
virDomainDriverNodeDeviceDetachFlags, in which case virPCIDeviceSetStubDriver
is never called to set the stub to pciback. Fix it by setting the driver to
"xen" if it is not specified when invoking NodeDeviceDetach.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Attempting to report error in case when we ran out of memory is
pointless.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
The libvirt libxl driver has no access to FDs associated with VM disks.
The disks are opened by libxl.so and any related FDs are not exposed to
applications. The prevents using virtlockd's auto-release feature to
release locks when the FD is closed. Acquiring and releasing locks is
explicitly handled by the libxl driver.
The current logic is structured such that locks are acquired in
libxlDomainStart and released in libxlDomainCleanup. This works well
except for migration, where the locks must be released on the source
host before the domain can be started on the destination host, but the
domain cannot be cleaned up until the migration confirmation stage.
When libxlDomainCleanup if finally called in the confirm stage, locks
are again released resulting in confusing errors from virtlockd and
libvirtd
virtlockd[8095]: resource busy: Lockspace resource 'xxxxxx' is not locked
libvirtd[8050]: resource busy: Lockspace resource 'xxxxxx' is not locked
libvirtd[8050]: Unable to release lease on testvm
The error is also encountered in some error cases, e.g. when
libxlDomainStart fails before acquiring locks and libxlDomainCleanup
is still used for cleanup.
In lieu of a mechanism to check if a lock has been acquired, this patch
takes an easy approach to fixing the unnecessary lock releases by adding
an indicator to the libxlDomainPrivate object that can be set when the
lock is acquired and cleared when the lock is released. libxlDomainCleanup
can then skip releasing the lock in cases where it was previously released
or never acquired in the first place.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Commit fa30ee04a2 caused a regression in normal domain shutown.
Initiating a shutdown from within the domain or via 'virsh shutdown'
does cause the guest OS running in the domain to shutdown, but libvirt
never reaps the domain so it is always shown in a running state until
calling 'virsh destroy'.
The shutdown thread is also an internal user of the driver shutdown
machinery and eventually calls libxlDomainDestroyInternal where
the ignoreDeathEvent inhibitor is set, but running in a thread
introduces the possibility of racing with the death event from
libxl. This can be prevented by setting ignoreDeathEvent before
running the shutdown thread.
An additional improvement is to handle the destroy event synchronously
instead of spawning a thread. The time consuming aspects of destroying
a domain have been completed when the destroy event is delivered.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
libxlNodeDeviceDetachFlags() and qemuNodeDeviceDetachFlags() are mostly
equal, aside from how the virHostdevmanager pointer is retrieved and
the PCI stub driver used.
Now that the PCI stub driver verification is done early in both functions,
we can use the virDomainDriverNodeDeviceDetachFlags() helper to reduce
code duplication between them. 'driverName' is checked inside the helper
to set the appropriate stub driver.
The helper is named with the 'Flags' suffix, even when the helper itself
isn't receiving the flags from the callers, to be compliant with the
ACL function virNodeDeviceDetachFlagsEnsureACL() that is being called
inside it and was called from the original functions. Renaming the helper
would implicate in renaming REMOTE_PROC_NODE_DEVICE_DETACH_FLAGS, and all the
related structs inside remote_protocol.x, to be compliant with the ACL
rules.
This is not being checked at this moment, but we'll fix check-aclrules.py to
verify all the helpers that calls ACL functions in domain_driver.c shortly.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
The validation of 'driverName' does not depend on any other state and can be
done right on the start of the function. We can fail earlier while avoiding
a cleanup jump.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
libxlNodeDeviceReAttach() and qemuNodeDeviceReAttach() are mostly equal,
differing only how the virHostdevManager pointer is retrieved.
Put the common code into virDomainDriverNodeDeviceReAttach() to reduce
code duplication.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
libxlNodeDeviceReset() and qemuNodeDeviceReset() are mostly equal,
differing only how the virHostdevManager pointer is retrieved.
Put the common code into virDomainDriverNodeDeviceReset() to reduce
code duplication.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
All of these strings are allocated once, freed once, and are never
returned out of the function where they are declared.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Our implementation was inspired by glib anyways. The difference is only
the order of arguments.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Our implementation was heavily inspired by the glib version so it's a
drop-in replacement.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Precalculate the lenght to avoid use of 'virStringListAdd' in a loop.
The code is also simplified by using APIs which don't return errors.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
