qemu: fix recording of vCPU pids for MTTCG

MTTCG is the new multi-threaded impl of TCG which follows
KVM in having one host OS thread per vCPU. Historically
we have discarded all PIDs reported for TCG guests, but
we must now selectively honour this data.

We don't have anything in the domain XML that indicates
whether a guest is using TCG or MTTCG. While QEMU does
have an option (-accel tcg,thread=single|multi), it is
not desirable to expose this in libvirt. QEMU will
automatically use MTTCG when the host/guest architecture
pairing is known to be safe. Only developers of QEMU TCG
have a strong reason to override this logic.

Thus we use two sanity checks to decide if the vCPU
PID information is usable. First we see if the PID
duplicates the main emulator PID, and second we see
if the PID duplicates any other vCPUs.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

lib: domain: Emphasise that users should wait for block job READY state via events

The transition to the ready state is best observed by events as it's
ansynchronous and does not hint users to do polling. As currently only
the qemu driver supports block copy and block commit and the ready state
event was introduced by qemu 1.3 we can fully switch to the new
approach.

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

lib: Clarify that any block job may block VM save or device detach

The documentation was only referring to a copy job, but in fact any
running blockjob will have the same results.

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

qemu: Don't reject making domain persistent if block copy is running

Add documentation that the 'VIR_DOMAIN_BLOCK_COPY_TRANSIENT_JOB' flag
is auto-assumed if the block copy job is started while the VM is
transient and remove the restriction to define the domain when copy
is running.

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

util: move virtual network firwall rules into private chains

The previous commit created new chains to hold the firewall rules. This
commit changes the code that creates rules to place them in the new
private chains instead of the builtin top level chains.

With two networks running, the rules in the filter table now look like

  -N LIBVIRT_FWI
  -N LIBVIRT_FWO
  -N LIBVIRT_FWX
  -N LIBVIRT_INP
  -N LIBVIRT_OUT
  -A INPUT -j LIBVIRT_INP
  -A FORWARD -j LIBVIRT_FWX
  -A FORWARD -j LIBVIRT_FWI
  -A FORWARD -j LIBVIRT_FWO
  -A OUTPUT -j LIBVIRT_OUT
  -A LIBVIRT_FWI -d 192.168.0.0/24 -o virbr0 -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT
  -A LIBVIRT_FWI -o virbr0 -j REJECT --reject-with icmp-port-unreachable
  -A LIBVIRT_FWI -d 192.168.1.0/24 -o virbr1 -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT
  -A LIBVIRT_FWI -o virbr1 -j REJECT --reject-with icmp-port-unreachable
  -A LIBVIRT_FWO -s 192.168.0.0/24 -i virbr0 -j ACCEPT
  -A LIBVIRT_FWO -i virbr0 -j REJECT --reject-with icmp-port-unreachable
  -A LIBVIRT_FWO -s 192.168.1.0/24 -i virbr1 -j ACCEPT
  -A LIBVIRT_FWO -i virbr1 -j REJECT --reject-with icmp-port-unreachable
  -A LIBVIRT_FWX -i virbr0 -o virbr0 -j ACCEPT
  -A LIBVIRT_FWX -i virbr1 -o virbr1 -j ACCEPT
  -A LIBVIRT_INP -i virbr0 -p udp -m udp --dport 53 -j ACCEPT
  -A LIBVIRT_INP -i virbr0 -p tcp -m tcp --dport 53 -j ACCEPT
  -A LIBVIRT_INP -i virbr0 -p udp -m udp --dport 67 -j ACCEPT
  -A LIBVIRT_INP -i virbr0 -p tcp -m tcp --dport 67 -j ACCEPT
  -A LIBVIRT_INP -i virbr1 -p udp -m udp --dport 53 -j ACCEPT
  -A LIBVIRT_INP -i virbr1 -p tcp -m tcp --dport 53 -j ACCEPT
  -A LIBVIRT_INP -i virbr1 -p udp -m udp --dport 67 -j ACCEPT
  -A LIBVIRT_INP -i virbr1 -p tcp -m tcp --dport 67 -j ACCEPT
  -A LIBVIRT_OUT -o virbr0 -p udp -m udp --dport 68 -j ACCEPT
  -A LIBVIRT_OUT -o virbr1 -p udp -m udp --dport 68 -j ACCEPT

