We go through the trouble of checking {old|new}Bandwidth[->in] and
storing the result in local @old_floor and @new_floor, but then
we don't use them. Instead we make derefs to the longer name. This
caused Coverity to note dereferencing newBandwidth->in without first
checking @newBandwidth like was done for new_floor could cause a
NULL dereference.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>
Reviewed-by: NDaniel Henrique Barboza <danielhb413@gmail.com>

Since g_strdup_printf will abort, we know @newfile won't be NULL.

Found by Coverity
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>
Reviewed-by: NDaniel Henrique Barboza <danielhb413@gmail.com>

The @def variable holds pointer to the domain defintion, but is
set only somewhere in the middle of the function. This is
suboptimal.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Signed-off-by: NJonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

This flag is not implied by g_mkstemp_full, only by g_mkstemp.
Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reported-by: NBjoern Walk <bwalk@linux.ibm.com>
Fixes: 4ac47730Reviewed-by: NPeter Krempa <pkrempa@redhat.com>

As part of an goal to eliminate Perl from libvirt build tools,
rewrite the prohibit-duplicate-header.pl tool in Python.

This was a straight conversion, manually going line-by-line to
change the syntax from Perl to Python. Thus the overall structure
of the file and approach is the same.
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

qemuDomainDefFormatBufInternal function wasn't testing whether the CPU
was actually defined in the XML and saving such a domain resulted in the
following backtrace:

0 in qemuDomainMakeCPUMigratable (cpu=0x0)
1 in qemuDomainDefFormatBufInternal()
2 in qemuDomainDefFormatXMLInternal()
3 in qemuDomainDefFormatLive()
4 in qemuDomainSaveInternal()
5 in qemuDomainSaveFlags()
6 in qemuDomainSave()
7 in virDomainSave()
Signed-off-by: NErik Skultety <eskultet@redhat.com>
Reviewed-by: NDaniel Henrique Barboza <danielhb413@gmail.com>

Commit <f136b831> reworked process
affinity setting but did not take cgroups into account which introduced
an issue when starting VM with custom cpuset.cpus for the whole machine
group.

If the machine group is limited to some pCPUs libvirt should not try to
set a VM to run on all pCPUs as it will result in permission denied when
writing to cpuset.cpus.

To fix this the affinity has to be set separately from cgroups cpuset.

Resolves: <https://bugzilla.redhat.com/show_bug.cgi?id=1746517>
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

In functions implemented here we fill this attr union (type of
bpf_attr) and just pass it to syscall(2). Thing is that some of
the union members are type of __aligned_u64. This is not regular
uint64_t. This one is explicitly aligned to 8 bytes, while
uint64_t can be aligned to 4 bytes (on 32 bits). We've used
explicit typecast to uint64_t to shut compiler which would
otherwise complain of assigning a pointer into an integer. Well,
we have uintptr_t just for that.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

In virCgroupV2DevicesReallocMap() we are debug printing both
arguments passed to the function. However, the @size argument is
type of size_t but '%lu' is used to format it.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Reviewed-by: NAndrea Bolognani <abologna@redhat.com>
Signed-off-by: NJonathon Jongsma <jjongsma@redhat.com>

There are some OSes which don't have syscall() nor
<sys/syscall.h>. We already check for the header file in
configure phase, so we just need to add check for
HAVE_SYS_SYSCALL_H to HAVE_DECL_BPF_PROG_QUERY.

While I'm at it, some header files we are including are not
needed, so their includes can be safely dropped.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Fedora now advertises supported firmwares via descriptor files.
Since the upstream spec file assumes recent Fedora, remove the
build-time list of firmwares, which can produce a warning after
commit 75597f02.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

Ensure that both x and y are non-zero when resolution is specified for a
video device.
Reviewed-by: NCole Robinson <crobinso@redhat.com>
Signed-off-by: NJonathon Jongsma <jjongsma@redhat.com>

Since this function is now only called when an 'acceleration' element is
present in the xml, any failure to parse the element will be considered
an error.

