]> xenbits.xensource.com Git - libvirt.git/log
libvirt.git
5 years agovirStoragePoolObjListForEach: Grab a reference for pool object
Michal Privoznik [Fri, 24 May 2019 14:35:38 +0000 (16:35 +0200)]
virStoragePoolObjListForEach: Grab a reference for pool object

Turns out there's one callback that might remove a storage pool
during its run: storagePoolUpdateAllState() call
storagePoolUpdateStateCallback() which may call
virStoragePoolUpdateInactive() which in turn may call
virStoragePoolObjRemove(). Problem is that the
UpdateStateCallback() sees a storage pool object with just two
references: one for each hash table holding the object. If the
function ends up calling ObjRemove() then upon removing the
object from hash tables those references are gone and thus any
subsequent call touching the object is invalid.

The solution to this problem is to grab reference for the object
we are running iterator with.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agovirStoragePoolObjRemove: Don't unlock pool object upon return
Michal Privoznik [Fri, 24 May 2019 14:35:37 +0000 (16:35 +0200)]
virStoragePoolObjRemove: Don't unlock pool object upon return

The fact that we're removing a pool object from the list of pools
doesn't mean we want to unlock it. It violates locking policy
too as object locking and unlocking is not done on the same
level.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agosecurity_util: Remove stale XATTRs
Michal Privoznik [Thu, 8 Aug 2019 08:17:45 +0000 (10:17 +0200)]
security_util: Remove stale XATTRs

It may happen that we leave some XATTRs behind. For instance, on
a sudden power loss, the host just shuts down without calling
restore on domain paths. This creates a problem, because when the
host starts up again, the XATTRs are there but they don't reflect
the true state and this may result in libvirt denying start of a
domain.

To solve this, save a unique timestamp (host boot time) among
with our XATTRs.

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

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
5 years agoutil: Introduce virhostuptime
Michal Privoznik [Thu, 8 Aug 2019 08:16:48 +0000 (10:16 +0200)]
util: Introduce virhostuptime

This module contains function to get host boot time.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
5 years agosecurity: Don't increase XATTRs refcounter on failure
Michal Privoznik [Wed, 21 Aug 2019 08:46:27 +0000 (10:46 +0200)]
security: Don't increase XATTRs refcounter on failure

If user has two domains, each have the same disk (configured for
RW) but each runs with different seclabel then we deny start of
the second domain because in order to do that we would need to
relabel the disk but that would cut the first domain off. Even if
we did not do that, qemu would fail to start because it would be
unable to lock the disk image for the second time. So far, this
behaviour is expected. But what is not expected is that we
increase the refcounter in XATTRs and leave it like that.

What happens is that when the second domain starts,
virSecuritySetRememberedLabel() is called, and since there are
XATTRs from the first domain it increments the refcounter and
returns it (refcounter == 2 at this point). Then callers
(virSecurityDACSetOwnership() and
virSecuritySELinuxSetFileconHelper()) realize that refcounter is
greater than 1 and desired seclabel doesn't match the one the
disk image already has and an error is produced. But the
refcounter is never decremented.

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

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
5 years agoqemuBuildTPMBackendStr: format device and alias separately
Ján Tomko [Tue, 20 Aug 2019 12:30:26 +0000 (14:30 +0200)]
qemuBuildTPMBackendStr: format device and alias separately

Also get rid of the temporary 'type' variable.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoqemu: move TPM vaildation to qemuDomainDeviceDefValidateTPM
Ján Tomko [Tue, 20 Aug 2019 11:42:13 +0000 (13:42 +0200)]
qemu: move TPM vaildation to qemuDomainDeviceDefValidateTPM

Simplify the command line formatter by complicating the validator.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoqemuxml2xmltest: switch TPM tests to use latest caps
Ján Tomko [Tue, 20 Aug 2019 11:42:03 +0000 (13:42 +0200)]
qemuxml2xmltest: switch TPM tests to use latest caps

