Keep the pointer to QEMU stderr output in qemuProcessQMP struct instead
of requiring the caller to provide it (and free it).
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Let's push the call to virQEMUCapsLogProbeFailure down the stack to
where the probing failure is detected.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
While qemuProcessQMPRun and virQEMUCapsInitQMPMonitor* functions called
from virQEMUCapsInit ignore some errors, the caller of virQEMUCapsInit
would report an error unless usedQMP is true anyway. And since usedQMP
can only be true if the probing code really succeeded (i.e., no errors
were ignored), we can just simplify the logic by not ignoring the errors
in the first place.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function contains two almost identical parts. Let's consolidate them
into a single helper function and call it twice.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In new process code, move from model where qemuProcessQMP struct can be
used to activate a series of Qemu processes to model where one
qemuProcessQMP struct is used for one and only one Qemu process.
By allowing only one process activation per qemuProcessQMP struct, the
struct can safely store process outputs like status and stderr, without
being overwritten, until qemuProcessQMPFree is called.
By doing this, process outputs like status and stderr can remain stored
in the qemuProcessQMP struct without being overwritten by subsequent
process activations.
The forceTCG parameter (use / don't use KVM) will be passed when the
qemuProcessQMP struct is initialized since the qemuProcessQMP struct
won't be reused.
Signed-off-by: Chris Venteicher <cventeic@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virQEMUCapsInitQMP now stops QEMU process in all execution paths,
before freeing the process structure.
The qemuProcessQMPStop function can be called multiple times without
problems... Won't attempt to stop processes and free resources multiple
times.
Follow the convention established in qemu_process of
1) alloc process structure
2) start process
3) use process
4) stop process
5) free process data structure
The process data structure persists after the process activation fails
or the process dies or is killed so stderr strings can be retrieved
until the process data structure is freed.
Signed-off-by: Chris Venteicher <cventeic@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
s/qemuProcessQMPAbort/qemuProcessQMPStop/ applied to change function
name used to stop QEMU processes in process code moved from
qemu_capabilities.
No functionality change.
The new name, qemuProcessQMPStop, is consistent with the existing
function qemuProcessStop used to stop Domain processes.
Signed-off-by: Chris Venteicher <cventeic@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
s/cmd/proc/ in process code imported from qemu_capabilities.
Signed-off-by: Chris Venteicher <cventeic@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add the const qualifier on non modified strings
(string only copied inside qemuProcessQMPNew)
so that const strings can be used directly in calls to
qemuProcessQMPNew in future patches.
Signed-off-by: Chris Venteicher <cventeic@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
QEMU process code in qemu_capabilities.c is moved to qemu_process.c in
order to make the code usable outside the original capabilities use
cases.
The moved code activates and manages QEMU processes without establishing
a guest domain.
This patch is a straight cut/paste move between files.
Signed-off-by: Chris Venteicher <cventeic@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function must return a pointer, not a boolean. Fortunately 'false'
is equivalent to 'NULL' so this bug no had ill effect previously.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Only one of the three callers of virPCIDeviceAddressFormat correctly
handles an error return status. Fortunately it can't fail so can be
made void.
Reviewed-by: Laine Stump <laine@laine.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The newline was pretty arbitrary, and we're better off
without it.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The memory allocated by VIR_REALLOC_N() is uninitialized,
which means it's not possible to figure out whether any
output was produced at all after the fact.
Since we don't care about the previous contents of buffers,
if any, use VIR_FREE() followed by VIR_ALLOC_N() instead.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Remove unused variable. Fix for [1]
[1] 821dd6d8: storage: Use VIR_AUTOFREE for storage backends
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
We dropped support in commit 8e91a40 (November 2015), but some
occurrences still remained, even in live code.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reported-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Now that virStorageSource is a subclass of virObject we can use
virObjectUnref and remove virStorageSourceFree which was a thin wrapper.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Since virStorageSource is now a subclass of virObject, we can use
VIR_AUTOUNREF instead.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Add helper for utilizing __attribute__(cleanup())) for unref-ing
instances of sublasses of virObject.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
To allow tracking a single virStorageSource in multiple structures
without extra hassle allow refcounting by turining it into an object.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Add virStorageSourceNew and refactor places allocating that structure to
use the helper.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Use the proper function to allocate a disk definition.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Now that we've moved all the actual code into helper
functions, we can turn it into a switch statement.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Minor tweaks to ensure compliance with our coding style.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Minor tweaks to ensure compliance with our coding style.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Minor tweaks to ensure compliance with our coding style.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The libvirt zonefile for firewalld (added in commit 3b71f2e4) does the
following:
1) lists specific services it wants to allow, then
2) uses a lower priority <reject/> rule to block all other services to
the host, and then finally,
3) relies on the zone's default "accept" policy to, accept all
forwarded traffic (since forwarded traffic is ignored by the
slightly higher priority <reject/> rule in (2)).
I had assumed that icmp traffic was either being allowed at the top of
the rules, or that it would be ignored by the <reject/> rule and
passed by the default accept policy (similar to forwarded traffic),
but this assumption was incorrect; the <reject/> rule does block icmp
traffic. This became apparent when DHCPv6 which requires ICMPv6 in
addition to udp/dhcpv6) failed to work.
This all means that in order to achieve our original goal of "similar
behavior to a default reject policy, but also allowing forwarded
traffic", we need to add rules to allow all icmp and icmpv6 traffic to
the libvirt zone, and that's what this patch does.
This is a further refinement of the resolution to
https://bugzilla.redhat.com/1650320
Signed-off-by: Laine Stump <laine@laine.org>
Acked-by: Eric Garver <eric@garver.life>
On some very basic installations (e.g. some container images) the
modprobe binary might be missing. If that is the case, don't fail
virkmodtest.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The root snapshot does not have a parent.
Use NULLSTR_EMPTY to pass an empty string instead of putting
too few columns in the table.
https://bugzilla.redhat.com/show_bug.cgi?id=1662849
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
My change in 112f3a8d0f was too drastic. The @charAlias
variable is initialized only if @monitor == true. However, it is
used even outside of that condition, at which point it's just
uninitialized pointer.
Reported-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Even if an error is reported by `udev_enumerate_scan_devices`,
e.g. because a driver of a device has an bug, we can still enumerate
all other devices. Additionally the documentation of
udev_enumerate_scan_devices says that on success an integer >= 0 is
returned (see man udev_enumerate_scan_devices(3)).
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Another misleadingly named macro.
Deprecate in favor of NULLSTR_STAR.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
This macro neither takes nor produces an empty string.
Remove it in favor of NULLSTR_MINUS.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Use the newly introduced macro in the few places that open-code it.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
NULLSTR_EMPTY, the quiet child,
NULLSTR_STAR, the famous one and
NULLSTR_MINUS, the grumpy one.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The @tmpChr is looked up in domain definition based on user
provided chardev XML. Therefore, the alias must have been
allocated already when domain was started up.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This is basically an old artefact from 24b0821926 when the idea
was:
1) Build device string only to see if chardev has any -device
associated with it and thus if device_del is needed
2) Detach chardev using chardev_del
Now, that DEVICE and DEVICE_DELETED capabilities are assumed for
every domain 1) does not make sense anymore.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Previous two commits demonstrate a hole in our test scenario.
Fix that.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1624204
The guestfwd channels are -netdevs really. Hotunplug them as
such. Also, DEVICE_DELETED event is not triggered (surprisingly,
since we're not issuing device_del rather than netdev_del) and
associated chardev is removed automagically too. This means that
we need to do qemuDomainRemoveChrDevice() minus monitor call to
remove the chardev.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1624204
The guestfwd channels are -netdevs really. Hotplug them as such.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Introduced by d86c876a66.
There is no real need to have "user-" prefix for chardev.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
So far we are passing @chr to qemuBuildChrDeviceStr. This is
suboptimal (in fact wrong) because @chr is just parsed XML
definition provided by user which by definition may lack some
information. On the other hand, @tmpChr is the one that was found
using @chr in domain definition so it contains the same amount of
information or more.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The code for creating external snapshots for an offline domain
called out to qemu-img without escaping commas in the manner
that qemu-img expects. This also fixes a typo in the comment.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
commit 3bba4825 added the new function virFirewallDInterfaceSetZone()
which calledsends virDBUSCallMethod a DBusMessage** for the reply
message, but doesn't use the reply, and also doesn't free it. Since
this arg is allowed to be NULL, this patch simply sets it to NULL so
we don't have to deal with it.
Signed-off-by: Laine Stump <laine@laine.org>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
