When building using -Og, gcc sees that some variables can be used
uninitialized It can be debatable whether it is possible with our
codeflow, but functions should be self-contained and initializations are
always good. The return instead of goto is due to actualType being used
in the cleanup.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
(cherry picked from commit 3470cd860d517760b13e26d97b6a842ff72687a1)
If this reminds you of a commit message from around a year ago, it's
41c2aa729f0af084ede95ee9a06219a2dd5fb5df and yes, we're dealing with
"the same thing" again. Or f309db1f4d51009bad0d32e12efc75530b66836b and
it's similar.
There is a logic in place that if there is no real need for
memory-backend-file, qemuBuildMemoryBackendStr() returns 0. However
that wasn't the case with hugepage backing. The reason for that was
that we abused the 'pagesize' variable for storing that information, but
we should rather have a separate one that specifies whether we really
need the new object for hugepage backing. And that variable should be
set only if this particular NUMA cell needs special treatment WRT
hugepages.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1372153
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
(cherry picked from commit 4372a7845acbc6974f6027ef68e7dd3eeb47f425)
CVE-2016-5008
Setting an empty graphics password is documented as a way to disable
VNC/SPICE access, but QEMU does not always behaves like that. VNC would
happily accept the empty password. Let's enforce the behavior by setting
password expiration to "now".
https://bugzilla.redhat.com/show_bug.cgi?id=1180092
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
(cherry picked from commit bb848feec0f3f10e92dd8e5231ae7aa89b5598f3)
Since commit 714080791778e3dfbd484ccb3953bffd820b8ba9 we are generating
socket path later than before -- when starting a domain. That makes one
particular inconsistent state of a chardev, which was not possible
before, currently valid. However, SELinux security driver forgot to
guard the main restoring function by a check for NULL-paths. So make it
no-op for NULL paths, as in the DAC driver.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1300532
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
(cherry picked from commit 1794a0103ae4fa91d9c11617e7981471173e27ce)
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
In my previous commit a70f3b1c779120129 I've tried to fix case
when building from VPATH and a file wasn't being installed.
However, my fix broke non-VPATH build.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
(cherry picked from commit 0d0e41a71910e0415a924648f44a0b00389a576a)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The libvirt-common.h is build time generated file from .in.
Obviously, it's generated into builddir and not srcdir. Problem
is, the list of header files to install, virinc_HEADERS contains
only $(srcdir)/*.h and this misses libvirt-common.h. This problem
is pretty obvious when doing a VPATH build.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
(cherry picked from commit a70f3b1c77912012905c6c5be3bf37b05592e80f)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Since we pass dummy variables @fdout and @fdoutlen into
virNetClientProgramCall() we make it alloc @fdout array (even
though it's an array of 0 elements since vitlogd can hardly pass
us some FDs at this stage). Nevertheless, it's an allocation not
followed by free():
==29385== 0 bytes in 60 blocks are definitely lost in loss record 2 of 1,009
==29385== at 0x4C2C070: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==29385== by 0x54B99EF: virAllocN (viralloc.c:191)
==29385== by 0x56821B1: virNetClientProgramCall (virnetclientprogram.c:359)
==29385== by 0x563B304: virLogManagerDomainReadLogFile (log_manager.c:272)
==29385== by 0x217CD613: qemuDomainLogContextRead (qemu_domain.c:2485)
==29385== by 0x217EDC76: qemuProcessReadLog (qemu_process.c:1660)
==29385== by 0x217EDE1D: qemuProcessReportLogError (qemu_process.c:1696)
==29385== by 0x217EE8C1: qemuProcessWaitForMonitor (qemu_process.c:1957)
==29385== by 0x217F6636: qemuProcessLaunch (qemu_process.c:4955)
==29385== by 0x217F71A4: qemuProcessStart (qemu_process.c:5152)
==29385== by 0x21846582: qemuDomainObjStart (qemu_driver.c:7396)
==29385== by 0x218467DE: qemuDomainCreateWithFlags (qemu_driver.c:7450)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
(cherry picked from commit c03fbecc7cd463900d0291034e873f00062a75db)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
So I can observe this crasher that with freshly started daemon
(and virtlogd enabled) I am trying to startup a domain that
immediately dies (because it's said to use huge pages but I
haven't allocated a single one in the pool). Hardly reproducible
with -O0 or under valgrind. But I just got lucky:
==20469== Invalid write of size 8
==20469== at 0x4C2E99B: memcpy@GLIBC_2.2.5 (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==20469== by 0x217EDD07: qemuProcessReadLog (qemu_process.c:1670)
==20469== by 0x217EDE1D: qemuProcessReportLogError (qemu_process.c:1696)
==20469== by 0x217EE8C1: qemuProcessWaitForMonitor (qemu_process.c:1957)
==20469== by 0x217F6636: qemuProcessLaunch (qemu_process.c:4955)
==20469== by 0x217F71A4: qemuProcessStart (qemu_process.c:5152)
==20469== by 0x21846582: qemuDomainObjStart (qemu_driver.c:7396)
==20469== by 0x218467DE: qemuDomainCreateWithFlags (qemu_driver.c:7450)
==20469== by 0x21846845: qemuDomainCreate (qemu_driver.c:7468)
==20469== by 0x5611CD0: virDomainCreate (libvirt-domain.c:6753)
==20469== by 0x125D9A: remoteDispatchDomainCreate (remote_dispatch.h:3613)
==20469== by 0x125CB7: remoteDispatchDomainCreateHelper (remote_dispatch.h:3589)
==20469== Address 0x27a52ad0 is 0 bytes after a block of size 5,584 alloc'd
==20469== at 0x4C29F80: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==20469== by 0x9B8D1DB: xdr_string (in /lib64/libc-2.21.so)
==20469== by 0x563B39C: xdr_virLogManagerProtocolNonNullString (log_protocol.c:24)
==20469== by 0x563B6B7: xdr_virLogManagerProtocolDomainReadLogFileRet (log_protocol.c:123)
==20469== by 0x164B34: virNetMessageDecodePayload (virnetmessage.c:407)
==20469== by 0x5682360: virNetClientProgramCall (virnetclientprogram.c:379)
==20469== by 0x563B30E: virLogManagerDomainReadLogFile (log_manager.c:272)
==20469== by 0x217CD613: qemuDomainLogContextRead (qemu_domain.c:2485)
==20469== by 0x217EDC76: qemuProcessReadLog (qemu_process.c:1660)
==20469== by 0x217EDE1D: qemuProcessReportLogError (qemu_process.c:1696)
==20469== by 0x217EE8C1: qemuProcessWaitForMonitor (qemu_process.c:1957)
==20469== by 0x217F6636: qemuProcessLaunch (qemu_process.c:4955)
This points to memmove() in qemuProcessReadLog(). Imagine we just
read the following string from qemu:
"abc\n2016-01-18T09:40:44.022744Z qemu-system-x86_64: Error\n"
After the first pass of the while() loop in the
qemuProcessReadLog() (in which we have taken the false branch in
the if) @buf still points to the beginning of the string,
@filter_next points to the beginning of the second line. So we
start second iteration because there is yet another newline
character at the end. In this iteration @eol points to it
actually. Now, the control gets inside true branch of if(). Just
to remind you:
got = 58
filter_next = buf + 5,
eol = buf + 58.
Therefore skip = 54 which is correct. The message we want to skip
is 54 bytes long. However:
memmove(filter_next, eol + 1, (got - skip) +1);
which is
memmove(filter_next, eol + 1, 5)
is obviously wrong as there is only one byte we can access, not 5!
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
(cherry picked from commit 105b51f42ecd26914186239f36e73ee1e5e990c1)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
A device tree binary file specified by /domain/os/dtb element is a
read-only resource similar to kernel and initrd files. We shouldn't
restore its label when destroying a domain to avoid breaking other
domains configure with the same device tree.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Kernel/initrd files are essentially read-only shareable images and thus
should be handled in the same way. We already use the appropriate label
for kernel/initrd files when starting a domain, but when a domain gets
destroyed we would remove the labels which would make other running
domains using the same files very unhappy.
https://bugzilla.redhat.com/show_bug.cgi?id=921135
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
We have this function qemuAgentNotifyEvent() which is supposed to
be called from thread pool responsible for processing qemu
monitor events. The function then should wake up other thread
that is waiting for a guest to shutdown or reboot. However, if we
have received a different error a warning is printed out. This
warning lacks info on which event is expected.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Commit id '90b721e43' moved where the virCgroupAddTask was made until
after the check for the vcpupin checks. However, in doing so it missed
an option where if the cpumap didn't exist, then the code would continue
back to the top of the current vcpu loop. The results was that the
virCgroupAddTask wouldn't be called.
Signed-off-by: John Ferlan <jferlan@redhat.com>
This reverts commit a41c00b472efaa192d2deae51ab732e65903238f.
After much testing and upstream discussion this has been deemed to be
the incorrect operation since it means we no longer have any guarantee
about which resource controllers the QEMU processes in general are in.
There is no need to deny writes on a readonly mount: write still
won't be accepted, even if the user remounts the folder as RW in
the guest as qemu sets the 9p mount as ro.
This deny rule was leading to problems for example with readonly /:
The qemu process had to write to a bunch of files in / like logs,
sockets, etc. This deny rule was also preventing auditing of these
denials, making it harder to debug.
Commit id '7bf3198df' neglected to initialize deflate leading to a
possibility if model allocation/checks fail, then the VIR_FREE(deflate)
would be erroneous. Noted by Jan Tomko.
In 50078cfbcbf3c8 I've tried to fix distcheck but accidentally
broke rpm build. The problem is that rpm build not only sets
DESTDIR but also passes plugindir path. This results in double
DESTDIR being in the plugin path, Drop one.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Even though the Makefile has WITH_WIRESHARK guards, the _SOURCES
variables are still processed when adding bits to the dist archive.
plugin.c is a generated file that is only built when wireshark is
enabled and it shouldn't be distributed, so use 'nodist'
So, you try to start a domain, but before we even get to the part
where chardev part of qemu command line is generated (and
possibly missing path to unix sockets is made up) an error occurs
which results in calling qemuProcessStop. This will then try to
clean up the mess and possibly ends up calling unlink(NULL).
==8085== Thread 3:
==8085== Syscall param unlink(pathname) points to unaddressable byte(s)
==8085== at 0xA85EA57: unlink (in /lib64/libc-2.21.so)
==8085== by 0x213D3C24: qemuProcessCleanupChardevDevice (qemu_process.c:2866)
==8085== by 0x558D6B1: virDomainChrDefForeach (domain_conf.c:22924)
==8085== by 0x213DA9AE: qemuProcessStop (qemu_process.c:5326)
==8085== by 0x213DA2F2: qemuProcessStart (qemu_process.c:5190)
==8085== by 0x2142957F: qemuDomainObjStart (qemu_driver.c:7396)
==8085== by 0x214297DB: qemuDomainCreateWithFlags (qemu_driver.c:7450)
==8085== by 0x21429842: qemuDomainCreate (qemu_driver.c:7468)
==8085== by 0x5611B95: virDomainCreate (libvirt-domain.c:6753)
==8085== by 0x125D9A: remoteDispatchDomainCreate (remote_dispatch.h:3613)
==8085== by 0x125CB7: remoteDispatchDomainCreateHelper (remote_dispatch.h:3589)
==8085== by 0x568BF41: virNetServerProgramDispatchCall (virnetserverprogram.c:437)
==8085== Address 0x0 is not stack'd, malloc'd or (recently) free'd
==8085==
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Commmit fd2e3c4c used the domctl version 8 structure for version 9
in the xen_getdomaininfolist union, resulting in insufficient buffer
size (and subsequent memory corruption) for the GETDOMAININFOLIST
ioctl.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
So after da176bf6b756 and friend we have switched to $(wildcard
some/path/*.xml) instead of enumerating the files explicitly.
This is nice, however it makes distcheck build from VPATH fail.
The reason is that it's is not obvious to what does the wildcard
refer to: srcdir or builddir?
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Autodeflate can be enabled/disabled for memballon device
of model 'virtio'.
xml:
<devices>
<memballoon model='virtio' autodeflate='on'/>
</devices>
qemu:
qemu -device virtio-balloon-pci,...,deflate-on-oom=on
Autodeflate cannot be enabled/disabled for running domain.
Excessive memory balloon inflation can cause invocation of OOM-killer,
when Linux is under severe memory pressure. QEMU memballoon device
has a feature to release some memory at the last moment before some
process will be get killed by OOM-killer.
Introduce a new optional balloon device attribute 'autodeflate' to
enable or disable this feature.
On every socket connect(2) attempt we were re-launching session
libvirtd, up to 100 times in 5 seconds.
This understandably caused some weird load races and intermittent
qemu:///session startup failures
https://bugzilla.redhat.com/show_bug.cgi?id=1271183
When we autolaunch libvirtd for session URIs, we spin in a retry
loop waiting for the daemon to start and the connect(2) to succeed.
However if we exceed the retry count, we don't explicitly raise an
error, which can yield a slew of different error messages elsewhere
in the code.
Explicitly raise the last connect(2) failure if we run out of retries.
- Add some debugging
- Make the loop dependent only on retries
- Make it explicit that connect(2) success exits the loop
- Invert the error checking logic
Commit 2b6f6ad introduced the virxdrdefs.h header with
common definitions to be included in the protocol files,
but logging/log_protocol.x was missed, so add it there as well.
Hopefully this fixes build on OS X.
When we are receiving data in smaller chunks it might happen that
virNetServerClientDispatchRead() will be called multiple times. And as
that happens, if it is a message that also transfer headers, we decode
the number of them every single time and, unfortunately, also allocate
the memory for them. That causes a leak, in the best scenario.
Best viewed with '-w'.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Rather than continually cut-n-paste the strings into each command,
create a common macro to be used generically. Note that not all
'{.name = "interface",' entries are replaced, just those that have the
common .help string of "interface name or MAC address".
Signed-off-by: John Ferlan <jferlan@redhat.com>
Rather than continually cut-n-paste the strings into each command,
create a common macro to be used generically. Note that not all
'{.name = "network",' entries are replaced, just those that have the
common .help string of "network name or uuid".
Signed-off-by: John Ferlan <jferlan@redhat.com>
Rather than continually cut-n-paste the strings into each command,
create common macros to be used generically. Replace the more commonly
used "vol" option with a macro. This also adjusts 2 commands that
didn't have the correct helpstr - 'vol-create-from' and 'vol-clone'.
Both are described in the man page as taking vol, path, or key and
the code uses the virshCommandOptVol instead of virshCommandOptVolBy.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Rather than continually cut-n-paste the strings into each command,
create a common macro to be used generically. The macro will take a
single argument _helpstr for the less common help string for each
command option. Note that only file options using "OT_DATA" and
"OFLAG_REQ" will be replace - others are left as is.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Rather than continually cut-n-paste the strings into each command,
create a common macro to be used generically. The macro will take a
single argument _helpstr which for many options in virsh-domain.c
is simply "affect current domain". So, create a second macro within that
file in order to define the more common use as a revector to the
common macro with the common _helpstr.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Rather than continually cut-n-paste the strings into each command,
create a common macro to be used generically. The macro will take a
single argument _helpstr which for many options in virsh-domain.c
is simply "affect running domain". So, create a second macro within that
file in order to define the more common use as a revector to the
common macro with the common _helpstr.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Rather than continually cut-n-paste the strings into each command,
create a common macro to be used generically. The macro will take a
single argument _helpstr which for many options in virsh-domain.c
is simply "affect next boot". So, create a second macro within that
file in order to define the more common use as a revector to the
common macro with the common _helpstr.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Rather than continually cut-n-paste the strings into each command,
create a common macro to be used generically. Note that not all
'{.name = "persistent",' entries are replaced, just those that have the
common .help string of "make live change persistent".
Non replaced instances are unique to the command.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Rather than continually cut-n-paste the strings into each command,
create a common macro to be used generically. The macro will take a
single argument _helpstr which will be used to pass the translatable
helpstr since not all domain options can take the same string.
The majority of the options take 'N_("domain name, id or uuid")', so
create a separate macro with a _FULL suffix while those that do not
take the same string will use the VIRSH_COMMON_OPT_DOMAIN macro.
Signed-off-by: John Ferlan <jferlan@redhat.com>
The macro is slightly adjusted to add an argument "_helpstr". This
will be passed as a translation macro string since other uses of the
option may not have the same exact help string (such as is the case
when the uuid is not suppliable for create commands).
In virsh-pool.c - we'll create a singular VIRSH_COMMON_OPT_POOL_FULL
in order to pass along the 'N_("pool name or uuid")'
In virsh-volume.c there will be a VIRSH_COMMON_OPT_POOL_FULL and a
VIRSH_COMMON_OPT_POOL_NAME, which passes 'N_("pool name")' for
the commands that can only pass a name. There will also be a
VIRSH_COMMON_OPT_POOL_OPTIONAL which is used for the command
options which use OT_STRING and don't require the --pool argument.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Commit id's 'cf793b00', 'e178688f', 'f9a6110f', '5372d49', and 'e193735'
added new VSH_POOL_ macros; however, it was pointed out after push that
commit id '834c5720' preferred use of VIRSH_ for the prefix over VSH_.
So this patch just changes the VSH_ to VIRSH_ and it changes the naming
format from VIRSH_<opt>_OPT_COMMON to VIRSH_COMMON_OPT_<opt>.
Signed-off-by: John Ferlan <jferlan@redhat.com>
if instanceId is NULL
When virNetDevVPortProfileGetStatus() was called with instanceId =
NULL (which is the case for all DISASSOCIATE requests in 802.1Qbh) it
would log the following error:
Could not find netlink response with expected parameters
even though the disassociate had been successfully completely. Then,
due to the fortunate coincidence of status having been initialized to
0 and then not changed when the "failure" was encountered, it would
still return a status of 0 (PORT_VDP_RESPONSE_SUCCESS), so the caller
would assume a successful operation.
This would result in a spurious log message though, and would fill in
LastErrorMessage, so that the API would return that error if it
happened during cleanup from some other error. That, in turn, would
lead to an incorrect supposition that the response to the port profile
disassociate was the cause of the failure.
During debugging, I noticed that the VF in question usually had *no
uuid* associated with it (big surprise)by the time the disassociate
completed, so the solution is *not* to send the previous instanceId
down.
This patch fixes virNetDevVPortProfileGetStatus() to only check the
VF's uuid in the status if it was given an instanceId to check against
when originally called. Otherwise it only checks that the particular
VF is present (it will be).
This does cause a slight difference in behavior - rather than
returning with status unchanged (and thus always 0) it will actually
get the IFLA_PORT_RESPONSE. This could lead to revelation of error
conditions we were previously ignoring. Or not. So far "not".
