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

This reverts commit cefb97fb.

The stateAutoStart callback will be removed in the next commit.
Therefore move autostarting of domains, networks and storage
pools back into stateInitialize callbacks.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Use of VIR_AUTOPTR and virString is confusing as it's a list and not a
single pointer. Replace it by VIR_AUTOSTRINGLIST as string lists are
basically the only sane NULL-terminated list we can have.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

The structure used to handle network entries was based on 'if,else'
conditions. This commit converts this ugly structure into a switch to
clearify each option of the handler.
Signed-off-by: NJulio Faracco <jcfaracco@gmail.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Extract out the network "type" processing into it's own method
rather than inline within lxcNetworkParseDataSuffix.
Signed-off-by: NJulio Faracco <jcfaracco@gmail.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

This commit removes the full network entry setting: "lxc.network.X" to
type only. Like "type", "name", "flags", etc. This will handle entries
regardless of whether they are prefixed by "lxc.network." (today) or
"lxc.net.X." (the future).
Signed-off-by: NJulio Faracco <jcfaracco@gmail.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Refactor lxcNetworkWalkCallback to be a simple method to handle
both possible network settings with indexes or the simple one. It is
better the decouple the whole algorithm to parse data to only parse
which entry type libvirt is handling.

The new method is responsible to verify is the settings correspond to
network entry. Right now, it is only verifying "lxc.network.", but in
the future, it can be used to verify "lxc.net.X." too. Any other case
would be rejected.

On the other hand, the idea here is working only with types. If we know
that entry is part of network settings, after we just need to know which
type is. It keeps the handler simple.
Signed-off-by: NJulio Faracco <jcfaracco@gmail.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

The new method called lxcNetworkParseDataIPs() is responsible to handle
IPv{4,6} settings now. The idea is let lxcNetworkWalkCallback() method
handle all entries related to network definition only.
Signed-off-by: NJulio Faracco <jcfaracco@gmail.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Many drivers had a comment that they did not validate the incoming
'flags' to virDomainGetXMLDesc() because they were relying on
virDomainDefFormat() to do it instead. This used to be the case
(at least since 461e0f1a and friends in 0.9.4 added unknown flag
checking in general), but regressed in commit 0ecd6851 (1.2.12),
when all of the drivers were changed to pass 'flags' through the
new helper virDomainDefFormatConvertXMLFlags(). Since this helper
silently ignores unknown flags, we need to implement flag checking
in each driver instead.

Annoyingly, this means that any new flag values added will silently
be ignored when targeting an older libvirt, rather than our usual
practice of loudly diagnosing an unsupported flag.  Add comments
in domain_conf.[ch] to remind us to be extra vigilant about the
impact when adding flags (a new flag to add data is safe if the
older server omitting the requested data doesn't break things in
the newer client; a new flag to suppress data rather than enhancing
the existing VIR_DOMAIN_XML_SECURE may form a data leak or even a
security hole).

In the qemu driver, there are multiple callers all funnelling to
qemuDomainDefFormatBufInternal(); many of them already validated
flags (and often only a subset of the full set of possible flags),
but for ease of maintenance, we can also check flags at the common
helper function.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Instead of repetitive:
  s ? s : ""
use NULLSTR_EMPTY.
Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>

If the container is really a simple one (init is just bash and
the whole root is passed through) then virDomainReboot and
virDomainShutdown will talk to the actual init within the host.
Therefore, 'virsh shutdown $dom' will result in shutting down the
host. True, at that point the container is shut down too but
looks a bit harsh to me.

The solution is to check if the init inside the container is or
is not the same as the init running on the host.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

