Commit Graph

19947 Commits

Author SHA1 Message Date
John Ferlan
8473859a47 network: Use @maxnames instead of @nnames
To be consistent with the API definition, use the @maxnames instead
of @nnames when describing/comparing against the maximum names to
be provided for the *ConnectList[Defined]Networks APIs.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
178ac3d133 network: Move virObjectRef during AssignDef processing
Move the virObjectRef in virNetworkObjAssignDefLocked to after
the virHashAddEntry to make it "clearer" why the @ref is being
incremented. Upon return from the ObjNew we will have 1 ref on
the object already, adding it to the hash table requires the
increment.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
7beef70871 network: Have virNetworkObjNew lock the returned object
Forces callers to use the virNetworkObjEndAPI properly.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
26c8fe4990 network: Consistent use of @obj for virnetworkobj
Consistently use @obj for a virNetworkObjPtr.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
bc9868aaf2 network: Introduce virNetworkObjIsPersistent
In preparation to privatize the virNetworkObj - create an accessor function
to get the current @persistent value.  Also change the value to a bool rather
than an unsigned int (since that's how it's generated anyway).

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
9e8227d76b network: Introduce virNetworkObj{Is|Set}Active
In order to privatize the virNetworkObj create accessors in virnetworkobj
in order to handle the get/set of the active value.

Also rather than an unsigned int, convert it to a boolean to match other
drivers representation and the reality of what it is.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
f57d8a7b32 network: Introduce virNetworkObj{Is|Set}Autostart
In preparation for privatizing the virNetworkObj structure, create
accessors for the obj->autostart.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
db207a6233 network: Add virNetworkObj Get/Set API's for @def and @newDef
In preparation for making the object private, create a couple of API's
to get the obj->def & obj->newDef and set the obj->def.

While altering networkxml2conftest.c to use the virNetworkObjSetDef
API, fix the name of the variable from @dev to @def

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
062c38ce75 network: Add virNetworkObj Get/Set API's for @floor_sum
In preparation for making the object private, create a couple of API's
to get/set the obj->floor_sum.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
7435404fd5 network: Introduce virNetworkObjGetClassIdMap
In preparation for privatizing virNetworkObj, create accessor function to
fetch the @classIdMap.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
d5d699ec18 network: Alter virNetworkObj @class_id to be @classIdMap
Change the variable name to be a bit more descriptive and less confusing
when used with the data.network.actual->class_id.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
8489d31cee network: Add virNetworkObj Get/Set API's for @dnsmasqPid and @radvdPid
In preparation for making the object private, create/use a couple of API's
to get/set the obj->dnsmasqPid and obj->radvdPid.

NB: Since the pid's can sometimes changed based on intervening functions,
be sure to always fetch the latest value.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
4f5e32d305 network: Unconditionally initialize macmap when stopping virtual network
Since we can only ever have one reference to obj->macmap, rather
than only clearing obj->macmap during virNetworkObjUnrefMacMap
(e.g. virtual network from networkShutdownNetwork), let's just
unconditionally clear the obj->macmap to ensure that some future
change that created it's own reference to obj->macmap wouldn't
have that reference disappear if virNetworkObjDispose got called.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
f4adeae09d network: Move macmap mgmt from bridge_driver to virnetworkobj
In preparation for having a private virNetworkObj - let's create/move some
API's that handle the obj->macmap. The API's will be renamed to have a
virNetworkObj prefix to follow conventions and the arguments slightly
modified to accept what's necessary to complete their task.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
070b6f7f55 network: Move and rename networkMacMgrFileName
Move networkMacMgrFileName into src/util/virmacmap.c and rename to
virMacMapFileName. We're about to move some more MacMgr processing
files into virnetworkobj and it doesn't make sense to have this helper
in the driver or in virnetworkobj.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
8e51b30148 network: Use consistent naming in bridge_driver for virNetwork objects
Use @obj for a virNetworkObjPtr

