Replace:
if (virBufferError(&buf)) {
virBufferFreeAndReset(&buf);
virReportOOMError();
...
}
with:
if (virBufferCheckError(&buf) < 0)
...
This should not be a functional change (unless some callers
misused the virBuffer APIs - a different error would be reported
then)
This modifies the formatting function of virInterface to be a proper
mirror of the parse function, including the addition of a
"parentIfType" arg so that we can decide whether or not it is
appropriate to emit the elements that are only in toplevel interfaces,
as well as the <link> element (which isn't allowed for bridge
interfaces).
Since the restructuring of the code necessarily changes the order of
some of the elements, some test case data had to be updated.
the switch cases for the 4 different interface types had repetitive
code which has now been pulled out as common. While touching those
lines, some extra usage of "!= NULL" etc has been eliminated to make
things more compact and inline with current coding practices.
NB: parentIfType == VIR_INTERFACE_TYPE_LAST means that this is a
toplevel interface (not a subordinate of a bridge or bond). Only
toplevel interfaces can have a start mode, mtu, or IP address element.
For some reason the bridge stp mode and delay were put directly into
the "bridge" case of the switch in virInterfaceDefParseXML(), although
they are inside the <bridge> element, and so should be parsed in the
function created for that purpose - virInterfaceBridgeDefFormat().
The interface state for bonds and vlans does seem to reflect the state
of the underlying physical devices, at least in some cases, so it
makes sense to allow reporting it (netcf now does).
The link state/speed for bridge devices is meaningless though, so we
don't even look for it.
Currently it is not possible to determine the speed of an interface
and whether a link is actually detected from the API. Orchestrating
platforms want to be able to determine when the link has failed and
where multiple speeds may be available which one the interface is
actually connected at. This commit introduces an extension to our
interface XML (without implementation to interface driver backends):
<interface type='ethernet' name='eth0'>
<start mode='none'/>
<mac address='aa:bb:cc:dd:ee:ff'/>
<link speed='1000' state='up'/>
<mtu size='1492'/>
...
</interface>
Where @speed is negotiated link speed in Mbits per second, and state
is the current NIC state (can be one of the following: "unknown",
"notpresent", "down", "lowerlayerdown","testing", "dormant", "up").
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
In "src/conf/" there are many enumeration (enum) declarations.
Similar to the recent cleanup to "src/util" directory, it's
better to use a typedef for variable types, function types and
other usages. Other enumeration and folders will be changed to
typedef's in the future. Most of the files changed in this commit
are reltaed to Network (network_conf.* and interface_conf.*) enums.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
These format functions needed the ability to be indented by an
arbitrary amount, but were written before the introduction of
virBufferAdjustIndent(). They instead used the much more clunky method
of adding a "level" arg to every format function, and padding with
spaces using the "%*s" printf format specifier (giving it the level,
and "", which has the effect of adding level spaces to the output).
While eliminating the hardcoded indentation in other xml, I decided it
was finally time to also modernize the interface formatter code to
make it more consistent.
This fixes a possible double free. In virNetworkAssignDef() if
virBitmapNew() fails, then virNetworkObjFree(network) is called.
However, with network->def pointing to actual @def. So if caller
frees @def again, ...
Moreover, this fixes one possible memory leak too. In
virInterfaceAssignDef() if appending to the list of interfaces
fails, we ought to call virInterfaceObjFree() instead of bare
VIR_FREE().
Although, in order to do that some array size variables needs
to be turned into size_t rather than int.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Currently, during XML parsing, when a call to a FromString() function to
get an enum value fails, the error which is reported is either
VIR_ERR_CONFIG_UNSUPPORTED, VIR_ERR_INTERNAL_ERROR or VIR_ERR_XML_ERROR.
This commit makes such conversion failures consistently return
VIR_ERR_CONFIG_UNSUPPORTED.
'const fooPtr' is the same as 'foo * const' (the pointer won't
change, but it's contents can). But in general, if an interface
is trying to be const-correct, it should be using 'const foo *'
(the pointer is to data that can't be changed).
Fix up remaining offenders in src/conf, and their fallout.
* src/conf/snapshot_conf.h (virDomainSnapshotAssignDef)
(virDomainSnapshotFindByName): Drop attempt at const.
* src/conf/interface_conf.h (virInterfaceObjIsActive)
(virInterfaceDefFormat): Use intended type.
(virInterfaceFindByMACString, virInterfaceFindByName)
(virInterfaceAssignDef, virInterfaceRemove): Drop attempt at
const.
* src/conf/network_conf.h (virNetworkObjIsActive)
(virNetworkDefFormat, virNetworkDefForwardIf)
(virNetworkDefGetIpByIndex, virNetworkIpDefPrefix)
(virNetworkIpDefNetmask): Use intended type.
(virNetworkFindByUUID, virNetworkFindByName, virNetworkAssignDef)
(virNetworkObjAssignDef, virNetworkRemoveInactive)
(virNetworkBridgeInUse, virNetworkSetBridgeName)
(virNetworkAllocateBridge): Drop attempt at const.
* src/conf/netdev_vlan_conf.h (virNetDevVlanFormat): Make
const-correct.
* src/conf/node_device_conf.h (virNodeDeviceHasCap)
(virNodeDeviceDefFormat): Use intended type.
(virNodeDeviceFindByName, virNodeDeviceFindBySysfsPath)
(virNodeDeviceAssignDef, virNodeDeviceObjRemove)
(virNodeDeviceGetParentHost): Drop attempt at const.
* src/conf/secret_conf.h (virSecretDefFormat): Use intended type.
* src/conf/snapshot_conf.c (virDomainSnapshotAssignDef)
(virDomainSnapshotFindByName): Fix fallout.
* src/conf/interface_conf.c (virInterfaceBridgeDefFormat)
(virInterfaceBondDefFormat, virInterfaceVlanDefFormat)
(virInterfaceProtocolDefFormat, virInterfaceDefDevFormat)
(virInterfaceDefFormat, virInterfaceFindByMACString)
(virInterfaceFindByName, virInterfaceAssignDef)
(virInterfaceRemove): Likewise.
* src/conf/network_conf.c
(VIR_ENUM_IMPL, virNetworkFindByName, virNetworkObjAssignDef)
(virNetworkAssignDef, virNetworkRemoveInactive)
(virNetworkDefGetIpByIndex, virNetworkIpDefPrefix)
(virNetworkIpDefNetmask, virNetworkDHCPHostDefParseXML)
(virNetworkIpDefFormat, virNetworkRouteDefFormat)
(virPortGroupDefFormat, virNetworkForwardNatDefFormat)
(virNetworkDefFormatInternal, virNetworkBridgeInUse)
(virNetworkAllocateBridge, virNetworkSetBridgeName)
(virNetworkDNSDefFormat, virNetworkDefFormat): Likewise.
* src/conf/netdev_vlan_conf.c (virNetDevVlanFormat): Likewise.
* src/conf/node_device_conf.c (virNodeDeviceHasCap)
(virNodeDeviceFindBySysfsPath, virNodeDeviceFindByName)
(virNodeDeviceAssignDef, virNodeDeviceObjRemove)
(virNodeDeviceDefFormat, virNodeDeviceGetParentHost): Likewise.
* src/conf/secret_conf.c (virSecretDefFormatUsage)
(virSecretDefFormat): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
Several places in virInterfaceDefParseProtoIPv6 clobber the
default 'ret' return value. So when jumping to cleanup on
error, 'ret' may mistakenly be set to 0 instead of -1. This
caused failure to report OOM errors, meaning data was silently
lost during parsing.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Convert the type of loop iterators named 'i', 'j', k',
'ii', 'jj', 'kk', to be 'size_t' instead of 'int' or
'unsigned int', also santizing 'ii', 'jj', 'kk' to use
the normal 'i', 'j', 'k' naming
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
I noticed several unusual spacings in for loops, and decided to
fix them up. See the next commit for the syntax check that found
all of these.
* examples/domsuspend/suspend.c (main): Fix spacing.
* python/libvirt-override.c: Likewise.
* src/conf/interface_conf.c: Likewise.
* src/security/virt-aa-helper.c: Likewise.
* src/util/virconf.c: Likewise.
* src/util/virhook.c: Likewise.
* src/util/virlog.c: Likewise.
* src/util/virsocketaddr.c: Likewise.
* src/util/virsysinfo.c: Likewise.
* src/util/viruuid.c: Likewise.
* src/vbox/vbox_tmpl.c: Likewise.
* src/xen/xen_hypervisor.c: Likewise.
* tools/virsh-domain-monitor.c (vshDomainStateToString): Drop
default case, to let compiler check us.
* tools/virsh-domain.c (vshDomainVcpuStateToString): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
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.
Noticed that parsing bond interface XML containing the miimon element
fails
<interface type="bond" name="bond0">
...
<bond mode="active-backup">
<miimon freq="100" carrier="netif"/>
...
</bond>
</interface>
This configuration does not contain the optional updelay and downdelay
attributes, but parsing will fail due to returning the result of
virXPathULong (a -1 when the attribute doesn't exist) from
virInterfaceDefParseBond after examining the updelay attribute.
While fixing this bug, cleanup the function to use virXPathInt instead
of virXPathULong, and store the result directly instead of using a tmp
variable. Using virXPathInt actually fixes a potential silent
truncation bug noted by Eric Blake.
Also, there is no cleanup in the error label. Remove the label,
returning failure where failure occurs and success if the end of the
function is reached.
The libvirt coding standard is to use 'function(...args...)'
instead of 'function (...args...)'. A non-trivial number of
places did not follow this rule and are fixed in this patch.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
https://www.gnu.org/licenses/gpl-howto.html recommends that
the 'If not, see <url>.' phrase be a separate sentence.
* tests/securityselinuxhelper.c: Remove doubled line.
* tests/securityselinuxtest.c: Likewise.
* globally: s/; If/. If/
Per the FSF address could be changed from time to time, and GNU
recommends the following now: (http://www.gnu.org/licenses/gpl-howto.html)
You should have received a copy of the GNU General Public License
along with Foobar. If not, see <http://www.gnu.org/licenses/>.
This patch removes the explicit FSF address, and uses above instead
(of course, with inserting 'Lesser' before 'General').
Except a bunch of files for security driver, all others are changed
automatically, the copyright for securify files are not complete,
that's why to do it manually:
src/security/security_selinux.h
src/security/security_driver.h
src/security/security_selinux.c
src/security/security_apparmor.h
src/security/security_apparmor.c
src/security/security_driver.c
Return statements with parameter enclosed in parentheses were modified
and parentheses were removed. The whole change was scripted, here is how:
List of files was obtained using this command:
git grep -l -e '\<return\s*([^()]*\(([^()]*)[^()]*\)*)\s*;' | \
grep -e '\.[ch]$' -e '\.py$'
Found files were modified with this command:
sed -i -e \
's_^\(.*\<return\)\s*(\(\([^()]*([^()]*)[^()]*\)*\))\s*\(;.*$\)_\1 \2\4_' \
-e 's_^\(.*\<return\)\s*(\([^()]*\))\s*\(;.*$\)_\1 \2\3_'
Then checked for nonsense.
The whole command looks like this:
git grep -l -e '\<return\s*([^()]*\(([^()]*)[^()]*\)*)\s*;' | \
grep -e '\.[ch]$' -e '\.py$' | xargs sed -i -e \
's_^\(.*\<return\)\s*(\(\([^()]*([^()]*)[^()]*\)*\))\s*\(;.*$\)_\1 \2\4_' \
-e 's_^\(.*\<return\)\s*(\([^()]*\))\s*\(;.*$\)_\1 \2\3_'
Commit 498d783 cleans up some of virtual file names for parsing strings
in memory. This patch cleans up (hopefuly) the rest forgotten by the
first patch.
This patch also changes all of the previously modified "filenames" to
valid URI's replacing spaces for underscores.
Changes to v1:
- Replace all spaces for underscores, so that the strings form valid
URI's
- Replace spaces in places changed by commit 498d783
We already have virAsprintf, so picking a similar name helps for
seeing a similar purpose. Furthermore, the prefix V before printf
generally implies 'va_list', even though this variant was '...', and
the old name got in the way of adding a new va_list version.
global rename performed with:
$ git grep -l virBufferVSprintf \
| xargs -L1 sed -i 's/virBufferVSprintf/virBufferAsprintf/g'
then revert the changes in ChangeLog-old.
The virConnectPtr is no longer required for error reporting since
that is recorded in a thread local. Remove use of virConnectPtr
from all APIs in interface_conf.{h,c} and update all callers to
match
