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

Instantiation of an nwfilter binding is only allowed when
the net->filter is defined for the network; however, the
teardown of the binding does not make this check. This
leaves open the possibility that the teardown could be
called during guest shutdown/teardown in session mode
resulting in the following error being logged:

    error : nwfilterConnectOpen:383 : internal error: unexpected
    nwfilter URI path '/session', try nwfilter:///system

So before going through the teardown processing, let's
be sure the network had a filter and then attempt to
get a connection. For session mode it's not even possible
create an nwfilter binding.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

Similar to nwfilterDefineXML, let's be sure the a filter binding
creation is not attempted in session mode and generate the proper
error message.

Failure to open nwfilter in session mode (nwfilterConnectOpen)
fails already, but that doesn't stop the free thinker from using
a different connection in order to attempt to attempt to create
the binding. Although even doing that would result in a failure:

$ virsh nwfilter-binding-create QEMUGuest1-binding.xml
error: Failed to create network filter from QEMUGuest1-binding.xml
error: internal error: Could not get access to ACL tech driver 'ebiptables'

$
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

A virtio device such as

  <controller type='scsi' model='virtio-scsi'/>

will be translated to one of four different QEMU devices
based on the address type. This behavior is the same for
all virtio devices, but unfortunately we have separate
ad-hoc code dealing with each and every one of them: not
only this is pointless duplication, but it turns out
that most of that code is not robust against new address
types being introduced and some of it is outright buggy.

Introduce a new function, qemuBuildVirtioDevStr(), which
deals with the issue in a generic fashion, and rewrite
all existing code to use it.

This fixes a bunch of issues such as virtio-serial-pci
being used with virtio-mmio addresses and virtio-gpu
not being usable at all with virtio-mmio addresses.

It also introduces a couple of minor regressions,
namely no longer erroring out when attempting to
use virtio-balloon and virtio-input devices with
virtio-s390 addresses; that said, virtio-s390 has
been superseded by virtio-ccw such a long time ago
that recent QEMU releases have dropped support for
the former entirely, so re-implementing such
device-specific validation is not worth it.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

The appropriate time to ensure the required capabilities are
present is validate rather than command line generation: add
a new qemuDomainDeviceDefValidateInput() function and move
all existing checks there.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

So far we've only formatted it for virtio-blk-pci and
virtio-blk-ccw, but other virtio-blk devices also support
the corresponding option; moreover, we've always formatted
it for all virtio-scsi devices.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

There are several functions where we pointlessly duplicate
parts of the format string and pass the same arguments:
refactor them so that the common parts are formatted separately
from the variable parts.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

A few qemuxml2xml tests for virtio-input devices are
missing the capabilities used for the corresponding
qemuxml2argv tests: this wasn't a problem until now
because capabilities were only checked at command line
generation time, but we're going to change that later.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

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

If a domain is configured to use <source type='file'/> under
<memoryBacking/> we have to honour that setting and produce
-mem-path on the command line. We are not doing so if domain has
no guest NUMA nodes nor hugepages.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Signed-off-by: NEric Blake <eblake@redhat.com>

The function to retrieve the file system info using QEMU-GA is using
some conditionals to retrieve the info. This is wrong because the error
of some conditionals will be raised if VIR_STRDUP return errors and not
if some problem occurred with JSON.
Signed-off-by: NJulio Faracco <jcfaracco@gmail.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

QEMU-GA supports get geust hostname command. This commit includes a
specific entry to inform this new feature for QEMU driver to 4.8.0
release.
Signed-off-by: NJulio Faracco <jcfaracco@gmail.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

This commit adds support to use the function qemuAgentGetHostname()
to obtain the domain hostname using QEMU-GA command.
Signed-off-by: NJulio Faracco <jcfaracco@gmail.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

This commit implements the function qemuAgentGetHostname() that uses
the QEMU guest agent command 'guest-get-host-name' to retrieve the
guest hostname of virtual machine running the QEMU-GA.
Signed-off-by: NJulio Faracco <jcfaracco@gmail.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

The new tests use DO_TEST_CAPS_ARCH_LATEST() with an input
XML describing a very simple headless guest and cover most
architectures and machine types we care about.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

We can add aarch64, ppc64 and riscv64 to the list of
supported architectures for the macro, since we have
capabilities data for all of them.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

virtio-serial is an alias for virtio-serial-pci, which
should not have been used for a PCIe-less aarch64/virt
guest but it ended up being used anyway because the
virtio-mmio capability was missing and the algorithm
is buggy.

Fix the test case so that we can fix the algorithm next.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

There's a single user for it which takes an existing
virPCIDeviceAddress, passes its various bits to the
function which in turn constructs a virPCIDevice and
then copies the string representation for the caller
to use: we can use virPCIDeviceAddressAsString()
instead and avoid creating the virPCIDevice in the
first place. Since the function ends up having no
users after the change, we can just drop it.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NMartin Kletzander <mkletzan@redhat.com>

The struct is called virPCIDeviceAddress and the
functions operating on it should be named accordingly.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NMartin Kletzander <mkletzan@redhat.com>

It's a better fit than conf/domain_conf.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NMartin Kletzander <mkletzan@redhat.com>

This will help in future expansions of the code when it is be
harder to track if @newName and/or @newLockspace were already
allocated or not and thus whether it is safe to 'return' or we
need to 'goto error'. By using the 'cleanup' label those two
cases merge into a single one.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