Use @net for a virNetworkPtr

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
f8f59d39cb network: Perform some formatting cleanup in bridge_driver.h
Alter prototypes to utilize some more recent guidelines.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
fdab78b574 qemu: Fix bug assuming usage of default UUID for certificate passphrase
If an environment specific _tls_x509_cert_dir is provided, then
do not VIR_STRDUP the defaultTLSx509secretUUID as that would be
for the "default" environment and not the vnc, spice, chardev, or
migrate environments. If the environment needs a secret to decode
it's certificate, then it must provide the secret. If the secrets
happen to be the same, then configuration would use the same UUID
as the default (but we cannot assume that nor can we assume that
the secret would be necessary).
2017-08-15 16:10:27 -04:00
John Ferlan
d3e17259e1 util: Add object checking for virObject{Ref|Unref}
Rather than assuming that what's passed to virObject{Ref|Unref}
would be a virObjectPtr as long as it's not NULL, let's do the
similar checks virObjectIsClass in order to prevent a possible
increment or decrement to some field at the obj->u.s.refs offset.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-14 21:41:59 -04:00
John Ferlan
dfa0efbb77 util: Add magic number check for object validity
The virObjectIsClass API has only ever checked object validity
based on if the @obj is not NULL and it was derived from some class.
While this has worked well in general, there is one additional
check that could be made prior to calling virClassIsDerivedFrom
which loops through the classes checking the magic number against
the klass expected magic number.

If by chance a non virObject is passed, rather than assuming the
void * @obj is a _virObject and thus offsetting to obj->klass,
obj->magic, and obj->parent, let's check that the void * @obj
has at least the "base part" of the magic number in the right
place and generate a more specific VIR_WARN message if not.

There are many consumers to virObjectIsClass, include the locking
primitives virObject{Lock|Unlock}, virObjectRWLock{Read|Write},
and virObjectRWUnlock. For those callers, the locking call will
not fail, but it also will not attempt a virMutex* call which
will "most likely" fail since the &obj->lock is used.

In order to avoid some possible future wrap on the 0xCAFExxxx
value, add a check during initialization that some new class
won't cause the wrap. Should be good for a few years at least!

It is still left up to the caller to handle the failed API calls
just as it would be if it passed a NULL opaque pointer anyobj.
2017-08-14 21:41:59 -04:00
John Ferlan
19f4395230 util: Create common error path for invalid object
If virObjectIsClass fails "internally" to virobject.c, create a
macro to generate the VIR_WARN describing what the problem is.
Also improve the checks and message a bit to indicate which was
the failure - whether the obj was NULL or just not the right class

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-14 21:41:59 -04:00
John Ferlan
045d712c4b util: Introduce and use virObjectRWUnlock
Rather than overload virObjectUnlock as commit id '77f4593b' has
done, create a separate virObjectRWUnlock API that will force the
consumers to make the proper decision regarding unlocking the
RWLock's. Similar to the RWLockRead and RWLockWrite, use the
virObjectGetRWLockableObj helper. This restores the virObjectUnlock
code to using the virObjectGetLockableObj.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-14 21:41:59 -04:00
John Ferlan
bf09f002b1 util: Introduce virObjectGetRWLockableObj
Introduce a helper to handle the error path more cleanly. The same
as virObjectGetLockableObj in order to essentially follow the original
logic of commit 'b545f65d' to ensure that the input argument at least
has some validity before using.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-14 21:41:59 -04:00
John Ferlan
8b03a609dd util: Only have virObjectLock handle virObjectLockable
Now that virObjectRWLockWrite exists to handle the virObjectRWLockable
objects, let's restore virObjectLock to only handle virObjectLockable
class locks. There still exists the possibility that the input @anyobj
isn't a valid object and the resource isn't truly locked, but that
also exists before commit id '77f4593b'.

This also restores some logic that commit id '77f4593b' removed
with respect to a common code path that commit id '10c2bb2b' had
introduced as virObjectGetLockableObj. This code path merely does
the same checks as the original virObjectLock commit 'b545f65d',
but in callable/reusable helper to ensure the @obj at least has
some validity before using.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-14 21:41:59 -04:00
John Ferlan
908b33644e util: Introduce and use virObjectRWLockWrite
Instead of making virObjectLock be the entry point for two
different types of locks, let's create a virObjectRWLockWrite API
which will only handle the virObjectRWLockableClass objects.