In preparation to moving the validation to the parser,
we need to supply the correct caps.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoqemuBuildHotpluggableCPUProps: use VIR_RETURN_PTR
Ján Tomko [Tue, 20 Aug 2019 10:40:09 +0000 (12:40 +0200)]
qemuBuildHotpluggableCPUProps: use VIR_RETURN_PTR

This lets us get rid of the error label.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoqemuBuildNumaArgStr: split variable declarations
Ján Tomko [Tue, 20 Aug 2019 11:05:24 +0000 (13:05 +0200)]
qemuBuildNumaArgStr: split variable declarations

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoqemuBuildSerialChrDeviceStr: rename cmd to buf
Ján Tomko [Tue, 20 Aug 2019 10:33:45 +0000 (12:33 +0200)]
qemuBuildSerialChrDeviceStr: rename cmd to buf

We usually use 'cmd' for a virCommand(Ptr) variable.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agovirt-aa-helper: Drop unnecessary AppArmor rule
Andrea Bolognani [Wed, 21 Aug 2019 07:42:39 +0000 (09:42 +0200)]
virt-aa-helper: Drop unnecessary AppArmor rule

Apparently /proc/self is automatically converted to /proc/@{pid}
before checking rules, which makes spelling it out explicitly
redundant.

Suggested-by: Jamie Strandboge <jamie@canonical.com>
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
5 years agosecurity_util: Document virSecurityMoveRememberedLabel
Michal Privoznik [Thu, 8 Aug 2019 11:45:41 +0000 (13:45 +0200)]
security_util: Document virSecurityMoveRememberedLabel

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
5 years agosecurity_util: Use more VIR_AUTOFREE()
Michal Privoznik [Thu, 8 Aug 2019 09:57:41 +0000 (11:57 +0200)]
security_util: Use more VIR_AUTOFREE()

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
5 years agovirUUIDFormat: s/VIR_UUID_RAW_LEN/VIR_UUID_BUFLEN/ in comment
Michal Privoznik [Thu, 8 Aug 2019 07:36:17 +0000 (09:36 +0200)]
virUUIDFormat: s/VIR_UUID_RAW_LEN/VIR_UUID_BUFLEN/ in comment

The function takes raw UUID and formats it into string
representation. However, the comment mistakenly states that the
expected size of raw UUID buffer is VIR_UUID_RAW_LEN bytes. We
don't have such constant since v0.3.2~24. It should have been
VIR_UUID_BUFLEN.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
5 years agoci: Stop using --workdir
Andrea Bolognani [Thu, 15 Aug 2019 18:56:24 +0000 (20:56 +0200)]
ci: Stop using --workdir

Now that we're using sudo, the initial work directory is no
longer relevant since the user will find themselves in their
home directory when they get control anyway.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agoci: Run $(CI_PREPARE_SCRIPT) as root
Andrea Bolognani [Thu, 15 Aug 2019 13:37:38 +0000 (15:37 +0200)]
ci: Run $(CI_PREPARE_SCRIPT) as root

In order for the prepare script to be really useful, it needs
to be able to perform privileged operations such as installing
additional packages or setting up custom mount points.

In order to achieve that, we now run the container as root,
run the prepare script with full privilege, and only then
switch to the unprivileged account with sudo.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agoci: Introduce $(CI_PREPARE_SCRIPT)
Andrea Bolognani [Thu, 15 Aug 2019 13:24:50 +0000 (15:24 +0200)]
ci: Introduce $(CI_PREPARE_SCRIPT)

This script is run before $(CI_BUILD_SCRIPT) and can be used
to tweak the environment as necessary before the build starts.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agoci: Generalize running commands inside the container
Andrea Bolognani [Thu, 15 Aug 2019 13:23:23 +0000 (15:23 +0200)]
ci: Generalize running commands inside the container

Both for ci-build and ci-shell we want to execute basically
the same setup and cleanup logic, the only difference being
that for the former we then run the build script and with the
latter a shell.

Rework the targets so that they both call the generic
ci-run-command rule passing an appropriate $(CI_COMMAND).

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agoci: Introduce $(CI_BUILD_SCRIPT)
Andrea Bolognani [Thu, 15 Aug 2019 12:28:17 +0000 (14:28 +0200)]
ci: Introduce $(CI_BUILD_SCRIPT)