If drvNew callback fails, nobody calls drvFree and thus private
data of the driver might leak.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Currently, there are only two types of resource. So effectively
this is a dead code. However, that assumption can change and we
shouldn't just silently ignore the error.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

The if() is completely useless since args.path is set to NULL in
the line just above.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

On daemon deinit only fileLockSpaceDir is freed. The other two
(scsiLockSpaceDir and lvmLockSpaceDir) are missing even though
they are allocated in virLockManagerLockDaemonLoadConfig().
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

28 bytes in 1 blocks are definitely lost in loss record 26 of 66
   at 0x4C2CF0F: malloc (vg_replace_malloc.c:299)
   by 0x7A02719: strdup (strdup.c:42)
   by 0x197DC1: virStrdup (virstring.c:961)
   by 0x12B478: virLockDaemonConfigFilePath (lock_daemon_config.c:44)
   by 0x12A759: main (lock_daemon.c:1270)

62 (32 direct, 30 indirect) bytes in 1 blocks are definitely lost in loss record 41 of 66
   at 0x4C2EF26: calloc (vg_replace_malloc.c:711)
   by 0x151B61: virAlloc (viralloc.c:144)
   by 0x12B56C: virLockDaemonConfigNew (lock_daemon_config.c:71)
   by 0x12A491: main (lock_daemon.c:1262)

13 bytes in 1 blocks are definitely lost in loss record 21 of 70
   at 0x4C2CF0F: malloc (vg_replace_malloc.c:299)
   by 0x7A02719: strdup (strdup.c:42)
   by 0x197E3F: virStrdup (virstring.c:961)
   by 0x12C86B: virLockSpaceProtocolDispatchRegister (lock_daemon_dispatch.c:291)
   by 0x12BB73: virLockSpaceProtocolDispatchRegisterHelper (lock_daemon_dispatch_stubs.h:152)
   by 0x1336AA: virNetServerProgramDispatchCall (virnetserverprogram.c:437)
   by 0x13320D: virNetServerProgramDispatch (virnetserverprogram.c:304)
   by 0x139E3E: virNetServerProcessMsg (virnetserver.c:144)
   by 0x13A1A2: virNetServerDispatchNewMessage (virnetserver.c:230)
   by 0x1350F5: virNetServerClientDispatchMessage (virnetserverclient.c:343)
   by 0x137680: virNetServerClientDispatchEvent (virnetserverclient.c:1498)
   by 0x147704: virNetSocketEventHandle (virnetsocket.c:2140)
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

The virSecurityStackAddNested() can fail in which case
virSecurityManagerNewStack() should fail too.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Use 'error' label to free allocated memory.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

The current socket test is rather crazy in that it sets up a server
listening for sockets and then runs a client connect call, relying on
the fact that the kernel will accept this despite the application
not having called accept() yet. It then closes the client socket and
calls accept() on the server. On Linux accept() will always see that
the client has gone and so skip the rest of the code. On FreeBSD,
however, the accept sometimes succeeds, causing us to then go into
code that attempts to read and write to the client which will fail
aborting the test. The accept() never succeeds on FreeBSD guests
with a single CPU, but as you add more CPUs, accept() becomes more and
more likely to succeed, giving a 100% failure rate for the test when
using 8 CPUs.

This completely rewrites the test so that it is avoids this designed in
race condition. We simply spawn a background thread to act as the
client, which will read a byte from the server and write it back again.
The main thread can now properly listen and accept the client in a
synchronous manner avoiding any races.
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

The test code for UNIX and TCP sockets will need to be rewritten and
extended later, and will benefit from code sharing.
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

If no JSON parser is available qemublocktest fails, so skip its execution.
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

The expected output strings from the vshtabletest.c are created on a
modern Linux host where unicode printing support is very good. On older
Linux platforms, or non-Linux platforms, some unicode characters will
not be considered printable. While the vsh table alignment code will
stil do the right thing with escaping & aligning in this case, the
result will not match the test's expected output.

Since we know the code is working correctly, do a check with iswprint()
to validate the platform's quality and skip the test if it fails. This
fixes the test on FreeBSD platforms.
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

The corresponding structs are declared there.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

These are simple predicates, which makes bool a more
appropriate return type than int.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

The function is called on a virDomainDeviceInfo, so it
should be declared along with it.

Moving this function requires moving and making public
virDomainDeviceCCWAddressIsValid() as well, but that's
perfectly fine since the same reasoning above also
applies to it, due to virDomainDeviceCCWAddress being
(correctly) declared in device_conf.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

It's used in virDomainDeviceInfo, which makes
domain_conf the wrong place to declare it.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

When switching the host architecture to something for which we do not
have any host CPU model defined, the mocked
virQEMUCapsProbeHostCPUForEmulator would just return the previous CPU
model resulting in strange combinations, such as "core2duo" host CPU
model in QEMU capabilities for "AArch64" architecture. It currently
doesn't break any test case, but we should fix it anyway to avoid future
surprises which would be quite hard to debug.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

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

When users want to update a path to a CDROM they tend to
construct a very minimal XML and feed the API with it. This is
not a good practice as it breaks the assumptions the API is built
on. Most notably, leaving an element out should be treated as a
request for removal of the corresponding setting. Just like
leaving out <bandwidth/> clears out any QoS previously set.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Test that we correctly accept 64-bit unsigned numbers for QEMU.
Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

We expect to get numbers as big as ULLONG_MAX from QEMU,
add a test for them.
Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>