Calling qemuProcessStop without a job opens a way to race conditions
with qemuDomainObjExitMonitor called in another thread. A real world
example of such a race condition:

  - migration thread (A) calls qemuMigrationWaitForSpice
  - another thread (B) starts processing qemuDomainAbortJob API
  - thread B signals thread A via qemuDomainObjAbortAsyncJob
  - thread B enters monitor (qemuDomainObjEnterMonitor)
  - thread B calls qemuMonitorSend
  - thread A awakens and calls qemuProcessStop
  - thread A calls qemuMonitorClose and sets priv->mon to NULL
  - thread B calls qemuDomainObjExitMonitor with priv->mon == NULL
  => monitor stays ref'ed and locked

Depending on how lucky we are, the race may result in a memory leak or
it can even deadlock libvirtd's event loop if it tries to lock the
monitor to process an event received before qemuMonitorClose was called.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

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

Stopping a domain without a job risks a race condition with another
thread which started a job a which does not expect anyone else to be
messing around with the same domain object.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

When destroying a domain we need to make sure we will be able to start a
job no matter what other operations are running or even stuck in a job.
This is done by killing the domain before starting the destroy job.

Let's introduce qemuProcessBeginStopJob which combines killing a domain
and starting a job in a single API which can be called everywhere we
need a job to stop a domain.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

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

Create a new module qemu_alias.c to handle the qemuAssign*Alias* APIs
and the qemuDomainDeviceAliasIndex

Move function to qemu_process.c, rename to qemuProcessNetworkPrepareDevices
and make it static.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Create new modules qemu_domain_address.c and qemu_domain_address.h to
contain all the new functions and header data. Additionally move any
supporting static functions.

Make qemuDomainSupportsPCI non static.

Also, move and rename the following:

qemuSetSCSIControllerModel to qemuDomainSetSCSIControllerModel
qemuCollectPCIAddress to qemuDomainCollectPCIAddress
qemuValidateDevicePCISlotsPIIX3 to qemuDomainValidateDevicePCISlotsPIIX3
qemuAssignDevicePCISlots to qemuDomainAssignDevicePCISlots
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

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

Since we already have virtio channel events, we know when guest
agent within guest has (dis-)connected. Instead of us blindly
connecting to a socket that no one is listening to, we can just
follow what qemu-ga does. This has a nice benefit that we don't
need to 'guest-ping' the agent just to timeout and find out
nobody is listening.

The way that this commit is implemented:
- don't connect in qemuProcessLaunch directly, defer that to event
  callback (which already follows the agent) -
  processSerialChangedEvent
- after migration is settled, before we resume vCPUs, ask qemu
  whether somebody is listening on the socket and if so, connect
  to it.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Rather than iterating 3 times for various settings this function
aggregates all the code into single place. One of the other advantages
is that it can then be reused for properly setting IOThread info on
hotplug.

Rather than iterating 3 times for various settings this function
aggregates all the code into single place. One of the other advantages
is that it can then be reused for properly setting vCPU info on hotplug.

With this approach autoCpuset is also used when setting the process
affinity rather than just via cgroups.

Similarly to previous commit change the way how iothread scheduler info
is stored and clean up a lot of unnecessary code.

Due to bad design the vcpu sched element is orthogonal to the way how
the data belongs to the corresponding objects. Now that vcpus are a
struct that allow to store other info too, let's convert the data to the
sane structure.

The helpers for the conversion are made universal so that they can be
reused for iothreads too.

This patch also resolves https://bugzilla.redhat.com/show_bug.cgi?id=1235180
since with the correct storage approach you can't have dangling data.

Now with the new struct the data can be stored in a much saner place.

This step can be omitted, so that drivers can decide what to do when the
user requests to use default vcpu pinning.

When starting a qemu process there are certain checks done to ensure
that the configuration makes sense. Extract them into a separate
function so that they can be reused in the test code.

Retrieval of the driver capabilities as well as emulator capabilities
does not require the complete qemuProcessStop to be executed on
failure.

So, systemd-machined has this philosophy that machine names are like
hostnames and hence should follow the same rules.  But we always allowed
international characters in domain names.  Thus we need to modify the
machine name we are passing to systemd.

In order to change some machine names that we will be passing to systemd,
we also need to call TerminateMachine at the end of a lifetime of a
domain.  Even for domains that were started with older libvirt.  That
can be achieved thanks to virSystemdGetMachineNameByPID().  And because
we can change machine names, we can get rid of the inconsistent and
pointless escaping of domain names when creating machine names.

So this patch modifies the naming in the following way.  It creates the
name as <drivername>-<id>-<name> where invalid hostname characters are
stripped out of the name and if the resulting name is longer, it
truncates it to 64 characters.  That way we can start domains we
couldn't start before.  Well, at least on systemd.

To make it work all together, the machineName (which is needed only with
systemd) is saved in domain's private data.  That way the generation is
moved to the driver and we don't need to pass various unnecessary
arguments to cgroup functions.

