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

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

Commit id 'dc692438' reverted the automagic addition of a SCSI
controller attempt during virDomainHostdevAssignAddress; however,
the logic to determine where to place the next_unit depended upon
the "new" controller being added.  Without the new controller the
the next time through the call for the next SCSI hostdev found
would result in the "next_unit" never changing from 0 (zero) and
as a result the addition of the device will fail due to being a
duplicate unit number of the first with the error message:

  virDomainDefCheckDuplicateDriveAddresses:$line : unsupported
      configuration: SCSI host address controller='0' bus='1'
      target='0' unit='0' in use by another SCSI host device

So instead of walking the controller list looking for SCSI
controllers, all we can do is "pretend" that they exist and
allow other code to create them later as necessary.

In virDomainDefMaybeAddHostdevSCSIcontroller when we add a new
controller because someone neglected to add one or we're adding
one because the existing one is full, we should copy over the
model number from the existing controller since whatever we
create should at least have the same characteristics as the one
we cannot use because it's full.

NB: This affects the existing hostdev-scsi-autogen-address test
which would add a default ('lsi') SCSI controller for the various
scsi_host's that would create a controller for the hostdev.

When qemuDomainFindOrCreateSCSIDiskController adds a controller,
let's use the same model as a currently found controller under the
assumption that the reason to add the controller in hotplug is
because virDomainHostdevAssignAddress determined that there were
too many devices on the existing controller, but only assigned a
new controller index and did not add a new controller and we
desire to use the same controller model as any existing controller
and not take a chance that qemuDomainSetSCSIControllerModel would
use a default that may be incompatible.

Introduced by commit <0eaa59dc>.  That comparison already returns
true or false.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>

Let's move the udevEnumerateDevices into a thread to "speed
up" the initialization process. If the enumeration fails we
can set the Quit flag to ensure that udevEventHandleCallback
will not run.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Replace virNetServerClientNeedAuth with
virNetServerClientIsAuthenticated because it makes it clearer what it
means.
Signed-off-by: NMarc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: NBoris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: NStefan Zimmermann <stzi@linux.vnet.ibm.com>

'Squash' virNetServerClientNeedAuthLocked into
virNetServerClientNeedAuth and remove virNetServerClientNeedAuthLocked
as it's not longer needed.
Signed-off-by: NMarc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: NBoris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: NStefan Zimmermann <stzi@linux.vnet.ibm.com>

Enable testing for 'auth_pending' in the virnetdaemon test case.
Signed-off-by: NMarc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: NBoris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

There is a race between virNetServerProcessClients (main thread) and
remoteDispatchAuthList/remoteDispatchAuthPolkit/remoteSASLFinish (worker
thread) that can lead to decrementing srv->nclients_unauth when it's
zero. Since virNetServerCheckLimits relies on the value
srv->nclients_unauth the underrun causes libvirtd to stop accepting
new connections forever.

Example race scenario (assuming libvirtd is using policykit and the
client is privileged):
  1. The client calls the RPC remoteDispatchAuthList =>
     remoteDispatchAuthList is executed on a worker thread (Thread
     T1). We're assuming now the execution stops for some time before
     the line 'virNetServerClientSetAuth(client, 0)'
  2. The client closes the connection irregularly. This causes the
     event loop to wake up and virNetServerProcessClient to be
     called (on the main thread T0). During the
     virNetServerProcessClients the srv lock is hold. The condition
     virNetServerClientNeedAuth(client) will be checked and as the
     authentication is not finished right now
     virNetServerTrackCompletedAuthLocked(srv) will be called =>
     --srv->nclients_unauth => 0
  3. The Thread T1 continues, marks the client as authenticated, and
     calls virNetServerTrackCompletedAuthLocked(srv) =>
     --srv->nclients_unauth => --0 => wrap around as nclient_unauth is
     unsigned
  4. virNetServerCheckLimits(srv) will disable the services forever

To fix it, add an auth_pending field to the client struct so that it
is now possible to determine if the authentication process has already
been handled for this client.

Setting the authentication method to none for the client in
virNetServerProcessClients is not a proper way to indicate that the
counter has been decremented, as this would imply that the client is
authenticated.

Additionally, adjust the existing test cases for this new field.
Signed-off-by: NMarc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: NBoris Fiuczynski <fiuczy@linux.vnet.ibm.com>

Combine virNetServerClientSetAuth(client,
VIR_NET_SERVER_SERVICE_AUTH_NONE) and virNetServerTrackCompletedAuth
into one new function named virNetServerSetClientAuthenticated.