So far the virInitctlSetRunLevel() is fully automatic. It finds
the correct fifo to use to talk to the init and it will set the
desired runlevel. Well, callers (so far there is just one) will
need to inspect the fifo a bit just before the runlevel is set.
Therefore, expose the internal list of fifos and also allow
caller to explicitly use one.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Due to a bug the seclabels are restored before any PID in the
container is killed. This should be done afterwards in
virLXCProcessCleanup.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Not that it would matter because LXC driver doesn't differentiate
the job types so far, but nevertheless the Destroy() should grab
LXC_JOB_DESTROY.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Missing semicolon at the end of macros can confuse some analyzers
(like cppcheck <filename>). VIR_ONCE_GLOBAL_INIT is almost
exclusively called without an ending semicolon, but let's
standardize on using one like the other macros.

Add a dummy struct definition at the end of the macro, so
the compiler will require callers to add a semicolon.
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
Signed-off-by: NCole Robinson <crobinso@redhat.com>

Missing semicolon at the end of macros can confuse some analyzers
(like cppcheck <filename>), and we have a mix of semicolon and
non-semicolon usage through the code. Let's standardize on using
a semicolon for VIR_ENUM_IMPL calls.

Move the verify() statement to the end of the macro and drop
the semicolon, so the compiler will require callers to add a
semicolon.

While we are touching these call sites, standardize on putting
the closing parenth on its own line, as discussed here:
https://www.redhat.com/archives/libvir-list/2019-January/msg00750.htmlReviewed-by: NJohn Ferlan <jferlan@redhat.com>
Signed-off-by: NCole Robinson <crobinso@redhat.com>

Missing semicolon at the end of macros can confuse some analyzers
(like cppcheck <filename>), and we have a mix of semicolon and
non-semicolon usage through the code. Let's standardize on using
a semicolon for VIR_ENUM_DECL calls.

Drop the semicolon from the final statement of the macro, so
the compiler will require callers to add a semicolon.
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
Signed-off-by: NCole Robinson <crobinso@redhat.com>

Now that we have replacement in the form of the image labeling function
we can drop the unnecessary functions by replacing all callers.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

The use of 'lxc://' was mistakenly broken in:

  commit 4c8574c8
  Author: Daniel P. Berrangé <berrange@redhat.com>
  Date:   Wed Mar 28 12:49:29 2018 +0100

    driver: ensure NULL URI isn't passed to drivers with whitelisted URIs

Allow it again for historical compatibility.
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

There is a limit on the number of lines in the /proc/<pid>/{g,u}id_map
files. In Linux 4.14 and earlier, this limit was (arbitrarily) set at
5 lines. Since Linux 4.15, which was released on 28 Jan 2018, the limit
is 340 lines.

This change is documented in user_namespaces(7).

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6397fac4915ab3002dc15aae751455da1a852f25Signed-off-by: NRadostin Stoyanov <rstoyanov1@gmail.com>

Require that all headers are guarded by a symbol named

  LIBVIRT_$FILENAME

where $FILENAME is the uppercased filename, with all characters
outside a-z changed into '_'.

Note we do not use a leading __ because that is technically a
namespace reserved for the toolchain.
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

This introduces a syntax-check script that validates header files use a
common layout:

  /*
   ...copyright header...
   */
  <one blank line>
  #ifndef SYMBOL
  # define SYMBOL
  ....content....
  #endif /* SYMBOL */

For any file ending priv.h, before the #ifndef, we will require a
guard to prevent bogus imports:

  #ifndef SYMBOL_ALLOW
  # error ....
  #endif /* SYMBOL_ALLOW */
  <one blank line>

The many mistakes this script identifies are then fixed.
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

In many files there are header comments that contain an Author:
statement, supposedly reflecting who originally wrote the code.
In a large collaborative project like libvirt, any non-trivial
file will have been modified by a large number of different
contributors. IOW, the Author: comments are quickly out of date,
omitting people who have made significant contribitions.

In some places Author: lines have been added despite the person
merely being responsible for creating the file by moving existing
code out of another file. IOW, the Author: lines give an incorrect
record of authorship.

With this all in mind, the comments are useless as a means to identify
who to talk to about code in a particular file. Contributors will always
be better off using 'git log' and 'git blame' if they need to  find the
author of a particular bit of code.