Instead of hardcoding build instructions into the Makefile,
move them to a separate script that's mounted into the
container.

This gives us a couple of advantages: we no longer have to
deal with the awkward quoting required when embedding shell
code in a Makefile, and we also provide the users with a way
to override the default build instructions with their own.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agoci: Move source directory under $(CI_USER_HOME)
Andrea Bolognani [Thu, 15 Aug 2019 18:52:07 +0000 (20:52 +0200)]
ci: Move source directory under $(CI_USER_HOME)

Now that we have a home directory for the user, storing the
source there rather than in a custom top-level directory is
the obvious choice.

Later on we're also going to add some more files related to
builds, and storing everything in the user's home directory
will keep things nice and tidy.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agoci: Create user's home directory in the container
Andrea Bolognani [Thu, 15 Aug 2019 18:34:20 +0000 (20:34 +0200)]
ci: Create user's home directory in the container

Some applications expect the user's home directory to be
present on the system and require workarounds when that's not
the case. Creating the home directory along with everything
else is easy enough for us, so let's just do that.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agoci: Move everything to a separate directory
Andrea Bolognani [Thu, 15 Aug 2019 12:06:14 +0000 (14:06 +0200)]
ci: Move everything to a separate directory

We're going to have a few more CI-related files in a second, and
it makes sense to have a separate directory for them rather than
littering the root directory.

$(CI_SCRATCHDIR) can now also be created inside the CI directory,
and as a bonus the make rune necessary to start CI builds without
running configure first becomes shorter.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agoci: Drop $(CI_SUBMODULES)
Andrea Bolognani [Thu, 15 Aug 2019 14:21:18 +0000 (16:21 +0200)]
ci: Drop $(CI_SUBMODULES)

We only use the list of submodules once, so no need to
store it in a variable.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agoci: Fix /etc/sub{u,g}id parsing
Andrea Bolognani [Thu, 15 Aug 2019 16:41:05 +0000 (18:41 +0200)]
ci: Fix /etc/sub{u,g}id parsing

The $ needs to be escaped when calling shell code from a
Makefile.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agoxml: namespaces: use uri instead of href
Ján Tomko [Wed, 21 Aug 2019 07:48:47 +0000 (09:48 +0200)]
xml: namespaces: use uri instead of href

Store the namespace URI as const char*, instead of in a function.

Suggested-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
5 years agoconf: domain: use virXMLNamespaceRegister
Ján Tomko [Tue, 20 Aug 2019 22:23:10 +0000 (00:23 +0200)]
conf: domain: use virXMLNamespaceRegister

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
5 years agoconf: storage: use virXMLNamespaceRegister
Ján Tomko [Tue, 20 Aug 2019 22:18:55 +0000 (00:18 +0200)]
conf: storage: use virXMLNamespaceRegister

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
5 years agoconf: network: use virXMLNamespaceRegister
Ján Tomko [Tue, 20 Aug 2019 20:14:46 +0000 (22:14 +0200)]
conf: network: use virXMLNamespaceRegister

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
5 years agoutil: xml: introduce virXMLNamespaceRegister
Ján Tomko [Tue, 20 Aug 2019 20:14:13 +0000 (22:14 +0200)]
util: xml: introduce virXMLNamespaceRegister

A wrapper around xmlXPathRegisterNs that will save us
from having to include xpathInternals.h everywhere
we want to use a custom namespace and open-coding
the strings already contained in virXMLNamespace.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
5 years agoconf: domain: use virXMLNamespaceFormatNS
Ján Tomko [Tue, 20 Aug 2019 22:14:53 +0000 (00:14 +0200)]
conf: domain: use virXMLNamespaceFormatNS

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
5 years agoconf: storage: use virXMLNamespaceFormatNS
Ján Tomko [Tue, 20 Aug 2019 22:08:33 +0000 (00:08 +0200)]
conf: storage: use virXMLNamespaceFormatNS

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
5 years agoconf: network: use virXMLNamespaceFormatNS
Ján Tomko [Tue, 20 Aug 2019 20:50:15 +0000 (22:50 +0200)]
conf: network: use virXMLNamespaceFormatNS

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
5 years agoutil: introduce virXMLNamespaceFormatNS
Ján Tomko [Tue, 20 Aug 2019 20:50:10 +0000 (22:50 +0200)]
util: introduce virXMLNamespaceFormatNS

