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().
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.
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
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.
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)
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.
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.
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.
When building libvirt with clang we get bogus warnings about
'double' being promoted to 'long double' when calling isnan().
https://bugzilla.redhat.com/show_bug.cgi?id=1472437
Detect this broken isnan() / compiler combination and disable
the -Wdouble-promotion flag.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1458638
This code is so complicated because we allow enabling the same
bits at many places. Just like in this case: huge pages can be
enabled by global <hugepages/> element under <memoryBacking> or
on per <memory/> basis. To complicate things a bit more, users
are allowed to omit the page size which case the default page
size is used. And this is what is causing this bug. If no page
size is specified, @pagesize is keeping value of zero throughout
whole function. Therefore we need yet another boolean to hold
[use, don't use] information as we can't sue @pagesize for that.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
So the hostdev manager has some lists to keep track which devices
are active (=assigned to a domain) or inactive. The manager and
its lists are allocated in myInit and freed in myCleanup but one
of them (activeSCSIHostdevs) was missing. Also, the order in
which the cleanup was done doesn't make it easy to spot it,
therefore reoder it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
virDomainDef is not an instance of virObject thus
virObjectUnref() is not the correct function to be called.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
In virDomainNetDefParseXML() the def->coalesce is parsed and
allocated by virDomainNetDefCoalesceParseXML() but in fact it's
never freed .
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1467245
Currently, there's a bug when undefining a domain with NVRAM
store. Basically, the unlink() of the NVRAM store file happens
during the undefine procedure iff domain is inactive. So, if
domain is running and undefine is called the file is left behind.
It won't be removed in the domain cleanup process either
(qemuProcessStop). One of the solutions is to remove if
regardless of the domain state and rely on qemu having the file
opened. This still has a downside that if the domain is defined
back the NVRAM store file is going to be new, any changes to the
current one are lost (just like with any other file that is
deleted while a process has it opened). But is it really a
downside?
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
For all machine types except i440fx, making a guest hotplug
capable requires some sort of planning. Add some information
to help users make educated choices when defining the PCI
topology of guests.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
If there's no content in <script></script>, the XSTL generator
will turn it into <script/> which is not permitted in XHTML.
Adding a single whitespace is enough to guarantee an explicit
closing tag. Without this, the scripts never get loaded by
the browser.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Slight refactor of the WMI serialization code to minimize mixing
openwsman and libxml2 APIs that triggered clang alignment warnings.
The only usage of libxml2 APIs now is in creating CDATA blocks,
because the openwsman API does not provide that functionality. The
clang alignment warning in this case is silenced by casting to a
void pointer first.
Validate that we can pass QEMU command line options using a default
namespace, instead of a prefixed namespace
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The website does not look good in a mobile device as the text is
far too small and the layout assumes a wide screen.
Make the style dynamically adapt based on viewport size, so a
mobile device gets a layout more suited to its dimensions,
also changing "Learn" to "Docs"
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The API docs for the various vir$OBJECTGetConnect functions
contain a warning
WARNING: When writing libvirt bindings in other languages, do
not use this function. Instead, store the connection and
the domain object together.
There is no reason why language bindings should not use this
method, and indeed the Perl, Python, and Go bindings all use
these methods.
This warning was originally added back in
commit 3edb4bc9fb
Author: Daniel Veillard <veillard@redhat.com>
Date: Tue Jul 24 15:32:55 2007 +0000
* libvirt.spec.in NEWS docs/* po/*: preparing release 0.3.1
* src/libvirt.c python/generator.py: some cleanup and warnings
from Richard W.M. Jones
IIUC, the rational was that these APIs do not need to be
directly exposed to the non-C language, as the language
can expose the same concept itself by storing the original
virConnectPtr object alongside the virDomainPtr. There's
no reason to mandate such an approach though - it is valid
for languages to expose this directly if that suits their
needs better.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
In testMessageSingleArrayRef the string is doubly referenced.
Therefore we have to free also the first pointer to the string.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Some tests take already prepared domain from previous tests. In
this case, the domain is freed by the first test that doesn't
keep the domain. However, if there's no such test case domain is
leaked.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
After reading the contents of a file some cleanup is performed.
However, the check for it might access a byte outside of the
string - if the file is empty in the first place. Then strlen()
is zero.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
We're storing the machine name in @priv but free it just in
qemuProcessStop, Therefore this may leak.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
When parsing boot options from domain XML in
virDomainDefParseBootOptions() initenv id stored to:
def->os.initenv[i]->name
def->os.initenv[i]->value
But these are never freed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
- Update the driver page with the information about using
autport for VNC ports
- Add a news entry
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This patch adds support for automatic VNC port assignment for bhyve guests.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The libxl library allows a libxl_domain_config object to be serialized
from/to a JSON string. Use this to allow testing of the XML to
libxl_domain_config conversion process. Test XML is converted to
libxl_domain_config, which is then serialized to json. A json template
corresponding to the test XML is converted to a libxl_domain_config
object using libxl_domain_config_from_json(), and then serialized
back to json using libxl_domain_config_to_json(). The two json
docs are then compared.
Using libxl to convert the json template to a libxl_domain_config
object and then back to json provides a simple way to account for
any changes or additions to the json representation across Xen
releases.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
[update to v3.5.0-rc1, improve error reporting, use /bin/true emulator]
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Most other top level objects have already had their limits increased
to 16384. Increase the storage pool, nwfilter & snapshot object
limits to match. For snapshots at least, we have seen hosts which
exceeded the current limit
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The code only currently handles writing an x86 default -cpu
argument, and doesn't know anything about other architectures.
Let's make this explicit rather than leaving ex. qemu ppc64 to
throw an error about -cpu qemu64
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
Certain XML features that aren't in the <cpu> block map to -cpu
flags on the qemu cli. If one of these is specified but the user
didn't explicitly pass an XML <cpu> model, we need to format a
default model on the command line.
The current code handles this by sprinkling this default cpu handling
among all the different flag string formatting. Instead, switch it
to do this just once.
This alters some test output slightly: the previous code would
write the default -cpu in some cases when no flags were actually
added, so the output was redundant.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
My commit 0c1d863 broke formatting of passthrough smartcard devices:
<smartcard mode='passthrough' type='spicevmc'/>
resulted in invalid XML:
<smartcard mode='passthrough'>
type='spicevmc'>
<address type='ccid' controller='0' slot='0'/>
</smartcard>
Split out chardev source formatting function into two -
one formatting the attributes and other formatting the subelements.
Reported-by: Cole Robinson <crobinso@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
It turns out that our implementation of the hashing function is
endian-dependent and thus if used on various architectures the testsuite
may have different results. Work this around by mocking virHashCodeGen
to something which does not use bit operations instead of just setting a
deterministic seed.