After using this new function the function
virNetServerTrackCompletedAuth was superfluous and is therefore
removed. In addition, it is not very common that a
'{{function}}' (virNetServerTrackCompletedAuth) does more than just
the locking compared to
'{{function}}Locked' (virNetServerTrackCompletedAuthLocked).

virNetServerTrackPendingAuth was already superfluous and therefore
it's also removed.
Signed-off-by: NMarc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: NBoris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: NStefan Zimmermann <stzi@linux.vnet.ibm.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

The lock for @client must not only be held for the duration of
checking whether the client wants to close, but also for as long as
we're closing the client. The same applies to the tracking of
authentications.
Signed-off-by: NMarc Hartmayer <mhartmay@linux.vnet.ibm.com>

Add virNetServerClientAuthMethodImpliesAuthenticated() for deciding
whether a authentication method implies that a client is automatically
authenticated or not. Use this new function in
virNetServerClientNeedAuthLocked().
Signed-off-by: NMarc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: NBoris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: NStefan Zimmermann <stzi@linux.vnet.ibm.com>

This makes the code more efficient.
Signed-off-by: NMarc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: NBjoern Walk <bwalk@linux.vnet.ibm.com>
Reviewed-by: NBoris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: NStefan Zimmermann <stzi@linux.vnet.ibm.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Be more precise in which cases the authentication is needed and
introduce *Locked.
Signed-off-by: NMarc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: NBoris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: NBjoern Walk <bwalk@linux.vnet.ibm.com>
Reviewed-by: NStefan Zimmermann <stzi@linux.vnet.ibm.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Add typedef for the anonymous enum used for the authentication methods
and remove the default case. This allows the usage of the type in a
switch statement and taking advantage of the compilers feature to
detect uncovered cases.
Signed-off-by: NMarc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: NBoris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: NStefan Zimmermann <stzi@linux.vnet.ibm.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Signed-off-by: NMarc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: NBoris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: NBjoern Walk <bwalk@linux.vnet.ibm.com>
Reviewed-by: NStefan Zimmermann <stzi@linux.vnet.ibm.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Direct leak of 104 byte(s) in 1 object(s) allocated from:
    #0 0x7f904bfbe12b  (/lib64/liblsan.so.0+0xe12b)
    #1 0x7f904ba0ad67 in virAlloc ../../src/util/viralloc.c:144
    #2 0x7f904bbc11a4 in virNetMessageNew ../../src/rpc/virnetmessage.c:42
    #3 0x7f904bbb8e77 in virNetServerClientNewInternal ../../src/rpc/virnetserverclient.c:392
    #4 0x7f904bbb9921 in virNetServerClientNew ../../src/rpc/virnetserverclient.c:440
    #5 0x402ce5 in testIdentity ../../tests/virnetserverclienttest.c:55
    #6 0x403bed in virTestRun ../../tests/testutils.c:180
    #7 0x402c1e in mymain ../../tests/virnetserverclienttest.c:146
    #8 0x404c80 in virTestMain ../../tests/testutils.c:1119
    #9 0x4030d5 in main ../../tests/virnetserverclienttest.c:152
    #10 0x7f9047f7f889 in __libc_start_main (/lib64/libc.so.6+0x20889)

Indirect leak of 4 byte(s) in 1 object(s) allocated from:
    #0 0x7f904bfbe12b  (/lib64/liblsan.so.0+0xe12b)
    #1 0x7f904ba0adc7 in virAllocN ../../src/util/viralloc.c:191
    #2 0x7f904bbb8ec7 in virNetServerClientNewInternal ../../src/rpc/virnetserverclient.c:395
    #3 0x7f904bbb9921 in virNetServerClientNew ../../src/rpc/virnetserverclient.c:440
    #4 0x402ce5 in testIdentity ../../tests/virnetserverclienttest.c:55
    #5 0x403bed in virTestRun ../../tests/testutils.c:180
    #6 0x402c1e in mymain ../../tests/virnetserverclienttest.c:146
    #7 0x404c80 in virTestMain ../../tests/testutils.c:1119
    #8 0x4030d5 in main ../../tests/virnetserverclienttest.c:152
    #9 0x7f9047f7f889 in __libc_start_main (/lib64/libc.so.6+0x20889)

SUMMARY: LeakSanitizer: 108 byte(s) leaked in 2 allocation(s).
Signed-off-by: NMarc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Signed-off-by: NMarc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: NBjoern Walk <bwalk@linux.vnet.ibm.com>
Reviewed-by: NBoris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: NStefan Zimmermann <stzi@linux.vnet.ibm.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

All calls to virDomainAuditCgroupPath() were passing 'rc == 0' as
argument, when it was supposed to pass the 'rc' value directly.

As a consequence, the audit events that were supposed to be
logged (actual cgroup changes) were never being logged, and bogus
audit events were logged when using regular files as disk image.

