The code of adding PID to the allocation could be reused, refactor it
for later reuse.
Signed-off-by: NWang Huaqiang <huaqiang.wang@intel.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Add interface for resctrl monitor to determine the path.
Signed-off-by: NWang Huaqiang <huaqiang.wang@intel.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

The code for determining resctrl allocation path could be reused
for monitor. Refactor it for reuse.
Signed-off-by: NWang Huaqiang <huaqiang.wang@intel.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Cache Monitoring Technology (aka CMT) provides the capability
to report cache utilization information of system task.

This patch introduces the concept of resctrl monitor through
data structure virResctrlMonitor.
Signed-off-by: NWang Huaqiang <huaqiang.wang@intel.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Refactor schemas and virresctrl to support optional <cache> element
in <cachetune>.

Later, the monitor entry will be introduced and to be placed
under <cachetune>. Either cache entry or monitor entry is
an optional element of <cachetune>.

An cachetune has no <cache> element is taking the default resource
allocating policy defined in '/sys/fs/resctrl/schemata'.
Signed-off-by: NWang Huaqiang <huaqiang.wang@intel.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

When no server name is provided in the URI, modern versions of libxml2
will set the port to '-1'. This is a change from behaviour with earlier
versions which set it to 0.

Libvirt expects the port to be 0 in these cases and as a result we get a
bug when connecting to URIs which lack a server name:

$ virsh  -c test+ssh:///default list
error: failed to connect to the hypervisor
error: Cannot recv data: Bad port '-1': Connection reset by peer

This libxml2 change was attempting to fix another bug identified by
libvirt where it didn't roundtrip URIs correctly in:

  https://github.com/GNOME/libxml2/commit/beb7281055dbf0ed4d041022a67c6c5cfd126f25

Essentially libxml2 was not expecting apps to look at the URI port
field when the server name is not provided. This was a reasonable
assumption, but none the less libvirt did look at it :-)

The fix is to ensure we explicitly set port to 0 when server name
is not present, avoiding undefined behaviour for the port field in
libxml2.
Reviewed-by: NErik Skultety <eskultet@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

This patch introduces a new shutdown reason "daemon" in order
to indicate that the daemon needed to force shutdown the domain
as the best course of action to take at the moment.

This action would occur during reconnection when processing
encounters an error once the monitor reconnection is successful.
Signed-off-by: NNikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

When libvirt configuration includes '--with-apparmor-profiles', the
make uninstall target fails

make[1]: Entering directory '/home/jim/upstream/libvirt/examples'
 ( cd '/etc/apparmor.d//abstractions' && rm -f libvirt-qemu libvirt-lxc )
 ( cd '/etc/apparmor.d/' && rm -f usr.lib.libvirt.virt-aa-helper usr.sbin.libvirtd )
make[1]: *** No rule to make target 'uninstall-apparmor-local', needed by
'uninstall-local'.  Stop.

Add missing 'uninstall-apparmor-local' target to the examples Makefile.am.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>

Return -1 and report an error message if no transaction is set and
virSecuritySELinuxTransactionCommit is called.

The function description of virSecuritySELinuxTransactionCommit says:

  "Also it is considered as error if there's no transaction set and this
   function is called."
Signed-off-by: NMarc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: NBoris Fiuczynski <fiuczy@linux.ibm.com>

In 4674fc6a I've implemented transactions for selinux driver.
Well, now that I am working in this area I've noticed a subtle
bug: @ret is initialized to 0 instead of -1. Facepalm.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NMarc Hartmayer <mhartmay@linux.ibm.com>

Signed-off-by: NAndrea Bolognani <abologna@redhat.com>

As it's currently impossible for us to create new automated
builds on Docker Hub (see [1]), and quay.io doesn't suffer
from the same problem while still having all the feature we
need, switch to the latter.

[1] https://github.com/docker/hub-feedback/issues/1676Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

