Currently, we are allocating buffer for RPC messages statically.
This is not such pain when RPC limits are small. However, if we want
ever to increase those limits, we need to allocate buffer dynamically,
based on RPC message len (= the first 4 bytes). Therefore we will
decrease our mem usage in most cases and still be flexible enough in
corner cases.

We had a distributed file (remote_protocol.h, which in turn was
a prereq to remote_driver.c) depending on a generated file
(libvirt_probes.h), which is a no-no for a VPATH build from a
read-only source tree (no wonder 'make distcheck' tests precisely
that situation):

     File `libvirt_driver_remote.la' does not exist.
       File `libvirt_driver_remote_la-remote_driver.lo' does not exist.
             Prerequisite `libvirt_probes.h' is newer than target `../../src/remote/remote_protocol.h'.
            Must remake target `../../src/remote/remote_protocol.h'.
Invoking recipe from Makefile:7464 to update target `../../src/remote/remote_protocol.h'.
make[3]: Entering directory `/home/remote/eblake/libvirt-tmp2/build/libvirt-0.9.12/_build/src'
  GEN    ../../src/remote/remote_protocol.h
cannot create ../../src/remote/remote_protocol.h: Permission denied at ../../src/rpc/genprotocol.pl line 31.
make[3]: *** [../../src/remote/remote_protocol.h] Error 13

Rather than making distributed .c files depend on generated files, we
really want to ensure that compilation into .lo files is not attempted
until the generated files are present, done by this patch.  Since there
were two different sets of conditionally generated files that both
feed the .lo file, I had to introduce a new variable REMOTE_DRIVER_PREREQS
to keep automake happy.

After that fix, the next issue was that make treats './foo' and 'foo'
differently in determining whether an implicit %foo rule is applicable,
with the result that locking/qemu-sanlock.conf wasn't properly being
built at the right times.  Also, the output for using the .aug test
files was a bit verbose.

After fixing the src directory, the next error is related to the docs
directory, where the tarball is missing a stamp file and thus tries to
regenerate files that are already present:

  GEN    ../../docs/apibuild.py.stamp
Traceback (most recent call last):
  File "../../docs/apibuild.py", line 2511, in <module>
    rebuild("libvirt")
  File "../../docs/apibuild.py", line 2495, in rebuild
    builder.serialize()
  File "../../docs/apibuild.py", line 2424, in serialize
    output = open(filename, "w")
IOError: [Errno 13] Permission denied: '../../docs/libvirt-api.xml'
make[5]: *** [../../docs/apibuild.py.stamp] Error 1

and fixing that exposed another case of a distributed file (generated
html) depending on a built file (libvirt.h), but only when doing an
in-tree build, because of a file glob.

* src/Makefile.am ($(srcdir)/remote/remote_driver.c): Change...
(libvirt_driver_remote_la-remote_driver.lo): ...to the real
dependency.
($(builddir)/locking/%-sanlock.conf): Drop $(builddir), so that
rule gets run in time for test_libvirt_sanlock.aug.
(test_libvir*.aug): Cater to silent build.
(conf_DATA): Don't ship qemu-sanlock.conf in the tarball, since it
is trivial to regenerate.
* docs/Makefile.am (EXTRA_DIST): Ship our stamp file.
($(APIBUILD_STAMP)): Don't depend on generated file.

I came across a bug that the command line generated for passthrough
of the host parallel port /dev/parport0 by libvirt for QEMU is incorrect.

It currently produces:
-chardev tty,id=charparallel0,path=/dev/parport0
-device isa-parallel,chardev=charparallel0,id=parallel0

The first parameter is "tty". It sould be "parport".

If I launch qemu with -chardev parport,... it works as expected.

I have already filled a bug report (
https://bugzilla.redhat.com/show_bug.cgi?id=823879 ), the topic was
already on the list some months ago:

https://www.redhat.com/archives/libvirt-users/2011-September/msg00095.htmlSigned-off-by: NEric Blake <eblake@redhat.com>

Noticed during the previous commit.

* src/util/command.c: Fix some spacing and break long lines.

It is possible to deadlock libvirt by having a domain with XML
longer than PIPE_BUF, and by writing a hook script that closes
stdin early.  This is because libvirt was keeping a copy of the
child's stdin read fd open, which means the write fd in the
parent will never see EPIPE (remember, libvirt should always be
run with SIGPIPE ignored, so we should never get a SIGPIPE signal).
Since there is no error, libvirt blocks waiting for a write to
complete, even though the only reader is also libvirt.  The
solution is to ensure that only the child can act as a reader
before the parent does any writes; and then dealing with the
fallout of dealing with EPIPE.

Thankfully, this is not a security hole - since the only way to
trigger the deadlock is to install a custom hook script, anyone
that already has privileges to install a hook script already has
privileges to do any number of other equally disruptive things
to libvirt; it would only be a security hole if an unprivileged
user could install a hook script to DoS a privileged user.

* src/util/command.c (virCommandRun): Close parent's copy of child
read fd earlier.
(virCommandProcessIO): Don't let EPIPE be fatal; the child may
be done parsing input.
* tests/commandhelper.c (main): Set up a SIGPIPE situation.
* tests/commandtest.c (test20): Trigger it.
* tests/commanddata/test20.log: New file.

Although src/util/viratomic.h has been added to the repo, up until now
it hasn't been used. Stefan Berger is using it in his proposed dhcp
snooping patches, and an rpm build with those patches failed due to
viratomic.h not being packed up with the rest of the sources.

EBADF errors are logged as warnings as they normally indicate a double
close bug. This patch also provides VIR_MASS_CLOSE helper to be user in
the only case of mass close after fork when EBADF should rather be
ignored.

Probably a result of copy&paste...

The stream lock is unlocked twice instead of being locked and then
unlocked. Probably a typo.

With support for multiple IP addresses per interface in place, this patch
now adds support for multiple IP addresses per interface for the DHCP
snooping code.


Testing:

Since the infrastructure I tested this with does not provide multiple IP
addresses per MAC address (anymore), I either had to plug the VM's interface
from the virtual bride connected directly to the infrastructure to virbr0
to get a 2nd IP address from dnsmasq (kill and run dhclient inside the VM)
or changed the lease file  (/var/run/libvirt/network/nwfilter.leases) and
restart libvirtd to have a 2nd IP address on an existing interface.
Note that dnsmasq can take a lease timeout parameter as part of the --dhcp-range
command line parameter, so that timeouts can be tested that way
(--dhcp-range 192.168.122.2,192.168.122.254,120). So, terminating and restarting
dnsmasq with that parameter is another choice to watch an IP address disappear
after 120 seconds.

Regards,
   Stefan

The goal of this patch is to prepare for support for multiple IP
addresses per interface in the DHCP snooping code.

Move the code for the IP address map that maps interface names to
IP addresses into their own file. Rename the functions on the way
but otherwise leave the code as-is. Initialize this new layer
separately before dependent layers (iplearning, dhcpsnooping)
and shut it down after them.

This patch adds DHCP snooping support to libvirt. The learning method for
IP addresses is specified by setting the "CTRL_IP_LEARNING" variable to one of
"any" [default] (existing IP learning code), "none" (static only addresses)
or "dhcp" (DHCP snooping).

Active leases are saved in a lease file and reloaded on restart or HUP.

The following interface XML activates and uses the DHCP snooping:

    <interface type='bridge'>
      <source bridge='virbr0'/>
      <filterref filter='clean-traffic'>
        <parameter name='CTRL_IP_LEARNING' value='dhcp'/>
      </filterref>
    </interface>

All filters containing the variable 'IP' are automatically adjusted when
the VM receives an IP address via DHCP. However, multiple IP addresses per
interface are silently ignored in this patch, thus only supporting one IP
address per interface. Multiple IP address support is added in a later
patch in this series.
Signed-off-by: NDavid L Stevens <dlstevens@us.ibm.com>
Signed-off-by: NStefan Berger <stefanb@linux.vnet.ibm.com>

Currently, monitoring QEMU virtual machines with standard Unix
sysadmin tools is harder than it has to be. The QEMU command line is
often miles long and mostly redundant, it's hard to tell which process
is which.

This patch reorders the QEMU -name argument to be the first, so it's
immediately visible in "ps x", htop and "atop -c" output.

This patch resolves:

   https://bugzilla.redhat.com/show_bug.cgi?id=827519

The problem is that an interface with type='hostdev' will have an
alias of the form "hostdev%d", while the function that looks through
existing netdevs to determine the name to use for a new addition will
fail if there's an existing entry that does not match the form
"net%d".

This is another of the handful of places that need an exception due to
the hybrid nature of <interface type='hostdev'> (which is not exactly
an <interface> or a <hostdev>, but is both at the same time).

If we migrate to fd, spec->fwdType is not MIGRATION_FWD_DIRECT,
we will close spec->dest.fd.local in qemuMigrationRun(). So we
should set spec->dest.fd.local to -1 in qemuMigrationRun().

Bug present since 0.9.5 (commit 32617617).

We should not set *outfd or *errfd if virExecWithHook() failed
because the caller may close these fds.

Bug present since v0.4.5 (commit 60ed1d2a).

Wen Congyang reported that we have a double-close bug if we fail
virFDStreamOpenInternal, since childfd duplicated one of the fds[]
array contents.  In truth, since we always transfer both members
of fds to other variables, we should close the fds through those
other names, and just use fds[] for pipe().

Bug present since 0.9.0 (commit e886237a).

* src/fdstream.c (virFDStreamOpenFileInternal): Swap scope of
childfd and fds[], to avoid a double close.

KAMEZAWA Hiroyuki reported a nasty double-free bug when virCommand
is used to convert a string into input to a child command.  The
problem is that the poll() loop of virCommandProcessIO would close()
the write end of the pipe in order to let the child see EOF, then
the caller virCommandRun() would also close the same fd number, with
the second close possibly nuking an fd opened by some other thread
in the meantime.  This in turn can have all sorts of bad effects.

The bug has been present since the introduction of virCommand in
commit f16ad06f.

This is based on his first attempt at a patch, at
https://bugzilla.redhat.com/show_bug.cgi?id=823716

* src/util/command.c (_virCommand): Drop inpipe member.
(virCommandProcessIO): Add argument, to avoid closing caller's fd
without informing caller.
(virCommandRun, virCommandNewArgs): Adjust clients.

virCommandRunAsync() will set errfd if it succeed. We should
close it if virFDStreamOpenInternal() fails.

If the system does not support bypass cache, we will close fd,
but it is uninitialized.

Apart from the non-sanlock check build, there is also a little fix for
qemu (EXTRA_DIST had qemu.conf and others inside even if the build was
supposed to be without qemu).

We generate *.aug from *.aug.in by augeas-gentest.pl, so this script
should be included in dist file.

Some of our rules used $(PERL), while others used 'perl'.  Always
using the variable allows a developer to point to a different (often
better) perl than the default one found on $PATH.

* daemon/Makefile.am ($(srcdir)/remote_dispatch.h): s/perl/$(PERL).
* src/Makefile.am ($(srcdir)/remote/remote_client_bodies.h)
(PDWTAGS, %protocol.c, %_probes.stp): Likewise.

Without this fix, a VPATH build (such as used by ./autobuild.sh)
fails with messages like:

make[3]: Entering directory `/home/remote/eblake/libvirt-tmp2/build/daemon'
../../build-aux/augeas-gentest.pl libvirtd.conf ../../daemon/test_libvirtd.aug.in test_libvirtd.aug
cannot read libvirtd.conf: No such file or directory at ../../build-aux/augeas-gentest.pl line 38.