Fix all calls to use the return value of
virCgroup{Allow,Deny}Device*() directly as the 'rc' argument.
Signed-off-by: NEduardo Habkost <ehabkost@redhat.com>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

We don't try to right align the backslashes anymore.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

This pulls in updates for the 2018 copyright year, among other things.
Signed-off-by: NEric Blake <eblake@redhat.com>

Fix rule introduced by commit 0f33025a:
  * to handle /var/run not being a symlink to /run
  * to be properly parsed: missing comma at the end.

After commit a693fdba 'vol-dumpxml' missed the ability to show backingStore
information. This commit adds a volume type for files that fixes this
problem.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1529663Signed-off-by: NJulio Faracco <jcfaracco@gmail.com>

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

If a domain has no numa nodes, that means we don't put any
memory-backend-file onto the qemu command line. That in turn
means we can't set access='shared'. Therefore, we should produce
an error instead of ignoring the setting silently.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

The PROBE macro used in qemuMonitorIOProcess and the VIR_DEBUG message
in qemuMonitorJSONIOProcess create a lot of logging churn when debug
logging is enabled during monitor communication.

The messages logged from the PROBE macro are rather useless since they
are reporting the partial state of receiving the reply from qemu. The
actual full reply is still logged in qemuMonitorJSONIOProcessLine once
the full message is received.

PROBE macro adds a logging entry, when used in places seeing a lot of
traffic this can cause a significant slowdown.

There are a few more description-related issues that commit @9026d115
forgot to address.
Signed-off-by: NChen Hanxiao <chenhanxiao@gmail.com>
Signed-off-by: NErik Skultety <eskultet@redhat.com>

Set a transient hostname on containers. The hostname is computed from
the container name, only keeping the valid characters [a-zA-Z0-9-] in it.
This filtering is based on RFC 1123 and allows a digit to start the
hostname.

Add a function to filter a string based on a list of valid characters.

There's no argument named @result, use @matches instead.
Signed-off-by: NChen Hanxiao <chenhanxiao@gmail.com>
Signed-off-by: NErik Skultety <eskultet@redhat.com>

On live migration with --p2p like:
 $ virsh migrate --live --p2p kvmguest-bionic-normal \
       qemu+ssh://10.6.221.80/system

We hit an apparmor deny like:
  apparmor="DENIED" operation="file_inherit"
  profile="/usr/sbin/libvirtd" pid=23477 comm="ssh" family="unix"
  sock_type="stream" protocol=0 requested_mask="send receive"
  denied_mask="send" addr=none peer_addr=none peer="unconfined"

The rule is not perfect, but can't be restricted further at the moment
(new upstream kernel features needed). For now the lack of a profile on the
peer as well as comm not being a conditional on rules do not allow to filter
further.
Signed-off-by: NChristian Ehrhardt <christian.ehrhardt@canonical.com>

These are already exported at header file level because of
VIR_ENUM_DECL being in numa_conf.h. However, they are not being
exported at object level because of missing libvirt_private.syms
record.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Signed-off-by: NAndrea Bolognani <abologna@redhat.com>

QEMU 2.7 and newer don't allow guests to start unless the initial
vCPUs count is a multiple of the vCPU hotplug granularity, so
validate it and report an error if needed.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1283700Signed-off-by: NAndrea Bolognani <abologna@redhat.com>

While at the moment we're only performing a single check that is
connected to vCPU hotplugging, we're going to introduce a second
one soon. Move the topology check underneath the capability check
to make that easier; since, after this change, the 'topologycpus'
variable doesn't need to have function scope, we move its
declaration to the inner scope as well.

The comments around the check are modified in order to explain
the different QEMU versions involved.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>

Introduced by commit <4ae9dbea>.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1528572Signed-off-by: NPavel Hrdina <phrdina@redhat.com>

Similar to qemuDomainAddChardevTLSObjects let's move the chardev
source must be TCP and it has the @haveTLS flag set checks before
trying to delete the TLS objects.

For the Chr device this represents no change; however, for RNG device
this is an additionaly check that was missed in commit id '68808516'.
Before adding the objects, TCP and haveTLS are checked.

Let's make a comment deletion helper similar to the Add helper
that can be called after the ExitMonitor.

The modify qemuDomainRemoveChrDevice and qemuDomainRemoveRNGDevice
to call the helper instead of inlining the copy and pasted code.

So far clients were closed when disposing the daemon, after the state
driver cleanup. This was leading to libvirtd crashing at shutdown due
to missing driver.

Moving the client close in virNetServerClose() fixes the problem.
Reviewed-by: NErik Skultety <eskultet@redhat.com>