A function to automatically format the xmlns:<prefix>='<uri>'
attribute for per-driver namespaces.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
5 years agoconf: storage: store namespace prefix
Ján Tomko [Tue, 20 Aug 2019 22:02:50 +0000 (00:02 +0200)]
conf: storage: store namespace prefix

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
5 years agoconf: domain: store namespace prefix
Ján Tomko [Tue, 20 Aug 2019 22:02:23 +0000 (00:02 +0200)]
conf: domain: store namespace prefix

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
5 years agoconf: network: store namespace prefix
Ján Tomko [Tue, 20 Aug 2019 20:14:32 +0000 (22:14 +0200)]
conf: network: store namespace prefix

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
5 years agoxml: virXMLNamespace: add prefix
Ján Tomko [Tue, 20 Aug 2019 22:03:15 +0000 (00:03 +0200)]
xml: virXMLNamespace: add prefix

We have hardcoded the namespace prefix in various places:
1) the xmlns string stored in the 'href' function
2) the xmlXPathRegisterNs call in each parser
3) all the parsing and formatting code actually dealing
   with these elements

While eliminating the third one is probably a job for an
actual XML-aware formatter, let's store the prefix separately
here in the virXMLNamespace structure so that future patches
can get rid of the first two bullets.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
5 years agoconf: domain: use generic XML namespace types
Ján Tomko [Tue, 20 Aug 2019 21:39:24 +0000 (23:39 +0200)]
conf: domain: use generic XML namespace types

Now that virDomainXMLNamespace matches virXMLNamespace,
we no longer need to keep both around.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
5 years agoconf: storage: use generic XML namespace types
Ján Tomko [Tue, 20 Aug 2019 21:17:37 +0000 (23:17 +0200)]
conf: storage: use generic XML namespace types

There is no need to copy and paste the same types pointing
to void all over the place.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
5 years agoconf: network: use generic XML namespace types
Ján Tomko [Tue, 20 Aug 2019 19:52:08 +0000 (21:52 +0200)]
conf: network: use generic XML namespace types

There is no need to copy and paste the same types pointing
to void all over the place.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
5 years agoutil: introduce virXMLNamespace
Ján Tomko [Tue, 20 Aug 2019 15:45:10 +0000 (17:45 +0200)]
util: introduce virXMLNamespace

For various XMLs, we allow a custom namespace for passing unsupported
configurations.

Introduce a single structure to hold all the driver-specific functions
to remove duplication.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
5 years agoconf: ns.parse: decouple call from condition
Ján Tomko [Tue, 20 Aug 2019 20:12:55 +0000 (22:12 +0200)]
conf: ns.parse: decouple call from condition

In the future we will perform more actions if ns.parse
is present. Decouple the condition from the actual call.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
5 years agovirDomainDefParseXML: remove unused parameter
Ján Tomko [Tue, 20 Aug 2019 21:35:59 +0000 (23:35 +0200)]
virDomainDefParseXML: remove unused parameter

We do not need to pass the root node, since it's already
included in the XPathContext.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
5 years agovirDomainDefNamespaceParse: remove unused attributes
Ján Tomko [Tue, 20 Aug 2019 21:30:40 +0000 (23:30 +0200)]
virDomainDefNamespaceParse: remove unused attributes

Neither the xmlDocPtr nor the root xmlNode (also passed
in the XPathContext) are interesting to the callees.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
5 years agolibxl: send lifecycle event on PMSuspend
Jim Fehlig [Tue, 13 Aug 2019 19:53:59 +0000 (13:53 -0600)]
libxl: send lifecycle event on PMSuspend

