resctrl object stored in def->resctrls is shared by cachetune and
memorytune. The domain xml configuration is parsed firstly for
cachetune then memorytune, and the resctrl object will not be created
in parsing settings for memorytune once it found sharing exists.
But resctrl is improperly freed when sharing happens.
Signed-off-by: Wang Huaqiang <huaqiang.wang@intel.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Since libvirt-jenkins-ci commit 3c5ac0af41ba, MinGW packages
are installed on Fedora 30 rather than Fedora Rawhide, so we
need to update the Travis CI configuration accordingly.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>
GitLab CI unfortunately doesn't use the standard Makefile.ci
machinery, so its configuration needs to be updated separately.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>
Since libvirt-dockerfile commit 7130ffe0a0e9, the containers
used for CI builds have been renamed from buildenv-* to
buildenv-libvirt-* in order to make it possible for projects
other than libvirt to be supported, so we need to update our
Makefile.ci scaffolding accordingly.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>
Mocking of the __open_2 function was added in
commit 459f071cac
Author: Michal Privoznik <mprivozn@redhat.com>
Date: Thu Aug 15 16:37:17 2019 +0200
virpcimock: Mock __open_2()
This function only exists in glibc, however, and the mocking code runs
on systems not using glibc, such as FreeBSD. Even Linux hosts might be
using a different libc impl, though we don't actively try to support
that.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
If LXC is disabled at build time then there is no
libvirt_driver_lxc_impl_la-*.lo to run the 'check-protocol'
against.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Fix some typos and grammar (calling something safer and error-prone is
odd, and 'ther eneeds' is an obvious typo), and reflow some long
lines.
Signed-off-by: Eric Blake <eblake@redhat.com>
Gnulib has added a patch that allows configmake.h to be included
without causing build failures on mingw if <winsock2.h> is later
included (whether directly, or indirectly such as through gnulib's
<unistd.h>).
This reverts commit fed58d83c6 ("build:
Fix checkpoint_conf on mingw"), now that we don't have to worry about
header inclusion ordering issues.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Since
commit 432faf259b
Author: Michal Privoznik <mprivozn@redhat.com>
Date: Tue Jul 2 19:49:51 2019 +0200
virCommand: use procfs to learn opened FDs
When spawning a child process, between fork() and exec() we close
all file descriptors and keep only those the caller wants us to
pass onto the child. The problem is how we do that. Currently, we
get the limit of opened files and then iterate through each one
of them and either close() it or make it survive exec(). This
approach is suboptimal (although, not that much in default
configurations where the limit is pretty low - 1024). We have
/proc where we can learn what FDs we hold open and thus we can
selectively close only those.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
v5.5.0-173-g432faf259b
programs using the virCommand APIs on Linux need read access to
/proc/self/fd, or they will fail like
error : virCommandWait:2796 : internal error: Child process
(LIBVIRT_LOG_OUTPUTS=3:stderr /usr/lib/libvirt/virt-aa-helper -c
-u libvirt-b20e9a8e-091a-45e0-8823-537119e98bc6) unexpected exit
status 1: libvirt: error : cannot open directory '/proc/self/fd':
Permission denied
virt-aa-helper: error: apparmor_parser exited with error
Update the AppArmor profile for virt-aa-helper so that read access
to the relevant path is granted.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The way we're processing the return status, using WIFEXITED() and
friends, only works when we have the raw return status; however,
virCommand defaults to processing the return status for us. Call
virCommandRawStatus() before virCommandRun() so that we get the raw
return status and the logic can actually work.
This results in guest startup failures caused by AppArmor issues
being reported much earlier: for example, if virt-aa-helper exits
with an error we're now reporting
error: internal error: cannot load AppArmor profile 'libvirt-b20e9a8e-091a-45e0-8823-537119e98bc6'
instead of the misleading
error: internal error: Process exited prior to exec: libvirt:
error : unable to set AppArmor profile 'libvirt-b20e9a8e-091a-45e0-8823-537119e98bc6'
for '/usr/bin/qemu-system-x86_64': No such file or directory
Suggested-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Right now we're using the virRun() convenience API, but that
doesn't allow the kind of control we want. Use the virCommand
APIs directly instead.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The nwfilter XML configs are not merely examples, they are data that is
actively shipped and used in production by users.
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The QEMU driver now supports Direct Mode for Hyper-V Synthetic timers
for Hyper-V guests.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
QEMU-4.1 supports 'Direct Mode' for Hyper-V synthetic timers
(hv-stimer-direct CPU flag): Windows guests can request that timer
expiration notifications are delivered as normal interrupts (and not
VMBus messages). This is used by Hyper-V on KVM.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Support 'Direct Mode' for Hyper-V Synthetic Timers in domain config.
Make it 'stimer' enlightenment option as it is not a separate thing.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
In particular, use DO_TEST_CAPS_LATEST which tests the canonical
'hv-feature' syntax instead of 'hv_feature' aliases and DO_TEST_CAPS_VER
with 4.0.0 to also test the old syntax.
Suggested-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
The pci-stub is so old school that no one uses it. All modern
systems have adapted VFIO. Switch our virpcitest too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
The pci-stub is so old school that no one uses it. All modern
systems have adapted VFIO. Switch our virhostdevtest too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
The pci-assign device is so old school that no one uses it. All
modern systems have adapted VFIO. Switch our xml2argv test too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
The virHostdevPreparePCIDevices() function works in several
steps. In the very first one, it checks if devices we want to
detach from the host are not taken already by some other domain.
However, this piece of code returns different results depending
on the stub driver used (which is not wrong per se, but keep on
reading). If the stub driver is KVM then
virHostdevIsPCINodeDeviceUsed() is called which basically checks
if a PCI device from the detach list is not used by any domain
(including the one we are preparing the device for). If that is
the case, an error is reported ("device in use") and -1 is
returned.
However, that is not what happens if the stub driver is VFIO. If
the stub driver is VFIO, then we iterate over all PCI devices
from the same IOMMU group and check if they are taken by some
other domain (because a PCI device, well IOMMU group, can't be
shared between two or more qemu processes). But we fail to check,
if the device we are trying to detach from the host is not
already taken by a domain. That is, calling
virHostdevPreparePCIDevices() over a hostdev device twice
succeeds the first time and fails too late in the second run
(fortunately, virHostdevResetAllPCIDevices() will throw an error,
but this is already too late because the PCI device in question
was moved to the list of inactive PCI devices and now it appears
in both lists).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
It may happen that there are two domains with the same name in
two separate drivers (e.g. qemu and lxc). That is why for PCI
devices we track both names of driver and domain combination
which has taken the device. However, when we check if given PCI
device is in use (or PCI devices from the same IOMMU group) we
compare only domain name. This means that we can mistakenly claim
device as free to use while in fact it isn't.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
So far, we don't need to create anything under
/sys/kernel/iommu_groups/N/devices directory (which is symlinked
from /sys/bus/pci/devices/DDDD:BB:DD.F/iommu_group directory)
because virhostdevtest still tests the old KVM assignment and
thus has no notion of IOMMU groups. This will change in near
future though. And in order to discover devices belonging to the
same IOMMU group we need to do what kernel does - create symlinks
to devices.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
So far, we are creating devices directly under
/sys/bus/pci/devices/*. There is not much problem with it, but if
we really want to model kernel behaviour we need to create them
under /sys/devices/pciDDDD:BB and then only symlink them from the
old location.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
In upcoming patches we will need only some portions of the PCI
address. To construct that easily, it's better if the PCI address
of a device is stored as four integers rather than one string.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Have just one function to generate path to a PCI driver so that
when we change it in near future there's only few of the places
we need to fix.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Have just one function to generate path to a PCI device so that
when we change it in near future there's only few of the places
we need to fix.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
In near future, we will be creating devices under different
location and just symlink them under devices/. Just like real
kernel does. But for that we need the directories to exist.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
We will need to create more directories and instead of
introducing bunch of new variables to hold their actual
paths, we can have one and reuse it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
The @fakesysfspcidir is derived from @fakerootdir. We don't need
two global variables that contain nearly the same content,
especially when we construct the actual path anyways.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
It saves us couple of lines.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
When creating a PCI device, the pciDevice structure contains @id
member which holds device address (DDDD.BB:DD.F) and is type of
'char *'. But the structure is initialized from a const char and
in fact we never modify or free the @id.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Newer kernels (v3.16-rc1~29^2~6^4) have 'driver_override' file
which simplifies way of binding a PCI device to desired driver.
Libvirt has support for this for some time too (v2.3.0-rc1~236),
but not our virpcimock. So far we did not care because our code
is designed to deal with this situation. Except for one.
hypothetical case: binding a device to the vfio-pci driver can be
successful only via driver_override. Any attempt to bind a PCI
device to vfio-pci driver using old method (new_id + unbind +
bind) will fail because of b803b29c1a. While on vanilla kernel
I'm able to use the old method successfully, it's failing on RHEL
kernels (not sure why).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
This reverts commit b70c093ffa.
In next commit the virpcimock is going to be extended and thus
binding a PCI device to vfio-pci driver will finally succeed.
Remove this test as it will no longer make sense.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
The pci_driver_bind() and pci_driver_unbind() functions are
"internal implementation", meaning other parts of the code should
be able to call them and get the job done. Checking for actions
(PCI_ACTION_BIND and PCI_ACTION_UNBIND) should be done in
handlers (pci_driver_handle_bind() and
pci_driver_handle_unbind()). Surprisingly, the other two actions
(PCI_ACTION_NEW_ID and PCI_ACTION_REMOVE_ID) are checked already
at this level.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
virErrorPreserveLast()/virErrorRestore() (added in commit 8333e7455
back in 2017), do a better better job of saving and restoring the last
libvirt error than virSaveLastError()/virErrorRestore() (they're
simpler, and they also save/restore the system errno).
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
During networkPortCreateXML, if networkAllocatePort() failed,
networkReleasePort() would be called, which would (in the case of
network pools of macvtap passthrough devices) attempt to find the
allocated device by comparing port->plug.direct.linkdev to each device
in the pool. Since port->plug.direct.linkdev was still NULL, the
attempted strcmp would result in a SEGV.
Calling networkReleasePort() during error cleanup is something that
should only be done if networkAllocatePort() has already succeeded. It
turns out there is one other possible error exit from
networkPortCreateXML() that happens after networkAllocatePort() has
succeeded, so the code to call networkReleasePort() was just moved
down to there.
Resolves: https://bugzilla.redhat.com/1741390
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Commit e69444e17 (first appeared in libvirt-5.5.0) added the new value
"VIR_ACCESS_PERM_NETWORK_SEARCH_PORTS" to the virAccessPerNetwork
enum, and also the string "search_ports" to the VIR_ENUM_IMPL() macro
for that enum. Unfortunately, the enum value was added in the middle
of the list, while the string was added to the end of the
VIR_ENUM_IMPL().
This patch corrects that error by moving the new value to the end of
the enum definition, so that the order matches that of the string
list.
Resolves: https://bugzilla.redhat.com/1741428
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Now that we support blockdev for qemuDomainBlockCopy we can allow
copying to remote destinations as well.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Implement job handling for the block copy job (drive/blockdev-mirror)
when using -blockdev. In contrast to the previously implemented
blockjobs the block copy job introduces new images to the running qemu
instance, thus requires a bit more handling.
When copying to new images the code now makes use of blockdev-create to
format the images explicitly rather than depending on automagic qemu
behaviour.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
QEMU finally exposes an interface which allows us to instruct it to
format or create arbitrary images. This is required for blockdev
integration of block copy and snapshots as we need to pre-format images
prior to use with blockdev-add.
This path introduces job handling and also helpers for formatting and
attaching a whole image described by a virStorageSource.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Rather than copying just the top level image, let's copy the full user
provided backing chain.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The only code path which calls the parser with the
VIR_DOMAIN_DEF_PARSE_DISK_SOURCE is from qemuDomainBlockCopy. Since that
code path can properly handle backing chains for the disk and it's
desired to pass the parsed chains to the block copy code remove the
condition which prevents parsing the <backingStore> element.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In commit 3f93884a4d where the job handling of commit jobs with
blockdev was added I've forgot to add a 'break' in the switch fomatting
the status XML. Thankfully this would not be a problem as the cases
where this fell through didn't have any code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The utility of the function is extremely limited as for block copy
we need to register the mirror chain earlier than when it's set with the
disk. This means that it would be open-coded in that case.
Avoid any weird usage and just open-code the only current usage, remove
the function, and reword the docs.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Commit 16ca234b56 refactored how the 'shallow' and 'reuse' flags
are accessed but neglected to fix the clearing of 'shallow' in case when
the disk has no backing chain. This means that we'd request a shallow
copy even without backing chain and also a few checks would work wrong.
Fix it by using the extracted variable everywhere.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Allow reusing original backing chain when doing a shallow copy without
reuse of external image. The existing logic didn't allow it but it will
be possible. Also add a note to explain that logic.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Rename qemuDomainObjPrivateXMLFormatBlockjobFormatChain to
qemuDomainObjPrivateXMLFormatBlockjobFormatSource and add a 'chain'
parameter which allows controlling whether the backing chain is
formatted.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
