This is going to extend virDomainLoader enum. The reason is that
once loader path is NULL its type makes no sense. However, since
value of zero corresponds to VIR_DOMAIN_LOADER_TYPE_ROM the
following XML would be produced:

  <os>
    <loader type='rom'/>
    ...
  </os>

To solve this, introduce VIR_DOMAIN_LOADER_TYPE_NONE which would
correspond to value of zero and then use post parse callback to
set the default loader type to 'rom' if needed.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NLaszlo Ersek <lersek@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

Except not really. At least for now.

In the future, the firmware will be selected automagically.
Therefore, it makes no sense to require the pathname of a
specific firmware binary in the domain XML. But since it is not
implemented do not really allow the path to be NULL. Only move
code around to prepare it for further expansion.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NLaszlo Ersek <lersek@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

In some cases, the string representing architecture is different
in qemu and libvirt. That is the reason why we have
virQEMUCapsArchFromString() and virQEMUCapsArchToString(). So
far, we did not need them outside of qemu_capabilities code, but
this will change shortly. Expose them then.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NLaszlo Ersek <lersek@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

Move the code that (possibly) generates filename of NVRAM VAR
store into a single function so that it can be re-used later.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NLaszlo Ersek <lersek@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

It's meant for testing, not for production builds. Also we have a helper
for reporting OOM errors. Introduced by 23e0bf1cSigned-off-by: NPeter Krempa <pkrempa@redhat.com>

In qemuxml2xmltest, both activeVcpus and blockjobs members
of the testInfo struct have been entirely unused ever since
commit d1a7fc8b.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>

In local testing, I accidentally introduced a self-test failure,
and spent way too much time debugging it. Make sure the testsuite
log includes some hint as to why command option validation failed.
Lone exception: allocation failure is unlikely during self-test,
and if it happens, we are better off asserting (vsh.c can do this,
even if libvirt.so cannot).
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>
Acked-by: NPeter Krempa <pkrempa@redhat.com>

Forgot to remove this before pushing.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

Similarly to CAT, when you set some values in an group, remove the group and
recreate it, the previous values will be kept there.  In order to not get values
from a previous setting (a previous VM, for example), we need to set them to
sensible defaults.  The same way we do that for CAT, just set the same values as
the default group has.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

For CAT we calculate unallocated parts of the cache, however with MBA this does
not make sense as the purpose of that is to limit the bandwidth and the setting
is only proportional relative to bandwidth settings for other groups.

This means it makes sense to set the values to 100% even if there are other
groups with some allocations and that we don't need to find the available
(unallocated) bandwidth in all the groups.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

So far we are providing the suppressions file with a relative
path to valgrind. This apparently doesn't work on some distros
like Ubuntu and its derivates. Providing the absolute path fixes
the problem.
Signed-off-by: NShotaro Gotanda <g.sho1500@gmail.com>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

The existing behavior for ppc64 guests is to always add a USB
keyboard and mouse combo if graphics are present; unfortunately,
this means any attempt to use a USB tablet will cause both pointing
devices to show up in the guest, which in turn will result in poor
user experience.

We can't just stop adding the USB mouse or start adding a USB tablet
instead, because existing applications and users might rely on the
current behavior; however, we can avoid adding the USB mouse if a USB
tablet is already present, thus allowing users and applications to
create guests that contain a single pointing device.

https://bugzilla.redhat.com/show_bug.cgi?id=1683681Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>

These are similar to the existing simple headless guests, but
also include a graphical output and some input devices.

Input files were generated by running

  $ virt-install \
    --name guest --os-variant fedora29 \
    --vcpus 4 --memory 4096 --disk size=5 \
    --graphics vnc \
    --print-xml

followed by minor tweaks.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>

Some devices (namely virtio-scsi, virtio-gpu, virtio-keyboard,
virtio-tablet and virtio-mouse, plus virtio-crypto which is
not supported by libvirt) don't follow the same rules as all
other virtio devices, which is something that ought to be
documented.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>

While the parser and schema have to accept all possible models,
virtio-(non-)transitional models are only applicable to
type=passthrough and should be otherwise rejected.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>

Reviewed-by: NErik Skultety <eskultet@redhat.com>
Signed-off-by: NCole Robinson <crobinso@redhat.com>

Only PCI devices have '/sys/class/net/<ifname>/device/resource' so we
need to skip this check for all other network devices.

Without this patch and RDMA enabled libvirt will not detect any network
device that doesn't have the path above which includes 'lo', 'virbr',
'tun', etc.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1639258Reviewed-by: NAndrea Bolognani <abologna@redhat.com>
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>

If we pass XML to virDomainDefineXML API with these two elements:

    ...
    <title></title>
    <description></description>
    ...

libvirt correctly ignores these two elements and they will not appear
in the parsed XML.

However, if we use virDomainSetMetadata API and with "" as value for
title or description we will end up with the parsed XML that contains
these empty elements.

Let's fix the behavior of this API to behave the same as
virDomainDefineXML.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1518042Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>

When running virt-host-validate on an s390x host, the tool currently warns
that it is "Unknown if this platform has IOMMU support". We can use the
common check for entries in /sys/kernel/iommu_groups here, too, but it only
makes sense to check it if there are also PCI devices available. It's also
common on s390x that there are no PCI devices assigned to the LPAR, and in
that case there is no need for the PCI-related IOMMU, so without PCI devices
we should simply skip this test.
Signed-off-by: NThomas Huth <thuth@redhat.com>
Reviewed-by: NBjoern Walk <bwalk@linux.ibm.com>
Reviewed-by: NBoris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

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>