Use the new virObjectRWLockWrite for the virdomainobjlist code
in order to handle the Add, Remove, Rename, and Load operations
that need to be very synchronous.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-14 21:41:59 -04:00
John Ferlan
99a72b3eb4 util: Rename virObjectLockRead to virObjectRWLockRead
Since the class it represents is based on virObjectRWLockableClass
and in order to make sure we differentiate just in case anyone somehow
believes they could use virObjectLockRead for a virObjectLockableClass,
let's rename the API to use the RW in the name. Besides the RW locks
refer to pthread_rwlock_{init|rdlock|wrlock|unlock|destroy} while the
other locks refer to pthread_mutex_{init|lock|unlock|destroy}.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-14 21:41:59 -04:00
Pavel Hrdina
40cc355c92 qemu: fix nwfilter deadlock in qemuProcessReconnect
The correct lock order is:

  nwfilter driver lock (not used in this code path)
  nwfilter update lock
  virt driver lock (not used in this code path)
  domain object lock

but the current code have this order:

  domain object lock
  nwfilter update lock

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-14 18:23:56 +02:00
Pavel Hrdina
231c710460 qemu: fix nwfilter deadlock while reverting to snapshot
Introduced by commit <41127244fb90f08cf5032a5d7553f5f0390d925e>.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-14 18:23:21 +02:00
Pavel Hrdina
771002669b conf: use virXMLPropString for Domain def parsing
XPath is good for random search of elements, not for accessing
attributes of one node.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-14 17:46:22 +02:00
Pavel Hrdina
4785a401f9 conf: use virXMLPropString for IOThread pin parsing
XPath is good for random search of elements, not for accessing
attributes of one node.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-14 17:46:21 +02:00
Pavel Hrdina
954b52cd8b conf: use virXMLPropString for IOThread parsing
XPath is good for random search of elements, not for accessing
attributes of one node.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-14 17:46:21 +02:00
Pavel Hrdina
db755227ae conf: use virXMLPropString for disk geometry parsing
XPath is good for random search of elements, not for accessing
attributes of one node.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-14 17:46:21 +02:00
Pavel Hrdina
aa99ea747e conf: use virXMLPropString for KeyWrapCipherDef parsing
XPath is good for random search of elements, not for accessing
attributes of one node.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-14 17:42:11 +02:00
Pavel Hrdina
78ac13860a conf: cleanup virDomainChrSourceDef parsing
The @remaining variable is leftover from old code and it's not
used anymore.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-14 14:31:52 +02:00
Pavel Hrdina
068606f85c use virXMLNodeNameEqual instead of xmlStrEqual
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-14 14:31:52 +02:00
Pavel Hrdina
5bd8a1c5d8 util: introduce virXMLNodeNameEqual
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-14 14:31:52 +02:00
Martin Kletzander
a045317680 util: Rename virResctrl to virResctrlInfo
This way later patches can add another structures with virResctrl
prefix without the meaning being even more confusing than it needs to
be.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2017-08-14 10:01:12 +02:00
Martin Kletzander
7c4b4f8905 util: Make virResctrlGetCacheControlType() behave like other functions
That means that returning negative values means error and non-negative
values differ in meaning, but are all successful.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2017-08-14 10:01:12 +02:00
Martin Kletzander
af4270400a Move resctrl-related code from conf/capabilities to util/virresctrl
It doesn't access anything from conf/ and ti will be needed to use
from other util/ places.  This split makes the separation clearer.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2017-08-14 10:01:12 +02:00
Martin Kletzander
62146d8532 virxml: Fix indentation
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2017-08-14 10:01:12 +02:00
Laine Stump
f5bc8b5436 util: eliminate superfluous saveVlan check in virNetDevSetNetConfig()
Commit 81fb440b further qualified an if statement by adding the
boolean saveVlan to the condition. Coverity pointed out that this
change in the logic eliminated the need to check saveVlan in an
argument to virAsprintf().
2017-08-13 23:07:13 -04:00
Laine Stump
83074cc917 util: fix improper assignment of return value in virHostdevReadNetConfig()
Commit 9a94af6d restructured virHostdevReadNetConfig() so that it
would manually set ret = 0 after successfully reading the device's
config, but Coverity pointed out that "ret = 0" was erroneously placed
outside of an "else" clause, meaning that the the value of ret set in
the "if" clause was unnecessarily and incorrectly overwritten.

