* src/util/threads.h (virThreadID): New prototype.
* src/util/threads-pthread.c (virThreadID): New function.
* src/util/threads-win32.c (virThreadID): Likewise.
* src/libvirt_private.syms (threads.h): Export it.
* daemon/event.c (virEventInterruptLocked): Use it to avoid
warning on BSD systems.
"virCommandRun": if "cmd->outbuf" or "cmd->errbuf" is NULL,
libvirtd will be crashed when trying to start a qemu domain
(which invokes "virCommandRun"), it caused by we try to use
"*cmd->outbuf" and "*cmd->errbuf" regardless of cmd->outbuf
or cmd->errbuf is NULL.
* src/util/command.c (virCommandRun)
This patch adds a mode_t parameter to virFileWriteStr().
If mode is different from 0, virFileWriteStr() will try
to create the file if it doesn't exist.
* src/util/util.h (virFileWriteStr): Alter signature.
* src/util/util.c (virFileWriteStr): Allow file creation.
* src/network/bridge_driver.c (networkEnableIpForwarding)
(networkDisableIPV6): Adjust clients.
* src/node_device/node_device_driver.c
(nodeDeviceVportCreateDelete): Likewise.
* src/util/cgroup.c (virCgroupSetValueStr): Likewise.
* src/util/pci.c (pciBindDeviceToStub, pciUnBindDeviceFromStub):
Likewise.
This proof of concept shows how two existing uses of virExec
and virRun can be ported to the new virCommand APIs, and how
much simpler the code becomes
This introduces a new set of APIs in src/util/command.h
to use for invoking commands. This is intended to replace
all current usage of virRun and virExec variants, with a
more flexible and less error prone API.
* src/util/command.c: New file.
* src/util/command.h: New header.
* src/Makefile.am (UTIL_SOURCES): Build it.
* src/libvirt_private.syms: Export symbols internally.
* tests/commandtest.c: New test.
* tests/Makefile.am (check_PROGRAMS): Run it.
* tests/commandhelper.c: Auxiliary program.
* tests/commanddata/test2.log - test15.log: New expected outputs.
* cfg.mk (useless_free_options): Add virCommandFree.
(msg_gen_function): Add virCommandError.
* po/POTFILES.in: New translation.
* .x-sc_avoid_write: Add exemption.
* tests/.gitignore: Ignore new built file.
The arguments passed to the thread function must be allocated on
the heap, rather than the stack, since it is possible for the
spawning thread to continue before the new thread runs at all.
In such a case, it is possible that the area of stack where the
thread args were stored is overwritten.
* src/util/threads-pthread.c, src/util/threads-win32.c: Allocate
thread arguments on the heap
Use macvtap specific functions depending on WITH_MACVTAP.
Use #if instead of #ifdef to check for WITH_MACVTAP, because
WITH_MACVTAP is always defined with value 0 or 1.
Also export virVMOperationType{To|From}String unconditional,
because they are used unconditional in the domain config code.
This patch introduces the usage of the pre-associate state of the IEEE 802.1Qbg standard on incoming VM migration on the target host. It is in response to bugzilla entry 632750.
https://bugzilla.redhat.com/show_bug.cgi?id=632750
For being able to differentiate the exact reason as to why a macvtap device is being created, either due to a VM creation or an incoming VM migration, I needed to pass that reason as a parameter from wherever qemudStartVMDaemon is being called in order to determine whether to send an ASSOCIATE (VM creation) or a PRE-ASSOCIATE (incoming VM migration) towards lldpad.
I am also fixing a problem with the virsh domainxml-to-native call on the way.
Gerhard successfully tested the patch with a recent blade network 802.1Qbg-compliant switch.
The patch should not have any side-effects on the 802.1Qbh support in libvirt, but Roopa (cc'ed) may want to verify this.
This reverts commit
Log all errors at level INFO to stop polluting syslog
04bd0360f3.
and makes virRaiseErrorFull() log errors at debug priority
when called from inside libvirtd. This stops libvirtd from
polluting it's own log with client errors at error priority
that'll be reported and logged on the client side anyway.
The stdio.h header has a function called 'remove' declared. This
clashes with the 'remove' parameter in virShrinkN
* src/util/memory.c: Rename 'remove' to 'toremove'
The QEMU logger appends a ':' to the timestamp when it deems
it neccessary, so the virTimestamp API should not duplicate
this
* src/util/util.c: Remove trailing ':' from timestamp
Everytime a public API returns an error, libvirtd pollutes
syslog with that error message. Reduce the error logging
level to INFO so these don't appear by default.
* src/util/virterror.c: Log all errors at INFO
The virFork call resets all logging handlers that may have been
set. Re-enable them after fork in virExec, so that env variables
fir LIBVIRT_LOG_OUTPUTS and LIBVIRT_LOG_FILTERS take effect
until the execve()
* src/util/util.c: Preserve logging in child in virExec
To allow messages from different threads to be untangled,
include an integer thread identifier in log messages.
* src/util/logging.c: Include thread ID
* src/util/threads.h, src/util/threads.h, src/util/threads-pthread.c:
Add new virThreadSelfID() function
* configure.ac: Check for sys/syscall.h
This patch makes two corrections to the newly-added QED support patch series:
- Correct the QED header field offsets
- Remove XML parsing for VIR_STORAGE_FILE_AUTO_SAFE
Signed-off-by: Adam Litke <agl@us.ibm.com>
Implement getBackingStore() for QED images. The header format is defined in
the QED spec: http://wiki.qemu.org/Features/QED .
Signed-off-by: Adam Litke <agl@us.ibm.com>
Cc: Stefan Hajnoczi <stefan.hajnoczi@uk.ibm.com>
Cc: Anthony Liguori <aliguori@linux.vnet.ibm.com>
Add an entry in fileTypeInfo for QED image files.
Signed-off-by: Adam Litke <agl@us.ibm.com>
Cc: Stefan Hajnoczi <stefan.hajnoczi@uk.ibm.com>
Cc: Anthony Liguori <aliguori@linux.vnet.ibm.com>
Disk image formats that wish to opt-out of version validation are supposed to
set versionOffset to -1 in their fileTypeInfo entry.
By unconditionally returning False for these formats,
virStorageFileMatchesVersion() incorrectly reports a version mismatch when the
test was actually skipped. The correct behavior is to return True so these
formats can be successfully probed using the magic bytes alone.
Signed-off-by: Adam Litke <agl@us.ibm.com>
Similarly to deprecating close(), I am now deprecating fclose() and
introduce VIR_FORCE_FCLOSE() and VIR_FCLOSE(). Also, fdopen() is replaced with
VIR_FDOPEN().
Most of the files are opened in read-only mode, so usage of
VIR_FORCE_CLOSE() seemed appropriate. Others that are opened in write
mode already had the fclose()< 0 check and I converted those to
VIR_FCLOSE()< 0.
I did not find occurrences of possible double-closed files on the way.
In a first step I am converting the netlink message construction in
macvtap code to use libnl. It's pretty much a 1:1 conversion except that
now the message needs to be allocated and deallocated.
The util/threads.c/h code already has APIs for mutexes,
condition variables and thread locals. This commit adds
in code for actually creating threads.
* src/libvirt_private.syms: Export new symbols
* src/util/threads.h: Define APIs virThreadCreate, virThreadSelf,
virThreadIsSelf and virThreadJoin
* src/util/threads-win32.c, src/util/threads-win32.h: Win32
impl of threads
* src/util/threads-pthread.c, src/util/threads-pthread.h: POSIX
impl of threads
To avoid the need for duplicating implementations of virStream
drivers, provide a generic implementation that can handle any
FD based stream. This code is copied from the existing impl
in the QEMU driver, with the locking moved into the stream
impl, and addition of a read callback
The FD stream code will refuse to operate on regular files or
block devices, since those can't report EAGAIN properly when
they would block on I/O
* include/libvirt/virterror.h, include/libvirt/virterror.h: Add
VIR_FROM_STREAM error domain
* src/qemu/qemu_driver.c: Remove code obsoleted by the new
generic streams driver.
* src/fdstream.h, src/fdstream.c, src/fdstream.c,
src/libvirt_private.syms: Generic reusable FD based streams
I am trying to use a qcow image with libvirt where the backing 'file' is a
qemu-nbd server. Unfortunately virDomainDiskDefForeachPath() assumes that
backingStore is always a real file so something like 'nbd:0:3333' is rejected
because a file with that name cannot be accessed. Note that I am not worried
about directly using nbd images. That would require a new disk type with XML
markup, etc. I only want it to be permitted as a backingStore
The following patch implements danpb's suggestion:
> I think I'm inclined to push the logic for skipping NBD one stage higher.
> I'd rather expect virStorageFileGetMetadata() to return all backing
> stores, even if not files. The virDomainDiskDefForeachPath() method
> should definitely ignore non-file backing stores though.
>
> So what I'm thinking is to extend the virStorageFileMetadata struct and
> just add a 'bool isFile' field to it. Default this field to true, unless
> you see the prefix of nbd: in which case set it to false. The
> virDomainDiskDefForeachPath() method can then skip over any backing
> store with isFile == false
Signed-off-by: Adam Litke <agl@us.ibm.com>
Cc: Daniel P. Berrange <berrange@redhat.com>
Using automated replacement with sed and editing I have now replaced all
occurrences of close() with VIR_(FORCE_)CLOSE() except for one, of
course. Some replacements were straight forward, others I needed to pay
attention. I hope I payed attention in all the right places... Please
have a look. This should have at least solved one more double-close
error.
Move existing routines about virSysinfoDef to an util module,
add a new entry point virSysinfoRead() to read the host values
with dmidecode
* src/conf/domain_conf.c src/conf/domain_conf.h src/util/sysinfo.c
src/util/sysinfo.h: move to a new module, add virSysinfoRead()
* src/Makefile.am: handle the new module build
* src/libvirt_private.syms: new internal symbols
* include/libvirt/virterror.h src/util/virterror.c: defined a new
error code for that module
* po/POTFILES.in: add new file for translations
virPipeReadUntilEOF is used to read the stdout of exec'ed
and this could fail to capture the full output and read only
1024 bytes.
The problem is that this is based on a poll loop, and in the
loop we read at most 1024 bytes per file descriptor, but we also
note in the loop if poll indicates that the process won't output
more than that on that fd by setting finished[i] = 1.
The simplest way is that if we read a full buffer make sure
finished[i] is still 0 because we will need another pass in the
loop.
NFS does not support file labelling, so ignore this error
for stdin_path when on NFS.
* src/security/security_selinux.c: Ignore failures on labelling
stdin_path on NFS
* src/util/storage_file.c, src/util/storage_file.h: Refine
virStorageFileIsSharedFS() to allow it to check for a
specific FS type.
When we mount any cgroup without "-o devices", we will fail to start vms:
error: Failed to start domain vm1
error: Unable to deny all devices for vm1: No such file or directory
When we mount any cgroup without "-o cpu", we will fail to get schedinfo:
Scheduler : posix
error: unable to get cpu shares tunable: No such file or directory
We should only use the cgroup controllers which are mounted on host.
So I add virCgroupMounted() for qemuCgroupControllerActive()
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
fix warning
CC libvirt_util_la-virtaudit.lo
cc1: warnings being treated as errors
util/virtaudit.c: In function 'virAuditEncode':
util/virtaudit.c:146: error: implicit declaration of function 'virAsprintf' [-Wimplicit-function-declaration]
util/virtaudit.c:146: error: nested extern declaration of 'virAsprintf' [-Wnested-externs]
Commit 9bd3cce0d2 added virFork and
virDriverLoadModule to libvirt_private.syms, but virFork didn't have
a body on Win32 and virDriverLoadModule was already correctly
exported conditional via libvirt_driver_modules.syms.
Add a helper API for ecscaping the value in audit log
messages
* src/util/virtaudit.h, src/util/virtaudit.c,
src/libvirt_private.syms: Add virAuditEncode
The network address was being set to 192.168.122.0 instead
of 192.168.122.0/24. Fix this by removing the unneccessary
'network' field from virNetworkDef and just pass the
network address and netmask into the iptables APIs directly.
* src/conf/network_conf.h, src/conf/network_conf.c: Remove
the 'network' field from virNEtworkDef.
* src/network/bridge_driver.c: Update for iptables API changes
* src/util/iptables.c, src/util/iptables.h: Require the
network address + netmask pair to be passed in
Instead of storing the IP address string in virNetwork related
structs, store the parsed virSocketAddr. This will make it
easier to add IPv6 support in the future, by letting driver
code directly check what address family is present
* src/conf/network_conf.c, src/conf/network_conf.h,
src/network/bridge_driver.c: Convert to use virSocketAddr
in virNetwork, instead of char *.
* src/util/bridge.c, src/util/bridge.h,
src/util/dnsmasq.c, src/util/dnsmasq.h,
src/util/iptables.c, src/util/iptables.h: Convert to
take a virSocketAddr instead of char * for any IP
address parameters
* src/util/network.h: Add macros to determine if an address
is set, and what address family is set.
It is useful to know where the client is connecting from,
so include the socket address in probe data.
* daemon/libvirtd.h: Use virSocketAddr for storing client
address and keep printable address handy for logging
* daemon/libvirtd.c: Include socket address in client
connect/disconnect probes
* daemon/probes.d: Add socket address to probes
* examples/systemtap/client.stp: Print socket address
* src/util/network.h: Add sockaddr_un to virSocketAddr union
The inet_pton and inet_ntop functions are obsolete, replaced
by getaddrinfo+getnameinfo with the AI_NUMERICHOST flag set.
These can be accessed via the virSocket APIs.
The bridge.c code had methods for fetching the IP address of
a bridge which used inet_ntop. Aside from the use of inet_ntop
these methods are broken, because a NIC can have multiple
addresses and this only returns one address. Since the methods
are never used, just remove them.
* src/conf/network_conf.c, src/nwfilter/nwfilter_learnipaddr.c:
Replace inet_pton and inet_ntop with virSocket APIs
* src/util/bridge.c, src/util/bridge.h: Remove unused methods
which called inet_ntop.
The virSocketParse method was not doing any error reporting
which meant the true cause of the problem was lost. Remove
all error reporting from callers, and push it into virSocketParse
* src/util/network.c: Add error reporting to virSocketParse
* src/conf/domain_conf.c, src/conf/network_conf.c,
src/network/bridge_driver.c: Remove error reporting in
callers of virSocketParse
The getnameinfo() function is more flexible than inet_ntop()
avoiding the need to if/else the code based on socket family.
Also make it support UNIX socket addrs and allow inclusion
of a port (service) address. Finally do proper error reporting
via normal APIs.
* src/conf/domain_conf.c, src/nwfilter/nwfilter_ebiptables_driver.c,
src/qemu/qemu_conf.c: Fix error handling with virSocketFormat
* src/util/network.c: Rewrite virSocketFormat to use getnameinfo
and cope with UNIX socket addrs.
There was a typo in the IPv6 path of virSocketCheckNetmask which
caused it to never execute.
* src/util/network.c: s/AF_INET/AF_INET6/ in virSocketCheckNetmask
The virSocketParseAddr function was accepting any AF_* constant
and using that to set the ai_flags field in struct addrinfo.
This is invalid, since address families must go in the ai_family
field of the struct.
* src/util/network.c: Fix handling of address family
* src/conf/network_conf.c, src/network/bridge_driver.c: Pass
AF_UNSPEC instead of relying on it being 0.
Some operations on socket addresses need to know the length of
the sockaddr struct for the particular address family. This
info was being discarded when passing around virSocketAddr
instances. Turn it from a union into a struct containing
union+socklen_t fields, so length is always kept around.
* src/util/network.h: Add socklen_t field to virSocketAddr
* src/util/network.c, src/network/bridge_driver.c,
src/conf/domain_conf.c: Update to take account of new
struct definition.
Integrate with libaudit.so for auditing of important operations.
libvirtd gains a couple of config entries for auditing. By
default it will enable auditing, if its enabled on the host.
It can be configured to force exit if auditing is disabled
on the host. It will can also send audit messages via libvirt
internal logging API
Places requiring audit reporting can use the VIR_AUDIT
macro to report data. This is a no-op unless auditing is
enabled
* autobuild.sh, mingw32-libvirt.spec.in: Disable audit
on mingw
* configure.ac: Add check for libaudit
* daemon/libvirtd.aug, daemon/libvirtd.conf,
daemon/test_libvirtd.aug, daemon/libvirtd.c: Add config
options to enable auditing
* include/libvirt/virterror.h, src/util/virterror.c: Add
VIR_FROM_AUDIT source
* libvirt.spec.in: Enable audit
* src/util/virtaudit.h, src/util/virtaudit.c: Simple internal
API for auditing messages
Since bugs due to double-closed file descriptors are difficult to track down in a multi-threaded system, I am introducing the VIR_CLOSE(fd) macro to help avoid mistakes here.
There are lots of places where close() is being used. In this patch I am only cleaning up usage of close() in src/conf where the problems were.
I also dare to declare close() as being deprecated in libvirt code base (HACKING).
* include/libvirt/libvirt.h.in: some of the function type description
were broken so they could not be automatically documented
* src/util/event.c docs/apibuild.py: event.c exports one public API
so it needs to be scanned too, avoid a few warnings
pciFindStubDriver currently returns 0 in one of the error cases.
While it's correct...NULL is more readable.
Signed-off-by: Chris Wright <chrisw@redhat.com>
The current code will go into an infinite loop if the printf generated
string is >= 1000, AND exactly 1 character smaller than the amount of free
space in the buffer. When this happens, we are dropped into the loop body,
but nothing will actually change, because count == (buf->size - buf->use - 1),
and virBufferGrow returns unchanged if count < (buf->size - buf->use)
Fix this by removing the '- 1' bit from 'size'. The *nprintf functions handle
the NULL byte for us anyways, so we shouldn't need to manually accommodate
for it.
Here's a bug where we are actually hitting this issue:
https://bugzilla.redhat.com/show_bug.cgi?id=602772
v2: Eric's improvements: while -> if (), remove extra va_list variable,
make sure we report buffer error if snprintf fails
v3: Add tests/virbuftest which reproduces the infinite loop before this
patch, works correctly after
When passing a NULL tapfd argument to brAddTap, we need to close the fd
of the tap device. If we don't, libvirt will keep the fd open
indefinitely and renders the the guest unable to configure its side of
the tap device.
Signed-off-by: Soren Hansen <soren@linux2go.dk>
This patch fixes a couple of complaints from valgrind when tickling libvirtd with SIGHUP.
The first two files contain fixes for memory leaks. The 3rd one initializes an uninitialized variable. The 4th one is another memory leak.
virDiskNameToIndex has a list of disk name prefixes that it uses in the
process of finding the disk's index. This list is missing "ubd" which
is the disk prefix used for UML domains.
Signed-off-by: Soren Hansen <soren@linux2go.dk>
That way it can be used to verify a numeric address without storing
the details
* src/util/network.c: change virSocketParseAddr to allow a null @addr
parameter
This patch attempts to take advantage of a newly added netfilter
module to correct for a problem with some guest DHCP client
implementations when used in conjunction with a DHCP server run on the
host systems with packet checksum offloading enabled.
The problem is that, when the guest uses a RAW socket to read the DHCP
response packets, the checksum hasn't yet been fixed by the IP stack,
so it is incorrect.
The fix implemented here is to add a rule to the POSTROUTING chain of
the mangle table in iptables that fixes up the checksum for packets on
the virtual network's bridge that are destined for the bootpc port (ie
"dhcpc", ie port 68) port on the guest.
Only very new versions of iptables will have this support (it will be
in the next upstream release), so a failure to add this rule only
results in a warning message. The iptables patch is here:
http://patchwork.ozlabs.org/patch/58525/
A corresponding kernel module patch is also required (the backend of
the iptables patch) and that will be in the next release of the
kernel.
When trying to assign a PCI device to a guest, we have
to check that all bridges upstream of that device support
ACS. That means that we have to find the parent bridge of
the current device, check for ACS, then find the parent bridge
of that device, check for ACS, etc. As it currently stands,
the code to do this iterates through all PCI devices on the
system, looking for a device that has a range of busses that
included the current device's bus.
That check is not restrictive enough, though. Depending on
how we iterated through the list of PCI devices, we could first
find the *topmost* bridge in the system; since it necessarily had
a range of busses including the current device's bus, we
would only ever check the topmost bridge, and not check
any of the intermediate bridges.
Note that this also caused a fairly serious bug in the
secondary bus reset code, where we could erroneously
find and reset the topmost bus instead of the inner bus.
This patch changes pciGetParentDevice() so that it first
checks if a bridge device's secondary bus exactly matches
the bus of the device we are looking for. If it does, we've
found the correct parent bridge and we are done. If it does not,
then we check to see if this bridge device's busses *include* the
bus of the device we care about. If so, we mark this bridge device
as best, and go on. If we later find another bridge device whose
busses include this device, but is more restrictive, then we
free up the previous best and mark the new one as best. This
algorithm ensures that in the normal case we find the direct
parent, but in the case that the parent bridge secondary bus
is not exactly the same as the device, we still find the
correct bridge.
This patch was tested by me on a 4-port NIC with a
bridge without ACS (where assignment failed), a 4-port
NIC with a bridge with ACS (where assignment succeeded),
and a 2-port NIC with no bridges (where assignment
succeeded).
Signed-off-by: Chris Lalancette <clalance@redhat.com>
valgrind was complaining that virUUIDParse was depending on
an uninitialized value. Indeed it was; virSetHostUUIDStr()
didn't initialize the dmiuuid buffer to 0's, meaning that
anything after the string read from /sys was uninitialized.
Clear out the dmiuuid buffer before use, and make sure to
always leave a \0 at the end.
Signed-off-by: Chris Lalancette <clalance@redhat.com>
If detecting the FLR flag of a pci device fails, then we
could run into the situation of trying to close a file
descriptor twice, once in pciInitDevice() and once in pciFreeDevice().
Fix that by removing the pciCloseConfig() in pciInitDevice() and
just letting pciFreeDevice() handle it.
Thanks to Chris Wright for pointing out this problem.
While we are at it, fix an error check. While it would actually
work as-is (since success returns 0), it's still more clear to
check for < 0 (as the rest of the code does).
Signed-off-by: Chris Lalancette <clalance@redhat.com>
During function test of the 802.1Qbg implementation in lldpad we came
across a small problem in the handling of the netlink message
corresponding to PORT_PROFILE_RESPONSE_INPROGRESS. This should not
result in returning the default rc=1.
- src/util/macvtap.c: fix getPortProfileStatus() to return 0 in that
case and also fix an indentation problem
Some buggy PCI devices actually support FLR, but
forget to advertise that fact in their PCI config space.
However, Virtual Functions on SR-IOV devices are
*required* to support FLR by the spec, so force has_flr
on if this is a virtual function.
Signed-off-by: Chris Lalancette <clalance@redhat.com>
When doing a PCI secondary bus reset, we must be sure that there are no
active devices on the same bus segment. The active device tracking is
designed to only track host devices that are active in use by guests.
This ignores host devices that are actively in use by the host. So the
current logic will reset host devices.
Switch this logic around and allow sbus reset when we are assigning all
devices behind a bridge to the same guest at guest startup or as a result
of a single attach-device command.
* src/util/pci.h: change signature of pciResetDevice to add an
inactive devices list
* src/qemu/qemu_driver.c src/xen/xen_driver.c: use (or not) the new
functionality of pciResetDevice() depending on the place of use
* src/util/pci.c: implement the interface and logic changes
The first conditional is always true which means the iterator will
never find another device on the same bus.
if (dev->domain != check->domain ||
dev->bus != check->bus ||
----> (check->slot == check->slot &&
check->function == check->function)) <-----
The goal of that check is to verify that the device is either:
in a different pci domain
on a different bus
is the same identical device
This means libvirt may issue a secondary bus reset when there are
devices
on that bus that actively in use by the host or another guest.
* src/util/pci.c: fix a bogus test in pciSharesBusWithActive()
A Linux software bridge will assume the MAC address of the enslaved
interface with the numerically lowest MAC addr. When the bridge
changes MAC address there is a period of network blackout, so a
change should be avoided. The kernel gives TAP devices a completely
random MAC address. Occassionally the random TAP device MAC is lower
than that of the physical interface (eth0, eth1etc) that is enslaved,
causing the bridge to change its MAC.
This change sets an explicit MAC address for all TAP devices created
using the configured MAC from the XML, but with the high byte set
to 0xFE. This should ensure TAP device MACs are higher than any
physical interface MAC.
* src/qemu/qemu_conf.c, src/uml/uml_conf.c: Pass in a MAC addr
for the TAP device with high byte set to 0xFE
* src/util/bridge.c, src/util/bridge.h: Set a MAC when creating
the TAP device to override random MAC
virDirCreate also previously returned 0 on success and errno on
failure. This makes it fit the recommended convention of returning 0
on success, -errno (ie a negative number) on failure.
virFileOperation previously returned 0 on success, or the value of
errno on failure. Although there are other functions in libvirt that
use this convention, the preferred (and more common) convention is to
return 0 on success and -errno (or simply -1 in some cases) on
failure. This way the check for failure is always (ret < 0).
* src/util/util.c - change virFileOperation and virFileOperationNoFork to
return -errno on failure.
* src/storage/storage_backend.c, src/qemu/qemu_driver.c
- change the hook functions passed to virFileOperation to return
-errno on failure.
Require the disk image to be passed into virStorageFileGetMetadata.
If this is set to VIR_STORAGE_FILE_AUTO, then the format will be
resolved using probing. This makes it easier to control when
probing will be used
* src/qemu/qemu_driver.c, src/qemu/qemu_security_dac.c,
src/security/security_selinux.c, src/security/virt-aa-helper.c:
Set VIR_STORAGE_FILE_AUTO when calling virStorageFileGetMetadata.
* src/storage/storage_backend_fs.c: Probe for disk format before
calling virStorageFileGetMetadata.
* src/util/storage_file.h, src/util/storage_file.c: Remove format
from virStorageFileMeta struct & require it to be passed into
method.
The virStorageFileGetMetadataFromFD did two jobs in one. First
it probed for storage type, then it extracted metadata for the
type. It is desirable to be able to separate these jobs, allowing
probing without querying metadata, and querying metadata without
probing.
To prepare for this, split out probing code into a new pair of
methods
virStorageFileProbeFormatFromFD
virStorageFileProbeFormat
* src/util/storage_file.c, src/util/storage_file.h,
src/libvirt_private.syms: Introduce virStorageFileProbeFormat
and virStorageFileProbeFormatFromFD
Instead of including a field in FileTypeInfo struct for the
disk format, rely on the array index matching the format.
Use verify() to assert the correct number of elements in the
array.
* src/util/storage_file.c: remove type field from FileTypeInfo
When QEMU opens a backing store for a QCow2 file, it will
normally auto-probe for the format of the backing store,
rather than assuming it has the same format as the referencing
file. There is a QCow2 extension that allows an explicit format
for the backing store to be embedded in the referencing file.
This closes the auto-probing security hole in QEMU.
This backing store format can be useful for libvirt users
of virStorageFileGetMetadata, so extract this data and report
it.
QEMU does not require disk image backing store files to be in
the same format the file linkee. It will auto-probe the disk
format for the backing store when opening it. If the backing
store was intended to be a raw file this could be a security
hole, because a guest may have written data into its disk that
then makes the backing store look like a qcow2 file. If it can
trick QEMU into thinking the raw file is a qcow2 file, it can
access arbitrary files on the host by adding further backing
store links.
To address this, callers of virStorageFileGetMeta need to be
told of the backing store format. If no format is declared,
they can make a decision whether to allow format probing or
not.
IPtables will seek to preserve the source port unchanged when
doing masquerading, if possible. NFS has a pseudo-security
option where it checks for the source port <= 1023 before
allowing a mount request. If an admin has used this to make the
host OS trusted for mounts, the default iptables behaviour will
potentially allow NAT'd guests access too. This needs to be
stopped.
With this change, the iptables -t nat -L -n -v rules for the
default network will be
Chain POSTROUTING (policy ACCEPT 95 packets, 9163 bytes)
pkts bytes target prot opt in out source destination
14 840 MASQUERADE tcp -- * * 192.168.122.0/24 !192.168.122.0/24 masq ports: 1024-65535
75 5752 MASQUERADE udp -- * * 192.168.122.0/24 !192.168.122.0/24 masq ports: 1024-65535
0 0 MASQUERADE all -- * * 192.168.122.0/24 !192.168.122.0/24
* src/network/bridge_driver.c: Add masquerade rules for TCP
and UDP protocols
* src/util/iptables.c, src/util/iptables.c: Add source port
mappings for TCP & UDP protocols when masquerading.
Any error message raised after the process has forked needs
to be followed by virDispatchError, otherwise we have no chance of
ever seeing it. This was selectively done for hook functions in the past,
but really applies to all post-fork errors.
