Fix an incorrect @xmlDesc comment, as well as adding more details
about which XML element should be root.
Signed-off-by: NEric Blake <eblake@redhat.com>

Add a new function to make it possible to parse a list of snapshots
at once.  This is a counterpart to an earlier patch making it
possible to produce all snapshots in a single XML string, and
intentionally parses the same top-level element <snapshots> with
an optional attribute current='name'.

Note that since we know we started with no relations at all, and
since checking parent relationships per-snapshot is not viable as
we don't control which order the snapshots appear in, that we are
fine with doing a final pass to update all parent/child
relationships among the definitions.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Pull out the portion of virDomainSnapshotRefinePrep() that deals
with definition sanity into a separate helper routine that can
be reused with bulk redefine, leaving behind only the code
specific to loop checking and in-place updates that are only
needed in single-definition handling.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Right now, the only callers of qemuDomainSnapshotDiscardAllMetadata()
are right before freeing the virDomainSnapshotObjList, so it did not
matter if the list's metaroot (which points to all the defined root
snapshots) is left inconsistent. But an upcoming patch will want to
clear all snapshots if a bulk redefine fails partway through, in
which case things must be reset.  Make this work by teaching the
existing virDomainSnapshotUpdateRelations() to be safe regardless of
the incoming state of the metaroot (since we don't want to leak that
internal detail into qemu code), then fixing the qemu code to use
it after deleting all snapshots. Additionally, the qemu code must
reset vm->current_snapshot if the current snapshot was removed,
regardless of whether the overall removal succeeded or failed later.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Add a new function to output all of the domain's snapshots in one
buffer.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Split out an internal helper that produces format into a
virBuffer, similar to what domain_conf.c does, and making
the next patch easier to write.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

virDomainSnapshotDefFormat currently takes two sets of knobs:
an 'unsigned int flags' argument that can currently just be
VIR_DOMAIN_DEF_FORMAT_SECURE, and an 'int internal' argument used as
a bool to determine whether to output an additional element.  It
then reuses the 'flags' knob to call into virDomainDefFormatInternal(),
which takes a different set of flags. In fact, prior to commit 0ecd6851
(1.2.12), the 'flags' argument actually took the public
VIR_DOMAIN_XML_SECURE, which was even more confusing.  Let's borrow
from the style of that earlier commit, by introducing a function
for translating from the public flags (VIR_DOMAIN_SNAPSHOT_XML_SECURE
was just recently introduced) into a new enum specific to snapshot
formatting, and adjust all callers to use snapshot-specific enum
values when formatting, and where the formatter now uses a new
variable 'domainflags' to make it obvious when we are translating
from snapshot flags back to domain flags.  We don't even have to
use the conversion function for drivers that don't accept the
public VIR_DOMAIN_SNAPSHOT_XML_SECURE flag.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Clean up the previous patch which abused switch on virDomainState
while working with a variable containing virDomainSnapshotState, by
converting the two affected switch statements to now use the right
enum.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

The existing virDomainSnapshotState is a superset of virDomainState,
adding one more state (disk-snapshot) on top of valid domain states.
But as written, the enum cannot be used for gcc validation that all
enum values are covered in a strongly-typed switch condition, because
the enum does not explicitly include the values it is adding to.

Copy the style used in qemu_blockjob.h of creating new enum names
for every inherited value, and update most clients to use the new
enum names anywhere snapshot state is referenced. The exception is
two switch statements in qemu code, which instead gain a fixme
comment about odd type usage (which will be cleaned up in the next
patch). The rest of the patch is fairly mechanical (I actually did
it by temporarily s/state/xstate/ in snapshot_conf.h to let the
compiler find which spots in the code used the field, did the
obvious search and replace in those functions, then undid the rename).
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

qemuDomainSnapshotWriteMetadata does not modify the directory name,
and making it const-correct aids in writing an upcoming patch.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

The current qemu code rejects the combination of the two flags
VIR_DOMAIN_SNAPSHOT_CREATE_LIVE in tandem with
VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE, but rather late in the cycle
(after the snapshot was already parsed), and with a rather confusing
message (complaining that live snapshots require external storage,
even if the redefined snapshot already declares external storage).
Hoist the rejection message to occur earlier (before parsing any
XML, which also aids upcoming patches that will implement bulk
redefine), and with a more typical error message about mutually
exclusive flags.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Failure would have occurred for @ctxt before in callers' other
virXPath calls and @def derefs.