While in the nat table:

  -N LIBVIRT_PRT
  -A POSTROUTING -j LIBVIRT_PRT
  -A LIBVIRT_PRT -s 192.168.0.0/24 -d 224.0.0.0/24 -j RETURN
  -A LIBVIRT_PRT -s 192.168.0.0/24 -d 255.255.255.255/32 -j RETURN
  -A LIBVIRT_PRT -s 192.168.0.0/24 ! -d 192.168.0.0/24 -p tcp -j MASQUERADE --to-ports 1024-65535
  -A LIBVIRT_PRT -s 192.168.0.0/24 ! -d 192.168.0.0/24 -p udp -j MASQUERADE --to-ports 1024-65535
  -A LIBVIRT_PRT -s 192.168.0.0/24 ! -d 192.168.0.0/24 -j MASQUERADE
  -A LIBVIRT_PRT -s 192.168.1.0/24 -d 224.0.0.0/24 -j RETURN
  -A LIBVIRT_PRT -s 192.168.1.0/24 -d 255.255.255.255/32 -j RETURN
  -A LIBVIRT_PRT -s 192.168.1.0/24 ! -d 192.168.1.0/24 -p tcp -j MASQUERADE --to-ports 1024-65535
  -A LIBVIRT_PRT -s 192.168.1.0/24 ! -d 192.168.1.0/24 -p udp -j MASQUERADE --to-ports 1024-65535
  -A LIBVIRT_PRT -s 192.168.1.0/24 ! -d 192.168.1.0/24 -j MASQUERADE

And finally the mangle table:

  -N LIBVIRT_PRT
  -A POSTROUTING -j LIBVIRT_PRT
  -A LIBVIRT_PRT -o virbr0 -p udp -m udp --dport 68 -j CHECKSUM --checksum-fill
  -A LIBVIRT_PRT -o virbr1 -p udp -m udp --dport 68 -j CHECKSUM --checksum-fill

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

util: create private chains for virtual network firewall rules

Historically firewall rules for virtual networks were added straight
into the base chains. This works but has a number of bugs and design
limitations:

  - It is inflexible for admins wanting to add extra rules ahead
    of libvirt's rules, via hook scripts.

  - It is not clear to the admin that the rules were created by
    libvirt

  - Each rule must be deleted by libvirt individually since they
    are all directly in the builtin chains

  - The ordering of rules in the forward chain is incorrect
    when multiple networks are created, allowing traffic to
    mistakenly flow between networks in one direction.

To address all of these problems, libvirt needs to move to creating
rules in its own private chains. In the top level builtin chains,
libvirt will add links to its own private top level chains.

Addressing the traffic ordering bug requires some extra steps. With
everything going into the FORWARD chain there was interleaving of rules
for outbound traffic and inbound traffic for each network:

  -A FORWARD -d 192.168.3.0/24 -o virbr1 -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT
  -A FORWARD -s 192.168.3.0/24 -i virbr1 -j ACCEPT
  -A FORWARD -i virbr1 -o virbr1 -j ACCEPT
  -A FORWARD -o virbr1 -j REJECT --reject-with icmp-port-unreachable
  -A FORWARD -i virbr1 -j REJECT --reject-with icmp-port-unreachable
  -A FORWARD -d 192.168.2.0/24 -o virbr0 -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT
  -A FORWARD -s 192.168.2.0/24 -i virbr0 -j ACCEPT
  -A FORWARD -i virbr0 -o virbr0 -j ACCEPT
  -A FORWARD -o virbr0 -j REJECT --reject-with icmp-port-unreachable
  -A FORWARD -i virbr0 -j REJECT --reject-with icmp-port-unreachable

The rule allowing outbound traffic from virbr1 would mistakenly
allow packets from virbr1 to virbr0, before the rule denying input
to virbr0 gets a chance to run.

What we really need todo is group the forwarding rules into three
distinct sets:

 * Cross rules - LIBVIRT_FWX

  -A FORWARD -i virbr1 -o virbr1 -j ACCEPT
  -A FORWARD -i virbr0 -o virbr0 -j ACCEPT

 * Incoming rules - LIBVIRT_FWI

  -A FORWARD -d 192.168.3.0/24 -o virbr1 -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT
  -A FORWARD -o virbr1 -j REJECT --reject-with icmp-port-unreachable
  -A FORWARD -d 192.168.2.0/24 -o virbr0 -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT
  -A FORWARD -o virbr0 -j REJECT --reject-with icmp-port-unreachable

 * Outgoing rules - LIBVIRT_FWO

  -A FORWARD -s 192.168.3.0/24 -i virbr1 -j ACCEPT
  -A FORWARD -i virbr1 -j REJECT --reject-with icmp-port-unreachable
  -A FORWARD -s 192.168.2.0/24 -i virbr0 -j ACCEPT
  -A FORWARD -i virbr0 -j REJECT --reject-with icmp-port-unreachable