This patch moves ret = 0 into the else clause, which should silence
Coverity.
2017-08-13 23:07:13 -04:00
Guido Günther
cd3c4eece2 vbox: fix typo in warning message
Acked-by: Laine Stump <laine@laine.org>
2017-08-14 02:56:35 +02:00
Laine Stump
489a937eb4 util: check for PF online status earlier in guest startup
When using a VF from an SRIOV-capable network card in a guest (either
in macvtap passthrough mode, or via VFIO PCI device assignment), The
associated PF netdev must be online in order for the VF to be usable
by the guest. The guest, however, is not able to change the state of
the PF. And libvirt *could* set the PF online as needed, but that
could lead to the host receiving unexpected IPv6 traffic (since the
default for an unconfigured interface is to participate in IPv6
autoconf). For this reason, before assigning a VF to a guest, libvirt
verifies that the related PF netdev is online - if it isn't, then we
log an error and don't allow the guest startup to continue.

Until now, this check was done during virNetDevSetNetConfig(). This
works nicely because the same function is called both for macvtap
passthrough and for VFIO device assignment. But in the case of VFIO,
the VF has already been unbound from its netdev driver by the time we
get to virNetDevSetNetConfig(), and in the case of dual port Mellanox
NICs that have their VFs setup in single port mode, the *only* way to
determine the proper PF netdev to query for online status is via the
"phys_port_id" file that is in the VF netdev's sysfs directory. *BUT*
if we've unbound the VF from the netdev driver, then it doesn't *have*
a netdev sysfs directory.

So, in order to check the correct PF netdev for online status, this
patch moved the check earlier in the setup, into
virNetDevSaveNetConfig(), which is called *before* unbinding the VF
from its netdev driver.

(Note that this implies that if you are using VFIO device assignment
for the VFs of a Mellanox NIC that has the VFs programmed in single
port mode, you must let the VFs be bound to their net driver and use
"managed='yes'" in the device definition. To be more specific, this is
only true if the VFs in single port mode are using port *2* of the PF
- if the VFs are using only port 1, then the correct PF netdev will be
arrived at by default/chance))

  This resolves: https://bugzilla.redhat.com/267191
2017-08-11 19:13:33 -04:00
Laine Stump
9a08168301 util: restructure virNetDevReadNetConfig() to eliminate false error logs
virHostdevRestoreNetConfig() calls virNetDevReadNetConfig() to try and
read the "original config" of a netdev, and if that fails, it tries
again with a different directory/netdev name. This achieves the
desired effect (we end up finding the config wherever it may be), but
for each failure, virNetDevReadNetConfig() places a nice error message
in the system logs. Experience has shown that false-positive error
logs like this lead to erroneous bug reports, and can often mislead
those searching for *real* bugs.

This patch changes virNetDevReadNetConfig() to explicitly check if the
file exists before calling virFileReadAll(); if it doesn't exist,
virNetDevReadNetConfig() returns a success, but leaves all the
variables holding the results as NULL. (This makes sense if you define
the purpose of the function as "read a netdev's config from its config
file *if that file exists*).

To take advantage of that change, the caller,
virHostdevRestoreNetConfig() is modified to fail immediately if
virNetDevReadNetConfig() returns an error, and otherwise to try the
different directory/netdev name if adminMAC & vlan & MAC are all NULL
after the preceding attempt.
2017-08-11 19:09:49 -04:00
Laine Stump
b67eaa6351 util: save the correct VF's info when using a dual port SRIOV NIC in single port mode
Mellanox ConnectX-3 dual port SRIOV NICs present a bit of a challenge
when assigning one of their VFs to a guest using VFIO device
assignment.

These NICs have only a single PCI PF device, and that single PF has
two netdevs sharing the single PCI address - one for port 1 and one
for port 2. When a VF is created it can also have 2 netdevs, or it can
be setup in "single port" mode, where the VF has only a single netdev,
and that netdev is connected either to port 1 or to port 2.

When the VF is created in dual port mode, you get/set the MAC
address/vlan tag for the port 1 VF by sending a netlink message to the
PF's port1 netdev, and you get/set the MAC address/vlan tag for the
port 2 VF by sending a netlink message to the PF's port 2 netdev. (Of
course libvirt doesn't have any way to describe MAC/vlan info for 2
ports in a single hostdev interface, so that's a bit of a moot point)

