I find it more readable that way.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Similarly to gethostbyname3(), the @addr must be freed on return
from the function.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

The findLease() function allocates @addr array iff no error
occurred and at least one satisfactory record was found.
Therefore, there is no need to call free() if findLease() failed,
or did not find any records as addr == NULL.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

When parsing leases file, appendAddr() is called to append parsed
tuple (address, expiry time, family) into an array. Whilst doing
so, the array is searched for possible duplicate. This is done by
comparing each item of the array by passed @family: if @family is
AF_INET then the item is viewed as IPv4 address. Similarly, if
@family is AF_INET6 then the item is viewed as IPv6 address. This
is not exactly right - the array can contain addresses of both
families and thus the address family of each item of the array
must be considered.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

These helped with debugging
https://bugzilla.redhat.com/show_bug.cgi?id=1612383Reviewed-by: NPavel Hrdina <phrdina@redhat.com>
Signed-off-by: NCole Robinson <crobinso@redhat.com>

On Fedora 31, starting a 'mock' build alters /proc/$pid/cgroup,
probably due to usage of systemd-nspawn.

Before:
$ cat /proc/self/cgroup
0::/user.slice/user-1000.slice/...

After:
$ cat /proc/self/cgroup
1:name=systemd:/
0::/user.slice/user-1000.slice/...

The cgroupv2 code mishandles that first line in the second case, which
causes VM startup to fail with: Unable to read from
'/sys/fs/cgroup/machine/cgroup.controllers': No such file or directory

The kernel docs[1] say that the cgroupv2 path will always start with
'0::', which in the code here controllers="". Only set the v2 placement
path when we see that cgroup file entry.

[1] https://www.kernel.org/doc/html/v5.3/admin-guide/cgroup-v2.html#processes

https://bugzilla.redhat.com/show_bug.cgi?id=1751120Reviewed-by: NPavel Hrdina <phrdina@redhat.com>
Signed-off-by: NCole Robinson <crobinso@redhat.com>

The code that gets the job to refresh disk sizes was not merged yet so
remove this artifact.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

'vm' is passed in which contains the definition which contains the UUID
so we don't need another parameter for this.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

'vm' is passed in which contains the definition which contains the UUID
so we don't need another parameter for this.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Move it to qemu_domain.c and rename it to qemuDomainObjFromDomain. This
will allow reusing it after splitting out checkpoint code from
qemu_driver.c.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Blacklist Perl and Shell code in favour of Python for
sake of readability and portability.
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

As it turns out, on my 32bit ARM machine size_t is not the same
size as ULL. However, @length argument for both functions is type
of size_t but it's treated as ULL - for instance when passed to
qemuMonitorJSONMakeCommand(). The problem is that because of
"U:size" the virJSONValueObjectAddVArgs() expects an ULL argument
but on the stack there are size_t and char * arguments (which
coincidentally add up to size of ULL). So the created command has
only two arguments "val" and incorrect "size" and no "path" which
is required.

I've tried to find other occurrences of this pattern but at the
rest of places where size_t is used it tracks size of an array so
that's safe.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
ACKed-by: NPeter Krempa <pkrempa@redhat.com>

We stopped generating a giant ChangeLog file in

  commit ce97c33a
  Author: Andrea Bolognani <abologna@redhat.com>
  Date:   Mon Apr 1 17:33:03 2019 +0200

      maint: Stop generating ChangeLog from git

so there is no reason to compress it anymore.
Reviewed-by: NLaine Stump <laine@laine.org>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Drop the 'driver' argument since it can be extracted from private data
to shorten the argument list.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

Now that qemu 4.1 was released we can update the capabilities to the
final form.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Add capabilities test data for upcoming qemu 4.2.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJiri Denemark <jdenemar@redhat.com>

Before the refactoring that properly separated the network driver from
the hypervisor driver and forced all interaction to go through public
APIs, all network usage counters were zeroed when the network driver
was initialized, and the network driver's now-deprecated
"semi-private" API networkNotifyActualDevice() was called for every
interface of every domain as each hypervisor "reconnected" its domains
during a libvirtd restart, and this would refresh the usage count for
each network.