There is thus no risk of outgoing rules for one network mistakenly
allowing incoming traffic for another network, as all incoming rules
are evalated first.

With this in mind, we'll thus need three distinct chains linked from
the FORWARD chain, so we end up with:

        INPUT --> LIBVIRT_INP   (filter)

       OUTPUT --> LIBVIRT_OUT   (filter)

      FORWARD +-> LIBVIRT_FWX   (filter)
              +-> LIBVIRT_FWO
              \-> LIBVIRT_FWI

  POSTROUTING --> LIBVIRT_PRT   (nat & mangle)

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

util: pass layer into firewall query callback

Some of the query callbacks want to know the firewall layer that was
being used for triggering the query to avoid duplicating that data.

Reviewed-by: Laine Stump <laine@laine.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

network: add platform driver callbacks around firewall reload

Allow the platform driver impls to run logic before and after the
firewall reload process.

Reviewed-by: Laine Stump <laine@laine.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

qemu: Don't double-free disk->mirror if block commit initialization fails

disk->mirror would not be cleared while the local pointer was freed in
qemuDomainBlockCommit if qemuDomainObjExitMonitor or qemuBlockJobDiskNew
would return a failure.

Since block job handling is executed in the separate handler which needs
a qemu job, we don't need to pre-set the mirror state prior to starting
the job. Similarly the block copy job does not do that.

Move the setting of the data after starting the job so that we avoid
this problem.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

qemu: Clear block copy mirror state explicitly

While this should not be necessary as we clear it in the event handler,
let's be sure and clear it prior to starting the job.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

qemu: blockjob: Mark job as started only when it's new

Switching a block job to some states (e.g. QEMU_BLOCKJOB_STATE_READY)
might not require a job, thus if it will become ready asynchronously we
should not overwrite the state any more.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

qemu: blockjob: Make sure that internal states are not reported as event

While the callers should make sure that they don't call
qemuBlockJobEmitEvents for any internal state or job, let's add checks
that prevents us from emitting wrong events altogether.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

lib: Fix docs generated for enum virDomainBlockJobType

Mixing documentation strings trailing the enum value and preceeding the
enum value ends in a big mixup. Fix docs string for
VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN so that it's not squished together
with the next one.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

virfile: Detect ceph as shared FS

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

Ceph can be mounted just like any other filesystem and in fact is
a shared and cluster filesystem. The filesystem magic constant
was taken from kernel sources as it is not in magic.h yet.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>

lib: Use more of VIR_STEAL_PTR()

We have this very handy macro called VIR_STEAL_PTR() which steals
one pointer into the other and sets the other to NULL. The
following coccinelle patch was used to create this commit:

  @ rule1 @
  identifier a, b;
  @@

  - b = a;
    ...
  - a = NULL;
  + VIR_STEAL_PTR(b, a);

Some places were clean up afterwards to make syntax-check happy
(e.g. some curly braces were removed where the body become a one
liner).

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>

news: document bhyve custom commandline support

Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

docs: bhyve: document commandline element

Document the <bhyve:commandline> element which allows
to inject custom command line arguments for bhyve.

Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

bhyve: implement support for commandline args

Implement support for passing custom command line arguments
to bhyve using the 'bhyve:commandline' element:

  <bhyve:commandline>
    <bhyve:arg value='-newarg'/>
  </bhyve:commandline>

 * Define virDomainXMLNamespace for the bhyve driver, which
   at this point supports only the 'commandline' element
   described above,
 * Update command generation code to inject these command line
   arguments between driver-generated arguments and the vmname
   positional argument.

Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

docs: add forgotten mentions of forward mode "open"

A couple places in the docs didn't get updated when the forward mode
"open" was added.

Signed-off-by: Laine Stump <laine@laine.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

network: remove stale function

networkMigrateStateFiles was added nearly 5 years ago when the network
state directory was moved from /var/lib/libvirt to /var/run/libvirt
just prior to libvirt-1.2.4). It was only required to maintain proper
state information for networks that were active during an upgrade that
didn't involve rebooting the host. At this point the likelyhood of
anyone upgrading their libvirt from pre-1.2.4 directly to 5.0.0 or
later *without rebooting the host* is probably so close to 0 that no
properly informed bookie would take *any* odds on it happening, so it
seems appropriate to remove this pointless code.

