In some cases we have a label that contains nothing but a return
statement. The amount of such labels rises as we use automagic
cleanup. Anyway, such labels are pointless and can be dropped.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.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>
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>
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>
We jump to the error label if the 'if' condition is true.
Remove the explicit else to make it more obvious that 'hostname'
is filled on both branches of 'if (!uri_in)'.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
In case no uri_in was supplied, we forgot to set the hostname
to the current hostname and formatted a useless uri_out.
src/util/glibcompat.h:57:26: error: ‘%s’ directive argument is null [-Werror=format-overflow=]
57 | # define g_strdup_printf vir_g_strdup_printf
src/openvz/openvz_driver.c:2136:16: note: in expansion of macro ‘g_strdup_printf’
2136 | *uri_out = g_strdup_printf("ssh://%s", hostname);
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reported-by: Jaroslav Suchanek <jsuchane@redhat.com>
Fixes: e3c626a61d6c3d808555653684c0fb1e7c4d74ec
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Compilers are not very good at detecting this problem. Fixed by manual
inspection of compilation warnings after replacing 'VIR_FREE' with an
empty macro.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com
Construct the command in multiple steps instead of using a sentinel
in the args array.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Include virutil.h in all files that use it,
instead of relying on it being pulled in somehow.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Most code now uses the virProcess / virCommand APIs, so
the need for sys/wait.h is quite limited. Removing this
include removes the dependency on GNULIB providing a
dummy sys/wait.h for Windows.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This parameter is now unused and can be removed entirely.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
To enable the virCapsPtr parameter to the post parse method to be
eliminated, the drivers must fetch the virCapsPtr from their own
driver via the opaque parameter, or use an alternative approach
to validate the parsed data.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Moving their instance parameter to be the first one, and give consistent
ordering of other parameters across all functions. Ensure that the xml
options are passed into both functions in prep for future work.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Glib implementation follows the ISO C99 standard so it's safe to replace
the gnulib implementation.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Replace all occurrences of
if (VIR_STRDUP(a, b) < 0)
/* effectively dead code */
with:
a = g_strdup(b);
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The callers expect '1' on a successful probe,
so return 1 just like VIR_STRDUP would.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Replace all the occurrences of
ignore_value(VIR_STRDUP(a, b));
with
a = g_strdup(b);
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use G_GNUC_UNUSED from GLib instead of ATTRIBUTE_UNUSED.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Since qemuDomainDeviceDefPostParse callback requires qemuCaps, we need
to make sure it gets the capabilities stored in the domain's private
data if the domain is running. Passing NULL may cause QEMU capabilities
probing to be triggered in case QEMU binary changed in the meantime.
When this happens while a running domain object is locked, QMP event
delivered to the domain before QEMU capabilities probing finishes will
deadlock the event loop.
QEMU capabilities lookup (via domainPostParseDataAlloc callback) is
hidden inside virDomainDeviceDefPostParseOne with no way to pass
qemuCaps to virDomainDeviceDef* functions. This patch fixes all
remaining paths leading to virDomainDeviceDefPostParse.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Many drivers had a comment that they did not validate the incoming
'flags' to virDomainGetXMLDesc() because they were relying on
virDomainDefFormat() to do it instead. This used to be the case
(at least since 461e0f1a and friends in 0.9.4 added unknown flag
checking in general), but regressed in commit 0ecd6851 (1.2.12),
when all of the drivers were changed to pass 'flags' through the
new helper virDomainDefFormatConvertXMLFlags(). Since this helper
silently ignores unknown flags, we need to implement flag checking
in each driver instead.
Annoyingly, this means that any new flag values added will silently
be ignored when targeting an older libvirt, rather than our usual
practice of loudly diagnosing an unsupported flag. Add comments
in domain_conf.[ch] to remind us to be extra vigilant about the
impact when adding flags (a new flag to add data is safe if the
older server omitting the requested data doesn't break things in
the newer client; a new flag to suppress data rather than enhancing
the existing VIR_DOMAIN_XML_SECURE may form a data leak or even a
security hole).
In the qemu driver, there are multiple callers all funnelling to
qemuDomainDefFormatBufInternal(); many of them already validated
flags (and often only a subset of the full set of possible flags),
but for ease of maintenance, we can also check flags at the common
helper function.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
These were not caught by our current regular expressions
but will be caught by the improved ones we're about to
introduce, so fix them ahead of time.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In many files there are header comments that contain an Author:
statement, supposedly reflecting who originally wrote the code.
In a large collaborative project like libvirt, any non-trivial
file will have been modified by a large number of different
contributors. IOW, the Author: comments are quickly out of date,
omitting people who have made significant contribitions.
In some places Author: lines have been added despite the person
merely being responsible for creating the file by moving existing
code out of another file. IOW, the Author: lines give an incorrect
record of authorship.
With this all in mind, the comments are useless as a means to identify
who to talk to about code in a particular file. Contributors will always
be better off using 'git log' and 'git blame' if they need to find the
author of a particular bit of code.
This commit thus deletes all Author: comments from the source and adds
a rule to prevent them reappearing.
The Copyright headers are similarly misleading and inaccurate, however,
we cannot delete these as they have legal meaning, despite being largely
inaccurate. In addition only the copyright holder is permitted to change
their respective copyright statement.
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
All of the ones being removed are pulled in by internal.h. The only
exception is sanlock which expects the application to include <stdint.h>
before sanlock's headers, because sanlock prototypes use fixed width
int, but they don't include stdint.h themselves, so we have to leave
that one in place.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Acked-by: Michal Privoznik <mprivozn@redhat.com>
It doesn't really make sense for us to have stdlib.h and string.h but
not stdio.h in the internal.h header.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Acked-by: Michal Privoznik <mprivozn@redhat.com>
When adding a new object to the domain object list, there should
have been 2 virObjectRef calls made one for each list into which
the object was placed to match the 2 virObjectUnref calls that
would occur during Remove as part of virHashRemoveEntry when
virObjectFreeHashData is called when the element is removed from
the hash table as set up in virDomainObjListNew.
Some drivers (libxl, lxc, qemu, and vz) handled this inconsistency
by calling virObjectRef upon successful return from virDomainObjListAdd
in order to use virDomainObjEndAPI when done with the returned @vm.
While others (bhyve, openvz, test, and vmware) handled this via only
calling virObjectUnlock upon successful return from virDomainObjListAdd.
This patch will "unify" the approach to use virDomainObjEndAPI
for any @vm successfully returned from virDomainObjListAdd.
Because list removal is so tightly coupled with list addition,
this patch fixes the list removal algorithm to return the object
as entered - "locked and reffed". This way, the callers can then
decide how to uniformly handle add/remove success and failure.
This removes the onus on the caller to "specially handle" the
@vm during removal processing.
The Add/Remove logic allows for some logic simplification such
as in libxl where we can Remove the @vm directly rather than
needing to set a @remove_dom boolean and removing after the
libxlDomainObjEndJob completes as the @vm is locked/reffed.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Rework the code such that virDomainObjListFindByID will always
return a locked/ref counted object so that the callers can
always do the same cleanup logic to call virDomainObjEndAPI.
Makes accessing the objects much more consistent.
NB:
There were 2 callers (lxcDomainLookupByID and qemuDomainLookupByID)
that were already using the ByID name, but not virDomainObjEndAPI -
these were changed as well in this update/patch.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Now that every caller is using virDomainObjListFindByUUIDRef,
let's just remove it and keep the name as virDomainObjListFindByUUID.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Remove the unnecessary goto error followed by goto cleanup
processing.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Avoid the need for the drivers to explicitly check for a NULL path by
making sure it is at least the empty string.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Ensuring that we don't call the virDrvConnectOpen method with a NULL URI
means that the drivers can drop various checks for NULL URIs. These were
not needed anymore since the probe functionality was split
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Declare what URI schemes a driver supports in its virConnectDriver
struct. This allows us to skip trying to open the driver entirely
if the URI scheme doesn't match.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Add a localOnly flag to the virConnectDriver struct which allows a
driver to indicate whether it is local-only, or permits remote
connections. Stateful drivers running inside libvirtd are generally
local only. This allows us to remote the check for uri->server != NULL
from most drivers.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Currently the virDrvConnectOpen method is supposed to handle both
opening an explicit URI and auto-probing a driver if no URI is
given. Introduce a dedicated virDrvConnectURIProbe method to enable the
probing functionality to be split from the driver opening functionality.
It is still possible for NULL to be passed to the virDrvConnectOpen
method after this change, because the remote driver needs special
handling to enable probing of the URI against a remote libvirtd daemon.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
For openvzDomObjFromDomainLocked and openvzDomainLookupByID
let's return a locked and referenced @vm object so that callers
can then use the common and more consistent virDomainObjEndAPI
in order to handle cleanup rather than needing to know that the
returned object is locked and calling virObjectUnlock.
The LookupByName already returns the ref counted and locked object,
so this will make things more consistent.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
If openvzDomainLookupByID or openvzDomainLookupByName fails
to find a vm, let's be a bit more descriptive by providing
the failing id or name in the error message.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Rather than repeat code throughout, create and use a couple of
accessors in order to lookup by UUID.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Some of the indents were only 2 spaces, make consistent w/ 4 spaces.
Also some indents didn't align properly. Fix them all up.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Add typedef for the anonymous enum used for the driver features. This
allows the usage of the type in a switch statement and taking
advantage of the compilers feature to detect uncovered cases.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The size argument accounts for the nul-byte to terminate
the string. Use sizeof and remove the pointless assignment.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1497396
In 0d3d020ba6c4f I've added capability to accept MAC addresses
for the API too. However, the implementation was faulty. It needs
to lookup the corresponding interface in the domain definition
and pass the ifname instead of MAC address.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1497396
The other APIs accept both, ifname and MAC address. There's no
reason virDomainInterfaceStats can't do the same.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