Post-driver-split, during libvirtd restart/reconnection of the running
domains, the function virDomainNetNotifyActualDevice() is called by
each hypervisor driver for every interface of every domain restart,
and this function has code to re-register interfaces, but it only
calls into the network driver to re-register those ports that don't
already have a valid portid (ie. one that is not simply all 0),
assuming that those with valid portids are already known (and counted)
by the network driver.

commit 7ab9bdd4 recently modified the network driver so that, in most
cases, it properly resyncs each network's connection count during
libvirtd (or maybe virtnetworkd) restart by iterating through the
network's port list. This doesn't account for the case where a network
is destroyed and restarted while there are running domains that have
active ports on the network. In that case, the entire port list and
connection count for that network is lost, and now even a restart of
libvirtd/virtnetworkd/virtqemud, which in the past would resync the
connection count, doesn't help (the network driver thinks there are no
active ports, while the hypervisor driver knows about all the active
ports, but mistakenly believes that the network driver also knows).

The solution to this is to not just bypass valid portids during the
call to virDomainNetworkNotifyActualDevice(). Instead, we query the
network driver about the portid that was preserved in the domain
status, and if it is not registered, we register it.

(NB: while it would technically be correct to just generate a new
portid for these cases, it makes for less churn in portids (and thus
may make troubleshooting simpler) if we make the small fix to
virDomainNetDefActualToNetworkPort() that preserves existing valid
portids rather than unconditionally generating a new one.)
Signed-off-by: NLaine Stump <laine@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

define a VIR_DEFINE_AUTOPTR_FUNC() to autofree virNetworkPortDefs, and
convert all uses of virNetworkPortDefPtr that are appropriate to use
it.

This coincidentally fixes multiple potential memory leaks (in failure
cases) in networkPortCreateXML()
Signed-off-by: NLaine Stump <laine@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

Reviewed-by: NCole Robinson <crobinso@redhat.com>
Suggested-by: NCole Robinson <crobinso@redhat.com>
Signed-off-by: NDaniel Henrique Barboza <danielhb413@gmail.com>

Reviewed-by: NCole Robinson <crobinso@redhat.com>
Suggested-by: NCole Robinson <crobinso@redhat.com>
Signed-off-by: NDaniel Henrique Barboza <danielhb413@gmail.com>

Reviewed-by: NCole Robinson <crobinso@redhat.com>
Suggested-by: NCole Robinson <crobinso@redhat.com>
Signed-off-by: NDaniel Henrique Barboza <danielhb413@gmail.com>

The existing QEMU and vbox URI path validation consider
that a privileged user can use both a "/system" and a
"/session" URI. This differs from all the other drivers
that forbids the root user to use "/session" URI.

Let's update virConnectValidateURIPath() to handle these
cases as exceptions, using the already existent 'entityName'
value to handle "QEMU" and "vbox" differently. This allows
us to use the validateURI function in these cases without
changing the existing behavior of other drivers.
Reviewed-by: NCole Robinson <crobinso@redhat.com>
Suggested-by: NCole Robinson <crobinso@redhat.com>
Signed-off-by: NDaniel Henrique Barboza <danielhb413@gmail.com>

Reviewed-by: NCole Robinson <crobinso@redhat.com>
Suggested-by: NCole Robinson <crobinso@redhat.com>
Signed-off-by: NDaniel Henrique Barboza <danielhb413@gmail.com>

Reviewed-by: NCole Robinson <crobinso@redhat.com>
Suggested-by: NCole Robinson <crobinso@redhat.com>
Signed-off-by: NDaniel Henrique Barboza <danielhb413@gmail.com>

Reviewed-by: NCole Robinson <crobinso@redhat.com>
Suggested-by: NCole Robinson <crobinso@redhat.com>
Signed-off-by: NDaniel Henrique Barboza <danielhb413@gmail.com>