The only thing this complicates a bit is the scope generation when
validating a cgroup where we must check both old and new naming, so a
slight modification was needed there.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1282846Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

The virDomainObjFormat and virDomainSaveStatus methods
both call into virDomainDefFormat, so should be providing
a non-NULL virCapsPtr instance.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

virDomainSaveConfig calls virDomainDefFormat which was setting the caps
to NULL, thus keeping the old behaviour (i.e. not looking at
netprefix). This patch adds the virCapsPtr to the function and allows
the configuration to be saved and skipping interface names that were
registered with virCapabilitiesSetNetPrefix().
Signed-off-by: NJoao Martins <joao.m.martins@oracle.com>

Future patches will tweak and reuse the function in different places so
move it separately first.

This should be the last offender.

This patch creates two bitmaps, one for macvlan device names and one
for macvtap. The bitmap position is used to indicate that libvirt is
currently using a device with the name macvtap%d/macvlan%d, where %d
is the position in the bitmap. When requested to create a new
macvtap/macvlan device, libvirt will now look for the first clear bit
in the appropriate bitmap and derive the device name from that rather
than just starting at 0 and counting up until one works.

When libvirtd is restarted, the qemu driver code that reattaches to
active domains calls the appropriate function to "re-reserve" the
device names as it is scanning the status of running domains.

Note that it may seem strange that the retry counter now starts at
8191 instead of 5. This is because we now don't do a "pre-check" for
the existence of a device once we've reserved it in the bitmap - we
move straight to creating it; although very unlikely, it's possible
that someone has a running system where they have a large number of
network devices *created outside libvirt* named "macvtap%d" or
"macvlan%d" - such a setup would still allow creating more devices
with the old code, while a low retry max in the new code would cause a
failure. Since the objective of the retry max is just to prevent an
infinite loop, and it's highly unlikely to do more than 1 iteration
anyway, having a high max is a reasonable concession in order to
prevent lots of new failures.

Counterintuitively the user would end up with a VM with maximum number
of vCPUs available.

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

Next patch will add minimum checking, so use a more generic name.
Refactor return values to the commonly used semantics.

The corresponding event in QEMU is called MIGRATION_PASS.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

