The initial variant of libxlDomainChangeEjectableMedia could just leave
the function earlier. With refcounting this does not work anymore.
Fixes commit a5bf06ba34dbb226ac1b2fb63f5026c5d493bc65
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Jim Fehlig <jfehlig@suse.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>
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>
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_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>
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: 574b9bc66b6b10cc4cf50f299c3f0ff55f2cbefb
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>
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>
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>
Back in commit 2c71d3826, which appeared in libvirt-1.2.3 in April
2014, the location used to store saved MAC addresses and vlan tags of
SRIOV VFs was changed from /var/run/libvirt/qemu to
/var/run/libvirt/hostdevmgr. For backward compatibility the code was
made to continue looking in the old location for the files when it
didn't find them in the new location.
It's now been 6 years, and even if there was somebody still running
libvirt-1.2.3 on their system, that system would now be out of support
for libvirt, so there would be no way for them to upgrade to a new
libvirt that no longer looks in "oldStateDir" for the files. So
let's no longer look in "oldStateDir" for the files!
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The current virPCIDeviceNew() signature, receiving 4 uints in sequence
(domain, bus, slot, function), is not neat.
We already have a way to represent a PCI address in virPCIDeviceAddress
that is used in the code. Aside from the test files, most of
virPCIDeviceNew() callers have access to a virPCIDeviceAddress reference,
but then we need to retrieve the 4 required uints (addr.domain, addr.bus,
addr.slot, addr.function) to satisfy virPCIDeviceNew(). The result is
that we have extra verbosity/boilerplate to retrieve an information that
is already available in virPCIDeviceAddress.
A better way is presented by virNVMEDeviceNew(), where the caller just
supplies a virPCIDeviceAddress pointer and the function handles the
details internally.
This patch changes virPCIDeviceNew() to receive a virPCIDeviceAddress
pointer instead of 4 uints.
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Instead of receiving 4 uints in order and write domain/bus/slot/function,
receive a virPCIDeviceAddressPtr instead and write into it.
This change will allow us to simplify the API for virPCIDeviceNew()
in the next patch.
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
libxlNodeDeviceGetPCIInfo() and qemuNodeDeviceGetPCIInfo() are equal.
Let's move the logic to a new virDomainDriverNodeDeviceGetPCIInfo()
info to be used by libxl_driver.c and qemu_driver.c.
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Now that this function can be called regardless of interface type (and
whether or not we have a conn for the network driver), let's actually
call it for all interface types. This will assure that we re-connect
any disconnected bridge devices for <interface type='bridge'> as
mentioned in https://bugzilla.redhat.com/show_bug.cgi?id=1730084#c26
(until now we've only been reconnecting bridge devices for <interface
type='network'>)
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Simplify ReserveName/GenerateName for macvlan and macvtap by using
common functions.
Signed-off-by: Shi Lei <shi_lei@massclouds.com>
Reviewed-by: Laine Stump <laine@redhat.com>
The next objective is to move virDomainDeviceDefValidate() to
domain_validate.c. First let's move all the static helpers.
The net device validation functions are used across multiple
drivers, so let's move them separately first.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
This function always returns zero, so it might as well be void.
Signed-off-by: Matt Coleman <matt@datto.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This function always returns zero, so it might as well be void.
Signed-off-by: Matt Coleman <matt@datto.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The function only returns zero or aborts, so it might as well be void.
This has the added benefit of simplifying the code that calls it.
Signed-off-by: Matt Coleman <matt@datto.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The function only returns zero or aborts, so it might as well be void.
This has the added benefit of simplifying the code that calls it.
Signed-off-by: Matt Coleman <matt@datto.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
This adds a new value to virConnectCompareCPUFlags,
"VIR_CONNECT_CPU_VALIDATE_XML", that governs XML document validation in
virCPUDefParseXML.
In src/conf/cpu_conf.c, include configmake.h for PKGDATADIR and
virfile.h for virFileFindResource.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
I left in a 'return' or 'goto cleanup' in a few places
where I did the conversion manually.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reported-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
CVE-2020-25637
Add a requirement for domain:write if source is set to
VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
The refactoring in commit de49d5bad3 accidentally dropped the statement
setting def to NULL after successfully adding it to the virDomainObjList,
causing it to be freed while still in use. The resulting memory
corruption caused unpredictable behavior, often resulting in a libvirtd
crash.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
When the xen driver loads, it probes libxl for some info about dom0 and
adds it to the virDomainObjList. The driver then looks for any domains
in stateDir and if they are still alive adds them to the list as well.
This logic is a bit flawed wrt handling driver reload and causes the
following error
internal error: unexpected domain Domain-0 already exists
A simple fix is to load all domains from stateDir first and then only
add dom0 if needed.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The include was introduced by:
commit 3d6fe99c5c3b862db1b345da3598a920f1079b6e
Add vcpu functions to libxl driver
which used ceil() and floor(), but these were later
removed by:
commit 3eb869a04bdeaddbe10e74568540e865fb79e412
libxl: avoid compiler warning
which did not remove the include.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
There have been some reports that, due to libvirt always trying to
assign the lowest numbered macvtap / tap device name possible, a new
guest would sometimes be started using the same tap device name as
previously used by another guest that is in the process of being
destroyed *as the new guest is starting.
In some cases this has led to, for example, the old guest's
qemuProcessStop() code deleting a port from an OVS switch that had
just been re-added by the new guest (because the port name is based on
only the device name using the port). Similar problems can happen (and
I believe have) with nwfilter rules and bandwidth rules (which are
both instantiated based on the name of the tap device).
A couple patches have been previously proposed to change the ordering
of startup and shutdown processing, or to put a mutex around
everything related to the tap/macvtap device name usage, but in the
end no matter what you do there will still be possible holes, because
the device could be deleted outside libvirt's control (for example,
regular tap devices are automatically deleted when the qemu process
terminates, and that isn't always initiated by libvirt but could
instead happen completely asynchronously - libvirt then has no control
over the ordering of shutdown operations, and no opportunity to
protect it with a mutex.)
But this only happens if a new device is created at the same time as
one is being deleted. We can effectively eliminate the chance of this
happening if we end the practice of always looking for the lowest
numbered available device name, and instead just keep an integer that
is incremented each time we need a new device name. At some point it
will need to wrap back around to 0 (in order to avoid the IFNAMSIZ 15
character limit if nothing else), and we can't guarantee that the new
name really will be the *least* recently used name, but "math"
suggests that it will be *much* less common that we'll try to re-use
the *most* recently used name.
This patch implements such a counter for macvtap/macvlan, replacing
the existing, and much more complicated, "ID reservation" system. The
counter is set according to whatever macvtap/macvlan devices are
already in use by guests when libvirtd is started, incremented each
time a new device name is needed, and wraps back to 0 when either
INT_MAX is reached, or when the resulting device name would be longer
than IFNAMSIZ-1 characters (which actually is what happens when the
template for the device name is "maccvtap%d"). The result is that no
macvtap name will be re-used until the host has created (and possibly
destroyed) 99,999,999 devices.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Commit <2020c6af8a8e4bb04acb629d089142be984484c8> fixed an issue with
QEMU driver by reporting offline CPUs as well. However, doing so it
introduced a regression into libxl and test drivers by completely
ignoring the passed `hostcpus` variable.
Move the virHostCPUGetAvailableCPUsBitmap() out of the helper into QEMU
driver so it will not affect other drivers which gets the number of host
CPUs differently.
This was uncovered by running libvirt-dbus test suite which counts on
the fact that test driver has hard-coded host definition and must not
depend on the host at all.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Similar to commits 55ce6564634 and 6c17606b7cc in the qemu driver, make
separate copies of persistent and live device config and normalize the MAC
address between the two. This avoids having different MAC address for the
persistent and live config, ensuring the device has the same address when
the persistent config takes affect after a VM restart.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Laine Stump <laine@redhat.com>