After a successful call to libxl_domain_suspend_only(), set domain
state to VIR_DOMAIN_PMSUSPENDED and send lifecycle event.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
5 years agoRevert "libxl: send lifecycle event on suspend"
Jim Fehlig [Tue, 13 Aug 2019 19:33:24 +0000 (13:33 -0600)]
Revert "libxl: send lifecycle event on suspend"

A libxl event with shutdown reason LIBXL_SHUTDOWN_REASON_SUSPEND
is sent after a domain is successfully suspended, which could result
from suspending the domain to file (virDomainSave), suspending it to
socket (virDomainMigrate), or suspending it to memory
(virDomainPMSuspendForDuration). Commit d00c77ae changed the event
handler to always set domain state to VIR_DOMAIN_PMSUSPENDED when
LIBXL_SHUTDOWN_REASON_SUSPEND is received. The causes a persistent
domain to show state "pmsuspended" after a successful migrate or save
operation. Revert the commit and ignore the suspend event as before.

This reverts commit d00c77ae45c7d9fd90384f01cd8b04c54f501e96.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
5 years agovircgroupv2: fix virCgroupV2GetCpuCfsQuota for "max" value
Pavel Hrdina [Tue, 20 Aug 2019 11:59:54 +0000 (13:59 +0200)]
vircgroupv2: fix virCgroupV2GetCpuCfsQuota for "max" value

If the first value in cpu.max is "max" return from function.

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

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
5 years agovircgroupv2: fix parsing multiple values in single file
Pavel Hrdina [Mon, 19 Aug 2019 14:01:50 +0000 (16:01 +0200)]
vircgroupv2: fix parsing multiple values in single file

Our virStrToLong* helpers converts string to integers where it wraps
strtol standard function.  After the conversion happens and there are
some remaining invalid characters our helpers will fail if the second
argument is NULL.

We need to pass pointer to string in cases where there are multiple
values in a single file.

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

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
5 years agoconf: resctrl object is not properly handled
Wang Huaqiang [Tue, 20 Aug 2019 10:06:03 +0000 (18:06 +0800)]
conf: resctrl object is not properly handled

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>
5 years agotravis: Perform MinGW builds on Fedora 30
Andrea Bolognani [Tue, 20 Aug 2019 08:12:28 +0000 (10:12 +0200)]
travis: Perform MinGW builds on Fedora 30

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>
5 years agogitlab: Adapt to container name changes
Andrea Bolognani [Mon, 19 Aug 2019 10:45:16 +0000 (12:45 +0200)]
gitlab: Adapt to container name changes

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>
5 years agoci: Adapt to container name changes
Andrea Bolognani [Mon, 12 Aug 2019 14:03:04 +0000 (16:03 +0200)]
ci: Adapt to container name changes

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>
5 years agotests: fix #ifdef indentation from previous commit
Daniel P. Berrangé [Tue, 20 Aug 2019 10:30:08 +0000 (11:30 +0100)]
tests: fix #ifdef indentation from previous commit

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
5 years agotests: don't try to mock __open_2 on non-GLibc builds
Daniel P. Berrangé [Tue, 20 Aug 2019 08:57:05 +0000 (09:57 +0100)]
tests: don't try to mock __open_2 on non-GLibc builds

Mocking of the __open_2 function was added in

  commit 459f071cacf30af9df93b7d090b1bda71b0ef20f
  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>
5 years agovirt-aa-helper: Actually fix AppArmor profile
Andrea Bolognani [Tue, 20 Aug 2019 07:54:12 +0000 (09:54 +0200)]
virt-aa-helper: Actually fix AppArmor profile

Tried previously in

  commit b1eb8b3e8fd1d4cb1da8e5e2b16f2c10837fd823
  Author: Andrea Bolognani <abologna@redhat.com>
  Date:   Mon Aug 19 10:23:42 2019 +0200

    virt-aa-helper: Fix AppArmor profile

  v5.6.0-243-gb1eb8b3e8f