Previously, we detected some types of errors, but we would only log an
error (virReportError()), but still return a partially-specified accel
object to the caller. This patch returns NULL for all parsing errors and
reports that error back up to the caller.
Reviewed-by: NCole Robinson <crobinso@redhat.com>
Signed-off-by: NJonathon Jongsma <jjongsma@redhat.com>

The current code doesn't properly handle errors when parsing a video
device's resolution.  We were returning a NULL structure for the case
where 'x' or 'y' were missing. But for the other error cases, we were
logging an error (virReportError()), but still returning an
under-specified structure. That under-specified structure was used by
the calling function rather than properly reporting an error.

This patch changes the parse function to return NULL on any parsing
error and changes the calling function to report an error when NULL is
returned.
Reviewed-by: NCole Robinson <crobinso@redhat.com>
Signed-off-by: NJonathon Jongsma <jjongsma@redhat.com>

Previously, we were passing the video "model" node to the "acceleration"
and "resolution" parsing functions and requiring them to iterate over
the children to discover and parse the appropriate node. It makes more
sense to move this responsibility up to the parent function and just
pass these functions the node that needs to be parsed.
Reviewed-by: NCole Robinson <crobinso@redhat.com>
Signed-off-by: NJonathon Jongsma <jjongsma@redhat.com>

Call first virCgroupNew on the parent group virCgroupNewPartition if
it is available on before the creation of the child group.  This
ensures that the creation of a first level group on the unified
architecture, as the check at virCgroupV2ParseControllersFile as the
parent file is there.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1760233Signed-off-by: NMiguel Ángel Arruga Vivas <rosen644835@gmail.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

The non-systemd configurations do not create system neither user
control groups.  The title of the diagram referenced systemd too.
Signed-off-by: NMiguel Ángel Arruga Vivas <rosen644835@gmail.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

Enables hosting a pool on an existing zfs pool without affecting
other datasets there.
Specify dataset instead of pool as source to use.
Parent of dataset must exist for pool-build to succeed.
Beware that pool-delete destroys the source dataset and all children.

Solves: https://www.redhat.com/archives/libvirt-users/2017-April/msg00041.htmlSigned-off-by: NGregor Kopka <gregor@kopka.net>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

Glib implementation follows the ISO C99 standard so it's safe to replace
the gnulib implementation.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NPeter Krempa <pkrempa@redhat.com>

We need to mock virCgroupV2DevicesAvailable() in order to remove any
dependency on kernel as BPF devices might not be available.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

So the issue here is that you can end up with configuration where
you have cgroup v1 and v2 enabled at the same time and the devices
controllers is enabled for cgroup v1.

In cgroup v2 there is no devices controller, the device access is
controlled using BPF and since it is not a cgroup controller both
of them can exists at the same time and both of them are applied while
resolving access to devices.

In order to avoid configuring both BPF and cgroup v1 devices we will
use BPF if possible and otherwise fallback to cgroup v1 devices.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

If we want to deny all devices we just need to replace any existing
program with new program with empty map.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

If we want to allow all devices with all permissions we need to replace
any existing program that has any rule configured, otherwise we just
need to add new rule which will for example allow read access to all
devices.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

In order to deny device we need to check if there is any entry in BPF
map and we need to load the current value from map if there is already
entry for that device.  If both values are same we can remove that entry
but if they are different we need to update the entry because we don't
have to deny all access, but for example only write access.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

In order to allow device we need to create key and value which will be
used to update BPF map.  virBPFUpdateElem() can override existing
entries in BPF map so we need to check if that entry exists in order to
track number of entries in our map.

This can add rule for specific device but major and minor can be both
-1 which follows the same behavior as in cgroup v1.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Device rules are stored in BPF map that is a hash type, this function
will create a key based on major and minor id of device.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

We need to close our FD that we have for BPF program and map in order
to let kernel remove all resources once the cgroup is removed as well.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

This function will be called for every virCgroup(Allow|Deny)* API in
order to prepare BPF program for guest.  Since libvirtd can be restarted
at any point we will first try to detect existing progam, if there is
none we will create a new empty BPF program and lastly if we don't have
any space left in the existing BPF map we will create a new copy of the
BPF map with more space and attach a new program with that map into the
guest cgroup.

