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

While reviewing a patch from Andrea that modified this test case, I
realized that although it was "properly failing" (it's a negative
test), that it was failing for the wrong reason (the MULTIFUNCTION cap
wasn't set in the test case, so it was saying that multifunction=on
wasn't supported by the QEMU binary; instead it should have been
complaining that it had run out of PCI slots of the appropriate type
and couldn't automatically add any more).

This improper failure had started when I added the patch to
automatically aggregate pcie-root-ports onto multiple functions of
each pcie-root slot, but I hadn't noticed it because the test still
failed.

This patch corrects the test case to 1) set the MULTIFUNCTION flag in
the caps, and 2) attempt to add 241 pcie-root-ports to a domain. Since
there are 30 slots available on a pcie-root (slot 0 is reserved, and
slot 31 is used by the integrated SATA controller), and a
pcie-root-port can only be placed on a function of a slot on
pcie-root, the maximum number of pcie-root-ports in any domain is 240.

The build system for libvirt correctly detects the location of blkid
using PKG_CONFIG_PATH environment variable. The file blkid.pc states
that the include flags should be: 'Cflags: -I${includedir}/blkid' but
libvirt searches for blkid.h inside ${includedir}/blkid/blkid, which is
wrong. Until now, the compilation for libvirt succeeded because of pure
luck, as it had -I/usr/include as a CFLAG. This issue was faced while
compiling libvirt on Ubuntu 16.04.2 with bare minimum dev packages and a
custom compiled blkid kept in a non-standard $prefix.
Signed-off-by: NNehal J Wani <nehaljw.kkd1@gmail.com>

The generated HTML will contain <ul></ul> otherwise, which
triggers an error during 'make check'.

The proper fix would be not to generate the problematic
HTML in the first place but, while I'm working on it, this
workaround will do.

In a few cases, we checked for VIR_ARCH_X86_64 and
VIR_ARCH_I686 separately: change all those to use the
ARCH_IS_X86() macro instead.

The ARCH_IS_*() macro are defined in a way that allows
them to be used if a parentheses-less if statement, but
we don't really want that to happen

virQEMUCapsHasPCIMultiBus() performs a version check on
the QEMU binary to figure out whether multiple buses are
supported, so to get the correct aliases assigned when
dealing with pSeries guests we need to spoof the version
accordingly in the test suite.

Due to the extra architecture-specific logic, it's already
necessary for users to call virQEMUCapsHasPCIMultiBus(),
so the capability itself is just a pointless distraction.