Signed-off-by: Laine Stump <laine@laine.org>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>

virjson: add convenience wrapper for appending string to array

Upcoming patches need an array of strings for use in QMP
block-dirty-bitmap-merge.  A convenience wrapper cuts down
on the verbosity of creating the array, similar to the
existing virJSONValueObjectAppendString().

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

virjson: always raise vir error on append failures

A function that returns -1 for multiple possible failures, but only
raises a libvirt error for some of those failures, can be hard to
use correctly. Yet both of our JSON object/array appenders fall in
that pattern.  True, the silent errors represent coding bugs that
none of the callers should ever trigger, while the noisy errors
represent memory failures that can happen anywhere, so we happened
to never end up failing without an error. But it is better to
either use the _QUIET memory allocation variants, and make callers
decide to report failure; or make all failure paths noisy. This
patch takes the latter approach.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

tools: Document completer callback

Strictly speaking, this should go near vshCompleter typedef
declaration. However, I find it more useful near actual completer
implementations.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

qemuBuildControllersCommandLine: use i instead of j as the counter

Now that the nested loop is gone.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>

rename qemuBuildControllerDevCommandLine

Use qemuBuildControllersCommandLine since it builds the command line
for (nearly) all controllers, not just one.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>

qemuBuildControllersByTypeCommandLine: free devstr in the cleanup
section

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>

qemu: format CCID controllers after USB hubs

Since they go on the USB bus, format them after USB hubs.

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

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>

qemu: Introduce qemuBuildControllersByTypeCommandLine

Now that the inner loop does not require any other variables,
it can be easily separated. Apart from reducing the indentation
level this will allow it to be called from different code paths.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>

qemuBuildControllerDevStr: remove nusbcontroller argument

Now that it's no longer needed, remove the argument.
This removes the last helper variable in
qemuBuildControllerDevCommandLine.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>

qemu: separate counting of USB controllers

qemuBuildLegacyUSBControllerCommandLine is the only place where
we need to count the USB controllers.

Count them again instead of keeping track in a variable passed to
qemuBuildControllerDevStr.

This removes the need for another variable in the loop in
qemuBuildControllerDevCommandLine.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>

qemu: separate counting of legacy USB controllers

Count them in qemuBuildLegacyUSBControllerCommandLine to remove
yet another variable accessed from the loop in
qemuBuildControllerDevCommandLine.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>

Add qemuBuildDomainForbidLegacyUSBController

Shorten some long conditions.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>

qemu: exit early if USB_CONTROLLER_MODEL_NONE is present

This removes the need to mark it in the 'usbcontroller' variable.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>

qemu: move out legacy USB controller formatting

Move out the code formatting "-usb" on the QEMU command line.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>

virPortAllocatorSetUsed: ignore port 0

Similar to what commit 86dba8f3 did for virPortAllocatorRelease,
ignore port 0 in virPortAllocatorSetUsed.

For all the reasonable use cases the callers already check that
the port is non-zero, however if the port from the XML overflows
unsigned short and turns into 0, it can be set as used by
virPortAllocatorSetUsed but not released by virPortAllocatorRelease.

Also skip port '0' in virPortAllocatorSetUsed to make this behavior
symmetric.

The serenity was disturbed by commit 5dbda5e9 which started using
virPortAllocatorRelease instead of virPortAllocatorSetUsed (false).

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

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>

docs/governance: Clarify the version number of the LGPL

There is no "GNU Lesser General Public License, version 2",
only version 2.1 and later. In "version 2", the license was
still called "Library" instead of "Lesser". So assume that
version 2.1 is meant here.

Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>

tools/virt-xml-validate: Fix GPL information

The tools/virt-xml-validate.in file is licensed under the terms of
the GPL, but then says "You should have received a copy of the
GNU *Lesser* General Public License". Thus scratch the "Lesser" here.

Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>

bootstrap.conf: Fix LGPL information

The bootstrap.conf is licensed under the terms of the LGPL, but then
suggests to "See the GNU General Public License for more details".
That should be the "GNU Lesser General Public License" instead, of
course.

Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>

qemu: command: Make BuildVirtioDevStr more generic

Switch qemuBuildVirtioDevStr to use virDomainDeviceSetData: callers
pass in the virDomainDeviceType and the void * DefPtr. This will
save us from having to repeatedly extend the function argument
list in subsequent patches.

Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>

conf: Add virDomainDeviceSetData

