Commit Graph

266 Commits

Author SHA1 Message Date
Eric Blake
1add9c78da maint: don't use config.h in .h files
Enforce the rule that .h files don't need to (redundantly)
include <config.h>.

* cfg.mk (sc_prohibit_config_h_in_headers): New rule.
(_virsh_includes): Delete; instead, inline a smaller number of
exclusions...
(exclude_file_name_regexp--sc_require_config_h)
(exclude_file_name_regexp--sc_require_config_h_first): ...here.
* daemon/libvirtd.h (includes): Fix offenders.
* src/driver.h (includes): Likewise.
* src/gnutls_1_0_compat.h (includes): Likewise.
* src/libxl/libxl_conf.h (includes): Likewise.
* src/libxl/libxl_driver.h (includes): Likewise.
* src/lxc/lxc_conf.h (includes): Likewise.
* src/lxc/lxc_driver.h (includes): Likewise.
* src/lxc/lxc_fuse.h (includes): Likewise.
* src/network/bridge_driver.h (includes): Likewise.
* src/phyp/phyp_driver.h (includes): Likewise.
* src/qemu/qemu_conf.h (includes): Likewise.
* src/util/virnetlink.h (includes): Likewise.

Signed-off-by: Eric Blake <eblake@redhat.com>
2013-06-05 05:53:25 -06:00
Eric Blake
0c8926daf9 syntax: fix broken error message in previous patch
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>
2013-05-28 09:52:03 -06:00
Eric Blake
134e685b1d syntax-check: mandate space after mid-line semicolon
Enforce the style cleanup in the previous patch.

* build-aux/bracket-spacing.pl: Enforce trailing spacing.
* cfg.mk (bracket-spacing-check): Tweak error wording.
* docs/hacking.html.in: Document the rule.
* HACKING: Regenerate.

Signed-off-by: Eric Blake <eblake@redhat.com>
2013-05-28 08:26:05 -06:00
Michal Privoznik
d1527ba5d7 Introduce syntax-check rule to prefer VIR_STRDUP over strdup 2013-05-24 10:13:02 +02:00
Eric Blake
0e55024e7b maint: enforce correct copyright usage
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>
2013-05-20 14:50:12 -06:00
Eric Blake
de483052a2 maint: follow recommended practice for using LGPL
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>
2013-05-20 14:15:21 -06:00
Daniel P. Berrange
8845d8dfa3 Remove & ban use of select() for waiting for I/O
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>
2013-05-13 17:32:43 +01:00
Laine Stump
bfe7721d50 util: move virFile* functions from virutil.c to virfile.c
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.
2013-05-10 13:09:30 -04:00
Ján Tomko
605a077244 syntax-check: forbid virBufferAsprintf with string literals 2013-05-07 17:49:59 +02:00
Eric Blake
25ae3d3015 build: avoid useless virAsprintf
virAsprintf(&foo, "%s", bar) is wasteful compared to
foo = strdup(bar) (or eventually, VIR_STRDUP(foo, bar),
but one thing at a time...).

Noticed while reviewing Laine's attempt to clean up broken
qemu:///session.

* cfg.mk (sc_prohibit_asprintf): Enhance rule.
* src/esx/esx_storage_backend_vmfs.c
(esxStorageBackendVMFSVolumeLookupByKey): Fix offender.
* src/network/bridge_driver.c (networkStateInitialize): Likewise.
* src/nwfilter/nwfilter_dhcpsnoop.c (virNWFilterSnoopDHCPOpen):
Likewise.
* src/storage/storage_backend_sheepdog.c
(virStorageBackendSheepdogRefreshVol): Likewise.
* src/util/vircgroup.c (virCgroupAddTaskStrController): Likewise.
* src/util/virdnsmasq.c (addnhostsAdd): Likewise.
* src/xen/block_stats.c (xenLinuxDomainDeviceID): Likewise.
* src/xen/xen_driver.c (xenUnifiedConnectOpen): Likewise.
* tools/virsh.c (vshGetTypedParamValue): Likewise.

