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: NChris Venteicher <cventeic@redhat.com>
Reviewed-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NJá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: NChris Venteicher <cventeic@redhat.com>
Reviewed-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NJá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: NChris Venteicher <cventeic@redhat.com>
Reviewed-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

s/cmd/proc/ in process code imported from qemu_capabilities.
Signed-off-by: NChris Venteicher <cventeic@redhat.com>
Reviewed-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NJá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: NChris Venteicher <cventeic@redhat.com>
Reviewed-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

s/virQEMUCapsInitQMPCommand/qemuProcessQMP/
Signed-off-by: NChris Venteicher <cventeic@redhat.com>
Reviewed-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NJá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: NChris Venteicher <cventeic@redhat.com>
Reviewed-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NJá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: NJán Tomko <jtomko@redhat.com>
Signed-off-by: NDaniel 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: NLaine Stump <laine@laine.org>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

The newline was pretty arbitrary, and we're better off
without it.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>
Reviewed-by: NJohn 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: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

Remove unused variable. Fix for [1]

[1] 821dd6d8: storage: Use VIR_AUTOFREE for storage backends
Signed-off-by: NNikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>

We dropped support in commit 8e91a400 (November 2015), but some
occurrences still remained, even in live code.
Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reported-by: NThomas Huth <thuth@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Commit dcda2bf4 forgot to fix this one instance.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@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: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Since virStorageSource is now a subclass of virObject, we can use
VIR_AUTOUNREF instead.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Add helper for utilizing __attribute__(cleanup())) for unref-ing
instances of sublasses of virObject.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NErik 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: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Add virStorageSourceNew and refactor places allocating that structure to
use the helper.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Use the proper function to allocate a disk definition.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NErik 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: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Minor tweaks to ensure compliance with our coding style.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Minor tweaks to ensure compliance with our coding style.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Minor tweaks to ensure compliance with our coding style.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJá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/1650320Signed-off-by: NLaine Stump <laine@laine.org>
Acked-by: NEric 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: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NDaniel 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=1662849Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

My change in 112f3a8d 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: NJohn Ferlan <jferlan@redhat.com>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJohn 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: NBjoern Walk <bwalk@linux.ibm.com>
Signed-off-by: NMarc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Instead of repetitive:
  s ? s : ""
use NULLSTR_EMPTY.
Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>

Another misleadingly named macro.
Deprecate in favor of NULLSTR_STAR.
Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>

This macro neither takes nor produces an empty string.
Remove it in favor of NULLSTR_MINUS.
Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>

Use the newly introduced macro in the few places that open-code it.
Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>

NULLSTR_EMPTY, the quiet child,
NULLSTR_STAR, the famous one and
NULLSTR_MINUS, the grumpy one.
Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NAndrea 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: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

This is basically an old artefact from 24b08219 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: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Previous two commits demonstrate a hole in our test scenario.
Fix that.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>