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

This is only a theoretical leak, but in virChrdevAlloc() we
initialize a mutex and if creating a hash table fails,
then virChrdevFree() is called which because of incorrect check
doesn't deinit the mutex.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

When opening a console to a domain, we put a tuple of {path,
virStreamPtr} into a hash table that's private to the domain.
This is to ensure only one client at most has the console stream
open. Later, when the console is closed, the tuple is removed
from the hash table and freed. Except, @path won't be freed.

==234102== 60 bytes in 5 blocks are definitely lost in loss record 436 of 651
==234102==    at 0x4836753: malloc (vg_replace_malloc.c:307)
==234102==    by 0x5549110: g_malloc (in /usr/lib64/libglib-2.0.so.0.6000.6)
==234102==    by 0x5562D1E: g_strdup (in /usr/lib64/libglib-2.0.so.0.6000.6)
==234102==    by 0x4A5A917: virChrdevOpen (virchrdev.c:412)
==234102==    by 0x17B64645: qemuDomainOpenConsole (qemu_driver.c:17309)
==234102==    by 0x4BC8031: virDomainOpenConsole (libvirt-domain.c:9662)
==234102==    by 0x13F854: remoteDispatchDomainOpenConsole (remote_daemon_dispatch_stubs.h:9211)
==234102==    by 0x13F72F: remoteDispatchDomainOpenConsoleHelper (remote_daemon_dispatch_stubs.h:9178)
==234102==    by 0x4AB0685: virNetServerProgramDispatchCall (virnetserverprogram.c:430)
==234102==    by 0x4AB01F0: virNetServerProgramDispatch (virnetserverprogram.c:302)
==234102==    by 0x4AB700B: virNetServerProcessMsg (virnetserver.c:136)
==234102==    by 0x4AB70CB: virNetServerHandleJob (virnetserver.c:153)
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

This previous commit introduced a simpler free callback for
hash data with only 1 arg, the value to free:

  commit 49288fac
  Author: Peter Krempa <pkrempa@redhat.com>
  Date:   Wed Oct 9 15:26:37 2019 +0200

    util: hash: Add possibility to use simpler data free function in virHash

It missed two functions in the hash table code which need
to call the alternate data free function, virHashRemoveEntry
and virHashRemoveSet.

After the previous patch though, there is no code that
makes functional use of the 2nd key arg in the data
free function. There is merely one log message that can
be dropped.

We can thus purge the current virHashDataFree callback
entirely, and rename virHashDataFreeSimple to replace
it.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

The virChrdevHashEntryFree method uses the hash 'key'
as the name of the logfile it has to remove. By storing
a struct as the value which contains the stream and
the dev path, we can avoid relying on the hash key
when free'ing entries.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NDaniel Henrique Barboza <danielhb413@gmail.com>

Replace all occurrences of
  if (VIR_STRDUP(a, b) < 0)
     /* effectively dead code */
with:
  a = g_strdup(b);
Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

Use G_GNUC_UNUSED from GLib instead of ATTRIBUTE_UNUSED.
Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@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>

There is no reason for it not to be in the utils, all global symbols
under that file already have prefix vir* and there is no reason for it
to be part of DRIVER_SOURCES because that is just a leftover from
older days (pre-driver modules era, I believe).
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

Our existing virHashForEach method iterates through all items disregarding the
fact, that some of the iterators might have actually failed. Errors are usually
dispatched through an error element in opaque data which then causes the
original caller of virHashForEach to return -1. In that case, virHashForEach
could return as soon as one of the iterators fail. This patch changes the
iterator return type and adjusts all of its instances accordingly, so the
actual refactor of virHashForEach method can be dealt with later.
Signed-off-by: NErik Skultety <eskultet@redhat.com>

Console/channel devices have their pty devices assigned when the emulator is
actually started. If time is spent in guest preparation, someone attempts
to open the console/channel, the libvirt crashes in virChrdevLockFilePath().
The patch attempts to fix the crash by adding a check before attempting to
open.
Signed-off-by: NShivaprasad G Bhat <sbhat@linux.vnet.ibm.com>

Since virStreamFree will call virObjectUnref anyway, let's just use that
directly so as to avoid the possibility that we inadvertently clear out
a pending error message when using the public API.