Reviewed-by: NCole Robinson <crobinso@redhat.com>
Suggested-by: NCole Robinson <crobinso@redhat.com>
Signed-off-by: NDaniel Henrique Barboza <danielhb413@gmail.com>

Reviewed-by: NCole Robinson <crobinso@redhat.com>
Suggested-by: NCole Robinson <crobinso@redhat.com>
Signed-off-by: NDaniel Henrique Barboza <danielhb413@gmail.com>

Reviewed-by: NCole Robinson <crobinso@redhat.com>
Suggested-by: NCole Robinson <crobinso@redhat.com>
Signed-off-by: NDaniel Henrique Barboza <danielhb413@gmail.com>

The code to validate the URI path is repeated across several
files. This patch creates a common validation code to be
used across all of them.
Reviewed-by: NCole Robinson <crobinso@redhat.com>
Suggested-by: NCole Robinson <crobinso@redhat.com>
Signed-off-by: NDaniel Henrique Barboza <danielhb413@gmail.com>

A virDomainNetDef object in a domain's nets array might contain a
virDomainHostdevDef, and when this is the case, the domain's hostdevs
array will also have a pointer to this embedded hostdev (this is done
so that internal functions that need to perform some operation on all
hostdevs won't leave out the type='hostdev' network interfaces).

When a network device was updated with virDomainUpdateDeviceFlags(),
we were replacing the entry in the nets array (and free'ing the
original) but forgetting about the pointer in the hostdevs array
(which would then point to the now-free'd hostdev contained in the old
net object.) This often resulted in a libvirtd crash.

The solution is to add a function, virDomainNetUpdate(), called by
qemuDomainUpdateDeviceConfig(), that updates the hostdevs array
appropriately along with the nets array.

Resolves: https://bugzilla.redhat.com/1558934Signed-off-by: NLaine Stump <laine@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

The private data for video definition is created in
virDomainVideoDefNew() and we attempt to free it in
virDomainVideoDefFree(). This seems to work, except
the free function calls clear function which zeroes
out the whole structure and thus virObjectUnref()
which is called on private data does nothing.

2,568 bytes in 107 blocks are definitely lost in loss record 207 of 213
   at 0x4A35476: calloc (vg_replace_malloc.c:752)
   by 0x50A6048: virAllocVar (viralloc.c:346)
   by 0x513CC5A: virObjectNew (virobject.c:243)
   by 0x4DC1DEE: qemuDomainVideoPrivateNew (qemu_domain.c:1337)
   by 0x51A6BD6: virDomainVideoDefNew (domain_conf.c:2831)
   by 0x51B9F06: virDomainVideoDefParseXML (domain_conf.c:15541)
   by 0x51CB761: virDomainDefParseXML (domain_conf.c:21158)
   by 0x51C5973: virDomainDefParseNode (domain_conf.c:21708)
   by 0x51C583A: virDomainDefParse (domain_conf.c:21663)
   by 0x51C58AE: virDomainDefParseFile (domain_conf.c:21688)
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

nodinfotest.c doesn't exist anymore

seclabeltest.c has changed substantially since this behavior was
added to the spec, and in my testing doesn't have any problems
running in mock
Reviewed-by: NErik Skultety <eskultet@redhat.com>
Signed-off-by: NCole Robinson <crobinso@redhat.com>

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

The macro now became unused so it was deleted.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

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

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

The open-coded version does not take much more space and additionally we
get rid of the hidden goto.

This also requires us to remove the 'cleanup' section.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

The bulk stats functions are specific as they pass around the list into
many sub-functions and also a substantial amount of the entries uses
formatted names for indexing purposes. This makes them ideal to be
converted to the new virTypedParamList helpers.

Unfortunately given how the functions are used this requires a big-bang
rewrite of all of the calls to add entries to the parameter list.

Given that a substantial simplification is achieved as well as a pretty
significant change to the original code is required some macros which
were used only sporadically were replaced by inline calls rather than
tweaking the macros first and deleting them later.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Use QEMU_ADD_BLOCK_PARAM_ULL instead since all parameters are now
unsigned.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

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