VFIO AP has a limitation on a single device per domain, however, when
commit 11708641 added the support for vfio-ap, check for this limitation
was performed as part of the post parse code. Generally, checks like that
should be performed within the driver's validation callback to eliminate
any slight chance of failing in post parse, which could potentially
result in the domain XML config vanishing.
Signed-off-by: NErik Skultety <eskultet@redhat.com>
Reviewed-by: NBoris Fiuczynski <fiuczy@linux.ibm.com>

Since we'll need to validate other models apart from VFIO PCI too,
having a helper for each model should keep the code base cleaner.
Signed-off-by: NErik Skultety <eskultet@redhat.com>
Reviewed-by: NBoris Fiuczynski <fiuczy@linux.ibm.com>

There's a lot of stuff going on in src/conf/nodedev_conf which is
sometimes not directly related to config and we're not really consistent
with putting only parser/formatter related stuff here, e.g. like we do
for domains. So, let's start simply by adding a new module
node_device_util containing some of the helpers. Unfortunately, even
though these helpers tend to open a secondary driver connection and would
be much therefore better suited as a nodedev driver module, we can't do
that without pulling headers from the driver into conf/ and that's wrong
because we want conf/ to stay driver-agnostic.
Signed-off-by: NErik Skultety <eskultet@redhat.com>
Acked-by: NMichal Privoznik <mprivozn@redhat.com>

The gotShutdown bool has been redundant since we started setting
VIR_DOMAIN_SHUTDOWN state after receiving SHUTDOWN event from QEMU.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

If gotShutdown is true, the domain state cannot be running because of
the following code in qemuProcessHandleShutdown:

    priv->gotShutdown = true;

    VIR_DEBUG("Transitioned guest %s to shutdown state",
              vm->def->name);
    virDomainObjSetState(vm,
                         VIR_DOMAIN_SHUTDOWN,
                         VIR_DOMAIN_SHUTDOWN_UNKNOWN);
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

On aarch64, lauch vm with the follow configuration:

<interface type="hostdev" managed="yes">
  <mac address="fa:16:3e:14:41:00"/>
  <source>
    <address type="pci" domain="0x0000" bus="0x01" slot="0x0b" function="0x2"/>
  </source>
</interface>

libvirtd will crash when accessing net->model.
Signed-off-by: NWang Yechao <wang.yechao255@zte.com.cn>
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>

Signed-off-by: NJim Fehlig <jfehlig@suse.com>
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>

If qemuDomainSnapshotDiscard() fails for any reason (rare,
but possible with an ill-timed ENOMEM or if
qemuDomainSnapshotForEachQcow2() has problems talking to the
qemu guest monitor), then an attempt to retry the snapshot
deletion API will crash because we didn't undo the effects
of virDomainSnapshotDropParent() temporarily rearranging the
internal list structures, and the second attempt to drop
parents will dereference NULL.  Fix it by instead noting that
there are only two callers to qemuDomainSnapshotDiscard(),
and only one of the two callers wants the parent to be updated;
thus we can move the call to virDomainSnapshotDropParent()
into a code path that only gets executed on success.
Signed-off-by: NEric Blake <eblake@redhat.com>
ACKed-by: NMichal Privoznik <mprivozn@redhat.com>

In accordance with our platform support policy, now that
Fedora 29 is out we no longer support building on Fedora 27.

This allows us to remove a few version checks.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Since commit v4.7.0-302-ge6d77a75 processing RESUME event is mandatory
for updating domain state. But the event handler explicitly ignored this
event in some cases. Thus the state would be wrong after a fake reboot
or when a domain was rebooted after it crashed.

BTW, the code to ignore RESUME event after SHUTDOWN didn't make sense
even before making RESUME event mandatory. Most likely it was there as a
result of careless copy&paste from qemuProcessHandleStop.

The corresponding debug message was clarified since the original state
does not have to be "paused" only and while we have a "resumed" event,
the state is called "running".

https://bugzilla.redhat.com/show_bug.cgi?id=1612943Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

Commit id 5eb61e68 neglected to change the name in the wrong value
output to virCgroupGetPercpuStats from virCgroupGetMemoryUsage.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