Found by Coverity due to commit 66a508d2 using VIR_XPATH_NODE_AUTORESTORE
to access @ctxt before the if condition. The @def was noted by review.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

If shutting down a container via setting the runlevel fails, the
control jumps right onto endjob label and doesn't even try
sending the signal. If flags allow it, we should try both
methods.
Signed-off-by: NMaxim Kozin <kolomaxes@gmail.com>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

We can use STRNEQ() instead of STRNEQLEN() since we're only
interested in the trailing part of the string and we've
already verified that the length of file, name and suffix
are those we expect.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
ACKed-by: NPeter Krempa <pkrempa@redhat.com>

It's a predicate, so bool is the appropriate return type.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
ACKed-by: NPeter Krempa <pkrempa@redhat.com>

While this function is not, strictly speaking, a predicate,
it still mostly behaves like one as evidenced by the vast
majority of its callers, so using bool rather than int as
the return type makes sense.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
ACKed-by: NPeter Krempa <pkrempa@redhat.com>

It's a predicate, so bool is the appropriate return type.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
ACKed-by: NPeter Krempa <pkrempa@redhat.com>

Since qemu 2.13 reports the target architecture in a property called
'target' additionally to the property 'arch', that has been used in
qemu 2.12 in the response data of 'query-cpus-fast'.
Libvirts monitor code prefers the 'target' property over 'arch'.

At least for s390(x), target is reported as 's390x' while arch is 's390'.
In a later step a comparison is performed against 's390' which fails for
qemu 2.13 and later.

In consequence the architecture specific data for s390 won't be extracted
from the returned data, leading to incorrect values being reported by
virsh domstats --vcpu.

Changing to check explicitly for 's390' and 's390x'.
Signed-off-by: NViktor Mihajlovski <mihajlov@linux.ibm.com>
Reviewed-by: NBjoern Walk <bwalk@linux.ibm.com>
Reviewed-by: NBoris Fiuczynski <fiuczy@linux.ibm.com>

Problem is that if there are no signatures for a CPU, then we
still allocate cpu->signatures (even though with size 0). Later,
we access cpu->signatures[0] if cpu->signatures is not NULL.

 Invalid read of size 4
    at 0x5F439D7: virCPUx86Translate (cpu_x86.c:2930)
    by 0x5F3C239: virCPUTranslate (cpu.c:927)
    by 0x57CE7A1: qemuProcessUpdateGuestCPU (qemu_process.c:5870)
    ...
  Address 0xf752d40 is 0 bytes after a block of size 0 alloc'd
    at 0x4C30EC6: calloc (vg_replace_malloc.c:711)
    by 0x5DBDE4E: virAllocN (viralloc.c:190)
    by 0x5F3E4FA: x86ModelCopySignatures (cpu_x86.c:990)
    by 0x5F3E60F: x86ModelCopy (cpu_x86.c:1008)
    by 0x5F3E7CB: x86ModelFromCPU (cpu_x86.c:1068)
    by 0x5F4397E: virCPUx86Translate (cpu_x86.c:2922)
    by 0x5F3C239: virCPUTranslate (cpu.c:927)
    by 0x57CE7A1: qemuProcessUpdateGuestCPU (qemu_process.c:5870)
    ...
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJiri Denemark <jdenemar@redhat.com>

There is a lot of documentation in the comments about how PPC64 handles
passthrough VFIO devices to calculate the @memLockLimit. And more will
be added with the PPC64 NVLink2 support code.

Let's remove the PPC64 code from qemuDomainGetMemLockLimitBytes()
body and put it into a helper function. This will simplify the
flow of qemuDomainGetMemLockLimitBytes() that handles all the other
platforms and improves readability of the PPC64 specifics.
Signed-off-by: NDaniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

@passthroughLimit is being calculated even if @usesVFIO is false. After
that, an if-else conditional is used to check if we're going to sum it
up with @baseLimit.