So I can observe this crasher that with freshly started daemon
(and virtlogd enabled) I am trying to startup a domain that
immediately dies (because it's said to use huge pages but I
haven't allocated a single one in the pool). Hardly reproducible
with -O0 or under valgrind. But I just got lucky:

==20469== Invalid write of size 8
==20469==    at 0x4C2E99B: memcpy@GLIBC_2.2.5 (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==20469==    by 0x217EDD07: qemuProcessReadLog (qemu_process.c:1670)
==20469==    by 0x217EDE1D: qemuProcessReportLogError (qemu_process.c:1696)
==20469==    by 0x217EE8C1: qemuProcessWaitForMonitor (qemu_process.c:1957)
==20469==    by 0x217F6636: qemuProcessLaunch (qemu_process.c:4955)
==20469==    by 0x217F71A4: qemuProcessStart (qemu_process.c:5152)
==20469==    by 0x21846582: qemuDomainObjStart (qemu_driver.c:7396)
==20469==    by 0x218467DE: qemuDomainCreateWithFlags (qemu_driver.c:7450)
==20469==    by 0x21846845: qemuDomainCreate (qemu_driver.c:7468)
==20469==    by 0x5611CD0: virDomainCreate (libvirt-domain.c:6753)
==20469==    by 0x125D9A: remoteDispatchDomainCreate (remote_dispatch.h:3613)
==20469==    by 0x125CB7: remoteDispatchDomainCreateHelper (remote_dispatch.h:3589)
==20469==  Address 0x27a52ad0 is 0 bytes after a block of size 5,584 alloc'd
==20469==    at 0x4C29F80: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==20469==    by 0x9B8D1DB: xdr_string (in /lib64/libc-2.21.so)
==20469==    by 0x563B39C: xdr_virLogManagerProtocolNonNullString (log_protocol.c:24)
==20469==    by 0x563B6B7: xdr_virLogManagerProtocolDomainReadLogFileRet (log_protocol.c:123)
==20469==    by 0x164B34: virNetMessageDecodePayload (virnetmessage.c:407)
==20469==    by 0x5682360: virNetClientProgramCall (virnetclientprogram.c:379)
==20469==    by 0x563B30E: virLogManagerDomainReadLogFile (log_manager.c:272)
==20469==    by 0x217CD613: qemuDomainLogContextRead (qemu_domain.c:2485)
==20469==    by 0x217EDC76: qemuProcessReadLog (qemu_process.c:1660)
==20469==    by 0x217EDE1D: qemuProcessReportLogError (qemu_process.c:1696)
==20469==    by 0x217EE8C1: qemuProcessWaitForMonitor (qemu_process.c:1957)
==20469==    by 0x217F6636: qemuProcessLaunch (qemu_process.c:4955)

This points to memmove() in qemuProcessReadLog(). Imagine we just
read the following string from qemu:

"abc\n2016-01-18T09:40:44.022744Z qemu-system-x86_64: Error\n"

After the first pass of the while() loop in the
qemuProcessReadLog() (in which we have taken the false branch in
the if) @buf still points to the beginning of the string,
@filter_next points to the beginning of the second line.  So we
start second iteration because there is yet another newline
character at the end. In this iteration @eol points to it
actually. Now, the control gets inside true branch of if(). Just
to remind you:

got = 58
filter_next = buf + 5,
eol = buf + 58.

Therefore skip = 54 which is correct. The message we want to skip
is 54 bytes long. However:

memmove(filter_next, eol + 1, (got - skip) +1);

which is

memmove(filter_next, eol + 1, 5)

is obviously wrong as there is only one byte we can access, not 5!
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

This reverts commit a41c00b4.

After much testing and upstream discussion this has been deemed to be
the incorrect operation since it means we no longer have any guarantee
about which resource controllers the QEMU processes in general are in.

So, you try to start a domain, but before we even get to the part
where chardev part of qemu command line is generated (and
possibly missing path to unix sockets is made up) an error occurs
which results in calling qemuProcessStop. This will then try to
clean up the mess and possibly ends up calling unlink(NULL).

==8085== Thread 3:
==8085== Syscall param unlink(pathname) points to unaddressable byte(s)
==8085==    at 0xA85EA57: unlink (in /lib64/libc-2.21.so)
==8085==    by 0x213D3C24: qemuProcessCleanupChardevDevice (qemu_process.c:2866)
==8085==    by 0x558D6B1: virDomainChrDefForeach (domain_conf.c:22924)
==8085==    by 0x213DA9AE: qemuProcessStop (qemu_process.c:5326)
==8085==    by 0x213DA2F2: qemuProcessStart (qemu_process.c:5190)
==8085==    by 0x2142957F: qemuDomainObjStart (qemu_driver.c:7396)
==8085==    by 0x214297DB: qemuDomainCreateWithFlags (qemu_driver.c:7450)
==8085==    by 0x21429842: qemuDomainCreate (qemu_driver.c:7468)
==8085==    by 0x5611B95: virDomainCreate (libvirt-domain.c:6753)
==8085==    by 0x125D9A: remoteDispatchDomainCreate (remote_dispatch.h:3613)
==8085==    by 0x125CB7: remoteDispatchDomainCreateHelper (remote_dispatch.h:3589)
==8085==    by 0x568BF41: virNetServerProgramDispatchCall (virnetserverprogram.c:437)
==8085==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
==8085==
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

While this is no functional change, whole channel definition is
going to be needed very soon. Moreover, while touching this obey
const correctness rule in qemuAgentOpen() - so far it was passed
regular pointer to channel config even though the function is
expected to not change pointee at all. Pass const pointer
instead.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

The structure actually contains migration statistics rather than just
the status as the name suggests. Renaming it as
qemuMonitorMigrationStats removes the confusion.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

My commit 674afcb0 moved computing the
default listen address from qemuMigrationPrepareAny to
qemuMigrationPrepareIncoming. However, I didn't notice listenAddress was
later passed to qemuMigrationStartNBDServer. Thus, it would be called
with the original value of listenAddress (NULL).

Let's add the updated listen address to qemuProcessIncomingDef and use
it when starting NBD servers.
Reported-by: NMichael Chapman <mike@very.puzzling.org>
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

In commit 686eb7a2, the break was not considered part of the
condition, hence breaking after first node when searching.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

The machine cgroup is a superset, a parent to the emulator and vcpuX
cgroups. The parent cgroup should never have any tasks directly in it.
In fact the parent cpuset might contain way more cpus than the sum of
emulatorpin and vcpupins. So putting tasks in the superset will allow
them to run outside of <cputune>.
Signed-off-by: NHenning Schild <henning.schild@siemens.com>

When user configures vhost-user interface and forgets to also configure
any shared memory, the search for the root cause of non-operational
interface might take unpleasantly long time.  Let's enhance user
experience by emitting a warning in the logs.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1266982Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

Instead of directly accessing the array add a helper to do this.

Add qemuDomainHasVCpuPids to do the checking and replace in place checks
with it.

We no longer need checking whether the thread contains fake data
(vcpupids[0] == vm->pid) as in b07f3d82
and 65686e5a this was removed.

Finalize the refactor by adding the 'virDomainDefGetVCpusMax' getter and
reusing it accross libvirt.

Often when debugging bug reports one is given a copy of the file
from /var/log/libvirt/qemu/$NAME.log along with other supporting
files. In a number of cases I've been given sets of files which
were from different machines. Including the hostname in the QEMU
log file will help identify when the bug reporter is providing
bad information.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>