Since the test files are not part of the tarball, we can generate
them into the build dir, but rather than create a subdirectory
just for the test file, it is easier to test them directly in
libvirt.git/src.

* daemon/Makefile.am (AUG_GENTEST): Factor out definition.
(test_libvirtd.aug): Look for correct file.
* src/Makefile.am (AUG_GENTEST): Use $(PERL).
(qemu/test_libvirtd_qemu.aug, lxc/test_libvirtd_lxc.aug)
(locking/test_libvirt_sanlock.aug): Rename to avoid subdirectories.
(check-augeas-qemu, check-augeas-lxc, check-augeas-sanlock): Reflect
location of built tests.
* configure.ac (PERL): Substitute perl.

Currently, we are logging only one side of pipes we
create in virCommandRequireHandshake(); This is enough
in cases where pipe2() returns two consecutive FDs. However,
it is not guaranteed and it may return any FDs.
Therefore, it's wise to log the other ends as well.

When getting number of CPUs the host has assigned, there was always
number "1" returned. Even though all lxc domains with no pinning
launched by libvirt run on all pCPUs (by default, no matter what's the
number), we should at least return the same number as the user
specified when creating the domain.

I added libvirt_qemu_probes.h into BUILT_SOURCES. That makes it
generated, but most probably it is not the clearest way how to do
that, but it fixes the build.