Add virFDStreamOpenPTY() function which is a wrapper around
virFDStreamOpenFileInternal() with putting the device it opens into a
raw mode.

Make virChrdevOpen() use virFDStreamOpenPTY() for
VIR_DOMAIN_CHR_TYPE_PTY devices.

This fixes mangled console output when libvirt runs on FreeBSD as it
requires device it opens to be placed into a raw mode explicitly.

Any source file which calls the logging APIs now needs
to have a VIR_LOG_INIT("source.name") declaration at
the start of the file. This provides a static variable
of the virLogSource type.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

Don't free the stream on error if we've successfully added it
to the hash table, since it will be freed by virChrdevHashEntryFree
callback.

Preserve the error message before calling virStreamFree, since it
resets the error.

Introduced by 47161382, crashing since 69218922.

Reported by Sergey Fionov on libvir-list.

The source code base needs to be adapted as well. Some files
include virutil.h just for the string related functions (here,
the include is substituted to match the new file), some include
virutil.h without any need (here, the include is removed), and
some require both.

This also changes the function signature to take a
virDomainChrSourceDefPtr instead of just a path, since it needs to
differentiate behavior based on source->type.

The functionality provided in virchrdev.c (previously virconsole.c) is
applicable to other types of character devices besides consoles, such
as channels.  This patch is just code motion, renaming things such as
"console" or "pty", instead using more general terms such as
"character device" or "device path".

This is just code motion, in preparation to rename identifiers to be
less console-specific.

https://www.gnu.org/licenses/gpl-howto.html recommends that
the 'If not, see <url>.' phrase be a separate sentence.

* tests/securityselinuxhelper.c: Remove doubled line.
* tests/securityselinuxtest.c: Likewise.
* globally: s/;  If/.  If/

Commit ba226d33 tried to fix crash of
the daemon when a domain with an open console was destroyed. The fix was
wrong as it tried to remove the callback also when the stream was
aborted, where at that point the fd stream driver was already freed and
removed.

This patch clears the callbacks with a helper right before the hash is
freed, so that it doesn't interfere with other codepaths where the
stream object is freed.

When a domain has a active console connection and is destroyed the
callback is called on private data that no longer exist causing a
segfault.

Per the FSF address could be changed from time to time, and GNU
recommends the following now: (http://www.gnu.org/licenses/gpl-howto.html)

  You should have received a copy of the GNU General Public License
  along with Foobar.  If not, see <http://www.gnu.org/licenses/>.

This patch removes the explicit FSF address, and uses above instead
(of course, with inserting 'Lesser' before 'General').

Except a bunch of files for security driver, all others are changed
automatically, the copyright for securify files are not complete,
that's why to do it manually:

  src/security/security_selinux.h
  src/security/security_driver.h
  src/security/security_selinux.c
  src/security/security_apparmor.h
  src/security/security_apparmor.c
  src/security/security_driver.c

This removes all the per-file error reporting macros
from the code in src/conf/
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

This patch adds a set of functions used in creating console streams for
domains using PTYs and ensures mutually exclusive access to the PTYs.

If mutually exclusive access is not used, two clients may open the same
console, which results in corruption on both clients as both of them
race to read data from the PTY.

Two approaches are used to ensure this:
1) Internal data structure holding open PTYs.
        This is used internally and enables the user to forcibly
        terminate another console connection eg. when somebody leaves
        the console open on another host.

2) UUCP style lock files:
        This uses UUCP lock files according to the  FHS
        ( http://www.pathname.com/fhs/pub/fhs-2.3.html#VARLOCKLOCKFILES )
        to check if other programs (like minicom) are not using the pty
        device of the console.

        This feature is disabled by default and may be enabled using
        configure parameter
        --with-console-lock-files=/path/to/lock/file/directory
        or --with-console-lock-files=auto (which tries to infer the
        location from OS used (currently only linux).

        On usual linux systems, normal users may not write to the
        /var/lock directory containing the locks. This poses problems
        while in session mode. If the current user has no access to the
        lockfile directory, check for presence of the file is still
        done, but no lock file is created. This does NOT result in an
        error.