The array "mount" inside lxc_container is not being checked before for
loop. Clang syntax scan is complaining about this segmentation fault.
Signed-off-by: NJulio Faracco <jcfaracco@gmail.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

The current qemuProcessReconnect logic paints a broad brush
determining that the shutdown reason must be crashed if it was
determined that the domain was started with -no-shutdown; however,
there's many other ways to get to the error label, so let's narrow
our reasoning window for using VIR_DOMAIN_SHUTOFF_CRASHED to the
period where we essentially know we've tried to create to the
monitor and before we were successful in opening the connection.

Failures that occur outside that window would thus be considered
as VIR_DOMAIN_SHUTOFF_UNKNOWN, at least for now.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>
ACKed-by: NMichal Privoznik <mprivozn@redhat.com>

When qemuProcessReconnectHelper was introduced (commit d38897a5)
reconnection failure used VIR_DOMAIN_SHUTOFF_FAILED; however, that
was changed in commit bda2f17d to either VIR_DOMAIN_SHUTOFF_CRASHED
or VIR_DOMAIN_SHUTOFF_UNKNOWN.

When QEMU_CAPS_NO_SHUTDOWN checking was removed in commit fe35b1ad
the conditional state was just left at VIR_DOMAIN_SHUTOFF_CRASHED.

So introduce qemuDomainIsUsingNoShutdown which will manage the
condition when the domain was started with -no-shutdown so that
when/if reconnection failure occurs we can restore the decision
point used to determine whether CRASHED or UNKNOWN is provided.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>
ACKed-by: NMichal Privoznik <mprivozn@redhat.com>

V2 of the libxl soft reset patch, which was pushed as commit da4b0fd9,
dropped the hunk that disposed of the libxl_domain_config object. Add
the missing hunk to properly dispose the object.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>

The pvops Linux kernel implements machine_ops.crash_shutdown as

static void xen_hvm_crash_shutdown(struct pt_regs *regs)
{
        native_machine_crash_shutdown(regs);
        xen_reboot(SHUTDOWN_soft_reset);
}

but currently the libxl driver does not handle the soft reset
shutdown event. As a result, the guest domain never proceeds
past xen_reboot(), making it impossible for HVM domains to save
a crash dump using kexec.

This patch adds support for handling the soft reset event by
calling libxl_domain_soft_reset() and re-enabling domain death
events, which is similar to the xl tool handling of soft reset
shutdown event.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>
ACKed-by: NMichal Privoznik <mprivozn@redhat.com>

There are too many goto labels in libxlDomainShutdownThread. Convert the
'destroy' and 'restart' labels to helper functions, leaving only the
commonly used pattern of 'endjob' and 'cleanup' labels.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>
ACKed-by: NMichal Privoznik <mprivozn@redhat.com>

In libxlDomainShutdownThread, virObjectEventStateQueue is needlessly
called in the destroy and restart labels. The cleanup label aready
queues whatever event was created based on libxl_shutdown_reason.
There is no need to handle destroy and restart differently.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>
ACKed-by: NMichal Privoznik <mprivozn@redhat.com>

Make it avaiable to other chardev types.
Signed-off-by: NJán Tomko <jtomko@redhat.com>

Remove repetition of the 'socket' device and its alias.
Signed-off-by: NJán Tomko <jtomko@redhat.com>

Pass the driver's xmlopt to allocate the chardev source private
data correctly.
Signed-off-by: NJán Tomko <jtomko@redhat.com>

Our HACKING guide forbids these.
There's no point in exempting these from the spacing check
if their existence is against our coding style.

Note that the non-usage of these comments itself is not enforced
by syntax check, probably because of the need to implement a C parser.
Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Recent patches added indentation checks that discovered some cosmetic
issues at the cost of making this check last as long as the rest of
syntax-check combined on my system. Also, they're moving closer
to us implementing yet another C parser (docs/apibuild.py being the
other one).

Revert the following commits:
commit 11e1f11d
    syntax-check: Check for incorrect indentation in function body
commit 2585a79e
    build-aux:check-spacing: Introduce a new rule to check misaligned stuff in parenthesises