This commit thus deletes all Author: comments from the source and adds
a rule to prevent them reappearing.

The Copyright headers are similarly misleading and inaccurate, however,
we cannot delete these as they have legal meaning, despite being largely
inaccurate. In addition only the copyright holder is permitted to change
their respective copyright statement.
Reviewed-by: NErik Skultety <eskultet@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

This commit fixes a bug when you have multiple network settings defined.
Basically, if you set an IPv6 or IPv4 gateway, it carries on next
network settings. It is happening because the data is not being
initialized when a new network type is defined. So, the old data still
persists into the pointer. Another way to initialized the data was
introduced using memset() to avoid missing attributes from the struct.
Signed-off-by: NJulio Faracco <jcfaracco@gmail.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Commit 017dfa27 changed a few switch statements in the LXC code to
have all possible enum values, and in the process changed the switch
statement in virLXCControllerGetNICIndexes() to return an error status
for unsupported interface types, but it erroneously put type='direct'
on the list of unsupported types.

type='direct' (implemented with a macvlan interface) is supported on
LXC, but it's interface shouldn't be placed on the list of interfaces
given to CreateMachineWithNetwork() because the interface is put
inside the container, while CreateMachineWithNetwork() only wants to
know about the parent veths of veth pairs (the parent veth remains on
the host side, while the child veth is put into the container).

Resolves: https://bugzilla.redhat.com/1656463Signed-off-by: NLaine Stump <laine@laine.org>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

virLXCControllerGetNICIndexes() was deciding whether or not to add the
ifindex for an interface's ifname to the list of ifindexes sent to
CreateMachineWithNetwork based on the interface type stored in the
config. This would be incorrect in the case of <interface
type='network'> where the network was giving out macvlan interfaces
tied to a physical device (i.e. when the actual interface type was
"direct").

Instead of checking the setting of "net->type", we should be checking
the setting of virDomainNetGetActualType(net).

I don't think this caused any actual misbehavior, it was just
technically wrong.
Signed-off-by: NLaine Stump <laine@laine.org>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Signed-off-by: NYuri Chornoivan <yurchor@ukr.net>
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Signed-off-by: NJán Tomko <jtomko@redhat.com>

This reverts commit 3e26b476.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

This patch introduce the new settings for LXC 3.0 or higher. The older
versions keep the compatibility to deprecated settings for LXC, but
after release 3.0, the compatibility was removed. This commit adds the
support to the refactored settings.
Signed-off-by: NJulio Faracco <jcfaracco@gmail.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

The array "mount" inside lxc_container is not being checked before for
loop. Clang syntax scan is complaining about this segmentation fault.
Signed-off-by: NJulio Faracco <jcfaracco@gmail.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Remove the "!params" check from the condition since it's possible
someone could pass a non NULL value there, but a 0 for the nparams
and thus continue on.  The external API only checks if @nparams is
non-zero, then check for NULL @params.

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

A deadlock situation can occur when autostarting a LXC domain 'guest'
due to two threads attempting to take opposing locks while holding
opposing locks (AB BA problem). Thread A takes and holds the 'vm' lock
while attempting to take the 'client' lock, meanwhile, thread B takes
and holds the 'client' lock while attempting to take the 'vm' lock.

The potential for this can be seen as follows:

Thread A:
virLXCProcessAutostartDomain (takes vm lock)
 --> virLXCProcessStart
  --> virLXCProcessConnectMonitor
   --> virLXCMonitorNew
    --> virNetClientSetCloseCallback (wants client lock)

Thread B:
virNetClientIncomingEvent (takes client lock)
 --> virNetClientIOHandleInput
  --> virNetClientCallDispatch
   --> virNetClientCallDispatchMessage
    --> virNetClientProgramDispatch
     --> virLXCMonitorHandleEventInit
      --> virLXCProcessMonitorInitNotify (wants vm lock)

Since these threads are scheduled independently and are preemptible it
is possible for the deadlock scenario to occur where each thread locks
their first lock but both will fail to get their second lock and just
spin forever. You get something like:

virLXCProcessAutostartDomain (takes vm lock)
 --> virLXCProcessStart
  --> virLXCProcessConnectMonitor
   --> virLXCMonitorNew
<...>
virNetClientIncomingEvent (takes client lock)
 --> virNetClientIOHandleInput
  --> virNetClientCallDispatch
   --> virNetClientCallDispatchMessage
    --> virNetClientProgramDispatch
     --> virLXCMonitorHandleEventInit
      --> virLXCProcessMonitorInitNotify (wants vm lock but spins)
<...>
    --> virNetClientSetCloseCallback (wants client lock but spins)

Neither thread ever gets the lock it needs to be able to continue
while holding the lock that the other thread needs.

The actual window for preemption which can cause this deadlock is
rather small, between the calls to virNetClientProgramNew() and
execution of virNetClientSetCloseCallback(), both in
virLXCMonitorNew(). But it can be seen in real world use that this
small window is enough.

By moving the call to virNetClientSetCloseCallback() ahead of
virNetClientProgramNew() we can close any possible chance of the
deadlock taking place. There should be no other implications to the
move since the close callback (in the unlikely event was called) will
spin on the vm lock. The remaining work that takes place between the
old call location of virNetClientSetCloseCallback() and the new
location is unaffected by the move.
Signed-off-by: NMark Asselstine <mark.asselstine@windriver.com>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

In cgroup v2 we need to handle processes and threads differently,
following patch will introduce virCgroupAddThread.
Reviewed-by: NFabiano Fidêncio <fidencio@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>

Commit 40b5c99a modified the virConfGetValue callers to use
virConfGetValueString. However, using the virConfGetValueString
resulted in leaking the returned @value string in each case.
So, let's modify each instance to use the VIR_AUTOFREE(char *)
syntax. In some instances changing the variable name since
@value was used more than once.

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

Since lxcConvertSize already creates an error message, there
is no need to use an error: label in lxcSetMemTune to just
overwrite or essentially rewrite the same error. So remove
the label.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

All of the ones being removed are pulled in by internal.h. The only
exception is sanlock which expects the application to include <stdint.h>
before sanlock's headers, because sanlock prototypes use fixed width
int, but they don't include stdint.h themselves, so we have to leave
that one in place.
Signed-off-by: NErik Skultety <eskultet@redhat.com>
Acked-by: NMichal Privoznik <mprivozn@redhat.com>

It doesn't really make sense for us to have stdlib.h and string.h but
not stdio.h in the internal.h header.
Signed-off-by: NErik Skultety <eskultet@redhat.com>
Acked-by: NMichal Privoznik <mprivozn@redhat.com>

Now that we know what metadata lock manager user wishes to use we
can load it when initializing security driver. This is achieved
by adding new argument to virSecurityManagerNewDriver() and
subsequently to all functions that end up calling it.

The cfg.mk change is needed in order to allow lock_manager.h
inclusion in security driver without 'syntax-check' complaining.
This is safe thing to do as locking APIs will always exist (it's
only backend implementation that changes). However, instead of
allowing the include for all other drivers (like cpu, network,
and so on) allow it only for security driver. This will still
trigger the error if including from other drivers.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

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

This reverts commit 0f80c718.

Turns out, our code relies on virCgroupFree(&var) setting
var = NULL.

Conflicts:
  src/util/vircgroup.c: context because 94f1855f is not
  reverted.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NPavel Hrdina <phrdina@redhat.com>

Modify virCgroupFree function signature to take a value of type
virCgroupPtr instead of virCgroupPtr * as the parameter.

Change the argument type in all calls to virCgroupFree function
from virCgroupPtr * to virCgroupPtr. This is a step towards
having consistent function signatures for Free helpers so that
they can be used with VIR_AUTOPTR cleanup macro.
Signed-off-by: NSukrit Bhatnagar <skrtbhtngr@gmail.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>