This solution allows us to start with reasonably small BPF map consuming
only small amount of memory and if needed we can easily extend the BPF
map if there is a lot of host devices used in guest or if user wants to
hot-plug a lot of devices once the guest is running.

Since there is no way how to reallocate existing BPF map we need to
create a new copy if we run out of space in current BPF map.

This overcomes all the limitations in BPF:

    - map used in program has to be created before the program is loaded
      into kernel

    - once map is created you cannot change its size

    - you cannot replace map in existing program

    - you cannot use an array of maps because it can store FD to maps
      of one specific size so we would not be able to use it to overcome
      the second issue
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

This function creates new BPF program with new empty BPF map with the
default size and attaches it to the guest cgroup.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

This function will be called if libvirtd was restarted while some
domains were running.  It will try to detect existing programs attached
to the guest cgroup.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

This function loads the BPF prog with prepared map into kernel and
attaches it into guest cgroup.  It can be also used to replace existing
program in the cgroup if we need to resize BPF map to store more rules
for devices. The old program will be closed and removed from kernel.

There are two possible ways how to create BPF program:

    - One way is to write simple C-like code which can by compiled into
      BPF object file which can be loaded into kernel using elfutils.

    - The second way is to define macros which look like assembler
      instructions and can be used directly to create BPF program that
      can be directly loaded into kernel.

Since the program is not too complex we can use the second option.

If there is no program, all devices are allowed, if there is some
program it is executed and based on the exit status the access is
denied for 0 and allowed for 1.

Our program will follow these rules:

    - first it will try to look for the specific key using major and
      minor to see if there is any rule for that specific device

    - if there is no specific rule it will try to look for any rule that
      matches only major of the device

    - if there is no match with major it will try the same but with
      minor of the device

    - as the last attempt it will try to look for rule for all devices
      and if there is no match it will return 0 to deny that access
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

There is no exact way how to figure out whether BPF devices support is
compiled into kernel.  One way is to check kernel configure options but
this is not reliable as it may not be available.  Let's try to do
syscall to which will list BPF cgroup device programs.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

In order to implement devices controller with cgroup v2 we need to
add support for BPF programs, cgroup v2 doesn't have devices controller.

This introduces required helpers wrapping linux syscalls.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Some of our tests try to validate domain XMLs they are working
with (not intentionally, simply because they call top level
domain XML parse function). Anyway, this implies that we build
domain capabilities also - see
virQEMUDriverGetDomainCapabilities(). And since some domain XMLs
are type of 'kvm' the control gets through
virQEMUCapsFillDomainCaps() and virHostCPUGetKVMMaxVCPUs() to
opening /dev/kvm which may be missing on the machine we're
running 'make check'.

Previously, we did not see this issue, because it was masked. If
building domain capabilities failed for whatever reason, we
ignored the failure. Only v5.9.0-207-gc69e6ede uncovered the
problem (it changed reval from 0 to -1 if
virQEMUDriverGetDomainCapabilities() fails). Since the referenced
commit is correct, we need to mock access to /dev/kvm in our
tests.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

Some layered products such as oVirt have requested a way to avoid being
blocked by guest agent commands when querying a loaded vm. For example,
many guest agent commands are polled periodically to monitor changes,
and rather than blocking the calling process, they'd prefer to simply
time out when an agent query is taking too long.

This patch adds a way for the user to specify a custom agent timeout
that is applied to all agent commands.

One special case to note here is the 'guest-sync' command. 'guest-sync'
is issued internally prior to calling any other command. (For example,
when libvirt wants to call 'guest-get-fsinfo', we first call
'guest-sync' and then call 'guest-get-fsinfo').

Previously, the 'guest-sync' command used a 5-second timeout
(VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT), whereas the actual command that
followed always blocked indefinitely
(VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK). As part of this patch, if a
custom timeout is specified that is shorter than
5 seconds,  this new timeout is also used for 'guest-sync'. If there is
no custom timeout or if the custom timeout is longer than 5 seconds, we
will continue to use the 5-second timeout.
Signed-off-by: NJonathon Jongsma <jjongsma@redhat.com>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NPeter Krempa <pkrempa@redhat.com>