When the VF is created in single port mode, you can *set* the MAC/vlan
info by sending a netlink message to *either* PF netdev - the driver
is smart enough to understand that there's only a single netdev, and
set the MAC/vlan for that netdev. When you want to *get* it, however,
the driver is more accurate - it will return 00:00:00:00:00:00 for the
MAC if you request it from the port 1 PF netdev when the VF was
configured to be single port on port 2, or if you request if from the
port 2 PF netdev when the VF was configured to be single port on port
1.

Based on this information, when *getting* the MAC/vlan info (to save
the original setting prior to assignment), we determine the correct PF
netdev by matching phys_port_id between VF and PF.

(IMPORTANT NOTE: this implies that to do PCI device assignment of the
VFs on dual port Mellanox cards using <interface type='hostdev'>
(i.e. if you want the MAC address/vlan tag to be set), not only must
the VFs be configured in single port mode, but also the VFs *must* be
bound to the host VF net driver, and libvirt must use managed='yes')

By the time libvirt is ready to set the new MAC/vlan tag, the VF has
already been unbound from the host net driver and bound to
vfio-pci. This isn't problematic though because, as stated earlier,
when a VF is created in single port mode, commands to configure it can
be sent to either the port 1 PF netdev or the port 2 PF netdev.

When it is time to restore the original MAC/vlan tag, again the VF
will *not* be bound to a host net driver, so it won't be possible to
learn from sysfs whether to use the port 1 or port 2 PF netdev for the
netlink commands. And again, it doesn't matter which netdev you
use. However, we must keep in mind that we saved the original settings
to a file called "${PF}_${VFNUM}". To solve this problem, we just
check for the existence of ${PF1}_${VFNUM} and ${PF2}_${VFNUM}, and
use whichever one we find (since we know that only one can be there)
2017-08-11 19:05:20 -04:00
Laine Stump
39d136b67b util: match phys_port_id when converting PF-netdev to/from VF-netdev
This patch updates functions in netdev.c to pay attention to
phys_port_id. It uses the new function virNetDevGetPhysPortID() to
learn the phys_port_id of a VF or PF, then sends that info to
virPCIGetNetName(), which has newly been modified to take an optional
phys_port_id.
2017-08-11 18:55:25 -04:00
Laine Stump
b3b5aa75ed util: make virPCIGetNetName() more versatile
A single PCI device may have multiple netdevs associated with it. Each
of those netdevs will have a different phys_port_id entry in
sysfs. This patch modifies virPCIGetNetName() to allow selecting one
of the potential many netdevs in two different ways:

1) by setting the "idx" argument, the caller can select the 1st (0),
2nd (1), etc. netdev from the PCI device's net subdirectory.

2) If the physPortID arg is set (to a null-terminated string) then
virPCIGetNetName() returns the netdev that has that phys_port_id in
the sysfs file of the same name in the netdev's directory.
2017-08-11 18:35:09 -04:00
Laine Stump
0dc67e6d2d util: Fix const'ness of 1st arg to virPCIGetNetName()
The first arg isn't modified in the function, so it should be const.
2017-08-11 18:30:14 -04:00
Laine Stump
48f33bb5df util: new function virNetDevGetPhysPortID()
On Linux each network device *can* (but not necessarily *does*) have
an attribute called phys_port_id which can be read from the file of
that name in the netdev's sysfs directory. The examples I've seen have
been a many-digit hexadecimal number (as an ASCII string).

This value can be useful when a single PCI device is associated with
multiple netdevs (e.g a dual port Mellanox SR-IOV NIC - this card has
a single PCI Physical Function (PF), and that PF has two netdevs
associated with it (the "net" subdirectory of the PF in sysfs has two
links rather than the usual single link to a netdev directory). Each
of the PF netdevs has a different phys_port_id. The Virtual Functions
(VF) are similar - the PF (a PCI device) has "n" VFs (also each of
these is a PCI device), each VF has two netdevs, and each of the VF
netdevs points back to the VF PCI device (with the "device" entry in
its sysfs directory) as well as having a phys_port_id matching the PF
netdev it is associated with.

virNetDevGetPhysPortID() simply attempts to read the phys_port_id for
the given netdev and return it to the caller. If this particular
netdev driver doesn't support phys_port_id, it returns NULL (*not* a
NULL-terminated string, but a NULL pointer) but still counts it as a
success.
2017-08-11 18:25:00 -04:00