This patch initializes @passthroughLimit to zero and always returns
@memKB = @baseLimit + @passthroughLimit. The conditional is then used to
calculate @passthroughLimit if @usesVFIO == true. This results in some
cycles being spared for the @usesVFIO == false scenario, but the real
motivation is to make the code simpler to add an alternative formula to
calculate @passthroughLimit for NVLink2.
Signed-off-by: NDaniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

When we create cgroup for qemu threads we need to enable cpuset
controller in order to use it.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>

Signed-off-by: NPavel Hrdina <phrdina@redhat.com>

Cgroups v2 don't have memory_migrate interface and the migration is
enabled by default.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>

Signed-off-by: NPavel Hrdina <phrdina@redhat.com>

It may happen that both symbols are present. Especially when
chaining mocks. For instance if a test is using virpcimock and
then both stat and __xstat would be present in the address space
as virpcimock implements both. Then, if the test would try to use
say virfilewrapper (which again uses VIR_MOCK_REAL_INIT_ALT() to
init real_stat and real___xstat) it would find stat() from
virpcimock and stop there. The virfilewrapper.c:real___xstat
wouldn't be initialized and thus it may result in a segfault.

The reason for segfault is that sys/stat.h may redefine stat() to
call __xstat().
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

Google is shutting down Google+, with no replacement, in the very near
future so we are losing the Libvirt community group there.
Reviewed-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Support for XFS reflink clone was added in:

  commit 8ed874b3
  Author: Julio Faracco <jcfaracco@gmail.com>
  Date:   Fri Jul 6 10:43:01 2018 -0300

    storage: Rename btrfsCloneFile to support other filesystems.

  commit 2e11298f
  Author: Julio Faracco <jcfaracco@gmail.com>
  Date:   Fri Jul 6 10:43:00 2018 -0300

    configure: Adding XFS library/headers check.

But these patches missed that the xfs/xfs.h header is not installed
unless you have xfsprogs-devel present.
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Signed-off-by: NPavel Hrdina <phrdina@redhat.com>

Capabilities should not duplicate data that are obvious from our
documentation and will not change with different QEMU binaries
or the way how we compile libvirt.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>

This way if the first test "full" fails we will run the second test as
well.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>

This will allow to use VIR_TEST_REGENERATE_OUTPUT.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>

When dealing with internal paths we don't need to worry about
whether or not suffixes are lowercase since we have full control
over them, which means we can avoid performing case-insensitive
string comparisons.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
ACKed-by: NPeter Krempa <pkrempa@redhat.com>

This is the case-sensitive counterpart of the existing
virStringHasCaseSuffix() function.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
ACKed-by: NPeter Krempa <pkrempa@redhat.com>

Despite its name, this is really just a general-purpose string
manipulation function, so it should be moved to the virstring
module and renamed accordingly.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
ACKed-by: NPeter Krempa <pkrempa@redhat.com>

Despite its name, this is really just a general-purpose string
manipulation function, so it should be moved to the virstring
module and renamed accordingly.

A few trivial whitespace changes are squashed in.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
ACKed-by: NPeter Krempa <pkrempa@redhat.com>

Despite its name, this is really just a general-purpose string
manipulation function, so it should be moved to the virstring
module and renamed accordingly.

In addition to the obvious s/File/String/, also tweak the name
to make it clear that the presence of the suffix is verified
using case-insensitive comparison.

A few trivial whitespace changes are squashed in.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
ACKed-by: NPeter Krempa <pkrempa@redhat.com>

In these cases the check that is removed has been done a few
lines above already (as can even be seen in the context). Drop
them.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Commit a3ab6d42 changed the libvirtd profile to a named profile
but neglected to accommodate the change in the qemu profile
ptrace and signal rules. As a result, libvirtd is unable to
signal confined qemu processes and hence unable to shutdown
or destroy VMs.

Add ptrace and signal rules that reference the libvirtd profile
by name in addition to full binary path.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>
Acked-by: NJamie Strandboge <jamie@canonical.com>

Signed-off-by: NJohn Ferlan <jferlan@redhat.com>
ACKed-by: NMichal Privoznik <mprivozn@redhat.com>