commit a033182f
    build-aux:check-spacing: Add wrapper function of CheckCurlyBrackets
commit 6225626b
    build-aux:check-spacing: Add wrapper function of CheckWhiteSpaces
commit c3875129
    build-aux:check-spacing: Add wrapper function of KillComments
commit e995904c
    build-aux:check-spacing: Add wrapper function of CheckFunctionBody
commit 11e1f11d
    syntax-check: Check for incorrect indentation in function body

This brings the speed of the script to a tolerable level and lets it
focus on the more visible issues.
Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

https://bugzilla.redhat.com/show_bug.cgi?id=1631606

Since commit 82592551 usage of a primary connection driver for
a virConnect has been modified to open (virConnectOpen) and use
a connection to the specific driver in order to handle the API
calls to/for that driver. This causes some confusion and issues
for ACL polkit rule scripts to know exactly which driver by
name will be used.

Add some documentation describing the processing of the primary
and secondary connection as well as the list of the connect_driver
names used for each driver.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>
ACKed-by: NMichal Privoznik <mprivozn@redhat.com>

https://bugzilla.redhat.com/show_bug.cgi?id=1631606

Changes made to manage and utilize a secondary connection
driver to APIs outside the scope of the primary connection
driver have resulted in some confusion processing polkit rules
since the simple "access denied" error message doesn't provide
enough of a clue when combined with the "authentication failed:
access denied by policy" as to which connection driver refused
or failed the ACL check.

In order to provide some context, let's modify the existing
"access denied" error returne from the various vir*EnsureACL
API's to provide the connection driver name that is causing
the failure. This should provide the context for writing the
polkit rules that would allow access via the driver.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>
ACKed-by: NMichal Privoznik <mprivozn@redhat.com>

Commit 57f5621f modified nwfilterInstantiateFilter to detect when
a filter binding was already present before attempting to add the
new binding and instantiate it. Additionally, the change to
nwfilterStateInitialize to call virNWFilterBindingObjListLoadAllConfigs
(from commit c21679fa) to load active domain filter bindings, but
not instantiate them eventually leads to a problem for the QEMU
driver reconnection logic after a daemon restart where the filter
bindings would no longer be instantiated.

Subsequent commit f14c37ce replaced the nwfilterInstantiateFilter
with virDomainConfNWFilterInstantiate which uses @ignoreExists to
detect presence of the filter and still did not restore the filter
instantiation call when making the new nwfilter bindings logic active.

Thus in order to instantiate any active domain filter, we will call
virNWFilterBuildAll with 'false' to indicate the need to go through
all the active bindings calling virNWFilterInstantiateFilter to
instantiate the filter bindings.
Signed-off-by: NNikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Commit cdbe1332 neglected to document the API. So let's add some
details about the algorithm and why it was used to help future
readers understand the issues encountered.

NB: Management of the processing udev device notification is a
delicate balance between the udev process, the scheduler, and when
exactly the data from/for the socket is received. The balance is
particularly important for environments when multiple devices are
added into the system more or less simultaneously such as is done
for mdev or SRIOV. In these cases old libudev blocking on the udev
recv() occurs more frequently. It's expected that future devices
will follow similar algorithms. Even though the algorithm does
present some challenges for older OS's (such as Centos 6), trying
to rewrite the algorithm to fit both models would be more complex
and involve pulling the monitor object out of the private data
lockable object and would need to be guarded by a separate lock.
Devising such an algorithm to work around issues with older OS's
at the expense of more modern OS algorithms in newer event processing
code may result in unexpected issues, so the choice is to encourage
use of newer OS's with newer udev event processing code.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

https://bugzilla.redhat.com/show_bug.cgi?id=1524230

The qemuBuildVhostuserCommandLine builds command line for
vhostuser type interfaces. It is duplicating some code of the
function it is called from (qemuBuildInterfaceCommandLine)
because of the way it's called. If we merge it into the caller
not only we save a few lines but we also enable checks that we
would have to duplicate otherwise (e.g. QoS availability).
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>