When running on FreeBSD, there's a bug in virCommandProcessIO
polling that is triggered by the commandtest.
A test that triggers EPIPE in commandtest (named "test20") hungs
forever on FreeBSD.
Apparently, this happens because FreeBSD sets POLLHUP flag on revents
when stdin in closed. And as the current implementation only checks for
POLLOUT and POLLERR, it ends up looping forever inside
virCommandProcessIO and not trying to do one more write() that would
trigger EPIPE.
To fix that check for the POLLHUP flag along with POLLOUT and POLLERR.
When a user would specify a backing chain index that is above the start
point libvirt would report a rather unhelpful error:
invalid argument: could not find backing store 1 in chain for 'sub/link2'
This patch adds an explicit check that the index is below start point in
the backing store and reports the following error if not:
invalid argument: requested backing store index 1 is above 'sub/../qcow2' in chain for 'sub/link2'
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177062
Some storage protocols allow to have the @path field in struct
virStorageSource set to NULL. Add NULLSTR() wrappers to handle this
possibility until I finish the storage source error formatter.
Build fails on non-Linux systems with this error:
CC util/libvirt_util_la-virnetdev.lo
util/virnetdev.c:364:1: error: unused function 'virNetDevReplaceMacAddress' [-Werror,-Wunused-function]
virNetDevReplaceMacAddress(const char *linkdev,
^
util/virnetdev.c:406:1: error: unused function 'virNetDevRestoreMacAddress' [-Werror,-Wunused-function]
virNetDevRestoreMacAddress(const char *linkdev,
^
2 errors generated.
The virNetDev{Restore,Replace}MacAddress() functions are only used
by VF-related routines that are available on Linux only. So move these
functions under the same #ifdef.
Because packets going through the egress from a bridge (where our
bandwidth limiting takes place) have no information about which
interface they came from, the QoS rules that we create instead
use the source MAC address of the packets to make their decisions
about which QDisc the packet should be in.
One flaw in this is that when a guest changed the MAC address it
used, packets from the guest would no longer be put into the
correct QDisc, but would instead be put in an "unprivileged"
class, resulting in the bandwidth "floor" (minimum guaranteed)
being no longer honored.
Now that libvirt has infrastructure to capture and respond to
RX_FILTER_CHANGE events from qemu (sent whenever a guest
interface modifies its MAC address, among other things), we can
notice when a guest MAC address changes, and update the QoS rules
accordingly, so that bandwidth floor is honored even after a
guest MAC address change.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
In one of my previous patches (b68a56bcfe) I made class_id to
format more frequently. Well, now it's formatting way too
frequent - even for regular active XML. Users don't need to see
it, so lets format it only for the status XML where it's really
needed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
qemuDomainSetMemoryFlags() would allow to set the initial memory greater
than the <maxMemory> field. While the configuration would not work as
memory hotplug requires NUMA to be enabled and the
qemuDomainSetMemoryFlags() API does not work on NUMA guests this just
fixes a corner case.
The fix is still worth though as it allows to induce an invalid
configuration and make the VM vanish on libvirt restart.
Additionally this tweaks error message to be more accurate.
Signed-off-by: Luyao Huang <lhuang@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Introduce libxl.conf configuration file, adding the 'autoballoon'
setting as the first knob for controlling the libxl driver.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
A further fix for:
https://bugzilla.redhat.com/show_bug.cgi?id=1113474
Since there is no possibility that any type of macvtap will work if
the parent physdev it's attached to is offline, we should bring the
physdev online at the same time as the macvtap. When taking the
macvtap offline, it's also necessary to take the physdev offline for
macvtap passthrough mode (because the physdev has the same MAC address
as the macvtap device, so could potentially cause problems with
misdirected packets during migration, as outlined in commits 829770
and 879c13). We can't set the physdev offline for other macvtap modes
1) because there may be other macvtap devices attached to the same
physdev (and/or the host itself may be using the device) in the other
modes whereas passthrough mode is exclusive to one macvtap at a time,
and 2) there's no practical reason to do so anyway.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1113474
When we set the MAC address of a network device as a part of setting
up macvtap "passthrough" mode (where the domain has an emulated netdev
connected to a host macvtap device that has exclusive use of the
physical device, and sets the device MAC address to match its own,
i.e. "<interface type='direct'> <source mode='passthrough' .../>"), we
use ioctl(SIOCSIFHWADDR) giving it the name of that device. This is
true even if it is an SRIOV Virtual Function (VF).
But, when we are setting the MAC address / vlan ID of a VF in
preparation for "hostdev network" passthrough (this is where we set
the MAC address and vlan id of the VF after detaching the host net
driver and before assigning the device to the domain with PCI
passthrough, i.e. "<interface type='hostdev'>", we do the setting via
a netlink RTM_SETLINK message for that VF's Physical Function (PF),
telling it the VF# we want to change. This sets an "administratively
changed MAC" flag for that VF in the PF's driver, and from that point
on (until the PF driver is reloaded, *not* merely the VF driver) that
VF's MAC address can't be changed using ioctl(SIOCSIFHWADDR) - the
only way to change it is via the PF with RTM_SETLINK.
This means that if a VF is used for hostdev passthrough, it will have
the admin flag set, and future attempts to use that VF for macvtap
passthrough will fail.
The solution to this problem is to check if the device being used for
macvtap passthrough is actually a VF; if so, we use the netlink
RTM_SETLINK message to the PF to set the VF's mac address instead of
ioctl(SIOCSIFHWADDR) directly to the VF; if not, behavior does not
change from previously.
There are three pieces to making this work:
1) virNetDevMacVLan(Create|Delete)WithVPortProfile() now call
virNetDev(Replace|Restore)NetConfig() rather than
virNetDev(Replace|Restore)MacAddress() (simply passing -1 for VF#
and vlanid).
2) virNetDev(Replace|Restore)NetConfig() check to see if the device is
a VF. If so, they find the PF's name and VF#, allowing them to call
virNetDev(Replace|Restore)VfConfig().
3) To prevent mixups when detaching a macvtap passthrough device that
had been attached while running an older version of libvirt,
virNetDevRestoreVfConfig() is potentially given the preserved name
of the VF, and if the proper statefile for a VF can't be found in
the stateDir (${stateDir}/${pfname}_vf${vfid}),
virNetDevRestoreMacAddress() is called instead (which will look in
the file named ${stateDir}/${vfname}).
This problem has existed in every version of libvirt that has both
macvtap passthrough and interface type='hostdev'. Fortunately people
seem to use one or the other though, so it hasn't caused any real
world problem reports.
- Remove all qemu emulators
- Restart libvirtd
- Install qemu emulators
- Call 'virsh version' -> errors
The only thing that will force the qemu driver to refresh it's cached
capablities info is an explict API call to GetCapabilities.
However in the case when the initial caps lookup at driver connect didn't
find a single qemu emulator to poll, the driver is effectively useless
and really can't do anything until it's populated some qemu capabilities
info.
With the above steps, the user would have to either know about the
magic refresh capabilities call, or restart libvirtd to pick up the
changes.
Instead, this patch changes things so that every time a part of th
driver requests access to capabilities info, check to see if
we've previously seen any emulators. If not, force a refresh.
In the case of 'still no emulators found', this is still very quick, so
I can't think of a downside.
https://bugzilla.redhat.com/show_bug.cgi?id=1000116
When building without lxc support enabled, build fails with:
CLD vircapstest
vircapstest.o: In function `test_virCapsDomainDataLookupLXC':
vircapstest.c:(.text+0x9ef): undefined reference to `testLXCCapsInit'
Fix that by hiding LXC tests under appropriate #ifdef. Same applies
for QEMU and XEN.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1171933
Adjust the processLU error returns to be a bit more logical. Currently,
the calling code cannot determine the difference between a non disk/lun
volume and a processed/found disk/lun. It can also not differentiate
between perhaps real/fatal error and one that won't necessarily stop
the code from finding other volumes.
After this patch virStorageBackendSCSIFindLUsInternal will stop processing
as soon as a "fatal" message occurs rather than continuting processing
for no apparent reason. It will also only set the *found value when
at least one of the processLU's was successful.
With the failed return, if the reason for the stop was that the pool
target path did not exist, was /dev, was /dev/, or did not start with
/dev, then iSCSI pool startup and refresh will fail.
Rather than passing/returning a pointer to a boolean to indicate that
perhaps we should try again - adjust the return of the call to return
the count of LU's found during processing, then let the caller decide
what to do with that value.
Use virStorageBackendPoolUseDevPath API to determine whether creation of
stable target path is possible for the volume.
This will differentiate a failed virStorageBackendStablePath which won't
need to be fatal. Thus, we'll add a -2 return value to differentiate that
the failure was a result of either the inability to find the symlink for
the device or failure to open the target path directory
For virStorageBackendStablePath, in order to make decisions in other code
split out the checks regarding whether the pool's target is empty, using /dev,
using /dev/, or doesn't start with /dev
Commit 70f446631f (from 2008) introduced
some functions for testing whether xend was returning correct sound
models. Those functions have long gone, but the function prototypes
remain. This commit removes the unused prototypes.
Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
So, in the example the cpu stats are collected within a function
called do_top. At the beginning of the function we ask the daemon for
how much vCPUs can we get stats, and how many stats for a vCPU can we
get. This is because it's how our API works - users are required to
preallocate a chunk of memory for the results. Now, at the end, we try
to free the allocated array, but we are not doing it correctly.
There's this virTypedParamsFree() function which gets a pointer to the
array and the length of the array. However, if there was an error in
getting vCPU stats we pass a negative number instead of the originally
computed value. This flaw results in SIGSEGV:
libvirt: QEMU Driver error : Requested operation is not valid: domain is not running
ERROR do_top:333 : Unable to get cpu stats
==29201== Invalid read of size 4
==29201== at 0x4F1DF8B: virTypedParamsClear (virtypedparam.c:1145)
==29201== by 0x4F1DFEB: virTypedParamsFree (virtypedparam.c:1165)
==29201== by 0x4023C3: do_top (domtop.c:349)
==29201== by 0x40260B: main (domtop.c:386)
==29201== Address 0x131cd7c0 is 16 bytes after a block of size 768 alloc'd
==29201== at 0x4C2C070: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==29201== by 0x401FF1: do_top (domtop.c:295)
==29201== by 0x40260B: main (domtop.c:386)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This needs to specified in way too many places for a simple validation
check. The ostype/arch/virttype validation checks later in
DomainDefParseXML should catch most of the cases that this was covering.
This revealed that GuestDefaultEmulator was a bit buggy, capable
of returning an emulator that didn't match the passed domain type. Fix
up the test suite input to continue to pass.
This is a helper function to look up all capabilities data for all
the OS bits that are relevant to <domain>. This is
- os type
- arch
- domain type
- emulator
- machine type
This will be used to replace several functions in later commits.
But the internal API stays the same, and we just convert the value as
needed. Not useful yet, but this is the beginning step of using an enum
for ostype throughout the code.
When parsing XML, we validate the passed ostype + arch combo against
the detected hypervisor capabilities. This has led to the following
problem:
- Define x86 qemu guest
- qemu is inadvertently removed from the host
- libvirtd is restarted. fails to parse VM config since arch is removed
- 'virsh list --all' is now empty, user is wondering where their VMs went
Add a new internal flag VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS. Use
it when loading VM and snapshot configs from disk.
https://bugzilla.redhat.com/show_bug.cgi?id=1043572
If no <os><type> was specified:
before: unknown OS type no OS type
after : xml error: an os <type> must be specified
If an <os><type> is specified that's not in our capabiliities data:
before: unknown OS type: $type
after : unsupported configuration: no support found for os <type> '$type'
VIR_ERR_OS_TYPE is now unused (as it should be frankly) so drop its strings
as well to save our translators some effort.
In Parallels we do not support device name hints
aka <target dev=../> option and full-fledged device
disk device addressing through
<address type=.. controller=.. bus=.. target=.. unit=../>
and have only one index instead.
In this situation to be consistent we can only take
one-to-one mapping from some reasonable subset
of full address. Values outside this subset are
invalid to create Parallels VMs.
Reasonable mapping is default one defined in virDomainDiskDefAssignAddress.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@parallels.com>
We should return VIR_DRV_OPEN_ERROR in case
if we handle scheme in query but some
error occur. Previously we sometimes
return VIR_DRV_OPEN_DECLINE.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@parallels.com>
# virsh -c lxc:/// start helloworld
error: Failed to start domain helloworld
error: internal error: guest failed to start: Unknown
failure in libvirt_lxc startup
Return success when there are no cpuset.mems to be set,
instead of failing without setting an error.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
# virsh -c lxc:/// start helloworld
error: Failed to start domain helloworld
error: internal error: guest failed to start: Invalid value '1-3'
for 'cpuset.mems': Invalid argument
Free the cpu mask to avoid reusing it as a mem mask
in virCgroupSetCpusetMems
if virDomainNumatuneMaybeFormatNodeset does not set a mask.
Signed-off-by: Luyao Huang <lhuang@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
According to docs, using 'lun' as a value for device attribute is only valid
with disk types 'block' and 'network'. However current RNG schema also allows
a combination type='file' device='lun' which results in a successfull
xml validation, but fails at qemuBuildCommandLine.
Besides fixing the RNG schema, this patch also adds a qemuxml2argvtest
for this case.
https://bugzilla.redhat.com/show_bug.cgi?id=1210669
https://bugzilla.redhat.com/show_bug.cgi?id=1209948
So we have this bug. The virConnectGetDomainCapabilities() API
performs a couple of checks before it produces any result. One of
the checks is if the architecture requested by user can be run by
the binary (again user provided). However, the check is pretty
dumb. It merely compares if the default binary architecture
matches the one provided by user. However, a qemu binary can run
multiple architectures. For instance: qemu-system-ppc64 can run:
ppc, ppcle, ppc64, ppc64le and ppcemb. The default is ppc64, so
if user requested something else, like ppc64le, the check would
have failed without obvious reason.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
When a qemu domain is to be rebooted, from outside, at libvirt
level it looks like regular shutdown. To really restart the
domain, libvirt needs to issue reset command on the monitor once
SHUTDOWN event appeared. So, in order to differentiate bare
shutdown and reboot libvirt uses a variable within domain private
data. It's called fakeReboot. When the reboot API is called, the
variable is set, but when the shutdown API is called it must be
cleared out. But it was not for every possible case. So if user
called virDomainReboot(), and there was no ACPI daemon running
inside the guest (so guest didn't initiated shutdown sequence)
and then virDomainShutdown(mode=agent) was called bad thing
happened. We remembered the fakeReboot and instead of shutting
the domain down, we just rebooted it.
Signed-off-by: Zhang Bo <oscar.zhangbo@huawei.com>
Signed-off-by: Wang Yufei <james.wangyufei@huawei.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This is a simple wrapper around virNetDevBandwidthManipulateFilter() that
will update the desired filter on an interface (usually a network bridge)
with a new MAC address. Although, the MAC address in question usually
refers to some other interface - the one that the filter is constructed
for. Yeah, hard to parse. Thing is, our NATed network has a bridge where
some part of QoS takes place. And vNICs from guests are plugged into
the bridge. However, if a guest decides to change the MAC of its vNIC,
the corresponding qemu process emits an event which we can use to
update the QoS configuration based on the new MAC address.. However,
our QoS hierarchy is currently not notified, therefore it falls apart.
This function (when called in response to the aforementioned event)
will update our QoS hierarchy and duct tape it together again.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Not only this simplifies the code a bit, it prepares the
environment for upcoming patches. The new
virNetDevBandwidthManipulateFilter() function is capable of both
removing a filter and adding a new one. At the same time! Yeah,
this is not currently used anywhere but look at the next commit
where you'll see it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Currently, when constructing traffic shaping rules, the ingress
filter is created without any priority specified on the command
line. This makes kernel to make up one. While this works, it
simplifies things a bit if we provide the filter priority. In
this case, since it's the root filter lets give it the highest
priority of number 1.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
After a360912179 the formatting of virDomainActualNetDefPtr was
changed a bit. However, during the function rewrite, iface's class_id
is not formatted as frequently as it could be. In fact, after rewrite
it's formatted only for iface of type VIR_DOMAIN_NET_TYPE_DIRECT where
it makes no sense and is unused. While where needed (_TYPE_NETWORK) is
not formatted at all. This makes the daemon forget it upon daemon
restart resulting in bad behaviour.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1211436
This reverts commit b7829f959b.
The previous fix was not correct. Like everywhere else, a driver is a
global variable allocated in stateInitialize function (or something
similar for stateless drivers). Later, when a driver API is called,
it's possible that the global variable is accessed and dereferenced.
Now, some drivers require root privileges because they undertake some
actions reserved only for the system admin (e.g. manipulating host
firewall). And here's the trouble, the NWFilter state initializer
exited too early when finding out it's running unprivileged, leaving
the global NWFilter driver variable uninitialized. Any subsequent
API call that tried to lock the driver resulted in dereferencing the
driver and thus crash.
On the other hand, in order to not resurrect the bug the original
commit was fixing, Let's forbid the nwfilter define in session mode.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Conflicts:
src/nwfilter/nwfilter_driver.c: Context. Code changed a bit
since 2013.
There is a possibility that we jump onto error label with @lockpath
still initialized to NULL. Here, the @lockpath should be unlink()-ed,
but passing there a NULL is not a good idea. Don't do that. In fact,
we should call unlink() only if we created the lock file successfully.
Reported-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The 802.11 interfaces can not be moved by themselves, their Phy has to move too.
If there are other interfaces, they have to move too -- hopefully it's not too
confusing. This is a less-invasive alternative to defining a new hostdev type
for PHYs.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