Signed-off-by: Eric Blake <eblake@redhat.com>
2013-05-02 13:35:26 -06:00
Michal Privoznik
7c9a2d88cd virutil: Move string related functions to virstring.c
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.
2013-05-02 16:56:55 +02:00
Eric Blake
1fbf190554 build: avoid unsafe functions in libgen.h
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>
2013-04-25 14:47:01 -06:00
Osier Yang
9969ade669 syntax-check: Only allows to include public headers in external tools
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.
2013-04-18 11:31:19 +08:00
Osier Yang
1d69c6334b syntax-check: Don't include public headers in internal source
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.
2013-04-18 11:24:46 +08:00
Osier Yang
83d801a7e6 syntax-check: Don't include duplicate header
gnulib is excluded.
2013-04-18 11:19:19 +08:00
Daniel P. Berrange
d14524701a Add a test suite for cgroups functionality
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>
2013-04-15 17:35:31 +01:00
Eric Blake
7af86379ef util: portably check for unchanged uid
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.
2013-03-15 10:55:51 -06:00
Eric Blake
dce95297e3 tests: uniformly report test failures
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.
2013-02-25 17:38:11 -07:00
John Ferlan
a141a43e33 hacking: Add some details to handle Valgrind output
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
2013-02-07 14:08:14 -05:00
Guido Günther
7f5c285082 Add syntax-check to make sure Python files don't contain trailing semicolon
We allow for a trailing semicolon in full line comments since
docs/index.py has some SQL statements in it.
2013-02-07 19:52:49 +01:00
Peter Krempa
2198dadc2f syntax-check: Don't check non-reentrant functions in docs
Otherwise constructions like "random (used by default)" end up breaking
syntax check by apparently using non-reentrant functions.
2013-02-07 18:00:47 +01:00
John Ferlan
97278ab472 vbox: Adjust the UTF FREE macros
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.
2013-01-25 16:16:35 -07:00
Eric Blake
c8f79c9b29 spec: indent %if to make it easier to see conditions
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.
2013-01-21 10:36:14 -07:00
Daniel P. Berrange
3d1596b048 Introduce an LXC specific public API & library
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>
2013-01-14 13:58:34 +00:00
Daniel P. Berrange
907a39e735 Add a test suite for validating SELinux labelling
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>
2013-01-14 13:40:04 +00:00
Daniel P. Berrange
cd699ed150 Add some autoconf helper macros for checking for libraries
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>
2013-01-11 11:03:22 +00:00
John Ferlan
fef8d1a010 util: Check for NULL 'dev' on input to usbFreeDevice
Added 'usbFreeDevice' to the useless_free_options list in cfg.mk
2013-01-08 08:45:40 -07:00
Daniel P. Berrange
f24404a324 Rename virterror.c virterror_internal.h to virerror.{c,h} 2012-12-21 11:19:50 +00:00
Daniel P. Berrange
556cf5f617 Rename xml.{c,h} to virxml.{c,h} 2012-12-21 11:19:50 +00:00
Daniel P. Berrange
44f6ae27fe Rename util.{c,h} to virutil.{c,h} 2012-12-21 11:19:49 +00:00
Daniel P. Berrange
226ad9815a Rename sexpr.{c,h} to virsexpr.{c,h} 2012-12-21 11:19:48 +00:00
Daniel P. Berrange
ab9b7ec2f6 Rename memory.{c,h} to viralloc.{c,h} 2012-12-21 11:17:14 +00:00
Daniel P. Berrange
04d9510f50 Rename command.{c,h} to vircommand.{c,h} 2012-12-21 11:17:13 +00:00
Viktor Mihajlovski
347a712ab1 tests: Add tests for sysinfo
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>
2012-12-17 17:37:17 +00:00
Daniel P. Berrange
ad39fd83a8 Define a wire protocol for talking to the virtlockd daemon
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>
2012-12-13 15:26:57 +00:00
Daniel P. Berrange
c57e3d8994 Introduce basic infrastructure for virtlockd daemon
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>
2012-12-13 15:26:57 +00:00
Laine Stump
85b22f528f util: add VIR_(APPEND|INSERT|DELETE)_ELEMENT
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.
2012-12-11 05:49:44 -05:00
Eric Blake
55dc872bd8 build: fix incremental autogen.sh when no AUTHORS is present
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.
2012-12-03 14:59:09 -07:00
Eric Blake
71d125620d build: rerun bootstrap if AUTHORS is missing
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.
2012-11-14 13:41:15 -07:00
Daniel P. Berrange
a3e95abeb5 Document bracket whitespace rules & add syntax-check rule
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>
2012-11-02 14:00:32 +00:00
Jiri Denemark
d055498d04 build: Do not ignore logging.c in sc_prohibit_mkstemp
Now that the offending code was removed, we may remove this as well.
2012-11-02 12:32:51 +01:00
Michal Privoznik
30b398d5ef logging.c: Properly indent and ignore one syntax-check rule
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.
2012-11-02 11:19:04 +01:00
Eric Blake
4dbd6e9654 build: prefer mkostemp for multi-thread safety
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.
2012-10-31 10:06:10 -06:00
Laine Stump
7bafe009d9 util: do a better job of matching up pids with their binaries
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.
2012-10-30 13:28:47 -04:00
Cole Robinson
7b21981cdb Autogenerate AUTHORS
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.
2012-10-19 12:44:56 -04:00
Eric Blake
c5f162200c build: avoid infinite autogen loop
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.
2012-10-01 09:47:38 -06:00
Martin Kletzander
9ac287f826 syntax-check: fix run.in
Two more problems in "run.in" made the syntax-check fail.
2012-09-18 13:59:53 +02:00
Eric Blake
2387aa26c1 maint: fix missing spaces in message
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.
2012-09-12 11:55:29 -06:00
Daniel P. Berrange
8d78fd04be Add helper library for testing the qemu monitor code
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>
2012-09-07 13:18:04 +01:00
Eric Blake
60efb60018 maint: avoid doubled name in syntax check failures
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).
2012-09-05 11:16:20 -06:00