This is essentially a wrapper for easily setting the variable
name in virDomainDeviceDef that matches its associated
VIR_DOMAIN_DEVICE_TYPE.

Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>

qemu: command: Convert vhost-{vsock,scsi} to qemuBuildVirtioDevStr

Current code essentially duplicates the same logic, but misses
some cases (like vhost-vsock-device).

Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>

qemu: command: Make vhost-scsi device string depend on address

The vhost-scsi device string should depend on the requested
address type, not strictly on the emulated arch. This is the
same logic used by qemuBuildVirtioDevStr, and this particular
path is already tested in the hostdev-scsi-vhost-scsi-ccw tests

Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>

qemu: Move <rng> validation out of qemu_command.c

Move the rng->model == VIRTIO check to parse time. This also
allows us to remove similar checks throughout the qemu driver

Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>

qemu: Move <memballoon> validation out of qemu_command.c

If we validate that memballoon is NONE|VIRTIO at parse time,
we can drop similar checks elsewhere in the qemu driver

Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>

conf: Add virDomainNetIsVirtioModel

This will be extended in the future, so let's simplify things by
centralizing the checks.

Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>

conf: Set net->model earlier

So later code can more easily access def->model

Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>

docs: Add more iscsi-direct references to storage pages

Found that it was missing in formatstorage and had a few typos
in the storage driver page.

Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>

conf: Add check to avoid a NULL compare for SysfsPath

If the two sysfs_path are both NULL, there may be an incorrect
object returned for virNodeDeviceObjListFindBySysfsPath().

This check exists in old interface virNodeDeviceFindBySysfsPath().
e.g.
virNodeDeviceFindBySysfsPath(virNodeDeviceObjListPtr devs,
                             const char *sysfs_path)
{
    ...
        if ((devs->objs[i]->def->sysfs_path != NULL) &&
            (STREQ(devs->objs[i]->def->sysfs_path, sysfs_path))) {
    ...
}

Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Cheng Lin <cheng.lin130@zte.com.cn>

domain_conf: Free egl render node in virDomainGraphicsDefFree

 13 bytes in 1 blocks are definitely lost in loss record 44 of 179
    at 0x4C2EE6F: malloc (vg_replace_malloc.c:299)
    by 0x9514A69: strdup (in /lib64/libc-2.27.so)
    by 0x5E60C0B: virStrdup (virstring.c:956)
    by 0x54C856F: virHostGetDRMRenderNode (qemuxml2argvmock.c:190)
    by 0x57CB4E3: qemuProcessGraphicsSetupRenderNode (qemu_process.c:4860)
    by 0x57CB571: qemuProcessSetupGraphics (qemu_process.c:4881)
    by 0x57CE01B: qemuProcessPrepareDomain (qemu_process.c:6040)
    by 0x57D102E: qemuProcessCreatePretendCmd (qemu_process.c:6975)
    by 0x114C1C: testCompareXMLToArgv (qemuxml2argvtest.c:611)
    by 0x134B90: virTestRun (testutils.c:174)
    by 0x123478: mymain (qemuxml2argvtest.c:1697)
    by 0x136BFA: virTestMain (testutils.c:1112)

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>

virpci: Fix memleak in virPCIDeviceIterDevices

This partially reverts 00dc991ca167302c7a72f4fb16be061d05b12a32.

 2,030 (1,456 direct, 574 indirect) bytes in 14 blocks are definitely lost in loss record 77 of 80
    at 0x4C30E96: calloc (vg_replace_malloc.c:711)
    by 0x50F83AA: virAlloc (viralloc.c:143)
    by 0x5178DFA: virPCIDeviceNew (virpci.c:1753)
    by 0x51753E9: virPCIDeviceIterDevices (virpci.c:468)
    by 0x5175EB5: virPCIDeviceGetParent (virpci.c:759)
    by 0x517AB55: virPCIDeviceIsBehindSwitchLackingACS (virpci.c:2476)
    by 0x517AC24: virPCIDeviceIsAssignable (virpci.c:2494)
    by 0x10BF27: testVirPCIDeviceIsAssignable (virpcitest.c:229)
    by 0x10D14C: virTestRun (testutils.c:174)
    by 0x10C535: mymain (virpcitest.c:422)
    by 0x10F1B6: virTestMain (testutils.c:1112)
    by 0x10CF93: main (virpcitest.c:455)

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>

virPCIGetNetName: Initialize @netname to NULL

This is a return argument that is to be compared against NULL on
successful return. However, it is not initialized and therefore
relies on callers setting it to NULL prior calling the function.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>

qemu: improve compile-time check of qemuBlockjobState mapping

Asserting the value we set four lines earlier in qemuBlockjobState
doesn't buy us any safety (if the public header adds a value, we end
up skipping that value without the compiler warning us of our gap);
what we really want is to assert that the value auto-assigned by the
compiler matches the actual last value in the public headers (as was
done below for qemuBlockJobType).  Add useful comments while at it.