with somewhat disappointing results.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agosrc: Don't check lxc_monitor_protocol-struct when LXC is disabled
Michal Privoznik [Wed, 14 Aug 2019 13:03:57 +0000 (15:03 +0200)]
src: Don't check lxc_monitor_protocol-struct when LXC is disabled

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>
5 years agoci: Comment tweaks in Makefile.ci
Eric Blake [Thu, 8 Aug 2019 14:31:40 +0000 (09:31 -0500)]
ci: Comment tweaks in Makefile.ci

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>
5 years agomaint: Improve use of configmake.h on mingw
Eric Blake [Thu, 8 Aug 2019 13:49:43 +0000 (08:49 -0500)]
maint: Improve use of configmake.h on mingw

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 fed58d83c60ff1c20292856bec006577788b7494 ("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>
5 years agovirt-aa-helper: Fix AppArmor profile
Andrea Bolognani [Mon, 19 Aug 2019 08:23:42 +0000 (10:23 +0200)]
virt-aa-helper: Fix AppArmor profile

Since

  commit 432faf259b696043ee5d7e8f657d855419a9a3fa
  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>
5 years agovirt-aa-helper: Call virCommandRawStatus()
Andrea Bolognani [Mon, 19 Aug 2019 07:05:58 +0000 (09:05 +0200)]
virt-aa-helper: Call virCommandRawStatus()

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>
5 years agovirt-aa-helper: Use virCommand APIs directly
Andrea Bolognani [Mon, 19 Aug 2019 07:02:10 +0000 (09:02 +0200)]
virt-aa-helper: Use virCommand APIs directly

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>
5 years agonwfilter: move standard XML configs out of examples dir
Daniel P. Berrangé [Fri, 19 Jul 2019 17:31:20 +0000 (18:31 +0100)]
nwfilter: move standard XML configs out of examples dir

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>
5 years agonews: mention Direct Mode for Hyper-V Synthetic timers support
Vitaly Kuznetsov [Fri, 9 Aug 2019 14:31:41 +0000 (16:31 +0200)]
news: mention Direct Mode for Hyper-V Synthetic timers support

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>
5 years agoqemu: add support for Direct Mode for Hyper-V Synthetic timers
Vitaly Kuznetsov [Fri, 9 Aug 2019 14:31:40 +0000 (16:31 +0200)]
qemu: add support for Direct Mode for Hyper-V Synthetic timers

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>
5 years agoconf: add support for Direct Mode for Hyper-V Synthetic timers
Vitaly Kuznetsov [Fri, 9 Aug 2019 14:31:39 +0000 (16:31 +0200)]
conf: add support for Direct Mode for Hyper-V Synthetic timers

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>
5 years agotests: qemuxml2argv: switch to DO_TEST_CAPS for Hyper-V tests
Vitaly Kuznetsov [Fri, 9 Aug 2019 14:31:38 +0000 (16:31 +0200)]
tests: qemuxml2argv: switch to DO_TEST_CAPS for Hyper-V tests

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>
5 years agotests: virpcimock: remove unused variable 'devid'
Ján Tomko [Mon, 19 Aug 2019 09:27:19 +0000 (11:27 +0200)]
tests: virpcimock: remove unused variable 'devid'

virpcimock.c:685:26: error: unused variable 'devid' [-Werror,-Wunused-variable]
    VIR_AUTOFREE(char *) devid = NULL;
                         ^

Fixes: 76b42294380d40282ed29560e4ae4a7491b9df05
Signed-off-by: Ján Tomko <jtomko@redhat.com>
5 years agovirpcitest: Use modern VFIO
Michal Privoznik [Wed, 14 Aug 2019 10:09:47 +0000 (12:09 +0200)]
virpcitest: Use modern VFIO

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>
5 years agovirhostdevtest: Use modern VFIO
Michal Privoznik [Mon, 12 Aug 2019 15:25:57 +0000 (17:25 +0200)]
virhostdevtest: Use modern VFIO

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>
5 years agoqemuxml2argvtest: Switch to modern vfio backend
Michal Privoznik [Mon, 12 Aug 2019 14:47:14 +0000 (16:47 +0200)]
qemuxml2argvtest: Switch to modern vfio backend

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>
5 years agovirhostdev: Unify virHostdevPreparePCIDevices behaviour for KVM and VFIO cases
Michal Privoznik [Wed, 14 Aug 2019 09:28:30 +0000 (11:28 +0200)]
virhostdev: Unify virHostdevPreparePCIDevices behaviour for KVM and VFIO cases

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>
5 years agovirhostdev: Check driver name too in virHostdevIsPCINodeDeviceUsed()
Michal Privoznik [Wed, 14 Aug 2019 09:13:21 +0000 (11:13 +0200)]
virhostdev: Check driver name too in virHostdevIsPCINodeDeviceUsed()

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>
5 years agovirpcimock: Create symlink in /sys/kernel/iommu_groups/N/devices dir
Michal Privoznik [Tue, 13 Aug 2019 15:10:50 +0000 (17:10 +0200)]
virpcimock: Create symlink in /sys/kernel/iommu_groups/N/devices dir

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>
5 years agovirpcimock: Create PCI devices under /sys/devices/pci*
Michal Privoznik [Tue, 13 Aug 2019 09:05:35 +0000 (11:05 +0200)]
virpcimock: Create PCI devices under /sys/devices/pci*

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>
5 years agovirpcimock: Store PCI address as ints not string
Michal Privoznik [Tue, 13 Aug 2019 11:22:58 +0000 (13:22 +0200)]
virpcimock: Store PCI address as ints not string

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>
5 years agovirpcimock: Introduce and use pci_driver_get_path()
Michal Privoznik [Tue, 13 Aug 2019 14:11:17 +0000 (16:11 +0200)]
virpcimock: Introduce and use pci_driver_get_path()

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>
5 years agovirpcimock: Introduce and use pci_device_get_path()
Michal Privoznik [Tue, 13 Aug 2019 13:31:09 +0000 (15:31 +0200)]
virpcimock: Introduce and use pci_device_get_path()

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>
5 years agovirpcimock: Create devices/ and drivers/ under /sys/bus/pci upfront
Michal Privoznik [Tue, 13 Aug 2019 08:51:05 +0000 (10:51 +0200)]
virpcimock: Create devices/ and drivers/ under /sys/bus/pci upfront

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>
5 years agovirpcimock: Rename @fakesysfspcidir
Michal Privoznik [Tue, 13 Aug 2019 08:44:53 +0000 (10:44 +0200)]
virpcimock: Rename @fakesysfspcidir

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>
5 years agovirpcimock: Eliminate use of @fakesysfspcidir
Michal Privoznik [Tue, 13 Aug 2019 08:37:08 +0000 (10:37 +0200)]
virpcimock: Eliminate use of @fakesysfspcidir

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>
5 years agovirpcimock: Use VIR_AUTOFREE()
Michal Privoznik [Tue, 13 Aug 2019 11:50:48 +0000 (13:50 +0200)]
virpcimock: Use VIR_AUTOFREE()

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>
5 years agovirpcimock: Drop needless typecast
Michal Privoznik [Tue, 13 Aug 2019 09:03:05 +0000 (11:03 +0200)]
virpcimock: Drop needless typecast

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>
5 years agovirpcimock: Create driver_override file in device dirs
Michal Privoznik [Mon, 17 Jun 2019 14:42:23 +0000 (16:42 +0200)]
virpcimock: Create driver_override file in device dirs

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 b803b29c1a5. 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>
5 years agoRevert "virpcitest: Test virPCIDeviceDetach failure"
Michal Privoznik [Mon, 17 Jun 2019 16:01:40 +0000 (18:01 +0200)]
Revert "virpcitest: Test virPCIDeviceDetach failure"

This reverts commit b70c093ffa00cd87c8d39d3652b798f033a81faf.

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>
5 years agovirpcimock: Move actions checking one level up
Michal Privoznik [Mon, 17 Jun 2019 15:33:06 +0000 (17:33 +0200)]
virpcimock: Move actions checking one level up

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>
5 years agonetwork: replace virSaveLastError() with virErrorPreserveLast()
Laine Stump [Fri, 16 Aug 2019 02:28:27 +0000 (22:28 -0400)]
network: replace virSaveLastError() with virErrorPreserveLast()

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>
5 years agonetwork: fix crash during cleanup from failure to allocate port
Laine Stump [Fri, 16 Aug 2019 01:52:28 +0000 (21:52 -0400)]
network: fix crash during cleanup from failure to allocate port

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>
5 years agoaccess: fix incorrect addition to virAccessPermNetwork
Laine Stump [Thu, 15 Aug 2019 20:34:21 +0000 (16:34 -0400)]
access: fix incorrect addition to virAccessPermNetwork

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>
5 years agoqemu: driver: allow remote destinations for block copy
Peter Krempa [Mon, 22 Jul 2019 11:59:35 +0000 (13:59 +0200)]
qemu: driver: allow remote destinations for block copy

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>
5 years agoqemu: Add blockdev support for the block copy job
Peter Krempa [Mon, 22 Jul 2019 11:59:01 +0000 (13:59 +0200)]
qemu: Add blockdev support for the block copy job

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>
5 years agoqemu: Introduce code for blockdev-create
Peter Krempa [Mon, 10 Jun 2019 16:13:09 +0000 (18:13 +0200)]
qemu: Introduce code for blockdev-create

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>
5 years agoqemu: blockjob: Copy non-detected chain fully in qemuBlockJobRewriteConfigDiskSource
Peter Krempa [Wed, 7 Aug 2019 14:31:19 +0000 (16:31 +0200)]
qemu: blockjob: Copy non-detected chain fully in qemuBlockJobRewriteConfigDiskSource

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>
5 years agoconf: domain: Parse backingStore with VIR_DOMAIN_DEF_PARSE_DISK_SOURCE
Peter Krempa [Fri, 2 Aug 2019 13:02:50 +0000 (15:02 +0200)]
conf: domain: Parse backingStore with VIR_DOMAIN_DEF_PARSE_DISK_SOURCE

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>
5 years agoqemu: domain: Add 'break' after formatting commit job status XML
Peter Krempa [Fri, 2 Aug 2019 11:01:32 +0000 (13:01 +0200)]
qemu: domain: Add 'break' after formatting commit job status XML

In commit 3f93884a4d0 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>
5 years agoqemu: blockjob: Remove qemuBlockJobDiskRegisterMirror
Peter Krempa [Thu, 1 Aug 2019 14:41:28 +0000 (16:41 +0200)]
qemu: blockjob: Remove qemuBlockJobDiskRegisterMirror

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>
5 years agoqemu: fix broken handling of shallow flag in qemuDomainBlockCopyCommon
Peter Krempa [Wed, 7 Aug 2019 15:00:02 +0000 (17:00 +0200)]
qemu: fix broken handling of shallow flag in qemuDomainBlockCopyCommon

Commit 16ca234b56fac82 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>
5 years agoqemu: Fix logic in qemuDomainBlockCopyCommonValidateUserMirrorBackingStore
Peter Krempa [Fri, 26 Jul 2019 13:58:26 +0000 (15:58 +0200)]
qemu: Fix logic in qemuDomainBlockCopyCommonValidateUserMirrorBackingStore

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>
5 years agoqemu: domain: Allow formatting top source only in qemuDomainObjPrivateXMLFormatBlockj...
Peter Krempa [Thu, 25 Jul 2019 13:54:48 +0000 (15:54 +0200)]
qemu: domain: Allow formatting top source only in qemuDomainObjPrivateXMLFormatBlockjobFormatChain

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>
5 years agoqemu: Don't report some ignored errors in qemuDomainGetStatsOneBlockFallback
Peter Krempa [Wed, 14 Aug 2019 16:46:09 +0000 (18:46 +0200)]
qemu: Don't report some ignored errors in qemuDomainGetStatsOneBlockFallback

The function ignores all errors from qemuStorageLimitsRefresh by calling
virResetLastError. This still logs them. Since qemuStorageLimitsRefresh
allows suppressing some, do so.

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