Emacs is fairly good about navigating across function and scope
boundaries, provided that the code has balanced {}. The vbox
code, however, violated that premise, by splitting 'if () {'
across several #ifdef branches, but sharing the '} else {...}'
outside of the branches. The extra lines of code is worth my
sanity, in a function that is already a horrendous 1100+ lines
long.
* src/vbox/vbox_tmpl.c (vboxDomainGetXMLDesc) Duplicate code
rather than trying to share else branch across #ifdef.
Signed-off-by: Eric Blake <eblake@redhat.com>
A earlier commit changed the global log buffer so that it only
records messages that are explicitly requested via the log
filters setting. This removes the performance burden, and
improves the signal/noise ratio for messages in the global
buffer. At the same time though, it is somewhat pointless, since
all the recorded log messages are already going to be sent to an
explicit log output like syslog, stderr or the journal. The
global log buffer is thus just duplicating this data on stderr
upon crash.
The log_buffer_size config parameter is left in the augeas
lens to prevent breakage for users on upgrade. It is however
completely ignored hereafter.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Currently the log filter strings are used in a string comparison
against the source filename each time log message is emitted.
If no log filters at all are set, there's obviously no string
comparison to be done. If any single log filter is set though,
this imposes a compute burden on every logging call even if logs
from the file in question are disabled. This string comparison
must also be done while the logging mutex is held, which has
implications for concurrency when multiple threads are emitting
log messages.
This changes the log filtering to be done based on the virLogSource
object name. The virLogSource struct is extended to contain
'serial' and 'priority' fields. Any time the global log filter
rules are changed a global serial number is incremented. When a
log message is emitted, the serial in the virLogSource instance
is compared with the global serial number. If out of date, then
the 'priority' field in the virLogSource instance is updated based
on the new filter rules. The 'priority' field is checked to see
whether the log message should be sent to the log outputs.
The comparisons of the 'serial' and 'priority' fields are done
with no locks held. So in the common case each logging call has
an overhead of 2 integer comparisons, with no locks held. Only
if the decision is made to forward the message to the log output,
or if the 'serial' value is out of date do locks need to be
acquired.
Technically the comparisons of the 'serial' and 'priority' fields
should be done with locks held, or using atomic operations. Both
of these options have a notable performance impact, however, and
since all writes a protected by a global mutex, it is believed
that worst case behaviour where the fields are read concurrently
with being written would merely result in an mistaken emission
or dropping of the log message in question. This is an acceptable
tradeoff for the performance benefit of avoiding locking.
As a quick benchmark, a demo program that registers 500 file
descriptors with the event loop (eg equiv of 500 QEMU monitor
commands), creates pending read I/O on every FD, and then runs
virEventRunDefaultImpl() took 4.6 seconds to do 51200 iterations.
After this optimization it only takes 3.3 seconds, with the log
APIs no longer being a relevant factor in the running time.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
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: Daniel P. Berrange <berrange@redhat.com>
As part of the goal to get away from doing string matching on
filenames when deciding whether to emit a log message, turn
the virLogSource enum into a struct which contains a log
"name". There will eventually be one virLogSource instance
statically declared per source file. To minimise churn in this
commit though, a single global instance is used.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The dtrace probe macros rely on the logging API. We can't make
the internal.h header include the virlog.h header though since
that'd be a circular include. Instead simply split the dtrace
probes into their own header file, since there's no compelling
reason for them to be in the main internal.h header.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The error reporting code will invoke a callback when any error
is raised and the default callback will print to stderr. The
virRaiseErrorFull method also sends all error messages on to the
logging code, which also prints to stderr by default. To avoid
duplicated data on stderr, the logging code has some logic to
skip emission when no log outputs are configured, which checks
whether the virLogSource == VIR_LOG_FROM_ERROR.
Meanwhile the libvirtd daemon can register another callback which
is used to reduce log message priority from error to a lower level.
When this is used we do want messages to end up on stderr, so the
error code will conditionally use either VIR_LOG_FROM_FILE or
VIR_LOG_FROM_ERROR depending on whether such a callback is provided.
This will all complicate later refactoring. By pushing the checks
for whether a log output is present up a level into the error code,
the special cases can be isolated in one place.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
With the vast number of log debug statements in the code, the
logging framework has a measurable performance impact on libvirt
code, particularly in the daemon event loop.
The global log buffer records every single log message triggered
whether anyone cares to see them or not. This makes it impossible
to eliminate the overhead of printf format expansions in any of
the logging code. It is possible to disable the global log buffer
in libvirtd itself, but this doesn't help client side library
code. Also even if disabled by the config file, the existence of
the feature makes other performance improvements in the logging
layer impossible.
Instead of logging every single message to the global buffer, only
log messages that pass the log filters. This if libvirtd is set
to have log_filters="1:libvirt 1:qemu" the global log buffer will
only get filled with those messages instead of everything. This
reduces the performance burden, as well as improving the signal
to noise ratio of the log buffer.
As a quick benchmark, a demo program that registers 500 file
descriptors with the event loop (eg equiv of 500 QEMU monitor
commands), creates pending read I/O on every FD, and then runs
virEventRunDefaultImpl() took 1 minute 40 seconds to do 51200
iterations with nearly all the time shown against the logging
code. After this optimization it only takes 4.6 seconds.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Coverity spotted a use of possibly undefined variable. If a server is
restarting as an result of update, the JSON file that keeps current
value of some variables will not contain the new variables. This is
the case of @max_anonymous_clients too. We are correctly querying if
there's "max_anonymous_clients" in the JSON, however, we are not
setting a sane default if there's none.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
We allow translation from no_bandwidth to has_bandwidth for a vnic.
However, going in the opposite direction is not implemented. It's not
limitation of the API rather than internal implementation. The problem
is, we correctly detect that user hasn't specified any outbound (say
he wants to clear out outbound). However, this gets overwritten by
current vnic outbound settings. Then, virNetDevBandwidthSet doesn't
change anything. We need to stop overwriting the outbound if users
don't want us to. Same applies for inbound.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
If there should be some sort of separator it is better to use comment
with the filename, copyright, description, license information and
authors.
Found by:
git grep -nH '^$' | grep '\.[ch]:1:'
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
This patch is not trying to fix every switch, just the ones I worked
with last time, because some of these were especially unreadable.
Covers enums virDomainGraphicsType and virDomainChrType (where
applicable).
Also sort its cases by their value.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Commit a1cbe4b5 added a check for spaces around assignments and this
patch extends it to checks for spaces around '=='. One exception is
virAssertCmpInt where comma after '==' is acceptable (since it is a
macro and '==' is its argument).
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
While running qemuxml2xmltest, it was found that valgrind pointed out
the following memory leak:
==21905== 26 bytes in 1 blocks are definitely lost in loss record 23 of 69
==21905== at 0x4A069EE: malloc (vg_replace_malloc.c:270)
==21905== by 0x3E782A754D: xmlStrndup (in /usr/lib64/libxml2.so.2.7.6)
==21905== by 0x4CD986D: virDomainChrSourceDefParseXML (domain_conf.c:7233)
==21905== by 0x4CE4199: virDomainChrDefParseXML (domain_conf.c:7512)
==21905== by 0x4CFAF3F: virDomainDefParseXML (domain_conf.c:12303)
==21905== by 0x4CFB46E: virDomainDefParseNode (domain_conf.c:13031)
==21905== by 0x4CFB5E9: virDomainDefParse (domain_conf.c:12973)
==21905== by 0x41E9D8: testCompareXMLToXMLFiles (qemuxml2xmltest.c:40)
==21905== by 0x41EBAA: testCompareXMLToXMLHelper (qemuxml2xmltest.c:93)
==21905== by 0x421D21: virtTestRun (testutils.c:199)
==21905== by 0x41FCE9: mymain.part.0 (qemuxml2xmltest.c:244)
==21905== by 0x42249D: virtTestMain (testutils.c:782)
==21905==
... and 7 more
Make virt-aa-helper create rules to allow VMs access to filesystem
mounts from the host.
Signed-off-by: Felix Geyer <debfx@fobos.de>
Signed-off-by: Hiroshi Miura <miurahr@linux.com>
Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com>
Signed-off-by: Guido Günther <agx@sigxcpu.org>
While running domainsnapshotxml2xmltest, it was found that valgrind pointed out
the following memory leak:
==32176== 42 (32 direct, 10 indirect) bytes in 1 blocks are definitely lost in loss record 42 of 66
==32176== at 0x4A069EE: malloc (vg_replace_malloc.c:270)
==32176== by 0x4A06B62: realloc (vg_replace_malloc.c:662)
==32176== by 0x4C65A07: virReallocN (viralloc.c:243)
==32176== by 0x4C65B2E: virExpandN (viralloc.c:292)
==32176== by 0x4C65E30: virInsertElementsN (viralloc.c:434)
==32176== by 0x4CD71F3: virDomainDiskSourceDefParse (domain_conf.c:5078)
==32176== by 0x4CF6EF4: virDomainSnapshotDefParseNode (snapshot_conf.c:151)
==32176== by 0x4CF7314: virDomainSnapshotDefParseString (snapshot_conf.c:410)
==32176== by 0x41FB8D: testCompareXMLToXMLHelper (domainsnapshotxml2xmltest.c:100)
==32176== by 0x420FD1: virtTestRun (testutils.c:199)
==32176== by 0x41F859: mymain (domainsnapshotxml2xmltest.c:222)
==32176== by 0x42174D: virtTestMain (testutils.c:782)
==32176==
... and one more.
The virNWFilterVarCombIterNext method will free its
parameter when it gets to the end of the iterator.
This is somewhat misleading design, making it appear
as if the caller has a memory leak. Remove the free'ing
of the parameter and ensure that the calling method
ebiptablesCreateRuleInstanceIterate free's it instead.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The ebiptablesAddRuleInst method would leak an instance
of ebiptablesRuleInstPtr if it hit OOM when adding it
to the list of instances. Remove the pointless helper
method virNWFilterRuleInstAddData and just inline the
call to VIR_APPEND_ELEMENT and free the instance on
failure.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The libxl driver reads /proc/xen/capabilities to see if it
is on a Dom0 kernel. If that file does not even exist though,
an error is logged. Check for the file existance before trying
to read its contents to avoid the log message.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Addition of the hostbridge device was mistakenly placed to
bhyveBuildNetArgStr(). This could result in hostbridge device not being
added to the commandline if there are no network devices specified, but
hostbridge device should be added unconditionally.
Fix by placing it to virBhyveProcessBuildBhyveCmd().
https://bugzilla.redhat.com/show_bug.cgi?id=992980
This config tunable allows users to determine the maximum number of
accepted but yet not authenticated users.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The counter gets incremented on each unauthenticated client added to the
server and decremented whenever the client authenticates.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
- As of commit 2ff4c137, all virGet*() functions in datatypes.c always
return pointers to new objects. Objects are not cached in a
per-connection hashtable.
- Fix variable names in comments for all vir*Dispose() functions in
datatypes.c.
- Add comments for virGetStream(), virStreamDispose(),
virGetDomainSnapshot(), virDomainSnapshotDispose().
Signed-off-by: Michael Chapman <mike@very.puzzling.org>
Our current pidfile acquire APis (virPidFileAcquire) simply return -1 upon
failure to acquire a lock. This patch adds a parameter 'bool waitForLock'
which instructs the APIs if we want to make it block and wait for the lock
or not.
Thre was a syntax error in checking virRegisterStateDriver in
the remote driver, and bogus checking of a void return type
of virDomainConfNWFilterRegister in nwfilter.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Coverity found an issue in lxc_driver and uml_driver that we don't
check the return value of register functions.
I've also updated all other places and unify the way we check the
return value.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Right now we are parsing the XML as though it's live, which for example
will choke on hardcoded XML like:
<seclabel type='dynamic' model='selinux' relabel='yes'/>
Erroring with:
$ sudo virsh domxml-to-native qemu-argv f
error: XML error: security label is missing
All drivers are fixed, but only qemu was tested.
We have to explicitly destroy TAP devices on FreeBSD because
they're not freed after being closed, otherwise we end up with
orphaned TAP devices after destroying a domain.
A recent change to openvz_driver.c caused Coverity to make additional
comparisons and find that the openvzRegister() was not checking the
status of virRegisterDriver() call like other callers and thus generated
a CHECKED_RETURN condition
There were a lot of changes here, but all very mechanical. For some
reason, the virBufferPtr had been named "xml" instead of "buf" in this
file, so since the indentation changing touched almost every line
using the buffer, I took this chance to change its name for "buf" for
consistency with every other file.
This file was using multiple virBuffers, inserting the contents of
buf3 into buf2, then inserting the contents of buf2 into buf1, rather
than the more conventional method of just passing around a single
virBufferPtr and streaming everything into that single buffer. This
was unnecessary, and also made it more difficult to make indentation
relative, because when you insert a string into a buffer, the
indentation of the buffer is only applied once at the beginning of the
string, *not* each time a newline is encountered in the string.
These format functions needed the ability to be indented by an
arbitrary amount, but were written before the introduction of
virBufferAdjustIndent(). They instead used the much more clunky method
of adding a "level" arg to every format function, and padding with
spaces using the "%*s" printf format specifier (giving it the level,
and "", which has the effect of adding level spaces to the output).
While eliminating the hardcoded indentation in other xml, I decided it
was finally time to also modernize the interface formatter code to
make it more consistent.
All leading spaces in domain snapshot xml format functions have been
replaced with appropriate calls to virBufferAdjustIndent(). This will
make it easier to call other similarly fixed format functions
(e.g. domain device format functions).
Many of the domain xml format functions (including all of the device
format functions) had hard-coded spaces, which made for incorrect
indentation when those functions were called in a different context
(for example, commit 2122cf39 added <interface> XML into the document
provided to a network hook script, and in this case it should have
been indented by 2 spaces, but was instead indented by 6 spaces).
To make it possible to insert a properly indented device anywhere into
an XML document, this patch removes hardcoded spaces from the
formatting functions, and calls virBufferAdjustIndent() at appropriate
places instead. (a regex search of domain_conf.c was done to assure
that all occurrences of hardcoded spaces were removed).
virDomainDiskSourceDefFormatInternal() is also called from
snapshot_conf.c, so two virBufferAdjustIndent() calls were temporarily
added around that call - those functions will have hardcoded spaces
removed in a separate patch.
This could cause some conflicts when backporting future changes to the
formatting functions to older branches, but fortunately the changes
are almost all trivial, so conflict resolution will be obvious.
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=862887
Add a netmask for the source and destination IP address for the
ebtables --arp-ip-src and --arp-ip-dst options. Extend the XML
parser with support for XML attributes for these netmasks similar
to already supported netmasks. Extend the documentation.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1072292
Fix a problem related to rule priorities that did not allow to
have rules applied that had a higher priority than the chain they
were in. In this case the chain did not exist yet when the rule
was instantiated. The solution is to adjust the priority of rules
if the priority of the chain is of higher value. That way the chain
will be created before the rule.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Commit 6b306d66 converted virHostdevManager to a virObject, but
missed adding a virObject field to the virHostdevManager struct.
Result is memory corruption when taking a reference on an instance
of the object, where atomic inc is done on the stateDir field.
Later use of stateDir crashes libvirtd.
When I played with virtlockd I was stunned by lacking
documentation. My frustration got bigger when I had to
read the patches to get the correct value to set in
qemu.conf.
Moreover, from pure libvirt-pride I'm changing commented
value from sanlock to lockd. We want to favor our own
implementation after all.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
When ABI stability check fails, we only log the error message describing
the incompatibility. Let's log both XMLs in case of an error to make it
easier to analyze where and why the stability check failed.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The kernel didn't support the unprivileged SGIO for SCSI generic
device finally, and since it's unknow whether the way to support
unprivileged SGIO for SCSI generic device will be similar as for
SCSI block device or not, even it's simliar (I.e. via sysfs, for
SCSI block device, it's /sys/dev/block/8\:0/queue/unpriv_sgio,
for example), the file name might be different, So it's better not
guess what it should be like currently.
This patch removes the related code (mainly about the "shareable"
checking on the "sgio" setting, it's not supported at all, why
we leave checking code there? :-), and error out if "sgio" is
specified in the domain config.
As soon as any guest mounts xenfs to /proc/xen, there is a capabilities
file in that directory. However it returns nothing when reading from it.
Change the test to actually check the contents of the file.
BugLink: http://bugs.launchpad.net/bugs/1248025
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
While running vircryptotest, it was found that valgrind pointed out the
following error:
==27453== Invalid write of size 1
==27453== at 0x4C7D7C9: virCryptoHashString (vircrypto.c:76)
==27453== by 0x401C4E: testCryptoHash (vircryptotest.c:41)
==27453== by 0x402A11: virtTestRun (testutils.c:199)
==27453== by 0x401AD5: mymain (vircryptotest.c:76)
==27453== by 0x40318D: virtTestMain (testutils.c:782)
==27453== by 0x3E6CE1ED1C: (below main) (libc-start.c:226)
==27453== Address 0x51f0541 is 0 bytes after a block of size 65 alloc'd
==27453== at 0x4A0577B: calloc (vg_replace_malloc.c:593)
==27453== by 0x4C69F2E: virAllocN (viralloc.c:189)
==27453== by 0x4C7D76B: virCryptoHashString (vircrypto.c:69)
==27453== by 0x401C4E: testCryptoHash (vircryptotest.c:41)
==27453== by 0x402A11: virtTestRun (testutils.c:199)
==27453== by 0x401AD5: mymain (vircryptotest.c:76)
==27453== by 0x40318D: virtTestMain (testutils.c:782)
==27453== by 0x3E6CE1ED1C: (below main) (libc-start.c:226)
==27453==
...and many more. Two observations: hashstrlen was already set
to include the trailing NUL byte (so writing to hashstrlen as
the array offset was indeed writing one byte beyond bounds), and
VIR_ALLOC_N already guarantees zero-initialization (so we already
have a trailing NUL without needing to explicitly write one).
Signed-off-by: Eric Blake <eblake@redhat.com>
Changes parameter from vm def to specific hostdevs info and name info, so that
it could be used more widely, e.g, could be used without full vm def info.
Change any variable names with Usb, Pci or Scsi to use
USB, PCI and SCSI since they are abbreviations.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Some virHostdevXXXX methods included the string Hostdev again
as a suffix. Change the latter to Device instead.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Change any method names with Usb, Pci or Scsi to use
USB, PCI and SCSI since they are abbreviations.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Various methods in virnetdev.c and virhostdev.c were missing
const-ness for several char * parameters.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