The previous patch fixed an incremental build, but missed that on
a fresh checkout, we now have nothing left that stops make from
nuking libvirt_qemu_probes.o.

* src/Makefile.am ($(libvirt_driver_qemu_la_SOURCES)): Delete,
since this variable is empty.
(.PRECIOUS): Add %_probes.o, so they don't get nuked as an
intermediate by-product after creating %_probes.lo.

The moment you specify a _DEPENDENCIES, older automake (stupidly)
assumes that you will specify _all_ dependencies for that target.
This stupidity has been fixed in automake 1.12, but we cannot rely on
newer automake everywhere.  For libvirt_la_DEPENDENCIES, we took
care of providing the full list, but for libvirt_qemu_la_DEPENDENCIES,
we were missing the dependency on libvirt_qemu_impl.la, which resulted
in a failed build:

make[3]: Entering directory `/home/ajia/Workspace/libvirt/src'
   CCLD   libvirt_driver_qemu.la
libtool: link: `libvirt_qemu_probes.lo' is not a valid libtool object

* src/Makefile.am (libvirt_driver_qemu_la_DEPENDENCIES): Delete;
automake does a better job if it does the entire job.

Otherwise, 'make rpm' fails with:

make[2]: *** No rule to make target `locking/test_libvirt_sanlock.aug.in', needed by `distdir'.  Stop.
make[2]: Leaving directory `/home/dipankar/src/powerpc/libvirt-ppc/src'
make[1]: *** [distdir] Error 1

Reported by Dipankar Sarma.

* src/Makefile.am (CLEANFILES): Clean only the generated file.

==3240== 23 bytes in 1 blocks are definitely lost in loss record 242 of 744
==3240==    at 0x4C2A4CD: malloc (vg_replace_malloc.c:236)
==3240==    by 0x8077537: __vasprintf_chk (vasprintf_chk.c:82)
==3240==    by 0x509C677: virVasprintf (stdio2.h:199)
==3240==    by 0x509C733: virAsprintf (util.c:1912)
==3240==    by 0x1906583A: qemudStartup (qemu_driver.c:679)
==3240==    by 0x511991D: virStateInitialize (libvirt.c:809)
==3240==    by 0x40CD84: daemonRunStateInit (libvirtd.c:751)
==3240==    by 0x5098745: virThreadHelper (threads-pthread.c:161)
==3240==    by 0x7953D8F: start_thread (pthread_create.c:309)
==3240==    by 0x805FF5C: clone (clone.S:115)

src/libvirt.c: s,//,/,

To ensure consistent error reporting of invalid arguments,
provide a number of predefined helper methods & macros.

 - An arg which must not be NULL:

   virCheckNonNullArgReturn(argname, retvalue)
   virCheckNonNullArgGoto(argname, label)

 - An arg which must be NULL

   virCheckNullArgGoto(argname, label)

 - An arg which must be positive (ie 1 or greater)

   virCheckPositiveArgGoto(argname, label)

 - An arg which must not be 0

   virCheckNonZeroArgGoto(argname, label)

 - An arg which must be zero

   virCheckZeroArgGoto(argname, label)

 - An arg which must not be negative (ie 0 or greater)

   virCheckNonNegativeArgGoto(argname, label)

* src/libvirt.c, src/libvirt-qemu.c,
  src/nodeinfo.c, src/datatypes.c: Update to use
  virCheckXXXX macros
* po/POTFILES.in: Add libvirt-qemu.c and virterror_internal.h
* src/internal.h: Define macros for checking invalid args
* src/util/virterror_internal.h: Define macros for reporting
  invalid args
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

Libtool is picky about linking against a module library (aka a .so);
giving lots of warnings like this in the tests directory:

  CCLD   networkxml2argvtest

*** Warning: Linking the executable networkxml2argvtest against the loadable module
*** libvirt_driver_network.so is not portable!

Fix that by splitting things into a convenience library which can
be used directly by the tests, and making the real .so just wrap
the convenience library.

Based on a suggestion by Daniel P. Berrange.

* configure.ac (--with-driver-modules): Fix help test.
* src/Makefile.am (libvirt_driver_xen.la, libvirt_driver_libxl.la)
(libvirt_driver_qemu.la, libvirt_driver_lxc.la)
(libvirt_driver_uml.la): Factor into new convenience libraries.
* tests/Makefile.am (xen_LDADDS, qemu_LDADDS, lxc_LDADDS)
(networkxml2argvtest_LDADD): Link to convenience libraries, not
shared libraries.

There was no rule forcing libvirt_qemu_probes.o to be built
before libvirt_qemu_probes.lo was used. Also libvirtd was
still referencing the .o file, rather than the .lo file.

Both the .lo and .o file must be listed as DEPENDENCIES,
otherwise libtool will unhelpfully delete the .o file
once the .lo file is created.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

The CoTaskMemFree function requires the ole32 DLL to be
linked against. Currently this is only done for the
VirtualBox driver. Also add it to libvirt_util.la

* configure.ac: Unconditionally add ole32 DLL to Win32
* src/Makefile.am: Link old32 to libvirt_util.la

When adding new config file parameters, the corresponding
additions to the augeas lens' are constantly forgotten.
Also there are augeas test cases, these don't catch the
error, since they too are never updated.

To address this, the augeas test cases need to be auto-generated
from the example config files.

* build-aux/augeas-gentest.pl: Helper to generate an
  augeas test file, substituting in elements from the
  example config files
* src/Makefile.am, daemon/Makefile.am: Switch to
  auto-generated augeas test cases
* daemon/test_libvirtd.aug, daemon/test_libvirtd.aug.in,
  src/locking/test_libvirt_sanlock.aug,
  src/locking/test_libvirt_sanlock.aug.in,
  src/lxc/test_libvirtd_lxc.aug,
  src/lxc/test_libvirtd_lxc.aug.in,
  src/qemu/test_libvirtd_qemu.aug,
  src/qemu/test_libvirtd_qemu.aug.in: Remove example
  config file data, replacing with a ::CONFIG:: placeholder
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

Currently all the config options are listed under a 'vnc_entry'
group. Create a bunch of new groups & move options to the
right place
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

Add nmissing 'host_uuid' entry to libvirtd.conf lens and
rename spice_passwd to spice_password in qemu.conf lens
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>