Osier Yang pointed out that I introduced a syntax error in my
syntax check (I really shouldn't make last-minute changes without
testing them....).
/bin/sh: -c: line 2: syntax error near unexpected token `;'
/bin/sh: -c: line 2: ` { echo 'maint.mk: incorrect whitespace, see HACKING for rules' 2>&; \'
make: *** [bracket-spacing-check] Error 1
* cfg.mk (bracket-spacing-check): Fix copy-and-paste error.
Signed-off-by: Eric Blake <eblake@redhat.com>
To ensure we don't regress and cause the need for further
cleanups, add a 'make syntax-check' rule that ensures new
files have proper copyright contents.
* cfg.mk (sc_copyright_address): Rename...
(sc_copyright_usage): ...and enhance.
Signed-off-by: Eric Blake <eblake@redhat.com>
https://www.gnu.org/licenses/gpl-howto.html states:
You should also include a copy of the license itself somewhere in the
distribution of your program. All programs, whether they are released
under the GPL or LGPL, should include the text version of the GPL. In
GNU programs the license is usually in a file called COPYING.
If you are releasing your program under the LGPL, you should also
include the text version of the LGPL, usually in a file called
COPYING.LESSER. Please note that, since the LGPL is a set of
additional permissions on top of the GPL, it's important to include
both licenses so users have all the materials they need to understand
their rights.
* configure.ac (COPYING): No more games with non-git file.
* COPYING: New file, copied from gnulib.
* COPYING.LIB: Rename...
* COPYING.LESSER: ...to this.
* .gitignore: Track licenses in git.
* cfg.mk (exclude_file_name_regexp--sc_copyright_address): Tweak
rule.
* libvirt.spec.in (daemon, client, python): Reflect rename.
Signed-off-by: Eric Blake <eblake@redhat.com>
Use of the select() system call is inherantly dangerous since
applications will hit a buffer overrun if any FD number exceeds
the size of the select set size (typically 1024). Replace the
two uses of select() with poll() and use cfg.mk to ban any
future use of select().
NB: This changes the phyp driver so that it uses an infinite
timeout, instead of busy-waiting for 1ms at a time.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
These all existed before virfile.c was created, and for some reason
weren't moved.
This is mostly straightfoward, although the syntax rule prohibiting
write() had to be changed to have an exception for virfile.c instead
of virutil.c.
This movement pointed out that there is a function called
virBuildPath(), and another almost identical function called
virFileBuildPath(). They really should be a single function, which
I'll take care of as soon as I figure out what the arglist should look
like.
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.
POSIX says that both basename() and dirname() may return static
storage (aka they need not be thread-safe); and that they may but
not must modify their input argument. Furthermore, <libgen.h>
is not available on all platforms. For these reasons, you should
never use these functions in a multi-threaded library.
Gnulib instead recommends a way to avoid the portability nightmare:
gnulib's "dirname.h" provides useful thread-safe counterparts. The
obvious dir_name() and base_name() are GPL (because they malloc(),
but call exit() on failure) so we can't use them; but the LGPL
variants mdir_name() (malloc's or returns NULL) and last_component
(always points into the incoming string without modifying it,
differing from basename semantics only on corner cases like the
empty string that we shouldn't be hitting in the first place) are
already in use in libvirt. This finishes the swap over to the safe
functions.
* cfg.mk (sc_prohibit_libgen): New rule.
* src/util/vircgroup.c: Fix offenders.
* src/parallels/parallels_storage.c (parallelsPoolAddByDomain):
Likewise.
* src/parallels/parallels_network.c (parallelsGetBridgedNetInfo):
Likewise.
* src/node_device/node_device_udev.c (udevProcessSCSIHost)
(udevProcessSCSIDevice): Likewise.
* src/storage/storage_backend_disk.c
(virStorageBackendDiskDeleteVol): Likewise.
* src/util/virpci.c (virPCIGetDeviceAddressFromSysfsLink):
Likewise.
* src/util/virstoragefile.h (_virStorageFileMetadata): Avoid false
positive.
Signed-off-by: Eric Blake <eblake@redhat.com>
With this patch, include public headers in "" form is only allowed
for "internal.h". And only the external tools (examples|tools|python
|include/libvirt) can include the public headers in <> form.
Directories python/tools/examples should include them in <> form,
though this patch allows "" form in these directories by excluding
them, a later patch will do the cleanup.
Some aspects of the cgroups setup / detection code are quite subtle
and easy to break. It would greatly benefit from unit testing, but
this is difficult because the test suite won't have privileges to
play around with cgroups. The solution is to use monkey patching
via LD_PRELOAD to override the fopen, open, mkdir, access functions
to redirect access of cgroups files to some magic stubs in the
test suite.
Using this we provide custom content for the /proc/cgroup and
/proc/self/mounts files which report a fixed cgroup setup. We
then override open/mkdir/access so that access to the cgroups
filesystem gets redirected into files in a temporary directory
tree in the test suite build dir.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
We've already scrubbed for comparisons of 'uid_t == -1' (which fail
on platforms where uid_t is a u16), but another one snuck in.
* src/util/virutil.c (virSetUIDGIDWithCaps): Correct uid comparison.
* cfg.mk (sc_prohibit_risky_id_promotion): New rule.
testutils.c likes to print summaries after a test completes,
including if it failed. But if the test outright exit()s,
this summary is skipped. Enforce that we return instead of exit.
* cfg.mk (sc_prohibit_exit_in_tests): New syntax check.
* tests/commandhelper.c (main): Fix offenders.
* tests/qemumonitorjsontest.c (mymain): Likewise.
* tests/seclabeltest.c (main): Likewise.
* tests/securityselinuxlabeltest.c (mymain): Likewise.
* tests/securityselinuxtest.c (mymain): Likewise.
* tests/testutils.h (VIRT_TEST_MAIN_PRELOAD): Likewise.
* tests/testutils.c (virtTestMain): Likewise.
(virtTestCaptureProgramOutput): Use symbolic name.
hacking: Add some text around the running of Valgrind along with example
output for "real" vs. "false positives".
cfg.mk: Add hacking.in.html to sc_prohibit_raw_allocation
Adjust the macros to free memory allocated during various calls to
perform the check if parameter is NULL prior to really freeing and to
set the pointer to NULL after done freeing.
Nested conditionals are hard to read if they are not indented.
We can't add arbitrary whitespace to everything in spec files,
but we CAN add spaces before %if and %define. Use this trick,
plus a fancy sed script that rewrites a spec file into a C
file, so we can use cppi to keep our spec file nice.
For reference, the sed script converts code like:
|# RHEL-5 builds are client-only for s390, ppc
|%if 0%{?rhel} == 5
| %ifnarch %{ix86} x86_64 ia64
| %define client_only 1
| %endif
|%endif
into the following for cppi:
|// # RHEL-5 builds are client-only for s390, ppc
|#if a // 0%{?rhel} == 5
|# if a // %{ix86} x86_64 ia64
|# define client_only 1
|# endif
|#endif
and errors from 'make syntax-check' look like:
spec_indentation
cppi: mingw-libvirt.spec.in: line 130: not properly indented
maint.mk: incorrect preprocessor indentation
* libvirt.spec.in: Add some indentation to make it easier to follow
various conditionals.
* mingw-libvirt-spec.in: Likewise.
* cfg.mk (sc_spec_indentation): New syntax check to enforce it.
This patch introduces support for LXC specific public APIs. In
common with what was done for QEMU, this creates a libvirt_lxc.so
library and libvirt/libvirt-lxc.h header file.
The actual APIs are
int virDomainLxcOpenNamespace(virDomainPtr domain,
int **fdlist,
unsigned int flags);
int virDomainLxcEnterNamespace(virDomainPtr domain,
unsigned int nfdlist,
int *fdlist,
unsigned int *noldfdlist,
int **oldfdlist,
unsigned int flags);
which provide a way to use the setns() system call to move the
calling process into the container's namespace. It is not
practical to write in a generically applicable manner. The
nearest that we could get to such an API would be an API which
allows to pass a command + argv to be executed inside a
container. Even if we had such a generic API, this LXC specific
API is still useful, because it allows the caller to maintain
the current process context, in particular any I/O streams they
have open.
NB the virDomainLxcEnterNamespace() API is special in that it
runs client side, so does not involve the internal driver API.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
There are many aspects of the guest XML which result in the
SELinux driver applying file labelling. With the increasing
configuration options it is desirable to test this behaviour.
It is not possible to assume that the test suite has the
ability to set SELinux labels. Most filesystems though will
support extended attributes. Thus for the purpose of testing,
it is possible to extend the existing LD_PRELOAD hack to
override setfilecon() and getfilecon() to simply use the
'user.libvirt.selinux' attribute for the sake of testing.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Most checks for libraries take the same format
* --with-libFOO=yes|no|check|/some/path argument
* check for a function NNN in libFOO.so
* check for a header file DDD/HHH.h
* Define a WITH_FOO config.h symbol
* Define a WITH_FOO make conditional
* Substitute FOO_CFLAGS and FOO_LIBS make variables
* Print CFLAGS & LIBS summary at the end
Doing all this correctly is rather difficult, typically
done by copy+paste of a previous usage. Further small
improvements people make are not applied to all previous
usages.
Improve this by creating some helper macros to apply
good practice. First, to perform the actual checks:
LIBVIRT_CHECK_LIB([SELINUX], [selinux],
[getfilecon], [selinux/selinux.h])
This checks for 'getfilecon' in -lselinux, and the
existence of 'selinux/selinux.h' header file. If successful
it sets SELINUX_CFLAGS and SELINUX_LIBS. The WITH_SELINUX
config.h macro and WITH_SELINUX make conditional are also
defined.
In some cases we need to check two variants of the same
library
LIBVIRT_CHECK_LIB_ALT([SASL], [sasl2],
[sasl_client_init], [sasl/sasl.h],
[SASL1], [sasl],
[sasl_client_init], [sasl/sasl.h])
This checks for sasl_client_init in libsasl2, and if that
is not found, checks sasl_client_init in libsasl. If the
first check succeeds WITH_SASL is set, while if the second
check succeeds *both* WITH_SASL and WITH_SASL1 are set.
If the library supports pkg-config, then another variant
is available
LIBVIRT_CHECK_PKG([AVAHI], [avahi-client], [0.6.0])
This checks for avahi-client >= 0.6.0 via pkg-config
and sets WITH_AVAHI if found.
Finally to print a summary of CFLAGS & LIBs found (if any):
LIBVIRT_RESULT_LIB([SELINUX])
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Test cases for virSysinfoRead. Initially, there are tests for
x86 (DMI based) and s390 (/proc/... based).
In lack of PPC data, I have stubbed out the test for it, but it
can be added with a minimal effort.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
The virtlockd daemon will be responsible for managing locks
on virtual machines. Communication will be via the standard
RPC infrastructure. This provides the XDR protocol definition
* src/locking/lock_protocol.x: Wire protocol for virtlockd
* src/Makefile.am: Include lock_protocol.[ch] in virtlockd
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The virtlockd daemon will maintain locks on behalf of libvirtd.
There are two reasons for it to be separate
- Avoid risk of other libvirtd threads accidentally
releasing fcntl() locks by opening + closing a file
that is locked
- Ensure locks can be preserved across libvirtd restarts.
virtlockd will need to be able to re-exec itself while
maintaining locks. This is simpler to achieve if its
sole job is maintaining locks
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
I noticed when writing the backend functions for virNetworkUpdate that
I was repeating the same sequence of memmove, VIR_REALLOC, nXXX-- (and
messed up the args to memmove at least once), and had seen the same
sequence in a lot of other places, so I decided to write a few
utility functions/macros - see the .h file for full documentation.
The intent is to reduce the number of lines of code, but more
importantly to eliminate the need to check the element size and
element count arithmetic every time we need to do this (I *always*
make at least one mistake.)
VIR_INSERT_ELEMENT: insert one element at an arbitrary index within an
array of objects. The size of each object is determined
automatically by the macro using sizeof(*array). The new element's
contents are copied into the inserted space, then the original copy
of contents are 0'ed out (if everything else was
successful). Compile-time assignment and size compatibility between
the array and the new element is guaranteed (see explanation below
[*])
VIR_INSERT_ELEMENT_COPY: identical to VIR_INSERT_ELEMENT, except that
the original contents of newelem are not cleared to 0 (i.e. a copy
is made).
VIR_APPEND_ELEMENT: This is just a special case of VIR_INSERT_ELEMENT
that "inserts" one past the current last element.
VIR_APPEND_ELEMENT_COPY: identical to VIR_APPEND_ELEMENT, except that
the original contents of newelem are not cleared to 0 (i.e. a copy
is made).
VIR_DELETE_ELEMENT: delete one element at an arbitrary index within an
array of objects. It's assumed that the element being deleted is
already saved elsewhere (or cleared, if that's what is appropriate).
All five of these macros have an _INPLACE variant, which skips the
memory re-allocation of the array, assuming that the caller has
already done it (when inserting) or will do it later (when deleting).
Note that VIR_DELETE_ELEMENT* can return a failure, but only if an
invalid index is given (index + amount to delete is > current array
size), so in most cases you can safely ignore the return (that's why
the helper function virDeleteElementsN isn't declared with
ATTRIBUTE_RETURN_CHECK). A warning is logged if this ever happens,
since it is surely a coding error.
[*] One initial problem with the INSERT and APPEND macros was that,
due to both the array pointer and newelem pointer being cast to void*
when passing to virInsertElementsN(), any chance of type-checking was
lost. If we were going to move in newelem with a memmove anyway, we
would be no worse off for this. However, most current open-coded
insert/append operations use direct struct assignment to move the new
element into place (or just populate the new element directly) - thus
use of the new macros would open a possibility for new usage errors
that didn't exist before (e.g. accidentally sending &newelemptr rather
than newelemptr - I actually did this quite a lot in my test
conversions of existing code).
But thanks to Eric Blake's clever thinking, I was able to modify the
INSERT and APPEND macros so that they *do* check for both assignment
and size compatibility of *ptr (an element in the array) and newelem
(the element being copied into the new position of the array). This is
done via clever use of the C89-guaranteed fact that the sizeof()
operator must have *no* side effects (so an assignment inside sizeof()
is checked for validity, but not actually evaluated), and the fact
that virInsertElementsN has a "# of new elements" argument that we
want to always be 1.
Commit 71d1256 tried to fix a problem where rebasing an old
branch on top of newer libvirt.git resulted in automake failing
because of a missing AUTHORS file. However, while the fix
worked for an incremental 'make', it did not work for someone
that directly reran './autogen.sh'. Reported by Laine Stump.
* autogen.sh (autoreconf): Check for same conditions as cfg.mk.
* cfg.mk (_update_required): Add comments.
Ever since commit 7b21981c started generating AUTHORS, we now have
the situation that if you flip between two branches in the same
git repository that cross that commit boundary, then 'make' will
fail due to automake complaining about AUTHORS not existing. The
simplest solution is to realize that if AUTHORS does not exist,
then we flipped branches so we will need to rerun bootstrap
anyways; and rerunning bootstrap ensures AUTHORS will exist in time.
* cfg.mk (_update_required): Also depend on AUTHORS.
This documents the following whitespace rules
if(foo) // Bad
if (foo) // Good
int foo (int wizz) // Bad
int foo(int wizz) // Good
bar = foo (wizz); // Bad
bar = foo(wizz); // Good
typedef int (*foo) (int wizz); // Bad
typedef int (*foo)(int wizz); // Good
int foo( int wizz ); // Bad
int foo(int wizz); // Good
There is a syntax-check rule extension to validate all these rules.
Checking for 'function (...args...)' is quite difficult since it
needs to ignore valid usage with keywords like 'if (...test...)'
and while/for/switch. It must also ignore source comments and
quoted strings.
It is not possible todo this with a simple regex in the normal
syntax-check style. So a short Perl script is created instead
to analyse the source. In practice this works well enough. The
only thing it can't cope with is multi-line quoted strings of
the form
"start of string\
more lines\
more line\
the end"
but this can and should be written as
"start of string"
"more lines"
"more line"
"the end"
with this simple change, the bracket checking script does not
have any false positives across libvirt source, provided it
is only run against .c files. It is not practical to run it
against .h files, since those use whitespace extensively to
get alignment (though this is somewhat inconsistent and could
arguably be fixed).
The only limitation is that it cannot detect a violation where
the first arg starts with a '*', eg
foo(*wizz);
since this generates too many false positives on function
typedefs which can't be supressed efficiently.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
With our fix of mkostemp (pushed as 2b435c15) we define a macro
to compile with uclibc. However, this definition is conditional
and thus needs to be properly indented. Moreover, with this definition
sc_prohibit_mkstemp syntax-check rule keeps yelling:
src/util/logging.c:63:# define mkostemp(x,y) mkstemp(x)
maint.mk: use mkostemp with O_CLOEXEC instead of mkstemp
Therefore we should ignore this file for this rule.
https://bugzilla.redhat.com/show_bug.cgi?id=871756
Commit cd1e8d1 assumed that systems new enough to have journald
also have mkostemp; but this is not true for uclibc.
For that matter, use of mkstemp[s] is unsafe in a multi-threaded
program. We should prefer mkostemp[s] in the first place.
* bootstrap.conf (gnulib_modules): Add mkostemp, mkostemps; drop
mkstemp and mkstemps.
* cfg.mk (sc_prohibit_mkstemp): New syntax check.
* tools/virsh.c (vshEditWriteToTempFile): Adjust caller.
* src/qemu/qemu_driver.c (qemuDomainScreenshot)
(qemudDomainMemoryPeek): Likewise.
* src/secret/secret_driver.c (replaceFile): Likewise.
* src/vbox/vbox_tmpl.c (vboxDomainScreenshot): Likewise.
This patch resolves: https://bugzilla.redhat.com/show_bug.cgi?id=871201
If libvirt is restarted after updating the dnsmasq or radvd packages,
a subsequent "virsh net-destroy" will fail to kill the dnsmasq/radvd
process.
The problem is that when libvirtd restarts, it re-reads the dnsmasq
and radvd pidfiles, then does a sanity check on each pid it finds,
including checking that the symbolic link in /proc/$pid/exe actually
points to the same file as the path used by libvirt to execute the
binary in the first place. If this fails, libvirt assumes that the
process is no longer alive.
But if the original binary has been replaced, the link in /proc is set
to "$binarypath (deleted)" (it literally has the string " (deleted)"
appended to the link text stored in the filesystem), so even if a new
binary exists in the same location, attempts to resolve the link will
fail.
In the end, not only is the old dnsmasq/radvd not terminated when the
network is stopped, but a new dnsmasq can't be started when the
network is later restarted (because the original process is still
listening on the ports that the new process wants).
The solution is, when the initial "use stat to check for identical
inodes" check for identity between /proc/$pid/exe and $binpath fails,
to check /proc/$pid/exe for a link ending with " (deleted)" and if so,
truncate that part of the link and compare what's left with the
original binarypath.
A twist to this problem is that on systems with "merged" /sbin and
/usr/sbin (i.e. /sbin is really just a symlink to /usr/sbin; Fedora
17+ is an example of this), libvirt may have started the process using
one path, but /proc/$pid/exe lists a different path (indeed, on F17
this is the case - libvirtd uses /sbin/dnsmasq, but /proc/$pid/exe
shows "/usr/sbin/dnsmasq"). The further bit of code to resolve this is
to call virFileResolveAllLinks() on both the original binarypath and
on the truncated link we read from /proc/$pid/exe, and compare the
results.
The resulting code still succeeds in all the same cases it did before,
but also succeeds if the binary was deleted or replaced after it was
started.
AUTHORS.in tracks the maintainers, as well as some folks who were
previously in AUTHORS but don't have a git commit with proper
attribution.
Generated output is sorted alphabetically and lacks pretty spacing, so
tweak AUTHORS.in to follow the same format.
Additionally, drop the syntax-check rule that previously validated
AUTHORS against git log.
Several people have reported that if the .gnulib submodule is dirty,
then 'make' will go into an infinite loop attempting to rerun bootstrap,
because that never cleans up the dirty submodule. By default, we
should halt and make the user investigate, but if the user doesn't
know why or care that the submodule is dirty, I also added the ability
to 'make CLEAN_SUBMODULE=1' to get things going again.
Also, while testing this, I noticed that when a submodule update was
needed, 'make' would first run autoreconf, then bootstrap (which
reruns autoreconf); adding a strategic dependency allows for less work.
* .gnulib: Update to latest, for maint.mk improvements.
* cfg.mk (_autogen): Also hook maint.mk, to run before autoreconf.
* autogen.sh (bootstrap): Refuse to run if gnulib is dirty, unless
user requests discarding gnulib changes.
I got an off-list report about a bad diagnostic:
Target network card mac 52:54:00:49:07:ccdoes not match source 52:54:00:49:07:b8
True to form, I've added a syntax check rule to prevent it
from recurring, and found several other offenders.
* cfg.mk (sc_require_whitespace_in_translation): New rule.
* src/conf/domain_conf.c (virDomainNetDefCheckABIStability): Add
space.
* src/esx/esx_util.c (esxUtil_ParseUri): Likewise.
* src/qemu/qemu_command.c (qemuCollectPCIAddress): Likewise.
* src/qemu/qemu_driver.c (qemuDomainSetMetadata)
(qemuDomainGetMetadata): Likewise.
* src/qemu/qemu_hotplug.c (qemuDomainChangeNetBridge): Likewise.
* src/rpc/virnettlscontext.c
(virNetTLSContextCheckCertDNWhitelist): Likewise.
* src/vmware/vmware_driver.c (vmwareDomainResume): Likewise.
* src/vbox/vbox_tmpl.c (vboxDomainGetXMLDesc, vboxAttachDrives):
Avoid false negatives.
* tools/virsh-domain.c (info_save_image_dumpxml): Reword.
Based on a report by Luwen Su.
To be able to test the QEMU monitor code, we need to have a fake
QEMU monitor server. This introduces a simple (dumb) framework
that can do this. The test case registers a series of items to
be sent back as replies to commands that will be executed. A
thread runs the event loop looking for incoming replies and
sending back this pre-registered data. This allows testing all
QEMU monitor code that deals with parsing responses and errors
from QEMU, without needing QEMU around
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Based on the similar gnulib commit 96ad9077. The use of
$(_sc_search_regexp) already injects $(ME) into any output
messages, so a failure of these rules would look like this,
pre-patch:
maint.mk: maint.mk: use virStrToLong_*, not strtol variants
* cfg.mk (sc_prohibit_strncmp, sc_prohibit_strtol)
(sc_libvirt_unmarked_diagnostics): Drop redundant $(ME).
Yesterday's commit 15d2c9f pointed out that virsh was still using
localtime(), which is not thread-safe, even though virsh is
definitely multi-threaded. Even if we only ever triggered it from
one thread, it's better safe than sorry for maintenance purposes.
* cfg.mk (exclude_file_name_regexp--sc_prohibit_nonreentrant):
Tighten the rule.
* tools/virsh.c (vshOutputLogFile): Avoid localtime.
(vshEditWriteToTempFile, vshEditReadBackFile, cmdCd, cmdPwd)
(vshCloseLogFile): Avoid strerror.
* tools/console.c (vshMakeStdinRaw): Likewise.
* tools/virsh-domain.c (vshGenFileName): Fix spacing in previous
patch.
Nothing in the testsuite or examples directory should be translated,
as it is not part of the normally installed binary. We already
meet this rule, but enforcing it will make it easier to remember.
Suggested by Daniel P. Berrange.
* cfg.mk (sc_prohibit_useless_translation): Enhance rule.
'make distcheck' was failing because a syntax check file,
.sc-start-sc_vulnerable_makefile_CVE-2012-3386, got left
behind. I traced it to the 'distdir' rule depending on a
shortcut syntax-check name rather than the full rule name
normally used during 'local-check' from maint.mk.
* cfg.mk (distdir): Depend on full rule, not shorthand name.
* tools/virsh.c: New macro vshStrcasecmp
* tools/virsh-domain-monitor.c: Use vshStrcasecmp instead of
strcasecmp
* tools/virsh-snapshot.c: Likewise
* cfg.mk: Only avoid doing strcase checking for virsh.c
This introduces a fairly basic reference counted virObject type
and an associated virClass type, that use atomic operations for
ref counting.
In a global initializer (recommended to be invoked using the
virOnceInit API), a virClass type must be allocated for each
object type. This requires a class name, a "dispose" callback
which will be invoked to free memory associated with the object's
fields, and the size in bytes of the object struct.
eg,
virClassPtr connclass = virClassNew("virConnect",
sizeof(virConnect),
virConnectDispose);
The struct for the object, must include 'virObject' as its
first member
eg
struct _virConnect {
virObject object;
virURIPtr uri;
};
The 'dispose' callback is only responsible for freeing
fields in the object, not the object itself. eg a suitable
impl for the above struct would be
void virConnectDispose(void *obj) {
virConnectPtr conn = obj;
virURIFree(conn->uri);
}
There is no need to reset fields to 'NULL' or '0' in the
dispose callback, since the entire object will be memset
to 0, and the klass pointer & magic integer fields will
be poisoned with 0xDEADBEEF before being free()d
When creating an instance of an object, one needs simply
pass the virClassPtr eg
virConnectPtr conn = virObjectNew(connclass);
if (!conn)
return NULL;
conn->uri = virURIParse("foo:///bar")
Object references can be manipulated with
virObjectRef(conn)
virObjectUnref(conn)
The latter returns a true value, if the object has been
freed (ie its ref count hit zero)
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The cfg.mk file rule to check for tab characters was not
applied to perl files. Much of our Perl code is full of
tabs as a result. Kill them, kill them all !
Any time we have a string with no % passed through gettext, a
translator can inject a % to cause a stack overread. When there
is nothing to format, it's easier to ask for a string that cannot
be used as a formatter, by using a trivial "%s" format instead.
In the past, we have used --disable-nls to catch some of the
offenders, but that doesn't get run very often, and many more
uses have crept in. Syntax check to the rescue!
The syntax check can catch uses such as
virReportError(code,
_("split "
"string"));
by using a sed script to fold context lines into one pattern
space before checking for a string without %.
This patch is just mechanical insertion of %s; there are probably
several messages touched by this patch where we would be better
off giving the user more information than a fixed string.
* cfg.mk (sc_prohibit_diagnostic_without_format): New rule.
* src/datatypes.c (virUnrefConnect, virGetDomain)
(virUnrefDomain, virGetNetwork, virUnrefNetwork, virGetInterface)
(virUnrefInterface, virGetStoragePool, virUnrefStoragePool)
(virGetStorageVol, virUnrefStorageVol, virGetNodeDevice)
(virGetSecret, virUnrefSecret, virGetNWFilter, virUnrefNWFilter)
(virGetDomainSnapshot, virUnrefDomainSnapshot): Add %s wrapper.
* src/lxc/lxc_driver.c (lxcDomainSetBlkioParameters)
(lxcDomainGetBlkioParameters): Likewise.
* src/conf/domain_conf.c (virSecurityDeviceLabelDefParseXML)
(virDomainDiskDefParseXML, virDomainGraphicsDefParseXML):
Likewise.
* src/conf/network_conf.c (virNetworkDNSHostsDefParseXML)
(virNetworkDefParseXML): Likewise.
* src/conf/nwfilter_conf.c (virNWFilterIsValidChainName):
Likewise.
* src/conf/nwfilter_params.c (virNWFilterVarValueCreateSimple)
(virNWFilterVarAccessParse): Likewise.
* src/libvirt.c (virDomainSave, virDomainSaveFlags)
(virDomainRestore, virDomainRestoreFlags)
(virDomainSaveImageGetXMLDesc, virDomainSaveImageDefineXML)
(virDomainCoreDump, virDomainGetXMLDesc)
(virDomainMigrateVersion1, virDomainMigrateVersion2)
(virDomainMigrateVersion3, virDomainMigrate, virDomainMigrate2)
(virStreamSendAll, virStreamRecvAll)
(virDomainSnapshotGetXMLDesc): Likewise.
* src/nwfilter/nwfilter_dhcpsnoop.c (virNWFilterSnoopReqLeaseDel)
(virNWFilterDHCPSnoopReq): Likewise.
* src/openvz/openvz_driver.c (openvzUpdateDevice): Likewise.
* src/openvz/openvz_util.c (openvzKBPerPages): Likewise.
* src/qemu/qemu_cgroup.c (qemuSetupCgroup): Likewise.
* src/qemu/qemu_command.c (qemuBuildHubDevStr, qemuBuildChrChardevStr)
(qemuBuildCommandLine): Likewise.
* src/qemu/qemu_driver.c (qemuDomainGetPercpuStats): Likewise.
* src/qemu/qemu_hotplug.c (qemuDomainAttachNetDevice): Likewise.
* src/rpc/virnetsaslcontext.c (virNetSASLSessionGetIdentity):
Likewise.
* src/rpc/virnetsocket.c (virNetSocketNewConnectUNIX)
(virNetSocketSendFD, virNetSocketRecvFD): Likewise.
* src/storage/storage_backend_disk.c
(virStorageBackendDiskBuildPool): Likewise.
* src/storage/storage_backend_fs.c
(virStorageBackendFileSystemProbe)
(virStorageBackendFileSystemBuild): Likewise.
* src/storage/storage_backend_rbd.c
(virStorageBackendRBDOpenRADOSConn): Likewise.
* src/storage/storage_driver.c (storageVolumeResize): Likewise.
* src/test/test_driver.c (testInterfaceChangeBegin)
(testInterfaceChangeCommit, testInterfaceChangeRollback):
Likewise.
* src/vbox/vbox_tmpl.c (vboxListAllDomains): Likewise.
* src/xenxs/xen_sxpr.c (xenFormatSxprDisk, xenFormatSxpr):
Likewise.
* src/xenxs/xen_xm.c (xenXMConfigGetUUID, xenFormatXMDisk)
(xenFormatXM): Likewise.
We were defining 'func_or' as '|VIR_ERROR|...', which when put
inside 'func_re' resulted in a regex that matches everything in
isolation. Thankfully, we always used func_re with a leading
anchor \<, and since the empty regex does not start a word, we
happened to get the result we wanted; but it's better to define
func_or without a leading space converted into a leading empty
alternation.
* cfg.mk (func_or): Strip leading space.
Pick up some build fixes in the latest gnulib. In particular,
we want to ensure that official tarballs are secure, but don't
want to penalize people who don't run 'make dist', since fixed
automake still hasn't hit common platforms like Fedora 17.
* .gnulib: Update to latest, for Automake CVE-2012-3386 detection.
* bootstrap: Resync from gnulib.
* bootstrap.conf (gnulib_extra_files): Drop missing, since gnulib
has dropped it in favor of Automake's version.
* cfg.mk (local-checks-to-skip): Conditionally skip the security
check in cases where it doesn't matter.
Commands in node device group moved from virsh.c to virsh-nodedev.c,
* virsh.c: Remove commands in node device group.
* virsh-nodedev.c: New file, filled with commands in node device group
* po/POTFILES.in: Add virsh-nodedev.c
* cfg.mk: Skip to check config.h including for virsh-nodedev.c
Commands in host group moved from virsh.c to virsh-host.c,
* virsh.c: Remove commands in host group.
* virsh-host.c: New file, filled with commands in host group
* po/POTFILES.in: Add virsh-host.c
* cfg.mk: Skip to check config.h including for virsh-host.c
Commands to manage domain snapshot are moved from virsh.c to
virsh-snapshot.c.
* virsh.c: Remove domain snapshot commands.
* virsh-snapshot.c: New file, filled with domain snapshot commands.
* po/POTFILES.in: Add virsh-snapshot.c
* cfg.mk: Skip strcase and config.h including checking for
virsh-snapshot.c
Commands to manage secret are moved from virsh.c to virsh-secret.c,
with a few helpers for secret command use.
* virsh.c: Remove secret commands and a few helpers.
(vshCommandOptSecret, and vshCommandOptSecretBy)
* virsh-secret.c: New file, filled with secret commands and its helpers.
* po/POTFILES.in: Add virsh-secret.c
* cfg.mk: Skip to check config.h including for virsh-secret.c
Commands to manage network filter are moved from virsh.c to virsh-nwfilter.c,
with a few helpers for network filter command use.
* virsh.c: Remove network filter commands and a few helpers.
(vshCommandOptNWFilter, and vshCommandOptNWFilterBy)
* virsh-nwfilter.c: New file, filled with network filter commands and its helpers.
* po/POTFILES.in: Add virsh-nwfilter.c
* cfg.mk: Skip to check config.h including for virsh-nwfilter.c
Commands to manage host interface are moved from virsh.c to
virsh-interface.c, with a few helpers for interface command use.
* virsh.c: Remove interface commands and a few helpers.
(vshCommandOptInterface, vshCommandOptInterfaceBy)
* virsh-interface.c: New file, filled with interface commands and
its helpers.
* cfg.mk: Skip to check config.h including for virsh-interface.c
* po/POTFILES.in: Add virsh-interface.c
Commands to manage network are moved from virsh.c to virsh-network.c,
with a few helpers for network command use.
* virsh.c: Remove network commands and a few helpers.
* virsh-network.c: New file, filled with network commands and its
helpers.
* po/POTFILES.in: Add virsh-network.c
* cfg.mk: Skip to check config.h including for virsh-network.c
This splits commands of storage pool group into virsh-pool.c,
The helpers not for common use are moved too. Standard copyright
is added for the new file.
* tools/virsh.c:
Remove commands for storage storage pool and a few helpers.
(vshCommandOptVol, vshCommandOptVolBy).
* tools/virsh-pool.c:
New file, filled with commands of storage pool group and its
helpers.
* po/POTFILES.in:
Add virsh-pool.c
* cfg.mk:
Skip to check config.h including for virsh-pool.c
This splits commands of storage volume group into virsh-volume.c,
The helpers not for common use are moved too. Standard copyright
is added for the new file.
* tools/virsh.c:
Remove commands for storage storage volume and a few helpers.
(vshCommandOptVol, vshCommandOptVolBy).
* tools/virsh-volume.c:
New file, filled with commands of storage volume group and its
helpers.
* po/POTFILES.in:
Add virsh-volume.c
* cfg.mk:
Skip to check config.h including for virsh-volume.c
This splits commands to manage domain into virsh-domain.c,The helpers
not for common use are moved into them too. Standard copyright is added
for the new file.
* tools/virsh.c:
- Remove commands for domain group, and one helper
(vshDomainVcpuStateToString)
- vshStreamSink is moved before commands's definition for it's
also used by commands not of domain group, such as volUpload.
* tools/virsh-domain.c:
- New file, commands for domain group and the one helper are
moved into it.
* po/POTFILES.in:
- Add virsh-domain.c
* cfg.mk:
- Skip to check config.h including for virsh-domain.c
This splits commands commands to monitor domain status into
virsh-domain-monitor.c. The helpers not for common use are moved too.
Standard copyright is added.
* tools/virsh.c:
- Remove commands for domain monitoring group and a few helpers (
vshDomainIOErrorToString, vshGetDomainDescription,
vshDomainControlStateToString, vshDomainStateToString) not for
common use.
- Remove (incldue "intprops.h").
* tools/virsh-domain-monitor.c:
- New file, filled with commands of domain monitor group.
- Add "intprops.h".
* cfg.mk:
- Skip strcase checking for virsh-domain-monitor.c
- Skip to check config.h including for virsh-domain-monitor.c
* po/POTFILES.in
- Add virsh-domain-monitor.c
Update the legacy Xen drivers to use virReportError instead of
the statsError, virXenInotifyError, virXenStoreError,
virXendError, xenUnifiedError, xenXMError custom macros
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Update the libvirtd config handling code to use virReportError
instead of the virConfError custom macro
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Update the nodeinfo helper code to use virReportError instead
of the nodeReportError custom macro
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Update the security drivers to use virReportError instead of
the virSecurityReportError custom macro
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Update the Power-Hypervisor driver to use virReportError
instead of the PHYP_ERROR custom macro
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Update the ESX driver to use virReportError instead of
the ESX_ERROR & ESX_VI_ERROR custom macros
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The Xen driver had a number of error reports which passed a
constant string without format specifiers and was missing
"%s". Furthermore the errors were related to failing system
calls, but virReportSystemError was not used. So the only
useful piece of info (the errno) was being discarded
Update the linux bridge driver to use virReportError instead
of the networkReportError custom macro
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Update the network filter driver to use virReportError instead
of the virNWFilterReportError custom macro
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Update the node device driver to use virReportError instead of
the virNodeDeviceReportError custom macro
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Update the secret driver to use virReportError instead of the
virSecretReportError custom macro
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Update the storage driver to use virReportError instead of
the virStorageReportError custom macro
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
This removes nearly all the per-file error reporting macros
from the code in src/util/. A few custom macros remain for the
case, where the file needs to report errors with a variety of
different codes or parameters
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Nearly every source file does something like
#define VIR_FROM_THIS VIR_FROM_FOO
#define virFooReportErorr(code, ...) \
virReportErrorHelper(VIR_FROM_THIS, code, __FILE__, \
__FUNCTION__, __LINE__, \
__VA_ARGS__)
This creates needless duplication and inconsistent error
reporting function names in each file. It is trivial to
just have virterror_internal.h provide a virReportError
macro that is equivalent
* src/util/virterror_internal.h: Define virReportError(code, ...)
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The only useful translation of "%s" as a format string is "%s" (I
suppose you could claim "%1$s" is also valid, but why bother). So
it is not worth translating; fixing this exposes some instances
where we were failing to translate real error messages. This makes
the fix of commit 097da1ab more generic, as well as ensuring no
future regressions.
* cfg.mk (sc_prohibit_useless_translation): New rule.
* src/lxc/lxc_driver.c (lxcSetVcpuBWLive): Fix offender.
* src/openvz/openvz_conf.c (openvzReadFSConf): Likewise.
* src/qemu/qemu_cgroup.c (qemuSetupCgroupForVcpu): Likewise.
* src/qemu/qemu_driver.c (qemuSetVcpusBWLive): Likewise.
* src/xenapi/xenapi_utils.c (xenapiSessionErrorHandle): Likewise.
I noticed this during 'make syntax-check':
prohibit_readlink
grep: Unmatched ( or \(
* cfg.mk (exclude_file_name_regexp--sc_prohibit_readlink): Fix
mismatched '('.
Currently, we either generate some cmd*Edit commands (cmdPoolEdit
and cmdNetworkEdit) via sed script or copy the body of cmdEdit
(e.g. cmdInterfaceEdit, cmdNWFilterEdit, etc.). This fact makes
it harder to implement any new feature to our editing system.
Therefore switch to new implementation - define macros to:
- dump XML (EDIT_GET_XML)
- take an action if XML wasn't changed,
usually just vshPrint() (EDIT_NOT_CHANGED)
- define new object (EDIT_DEFINE) - the edited XML is in @doc_edited
- free object defined by EDIT_DEFINE (EDIT_FREE)
and #include "virsh-edit.c"
strncpy is generally evil - it runs the risk of missing NUL
termination, and more often than not wastes time zeroing way
more bytes than strictly necessary. We've avoided this evil
in our virStrncpy wrapper, except for places where we forgot
to use the wrapper; meanwhile, we have also added an even
higher layer wrapper for setting virTypedParameter values.
* tools/virsh.c (cmdMemtune, cmdBlkdeviotune): Use modern API.
* cfg.mk (exclude_file_name_regexp--sc_prohibit_strncpy): Tighten.
Gnulib finally relaxed the isatty license, needed as first mentioned here:
https://www.redhat.com/archives/libvir-list/2012-February/msg01022.html
Other improvements include better syntax-check rules (we can delete one
of ours now that it is a duplicate) and better compiler warning usage.
* .gnulib: Update to latest, for isatty.
* cfg.mk (sc_prohibit_strncpy): Drop a now-redundant rule.
* bootstrap.conf (gnulib_modules): Add isatty.
* bootstrap: Resync from gnulib.
Remove a number of pointless checks against PATH_MAX and
add a syntax-check rule to prevent its use in future
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The use of readlink() in lxc_container.c is intentional; we don't
want an absolute pathname there.
* src/util/cgroup.h (VIR_CGROUP_SYSFS_MOUNT): Indent properly.
* cfg.mk (exclude_file_name_regexp--sc_prohibit_readlink): Add
exemption.
Thanks to this new option we are now able to use modern CPU models (such
as Westmere) defined in external configuration file.
The qemu-1.1{,-device} data files for qemuhelptest are filled in with
qemu-1.1-rc2 output for now. I will update those files with real
qemu-1.1 output once it is released.
Test 2 data grabbed from a 2-core 1-node laptop.
Test 3 data grabbed from a 48-cpu AMD Magny Cours box.
* tests/nodeinfodata/linux-nodeinfo-sysfs-test-2*: New test data.
* tests/nodeinfodata/linux-nodeinfo-sysfs-test-3*: Likewise.
* tests/nodeinfotest.c (mymain): Run them.
* cfg.mk
(exclude_file_name_regexp--sc_prohibit_empty_lines_at_EOF): Exempt
new test files.
Otherwise, a string such as _("Don't use \"" VAR "\".") would
complain about unmarked diagnostics.
* cfg.mk (sc_libvirt_unmarked_diagnostics): Handle \" in message.
Ensure we don't introduce any more lousy integer parsing in new
code, while avoiding a scrub-down of existing legacy code.
Note that we also need to enable sc_prohibit_atoi_atof (see cfg.mk
local-checks-to-skip) before we are bulletproof, but that also
entails scrubbing I'm not ready to do at the moment.
* src/util/util.c (virStrToLong_i, virStrToLong_ui)
(virStrToLong_l, virStrToLong_ul, virStrToLong_ll)
(virStrToLong_ull, virStrToDouble): Mark exemptions.
* src/util/virmacaddr.c (virMacAddrParse): Likewise.
* cfg.mk (sc_prohibit_strtol): New syntax check.
(exclude_file_name_regexp--sc_prohibit_strtol): Ignore files that
I'm not willing to fix yet.
(local-checks-to-skip): Re-enable sc_prohibit_atoi_atof.
We are so close to a release that we don't want to pull in a
gnulib submodule update and risk regressions, since there has
been a lot of other gnulib churn upstream. However, there are
a couple of gnulib issues that are worth fixing in isolation,
by applying local patches to gnulib.
There was an upstream gnulib bug in maint.mk that rendered most
of our syntax checks ineffective (and fixing it flushed out a
minor bug in our code):
https://lists.gnu.org/archive/html/bug-gnulib/2012-03/msg00194.html
There is still an upstream bug where gnulib uses the wrong type
for ssize_t on mingw; we need the fix now even though it has not
yet been accepted into gnulib:
https://lists.gnu.org/archive/html/bug-gnulib/2012-03/msg00188.html
* gnulib/local/top/maint.mk.diff: Pick up upstream gnulib
maint.mk.
* gnulib/local/m4/ssize_t.m4.diff: Work around gnulib bug.
* src/libvirt.c: Remove unused header.
* cfg.mk
(exclude_file_name_regexp--sc_prohibit_empty_lines_at_EOF): Exempt
gnulib local files.
With latest gnulib we are checking even the lowest level functions
whether they check flags. Moreover, we are shadowing the real error
on system without TUNSETIFF support.
The code is splattered with a mix of
sizeof foo
sizeof (foo)
sizeof(foo)
Standardize on sizeof(foo) and add a syntax check rule to
enforce it
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
After cleanup introduced with previous commit, there is a need for
syntax-check rule taking care of return(). Regexp used in 'prohibit'
parameter is taken from the cleanup commit and modified so it fits
'grep -E' format. Semicolon at the end is needed, otherwise the regexp
could match return with cast.
Exception is created for python source files because we don't have any
documentation restricting the use of return that matches this case.
No thanks to 64-bit windows, with 64-bit pid_t, we have to avoid
constructs like 'int pid'. Our API in libvirt-qemu cannot be
changed without breaking ABI; but then again, libvirt-qemu can
only be used on systems that support UNIX sockets, which rules
out Windows (even if qemu could be compiled there) - so for all
points on the call chain that interact with this API decision,
we require a different variable name to make it clear that we
audited the use for safety.
Adding a syntax-check rule only solves half the battle; anywhere
that uses printf on a pid_t still needs to be converted, but that
will be a separate patch.
* cfg.mk (sc_correct_id_types): New syntax check.
* src/libvirt-qemu.c (virDomainQemuAttach): Document why we didn't
use pid_t for pid, and validate for overflow.
* include/libvirt/libvirt-qemu.h (virDomainQemuAttach): Tweak name
for syntax check.
* src/vmware/vmware_conf.c (vmwareExtractPid): Likewise.
* src/driver.h (virDrvDomainQemuAttach): Likewise.
* tools/virsh.c (cmdQemuAttach): Likewise.
* src/remote/qemu_protocol.x (qemu_domain_attach_args): Likewise.
* src/qemu_protocol-structs (qemu_domain_attach_args): Likewise.
* src/util/cgroup.c (virCgroupPidCode, virCgroupKillInternal):
Likewise.
* src/qemu/qemu_command.c(qemuParseProcFileStrings): Likewise.
(qemuParseCommandLinePid): Use pid_t for pid.
* daemon/libvirtd.c (daemonForkIntoBackground): Likewise.
* src/conf/domain_conf.h (_virDomainObj): Likewise.
* src/probes.d (rpc_socket_new): Likewise.
* src/qemu/qemu_command.h (qemuParseCommandLinePid): Likewise.
* src/qemu/qemu_driver.c (qemudGetProcessInfo, qemuDomainAttach):
Likewise.
* src/qemu/qemu_process.c (qemuProcessAttach): Likewise.
* src/qemu/qemu_process.h (qemuProcessAttach): Likewise.
* src/uml/uml_driver.c (umlGetProcessInfo): Likewise.
* src/util/virnetdev.h (virNetDevSetNamespace): Likewise.
* src/util/virnetdev.c (virNetDevSetNamespace): Likewise.
* tests/testutils.c (virtTestCaptureProgramOutput): Likewise.
* src/conf/storage_conf.h (_virStoragePerms): Use mode_t, uid_t,
and gid_t rather than int.
* src/security/security_dac.c (virSecurityDACSetOwnership): Likewise.
* src/conf/storage_conf.c (virStorageDefParsePerms): Avoid
compiler warning.
Make it easier to detect invalid cross-directory includes, by
adding a syntax check. The check is designed to be extensible:
the default case lists only the non-driver directories, and
specific directories can list a different set (for example,
util/ can only use itself, network/ can only use itself, util/,
or conf/).
* .gnulib: Update to latest, for syntax check improvment.
* cfg.mk (sc_prohibit_cross_inclusion): New check.
(sc_prohibit_strncmp, sc_libvirt_unmarked_diagnostics): Simplify.
It's been a while, and we're between releases, so now's as good
a time as any to resync. This also fixes a build-breaker on
cygwin, where cygwin 1.7.11 introduced a header bug in <termios.h>.
* .gnulib: Update to latest.
* bootstrap: Resync.
* cfg.mk (sc_prohibit_strncmp): Copy upstream changes to
sc_prohibit_strcmp.
Function xmlParseURI does not remove square brackets around IPv6
address when parsing. One of the solutions is making wrappers around
functions working with xmlURI*. This assures that uri->server will be
always properly assigned and it doesn't have to be changed when used
on some new place in the code.
For this purpose, functions virParseURI and virSaveURI were
added. These function are wrappers around xmlParseURI and xmlSaveUri
respectively.
Also there is one new syntax check function to prohibit these functions
anywhere else.
File changes:
- src/util/viruri.h -- declaration
- src/util/viruri.c -- definition
- src/libvirt_private.syms -- symbol export
- src/Makefile.am -- added source and header files
- cfg.mk -- added sc_prohibit_xmlURI
- all others -- ID name and include fixes
This patch starts the process of elevating the python binding code
to be on the same level as the rest of libvirt when it comes to
requiring good coding styles. Statically linking against the
libvirt_util library makes it much easier to write good code,
rather than having to open-code and reinvent things locally.
Done by global search and replace of s/free(/VIR_FREE(/, followed
by hand-inspection of remaining malloc and redundant memset.
* cfg.mk (exclude_file_name_regexp--sc_prohibit_raw_allocation):
Remove python from exemption.
* python/Makefile.am (INCLUDES): Add gnulib and src/util. Drop
$(top_builddir)/$(subdir), as automake already guarantees that.
(mylibs, myqemulibs): Pull in libvirt_util and gnulib.
(libvirtmod_la_CFLAGS): Catch compiler warnings if configured to
use -Werror.
* python/typewrappers.c (libvirt_charPtrSizeWrap)
(libvirt_charPtrWrap): Convert free to VIR_FREE.
* python/generator.py (print_function_wrapper): Likewise.
* python/libvirt-override.c: Likewise.
Our HACKING discourages use of malloc and free, for at least
a couple of years now. But we weren't enforcing it, until now :)
For now, I've exempted python and tests, and will clean those up
in subsequent patches. Examples should be permanently exempt,
since anyone copying our examples won't have use of our
internal-only memory.h via libvirt_util.la.
* cfg.mk (sc_prohibit_raw_allocation): New rule.
(exclude_file_name_regexp--sc_prohibit_raw_allocation): and
exemptions.
* src/cpu/cpu.c (cpuDataFree): Avoid false positive.
* src/conf/network_conf.c (virNetworkDNSSrvDefParseXML): Fix
offenders.
* src/libxl/libxl_conf.c (libxlMakeDomBuildInfo, libxlMakeVfb)
(libxlMakeDeviceModelInfo): Likewise.
* src/rpc/virnetmessage.c (virNetMessageSaveError): Likewise.
* tools/virsh.c (_vshMalloc, _vshCalloc): Likewise.
When converting a linear enum to a string, we have checks in
place in the VIR_ENUM_IMPL macro to ensure that there is one
string for every value, which lets us quickly flag if a user
added a value but forgot to add a counterpart string. However,
this only works if we use the _LAST marker.
* cfg.mk (sc_require_enum_last_marker): New syntax check.
* src/conf/domain_conf.h (virDomainSnapshotState): Add new marker.
* src/conf/domain_conf.c (virDomainSnapshotState): Fix offender.
* src/qemu/qemu_monitor_json.c (qemuMonitorWatchdogAction)
(qemuMonitorIOErrorAction, qemuMonitorGraphicsAddressFamily):
Likewise.
* src/util/virtypedparam.c (virTypedParameter): Likewise.
* .gnulib: Update to latest, for improved 'make syntax-check' and
compiler warnings.
* m4/virt-compile-warnings.m4 (LIBVIRT_COMPILE_WARNINGS):
Re-silence -Wformat-nonliteral.
* cfg.mk (_test_script_regex): Recognize our test scripts.
* gnulib/local/lib/*.diff: Drop, now that gnulib has this.
* tests/virsh-optparse: Fix use of compare.
* tests/virsh-schedinfo: Likewise.
Commit f7bd00c12 pulled in a gnulib module that fails to compile
on mingw. While it would be nice to pull in a newer version of
.gnulib that fixes this, it is difficult to backport any .gnulib
update to older releases. So, it makes sense to take advantage
of gnulib-tool's ability to support local diffs, where we can
apply specific diffs in our use of gnulib without waiting for
upstream gnulib to pick up those changes, as well as avoiding
a wholesale .gnulib update. The existence of local diffs will
also make it easier to backport fixes against a tarball (as long
as a tarball and libvirt.git share the same .gnulib commit, then
the tarball can be patched by applying the same local diffs as
a post-release libvirt.git commit, without having to rerun an
entire gnulib-tool bootstrap).
This patch introduces the framework for supporting local diffs,
without actually introducing any.
* bootstrap.conf (local_gl_dir): New variable.
* autogen.sh (bootstrap_hash): Hash any local diffs, to force a
re-bootstrap if just diffs change.
* cfg.mk (_update_required): Likewise.
The current I/O code for LXC uses a hand crafted event loop
to forward I/O between the container & host app, based on
epoll to handle EOF on PTYs. This event loop is not easily
extensible to add more consoles, or monitor other types of
file descriptors.
Remove the custom event loop and replace it with a normal
libvirt event loop. When detecting EOF on a PTY, disable
the event watch on that FD, and fork off a background thread
that does a edge-triggered epoll() on the FD. When the FD
finally shows new incoming data, the thread re-enables the
watch on the FD and exits.
When getting EOF from a read() on the PTY, the existing code
would do waitpid(WNOHANG) to see if the container had exited.
Unfortunately there is a race condition, because even though
the process has closed its stdio handles, it might still
exist.
To deal with this the new event loop uses a SIG_CHILD handler
to perform the waitpid only when the container is known to
have actually exited.
* src/lxc/lxc_controller.c: Rewrite the event loop to use
the standard APIs.
This removes the old example for legacy probes and adds two
new scripts demonstrating many of the new probe point facilities.
The rpc-monitor.stp script will print out friendly details of all
RPC traffic between a libvirt client/server. This is incredibly
useful in seeing what RPC calls are being made, and also debugging
problems in the RPC protocol code
The events.stp script will print out lots of info about the poll
event loop, which is useful for debugging event handling problems
* examples/systemtap/events.stp, examples/systemtap/rpc-monitor.stp:
New examples
* examples/systemtap/client.stp: Remove obsolete example
The libvirtd daemon had a few crude system tap probes. Some of
these were broken during the RPC rewrite. The new modular RPC
code is structured in a way that allows much more effective
tracing. Instead of trying to hook up the original probes,
define a new set of probes for the RPC and event code.
The master probes file is now src/probes.d. This contains
probes for virNetServerClientPtr, virNetClientPtr, virSocketPtr
virNetTLSContextPtr and virNetTLSSessionPtr modules. Also add
probes for the poll event loop.
The src/dtrace2systemtap.pl script can convert the probes.d
file into a libvirt_probes.stp file to make use from systemtap
much simpler.
The src/rpc/gensystemtap.pl script can generate a set of
systemtap functions for translating RPC enum values into
printable strings. This works for all RPC header enums (program,
type, status, procedure) and also the authentication enum
The PROBE macro will automatically generate a VIR_DEBUG
statement, so any place with a PROBE can remove any existing
manual DEBUG statements.
* daemon/libvirtd.stp, daemon/probes.d: Remove obsolete probing
* daemon/libvirtd.h: Remove probe macros
* daemon/Makefile.am: Remove all probe buildings/install
* daemon/remote.c: Update authentication probes
* src/dtrace2systemtap.pl, src/rpc/gensystemtap.pl: Scripts
to generate STP files
* src/internal.h: Add probe macros
* src/probes.d: Master list of probes
* src/rpc/virnetclient.c, src/rpc/virnetserverclient.c,
src/rpc/virnetsocket.c, src/rpc/virnettlscontext.c,
src/util/event_poll.c: Insert probe points, removing any
DEBUG statements that duplicate the info
Bug introduced in commit 675464b. On an OOM, this would try to
dereference a char* and free the contents as a pointer, which is
doomed to failure.
Adding a syntax check will prevent mistakes like this in the future.
* cfg.mk (sc_prohibit_internal_functions): New syntax check.
(exclude_file_name_regexp--sc_prohibit_internal_functions): Add
exemptions.
* daemon/remote.c (remoteRelayDomainEventIOError)
(remoteRelayDomainEventIOErrorReason)
(remoteRelayDomainEventGraphics, remoteRelayDomainEventBlockJob):
Use correct free function.
When libvirt calls virInitialize it creates a thread local
for the virErrorPtr storage, and registers a callback to
cleanup memory when a thread exits. When libvirt is dlclose()d
or otherwise made non-resident, the callback function is
removed from memory, but the thread local may still exist
and if a thread later exists, it will invoke the callback
and SEGV. There may also be other thread locals with callbacks
pointing to libvirt code, so it is in general never safe to
unload libvirt.so from memory once initialized.
To allow dlclose() to succeed, but keep libvirt.so resident
in memory, link with '-z nodelete'. This issue was first
found with the libvirt CIM provider, but can potentially
hit many of the dynamic language bindings which all ultimately
involve dlopen() in some way, either on libvirt.so itself,
or on the glue code for the binding which in turns links
to libvirt
* configure.ac, src/Makefile.am: Ensure libvirt.so is linked
with -z nodelete
* cfg.mk, .gitignore, tests/Makefile.am, tests/shunloadhelper.c,
tests/shunloadtest.c: A test case to unload libvirt while
a thread is still running.
Commit 2a0d75e5 added file python/libvirt-qemu-override.c that contains
code that does not pass "make syntax-check". This patch adds an
exception for this file and the check.
prohibit_always_true_header_tests
python/libvirt-qemu-override.c:17:#undef HAVE_PTHREAD_H
maint.mk: do not test the above HAVE_<header>_H symbol(s);
with the corresponding gnulib module, they are always true
make: *** [sc_prohibit_always_true_header_tests] Error 1
These functions parse given XML node and return pointer to the
output. Unknown elements are silently ignored. Attributes must
be integer and must fit in unsigned long long.
Free function frees elements of virBandwidth structure.
This test case checks certification validation rules for
- Basic constraints
- Key purpose
- Key usage
- Start/expiry times
It checks initial context creation sanity checks, and live
session validation
O_DIRECT has stringent requirements. Rather than make lots of changes
at each site that wants to use O_DIRECT, it is easier to offload
the work through a helper process that mirrors the I/O between a
pipe and the actual direct fd, so that the other end of the pipe
no longer has to worry about constraints.
Plus, if the kernel ever gains better posix_fadvise support, then we
only have to touch a single file to let all callers benefit from a
more efficient way to avoid file system caching.
* src/util/virfile.h (virFileDirectFdFlag, virFileDirectFdNew)
(virFileDirectFdClose, virFileDirectFdFree): New prototypes.
* src/util/virdirect.c: Implement new wrapper object.
* src/libvirt_private.syms (virfile.h): Export new symbols.
* cfg.mk (useless_free_options): Add to list.
* po/POTFILES.in: Add new translations.
the domain XML <interface> element is updated in the following ways:
1) <virtualportprofile> can be specified when source type='network'
(previously it was only valid for source type='direct')
2) A new attribute "portgroup" has been added to the <source>
element. When source type='network' (the only time portgroup is
recognized), extra configuration information will be taken from the
<portgroup> element of the given name in the network definition.
3) Each virDomainNetDef now also potentially has a
virDomainActualNetDef which is a private object (never
exported/imported via the public API, and not defined in the RNG) that
is used to maintain information about the physical device that was
actually used for a NetDef of type VIR_DOMAIN_NET_TYPE_NETWORK.
The virDomainActualNetDef will only be parsed/formatted if the
parse/format function is called with the
VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET flag set (which is only needed when
saving/loading a running domain's state info to the stateDir).
Enforce the recent flags cleanups - we want to use 'unsigned int flags'
in any of our APIs (except where backwards compatibility is important,
in the public migration APIs), and that all flags are checked for
validity (except when there are stub functions that completely
ignore the flags argument).
There are a few minor tweaks done here to avoid false positives:
signed arguments passed to open() are renamed oflags, and flags
arguments that are legitimately ignored are renamed flags_unused.
* cfg.mk (sc_flags_usage): New rule.
(exclude_file_name_regexp--sc_flags_usage): And a few exemptions.
(sc_flags_debug): Tweak wording.
* src/util/iohelper.c (runIO, main): Rename variable.
* src/util/util.c (virSetInherit): Likewise.
* src/fdstream.h (virFDStreamOpenFile, virFDStreamCreateFile):
Likewise.
* src/fdstream.c (virFDStreamOpenFileInternal)
(virFDStreamOpenFile, virFDStreamCreateFile): Likewise.
* src/util/command.c (virExecWithHook) [WIN32]: Likewise.
* src/util/util.c (virFileOpenAs, virDirCreate) [WIN32]: Likewise.
* src/locking/lock_manager.c (virLockManagerPluginNew)
[!HAVE_DLFCN_H]: Likewise.
* src/locking/lock_driver_nop.c (virLockManagerNopNew)
(virLockManagerNopAddResource, virLockManagerNopAcquire)
(virLockManagerNopRelease, virLockManagerNopInquire): Likewise.
Getting metadata on storage allocates a memory (path) which need to
be freed after use otherwise it gets leaked. This means after use of
virStorageFileGetMetadataFromFD or virStorageFileGetMetadata one
must call virStorageFileFreeMetadata to free it. This function frees
structure internals and structure itself.
The compiler might optimize based on our declaration that something
is unused. Putting that declaration in the header risks getting
out of sync with the actual implementation, so it belongs better
only in the .c files. We were mostly compliant, and a new syntax
check will help us in the future.
* cfg.mk (sc_avoid_attribute_unused_in_header): New syntax check.
* src/nodeinfo.h (nodeGetCPUStats, nodeGetMemoryStats): Delete
attribute already present in .c file.
* src/qemu/qemu_domain.h (qemuDomainEventFlush): Likewise.
* src/util/virterror_internal.h (virReportErrorHelper): Parameters
are actually used by .c file.
* src/xenxs/xen_sxpr.h (xenFormatSxprDisk): Adjust prototype.
* src/xenxs/xen_sxpr.c (xenFormatSxprDisk): Delete unused argument.
(xenFormatSxpr): Adjust caller.
* src/xen/xend_internal.c (xenDaemonAttachDeviceFlags)
(xenDaemonUpdateDeviceFlags): Likewise.
Suggested by Daniel Veillard.