Signed-off-by: Eric Blake <eblake@redhat.com>
ACKed-by: Peter Krempa <pkrempa@redhat.com>

apparmor: convert libvirtd profile to a named profile

Signed-off-by: Jim Fehlig <jfehlig@suse.com>

apparmor: Add support for named profiles

Upstream apparmor is switching to named profiles. In short,

/usr/sbin/dnsmasq {

becomes

profile dnsmasq /usr/sbin/dnsmasq {

Consequently, any profiles that reference profiles in a peer= condition
need to be updated if the referenced profile switches to a named profile.
Apparmor commit 9ab45d81 switched dnsmasq to a named profile. ATM it is
the only named profile switch that has affected libvirt. Add rules to the
libvirtd profile to reference dnsmasq in peer= conditions by profile name.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>

libxl: Set current memory value after successful balloon

The libxl driver does not set the new memory value in the active domain def
after a successful balloon. This results in the old memory value in
<currentMemory>. E.g.

virsh dumpxml test | grep currentMemory
  <currentMemory unit='KiB'>20971520</currentMemory>
virsh setmem test 16777216 --live
virsh dumpxml test | grep currentMemory
  <currentMemory unit='KiB'>20971520</currentMemory>

Set the new memory value in active domain def after a successful call to
libxl_set_memory_target().

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemu: process: Handle all failure values for dimms in qemuProcessHandleAcpiOstInfo

Hanlde all the possible failure codes as per ACPI standard documented in
the function header.

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

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

qemu: process: Improve documentation of values handled by qemuProcessHandleAcpiOstInfo

We forgot to document the specific fields for the 0x103 and 0x200
sources which are tied to device removal and device hotplug
respectively.

The value description is based on the ACPI 6.2A standard Table 6-207 and
Table 6-208. At the time of writing of this patch the standard can be
accessed e.g. at:

https://www.uefi.org/sites/default/files/resources/ACPI%206_2_A_Sept29.pdf

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

util: Fixing invalid error checking from virPCIGetNetname()

The @linkdev is In/Out function parameter as second order
reference pointer so requires first order dereference for
checking NULL which can be the result of virPCIGetNetName().

Fixes: d6ee56d7237 (util: change virPCIGetNetName() to not return error if device has no net name)
Signed-off-by: Radoslaw Biernacki <radoslaw.biernacki@linaro.org>
Signed-off-by: dann frazier <dann.frazier@canonical.com>

util: Fix for NULL dereference

The device xml parser code does not set "model" while parsing the
following XML:

  <interface type='hostdev'>
    <source>
      <address type='pci' domain='0x0002' bus='0x01' slot='0x00' function='0x2'/>
    </source>
  </interface>

The net->model can be NULL and therefore must be compared using
STREQ_NULLABLE instead of plain STREQ.

Fixes: ac47e4a6225 (qemu: replace "def->nets[i]" with "net" and "def->sounds[i]" with "sound")
Fixes: c7fc151eec7 (qemu: assign virtio devices to PCIe slot when appropriate)
Signed-off-by: Radoslaw Biernacki <radoslaw.biernacki@linaro.org>
Signed-off-by: dann frazier <dann.frazier@canonical.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

util: Code simplification

Removing redundant sections of the code

Signed-off-by: Radoslaw Biernacki <radoslaw.biernacki@linaro.org>
Signed-off-by: dann frazier <dann.frazier@canonical.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

util: fixing wrong assumption that PF has to have netdev assigned

libvirt wrongly assumes that VF netdev has to have the
netdev assigned to PF. There is no such requirement in SRIOV standard.
This patch change the virNetDevSwitchdevFeature() function to deal
with SRIOV devices which does not have netdev on PF. Also corrects
one comment about PF netdev assumption.

One example of such devices is ThunderX VNIC.
By applying this change, VF device is used for virNetlinkCommand() as
it is the only netdev assigned to VNIC.

Signed-off-by: Radoslaw Biernacki <radoslaw.biernacki@linaro.org>
Signed-off-by: dann frazier <dann.frazier@canonical.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

virt-aa-helper: generate rules for gl enabled graphics devices

This adds the virt-aa-helper support for gl enabled graphics devices to
generate rules for the needed rendernode paths.

Example in domain xml:
<graphics type='spice'>
  <gl enable='yes' rendernode='/dev/dri/bar'/>
</graphics>

results in:
  "/dev/dri/bar" rw,

Special cases are:
- multiple devices with rendernodes -> all are added
- non explicit rendernodes -> follow recently added virHostGetDRMRenderNode
- rendernode without opengl (in egl-headless for example) -> still add
  the node

Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1757085
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Acked-by: Jamie Strandboge <jamie@canonical.com>
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>

qemu: error out when vnc vncTLSx509secretUUID is unsupported

Add a capability check to qemuDomainDefValidate and refuse to start
a domain with VNC graphics if the TLS secret was set in qemu.conf
and it's not supported.

Note that qemuDomainSecretGraphicsPrepare does not generate any
secret data if the capability is not present and qemuBuildTLSx509BackendProps
is not called at all.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

qemu: add support for encrypted VNC TLS keys

Use the password stored in the secret driver under
the uuid specified by the vnc_tls_x509_secret_uuid
option in qemu.conf.

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

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

qemu.conf: add vnc_tls_x509_secret_uuid

Add an option that lets the user specify the secret
that unlocks the server TLS key.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

qemu_process: fix debug message

Be generic instead of trying to enumerate all the involved
device types.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

qemu: prepare secret for the graphics upfront

Instead of hardcoding the TLS creds alias in
qemuBuildGraphicsVNCCommandLine, store it
in the domain private data.

Given that we only support one VNC graphics
and thus have only one alias per-domain,
this is overengineered, but it will allow us
to prepare the secret upfront when we start
supporting encrypted server TLS keys.

Note that the alias is not formatted anywhere
since we won't need to access it after domain
startup.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

qemu: add qemuDomainGraphicsPrivate data with a tlsAlias

Also introduce the necessary callbacks.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

conf: add privateData to virDomainGraphicsDef

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

conf: introduce virDomainGraphicsNew

A helper function for allocating the virDomainGraphicsDef structure.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

rpm spec: remove %{extra_release} from spec

The %{extra_release} field was previously populated by data from the old
autobuild.sh file but is no longer used.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

tests: Add capabilities data for QEMU 4.0.0 x86_64

The next release of QEMU is going to be 4.0.0. A bit early, but
this adds capabilities data for x86_64 from current qemu git
15bede554162dda822cd762c689edb6fa32b6e3b

Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>

docs: use JavaScript based PolicyKit .rules files

PolicyKit authentication rules have switched to a JavaScript based
format quite some time ago. See:

http://davidz25.blogspot.com/2012/06/authorization-rules-in-polkit.html

While backwards compat for the old .pkla format is still available, it
makes sense to point people first at the new format.

The SSHPolicyKitSetup wiki page seems pretty stale, so remove the
reference to it.

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Mark McLoughlin <markmc@redhat.com>

virQEMUDriverConfigLoadSWTPMEntry: use VIR_AUTOFREE

Switch the function to use VIR_AUTOFREE and VIR_AUTOPTR macros
to get rid of the cleanup section.

Requested-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>

virQEMUDriverConfigLoadSecurityEntry: use VIR_AUTOFREE

Switch the function to use VIR_AUTOFREE and VIR_AUTOPTR macros
to get rid of the cleanup section.

Requested-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>

virQEMUDriverConfigLoadNVRAMEntry: use VIR_AUTOFREE

Switch the function to use VIR_AUTOFREE and VIR_AUTOPTR macros
to get rid of the cleanup section.

Requested-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>

virQEMUDriverConfigLoadProcessEntry: use VIR_AUTOFREE

Switch the function to use VIR_AUTOFREE and VIR_AUTOPTR macros
to get rid of the cleanup section.

Requested-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>

qemu: command: Don't format image properties for empty -drive

If a -drive has no image, using image properties makes qemu whine that
they should not be used.

This patch stops formating cache/readonly/... for empty drives
for the pre-blockdev syntax. Unfortunately those parameters can't be
added later when inserting media, but on the other hand qemu will start
with an empty drive.

Since we already were able to start a VM with such config previously due
to qemu ignoring them I've opted just to skip formatting them.
Additionally with -blockdev support it will work as expected as the
image properties will be formatted when adding the image itself which is
not possible without it.

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

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

tests: qemuxml2argv: Add test case for empty CDROM with cache mode

Upcomming change will influence CDROM with cache mode so add a test
case.

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

rpm spec: don't assume %{fedora} exists as a macro

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

config-post: Remove duplicated 'undef WITH_CAPNG'

Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>

rpm spec: disable ceph on 32-bit architectures

Ceph in upstream and Fedora has dropped support for building on host
architectures which are 32-bit.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

qemu: fix i6300esb watchdog hotplug on Q35

When commit 361c8dc17 added support for hotplugging the i6300esb
watchdog device (first in libvirt-3.9.0), it accidentally contstructed
the commandline for the device_add command before allocating a PCI
address for the device. With no PCI address specified in the command,
the watchdog would simply be placed at the lowest unused PCI slot.

On a 440fx guest, this doesn't cause a problem, because libvirt's PCI
address allocation algorithm would most likely give the same address
anyway (usually a slot on pci-root), so nobody noticed the omission of
address from the command.

But on a Q35 guest, the lowest unused PCI slot is on pcie-root, which
doesn't support hotplug; libvirt knows enough to assign a PCI address
that is on a pcie-to-pci-bridge (because its slots *do* support
hotplug), but qemu doesn't, so if there is no PCI address in the
command, qemu just tries to plug the new device into pcie-root, and
fails because it doesn't support hotplug, e.g.:

  error: Failed to attach device from watchdog.xml
  error: internal error: unable to execute QEMU command 'device_add':
  Bus 'pcie.0' does not support hotplugging

The solution is simply to build the command string after assigning a
PCI address, not before.

Resolves: https://bugzilla.redhat.com/1666559
Signed-off-by: Laine Stump <laine@laine.org>
Reviewed-by: John Ferlan <jferlan@redhat.com>

qemu: Assign device addresses earlier in qemuDomainAttachNetDevice

If code in the @actualType switch needs to have/know which PCI
Address is being used, then we must assign it earlier. In particular
a vhost-user device needs to call qemuDomainSupportsNicdev which
requires an address to be defined.

Signed-off-by: Wang Yechao <wang.yechao255@zte.com.cn>
Reviewed-by: John Ferlan <jferlan@redhat.com>

qemu_conf: rename checkdefaultTLSx509certdir

Use defaultTLSx509certdirPresent for consistencty.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

qemu_conf: split out virQEMUDriverConfigLoadDefaultTLSEntry

Split out parts of the config parsing code to make
the parent function easier to read.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

qemu_conf: split out virQEMUDriverConfigLoadVNCEntry

Split out parts of the config parsing code to make
the parent function easier to read.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

qemu_conf: split out virQEMUDriverConfigLoadNographicsEntry

Split out parts of the config parsing code to make
the parent function easier to read.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

qemu_conf: split out virQEMUDriverConfigLoadSPICEEntry

Split out parts of the config parsing code to make
the parent function easier to read.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

qemu_conf: split out virQEMUDriverConfigLoadSpecificTLS

Split out parts of the config parsing code to make
the parent function easier to read.

This is the only patch that mixes various augeas entry
groups in one function.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

qemu_conf: split out virQEMUDriverConfigLoadRemoteDisplayEntry

Split out parts of the config parsing code to make
the parent function easier to read.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

qemu_conf: split out virQEMUDriverConfigLoadSaveEntry

Split out parts of the config parsing code to make
the parent function easier to read.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

qemu_conf: split out virQEMUDriverConfigLoadProcessEntry

Split out parts of the config parsing code to make
the parent function easier to read.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

qemu_conf: split out virQEMUDriverConfigLoadDeviceEntry

Split out parts of the config parsing code to make
the parent function easier to read.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

qemu_conf: split out virQEMUDriverConfigLoadRPCEntry

Split out parts of the config parsing code to make
the parent function easier to read.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

qemu_conf: split out virQEMUDriverConfigLoadNetworkEntry

Split out parts of the config parsing code to make
the parent function easier to read.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

qemu_conf: split out virQEMUDriverConfigLoadLogEntry

Split out parts of the config parsing code to make
the parent function easier to read.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

qemu_conf: split out virQEMUDriverConfigLoadNVRAMEntry

Split out parts of the config parsing code to make
the parent function easier to read.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

qemu_conf: split out virQEMUDriverConfigLoadGlusterDebugEntry

Split out parts of the config parsing code to make
the parent function easier to read.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

qemu_conf: split out virQEMUDriverConfigLoadSecurityEntry

Split out parts of the config parsing code to make
the parent function easier to read.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>