* docs/news.xml: updated and regenerated NEWS
* po/*.po*: regenerated

Our documentation states that the chardev logging file is truncated
unless append='on' is specified. QEMU also behaves the same way and
truncates the file unless we provide the argument. The new virlogd
implementation did not honor if the argument was missing and continued
to append to the file.

Truncate the file even when the 'append' attribute is not present to
behave the same with both implementations and adhere to the docs.

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

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

This has worked in previous releases.
My commit c266b604 broke it.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

This function is calling public APIs (virNodeDeviceLookupByName
etc.). That requires the driver lock to be unlocked and locked
again. If we, however, replace the public APIs calls with the
internal calls (that public APIs call anyway), we can drop the
lock/unlock exercise.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

The 'nodes' is overwritten after the first usage and possibly leaked
if any code in the first set of parsing goes to error.

Found by Coverity.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Arguably though, function returning only on success is a very
interesting, although quite impractical concept. Also, the errno isn't
and shouldn't be preserved in this case, since the errno can be directly
fed to the virReportSystemError.
Signed-off-by: NErik Skultety <eskultet@redhat.com>

After eca76884 in case of error in qemuDomainSetPrivatePaths()
in pretended start we jump to stop. I've changed this during
review from 'cleanup' which turned out to be correct. Well, sort
of. We can't call qemuProcessStop() as it decrements
driver->nactive and we did not increment it. However, it calls
virDomainObjRemoveTransientDef() which is basically the only
function we need to call. So call that function and goto cleanup;
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

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

While "x86" is a CPU sub driver name, it is not a recognized name of any
architecture known to libvirt. Let's use "x86_64" prefix which can be
used with virArch APIs.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

The API is useful for creating virCPUData in a hypervisor driver from
data we got by querying the hypervisor.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

The API is useful for creating virCPUData in a hypervisor driver from
data we got by querying the hypervisor.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

The API is useful for creating virCPUData in a hypervisor driver from
data we got by querying the hypervisor.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

The CPU driver provides APIs to create and free virCPUDataPtr. Thus all
APIs exported from the driver should work with that rather than
requiring the caller to pass a pointer to an internal part of the
structure.

In other words

    virCPUx86DataAddCPUID(cpudata, &cpuid)

is much better than the original

    virCPUx86DataAddCPUID(&cpudata->data.x86, &cpuid)
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

The new API is called virCPUDataFree. Individual CPU drivers are no
longer required to implement their own freeing function unless they need
to free architecture specific data from virCPUData.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

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

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

Our documentation of the domain capabilities XML says that the fallback
attribute of a CPU model is used to indicate whether the CPU model was
detected by libvirt itself (fallback="allow") or by asking the
hypervisor (fallback="forbid"). We need to properly set
fallback="forbid" when CPU model comes from QEMU to match the
documentation.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

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

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1352529Signed-off-by: NPavel Hrdina <phrdina@redhat.com>

This will eventually replace virQEMUBuildBufferEscapeComma, however
it's not possible right now.  Some parts of the code that uses the
old function needs to be refactored.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>

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

Now that QEMU_CAPS_DEVICE_PCI_BRIDGE is no longer checked
unless a pci-bridge is really part of the configuration,
and most uses of the legacy PCI controller combo have been
dropped from tests that use PCIe machine types, we can
drop the corresponding capabilities from a lot of test
cases.

In some cases, only one of the two transformations was
checked; in other cases, the capabilities set differed.

The port is stored in graphics configuration and it will
also get released in qemuProcessStop in case of error.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1397440Signed-off-by: NPavel Hrdina <phrdina@redhat.com>

Previously the code called virStorageSourceUpdateBlockPhysicalSize which
did not do anything on empty drives since it worked only on block
devices. After the refactor in c5f61513 it's called for all devices
and thus attempts to deref the NULL path of empty drives.

Add a check that skips the update of the physical size if the storage
source is empty.

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

Fix incorrect jump labels in error paths as the stop jump is only
needed if the driver has already changed the state. For example
'virAtomicIntInc(&driver->nactive)' will be 'reverted' in the
qemuProcessStop call.
Signed-off-by: NMarc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: NBoris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

When a domain needs an access to some device (be it a disk, RNG,
chardev, whatever), we have to allow it in the devices CGroup (if
it is available), because by default we disallow all the devices.
But some of the functions that are responsible for setting up
devices CGroup are lacking check whether there is any CGroup
available. Thus users might be unable to hotplug some devices:

  virsh # attach-device fedora rng.xml
  error: Failed to attach device from rng.xml
  error: internal error: Controller 'devices' is not mounted
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

In GCC 7 there is a new warning triggered when a switch
case has a conditional statement (eg if ... else...) and
some of the code paths fallthrough to the next switch
statement. e.g.

conf/domain_conf.c: In function 'virDomainChrEquals':
conf/domain_conf.c:14926:12: error: this statement may fall through [-Werror=implicit-fallthrough=]
         if (src->targetTypeAttr != tgt->targetTypeAttr)
            ^
conf/domain_conf.c:14928:5: note: here
     case VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE:
     ^~~~
conf/domain_conf.c: In function 'virDomainChrDefFormat':
conf/domain_conf.c:22143:12: error: this statement may fall through [-Werror=implicit-fallthrough=]
         if (def->targetTypeAttr) {
            ^
conf/domain_conf.c:22151:5: note: here
     default:
     ^~~~~~~

GCC introduced a __attribute__((fallthrough)) to let you
indicate that this is intentionale behaviour rather than
a bug.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

One of the conditions in qemuDomainDeviceCalculatePCIConnectFlags
was missing a break that could result it in falling through to
an incorrect codepath.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

The libxl code was checking that a 'char *' was != '\0', instead
of checking the first element in the string
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

GCC 7 gets upset by

   if (!tmp && (size * count))

warning

  util/viralloc.c: In function 'virReallocN':
  util/viralloc.c:246:23: error: '*' in boolean context, suggest '&&' instead [-Werror=int-in-bool-context]
     if (!tmp && (size * count)) {
                 ~~~~~~^~~~~~~~

Keep it happy by adding != 0 to the right hand expression
so it realizes we really are wanting to treat the result
of the arithmetic expression as a boolean
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>