They all need virCgroupV1GetMemoryUnlimitedKB() so it's easier to
move them in one commit.
Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
We need to update one test-case because now new cgroup object will be
created only if there is any cgroup backend available.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
We will need to extract current cgroup v1 implementation into separate
backend because there will be new cgroup v2 implementation and both will
have to co-exist.
Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
This will be required once cgroup v2 is introduced. The cgroup
detection is not simple and we will have multiple backends so we
should not just jump into the middle of the detection code.
In order to use virCgroupNewSelf we need to create all the remaining
data files:
- {name}.cgroups represents /proc/cgroups, it is a list of cgroup
controllers compiled into kernel
- {name}.self.cgroup represents /proc/self/cgroup, it describes
cgroups to which the process belongs
For "no-cgroups" we need to modify the expected behavior because
virCgroupNewSelf() will fail if there are no controllers available.
Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Because we can set which files to return for cgroup tests there
is no need to have special function tailored to run tests.
Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Once we introduce cgroup v2 support we need to handle processes and
threads differently.
Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Use flags in virCgroupAddTaskInternal instead of boolean parameter.
Following patch will add new flag to indicate thread instead of process.
Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
In cgroup v2 we need to handle processes and threads differently,
following patch will introduce virCgroupAddThread.
Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
If we are on host with systemd we need to build cgroup hierarchy
ourselves for controllers that are not managed by systemd.
As a starting parent we need to force root group because
virCgroupMakeGroup() takes that parent in order to inherit values
for cpuset controller.
By default cpuset controller is managed by systemd so we will never
hit the issue but for v2 cgroups we need to use parent cgroup every
time.
Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
If virCgroupEnableMissingControllers() fails it could have already
created some directories, we should clean it up as well.
Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
If available, use b_info->nested_hvm instead of
b_info->u.hvm.nested_hvm. This will make nested HVM config available
also for PVH domains.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Otherwise starting PVH guest will result in "arch_setup_bootlate:
mapping shared_info failed (pfn=..., rc=-1, errno: 12): Internal error".
After this change the behavior is the same as in `xl`.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Commit 40b5c99a modified the virConfGetValue callers to use
virConfGetValueString. However, using the virConfGetValueString
resulted in leaking the returned @value string in each case.
So, let's modify each instance to use the VIR_AUTOFREE(char *)
syntax. In some instances changing the variable name since
@value was used more than once.
Found by Coverity
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Since lxcConvertSize already creates an error message, there
is no need to use an error: label in lxcSetMemTune to just
overwrite or essentially rewrite the same error. So remove
the label.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
In virDomainMemoryDefParseXML and virDomainVideoDefParseXML if
the VIR_ALLOC fails and NULL is returned, then the alteration
to ctxt->node isn't reversed.
Found by Coverity
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
In a following case:
virsh start $domain
service libvirtd stop
<shutdown> the guest from within the $domain
service libvirtd start
Notice that PCI devices which have been assigned to the $domain will
still be bound to stub drivers instead rebound to host drivers.
In that case the call stack is like below:
libvirtd start
qemuProcessReconnect
qemuProcessStop (because $domain was shutdown without
libvirtd event to process that)
qemuHostdevReAttachDomainDevices
qemuHostdevReAttachPCIDevices
virHostdevReAttachPCIDevices
However, because qemuHostdevUpdateActiveDomainDevices was called
after the qemuConnectMonitor, the setup of the tracking of each
host device in the $domain on either the activePCIHostdevs list
or inactivePCIHostdev list will not occur in an orderly manner.
Therefore, virHostdevReAttachPCIDevices just neglects these host PCI
devices which are bound to stub drivers and doesn't rebind them to
host drivers.
This patch fixs that by moving qemuHostdevUpdateActiveDomainDevices before
qemuConnectMonitor during libvirtd reconnection processing.
Signed-off-by: Wu Zongyong <cordius.wu@huawei.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Use the new qemuDomainRemoveInactiveJobLocked to remove the
@obj during the virDomainObjListForEach call which holds a
lock on the domain object list.
Signed-off-by: Wang Yechao <wang.yechao255@zte.com.cn>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Create a qemuDomainRemoveInactiveJobLocked which copies
qemuDomainRemoveInactiveJob except of course calling
another new helper qemuDomainRemoveInactiveLocked.
The qemuDomainRemoveInactiveLocked is a copy of
qemuDomainRemoveInactive except that instead of calling
virDomainObjListRemove it calls virDomainObjListRemoveLocked.
Signed-off-by: Wang Yechao <wang.yechao255@zte.com.cn>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Introduce qemuDomainRemoveInactiveJobCommon to handle what will
be the common parts of the code with a new function that will
be used to call virDomainObjListRemoveLocked instead of the
unlocked variant.
Signed-off-by: Wang Yechao <wang.yechao255@zte.com.cn>
Reviewed-by: John Ferlan <jferlan@redhat.com>
It was already available in 1.5.0, so we can assume it's
present and avoid checking for it at runtime.
This commit is best viewed with 'git show -w'.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
We already prefer them in capabilities, and domcapabilities
should be consistent with that.
This commit is best viewed with 'git show -w'.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
The new implementation contains less duplicated code and
is easier to extend.
This commit is best viewed with 'git show -w'.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Now that we have reduced the number of sensible options down
to either the native QEMU binary or RHEL's qemu-kvm, we can
make virQEMUCapsInitGuest() a bit simpler.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Both Fedora's qemu-kvm and Debian's/Ubuntu's kvm are nothing
more than paper-thin wrappers around the native QEMU binary,
so we gain nothing by looking for them.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
We're only ever passing a single binary when calling this
function, so we can remove all code dealing with the
possibility of a second binary being specified.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
When the guest is native, we are currently looking at
potential KVM binaries regardless of whether or not we have
already located a QEMU binary suitable to run the guest.
This made sense back when KVM support was not part of QEMU
proper, but these days the KVM binaries are in most cases
just trivial wrapper scripts around the native QEMU binary
so it doesn't make sense to poke at them unless they're
the only binaries on the system, such as when running on
RHEL.
This will allow us to simplify both virQEMUCapsInitGuest()
and virQEMUCapsInitGuestFromBinary().
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
When running an armv7l guest on an aarch64 hosts, the
qemu-system-aarch64 binary should be our first choice instead
of qemu-system-arm since the former can take advantage of KVM
acceleration.
Move the special case to virQEMUCapsFindBinaryForArch() so
that it's handled along with all other cases rather than on
its own later on.
Doing so will also make further refactoring easier.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
virCapabilitiesAddGuestDomain() takes an optional binary
name: this is intended for cases where a certain domain
type can't use the default one registered for the guest
architecture, but has to use a special binary instead.
The current code, however, will pass 'binary' again when
'kvmbin' is not defined, which is unnecessary as 'binary'
has been registered as default earlier, and will result
in capabilities output such as
<emulator>/usr/bin/qemu-system-x86_64</emulator>
<domain type='qemu'/>
<domain type='kvm'>
<emulator>/usr/bin/qemu-system-x86_64</emulator>
</domain>
with the second <emulator> element providing no additional
information.
Change it so that, when 'kvmbin' is not defined, NULL is
passed and so the default emulator will be used instead.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
The function performing the checks, rather than its callers,
should contain comments explaining the rationale behind said
checks.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1630164
Since 2a13a0a103 we are querying the vhostuser's interface name
when building qemu command line. However, we forgot to do so on
hotplug.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The `if(!list || list->type != VIR_CONF_LIST)` check couldn't be
written in a 100% similar way. Instead, we're just checking whether
`virConfGetValueStringList() <= 0` and creating a new function to:
- return -1 in case virConfGetValueStringList fails either due to some
allocation failure or when traversing the list;
- resetting the last error and return 0 otherwise;
Taking this approach we can have the behaviour with the new code as
close as possible to the old one.
Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This change actually changes the behaviour of xenConfigGetString() as
now it returns a newly-allocated string.
Unfortunately, there's not much that can be done in order to avoid that
and all the callers have to be changed in order to avoid leaking the
return value.
Also, as a side-effect of the change above, the function now takes a
"char **" argument instead of a "const char **" one.
Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
There seems to be no need to add the ignore_value wrapper or
caste with (void) to the unlink() calls, so let's just remove
them. I assume at one point in time Coverity complained. So,
let's just be consistent - those that care to check the return
status can and those that don't can just have the naked unlink.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
This patch is introducing cache monitor(CMT) to cache and
memory bandwidth monitor(MBM) for monitoring CPU memory
bandwidth.
The host capability of the two monitors is also introduced
in this patch.
For CMT, the host capability is shown like:
<host>
...
<cache>
<bank id='0' level='3' type='both' size='15' unit='MiB' cpus='0-5'>
<control granularity='768' min='1536' unit='KiB' type='both' maxAllocs='4'/>
</bank>
<monitor level='3' 'reuseThreshold'='270336' maxMonitors='176'>
<feature name='llc_occupancy'/>
</monitor>
</cache>
...
</host>
For MBM, the capability is shown like this:
<host>
...
<memory_bandwidth>
<node id='1' cpus='6-11'>
<control granularity='10' min ='10' maxAllocs='4'/>
</node>
<monitor maxMonitors='176'>
<feature name='mbm_total_bytes'/>
<feature name='mbm_local_bytes'/>
</monitor>
</memory_bandwidth>
...
</host>
Signed-off-by: Wang Huaqiang <huaqiang.wang@intel.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Move memory bandwidth capability nodes into one data structure,
this allows us to add a monitor for memory bandwidth.
Signed-off-by: Wang Huaqiang <huaqiang.wang@intel.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Move all cache banks into one data structure, this allows
us to add other cache component, such as cache monitor.
Signed-off-by: Wang Huaqiang <huaqiang.wang@intel.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This patch introduces the resource monitor and creates the interface
for getting host capability of resource monitor from the system resource
control file system.
The resource monitor takes the role of RDT monitoring group and could be
used to monitor the resource consumption information, such as the last
level cache occupancy and the utilization of memory bandwidth.
Signed-off-by: Wang Huaqiang <huaqiang.wang@intel.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1614283
Save the error from the refresh failure because the stopPool
processing may overwrite the error or even worse clear it
due to calling an external libvirt API that resets the last
error such as is the case with the SCSI pool which may call
virGetConnectNodeDev (see commit decaeb288) in order to
process deleting an NPIV vport.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Create a common pool refresh failure handling method as the
same code is repeated multiple times.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Rather than duplicate the error code, let's create an error
label to keep code common.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Alter the code path to remove the need to to go cleanup and thus
remove the label completely.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
If the virStoragePoolRefresh fails and we call stopPool, the
code neglected to clean up the state file leading to the next
libvirtd restart attempting to start the pool. For a transient
pool this could make it unexpectedly reappear.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1607202
It's essentially stated in the nwfilterBindingDelete that we
will allow the admin to shoot themselves in the foot by deleting
the nwfilter binding which then allows them to undefine the
nwfilter that is in use for the running guest...
However, by allowing this we cause a problem for libvirtd
restart reconnect processing which would then try to recreate
the missing binding attempting to use the deleted filter
resulting in an error and thus shutting the guest down.
So rather than keep adding virDomainConfNWFilterInstantiate
flags to "ignore" specific error conditions, modify the logic
to ignore, but VIR_WARN errors other than ignoreExists. This
will at least allow the guest to not shutdown for only nwfilter
binding errors that we can now perhaps recover from since we
have the binding create/delete capability.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
All of the ones being removed are pulled in by internal.h. The only
exception is sanlock which expects the application to include <stdint.h>
before sanlock's headers, because sanlock prototypes use fixed width
int, but they don't include stdint.h themselves, so we have to leave
that one in place.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Acked-by: Michal Privoznik <mprivozn@redhat.com>
It doesn't really make sense for us to have stdlib.h and string.h but
not stdio.h in the internal.h header.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Acked-by: Michal Privoznik <mprivozn@redhat.com>
QEMU commits:
e37a5c7fa4 (v2.12.0)
i386: Add Intel Processor Trace feature support
c2f193b538 (v2.7.0)
target-i386: Add support for UMIP and RDPID CPUID bits
aff9e6e46a (v2.12.0)
x86/cpu: Enable new SSE/AVX/AVX512 cpu features
f77543772d (v2.9.0)
x86: add AVX512_VPOPCNTDQ features
5131dc433d (v3.1.0)
i386: Add CPUID bit for PCONFIG
59a80a19ca (v3.1.0)
i386: Add CPUID bit for WBNOINVD
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
When restoring a domain from a compressed image, we launch an
intermediate process for decompressing the saved data. If QEMU fails to
load the data for some reason, we force close the stdin/stdout file
descriptors of the intermediate process and wait for it to die. However,
virCommandWait can report various errors which would overwrite the real
error from QEMU. Thus instead of getting something useful:
internal error: process exited while connecting to monitor:
2018-09-17T15:17:29.998910Z qemu-system-x86_64: can't apply global
Skylake-Client-x86_64-cpu.osxsave=off: Property '.osxsave' not found
we could get an irrelevant error message:
internal error: Child process (lzop -dc --ignore-warn) unexpected
fatal signal 13
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Lock all the paths we want to relabel to mutually exclude other
libvirt daemons.
The only hitch here is that directories can't be locked.
Therefore, when relabeling a directory do not lock it (this
happens only when setting up some domain private paths anyway,
e.g. huge pages directory).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
So far the whole transaction handling is done
virSecuritySELinuxSetFileconHelper(). This needs to change for
the sake of security label remembering and locking. Otherwise we
would be locking a path when only appending it to transaction
list and not when actually relabelling it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Firstly, the following code pattern is harder to follow:
if (func() < 0) {
error();
} else {
/* success */
}
We should put 'goto cleanup' into the error branch and move the
else branch one level up.
Secondly, 'rc' should really be named 'ret' because it holds
return value of the function. Not some intermediate value.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This label is used in both successful and error paths. Therefore
it should be named 'cleanup' and not 'err'.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Lock all the paths we want to relabel to mutually exclude other
libvirt daemons.
The only hitch here is that directories can't be locked.
Therefore, when relabeling a directory do not lock it (this
happens only when setting up some domain private paths anyway,
e.g. huge pages directory).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Firstly, the message that says we're setting uid:gid shouldn't be
called from virSecurityDACSetOwnershipInternal() because
virSecurityDACRestoreFileLabelInternal() is calling it too.
Secondly, there are places between us reporting label restore and
us actually doing it where we can quit. Don't say we're doing
something until we are actually about to do it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
So far the whole transaction handling is done
virSecurityDACSetOwnershipInternal(). This needs to change for
the sake of security label remembering and locking. Otherwise we
would be locking a path when only appending it to transaction
list and not when actually relabeling it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Two new APIs are added so that security driver can lock and
unlock paths it wishes to touch. These APIs are not for other
drivers to call but security drivers (DAC and SELinux). That is
the reason these APIs are not exposed through our
libvirt_private.syms file.
Three interesting things happen in this commit. The first is the
global @lockManagerMutex. Unfortunately, this has to exist so that
there is only one thread talking to virtlockd at a time. If there
were more threads and one of them closed the connection
prematurely, it would cause virtlockd killing libvirtd. Instead
of complicated code that would handle that, let's have a mutex
and keep the code simple.
The second interesting thing is keeping connection open between
lock and unlock API calls. This is achieved by duplicating client
FD and keeping it open until unlock is called. This trick is used
by regular disk content locking code when the FD is leaked to
qemu.
Finally, the third thing is polling implemented at client side.
Since virtlockd has only one thread that handles locking
requests, all it can do is either acquire lock or error out.
Therefore, the polling has to be implemented in client. The
polling is capped at 60 second timeout, which should be plenty
since the metadata lock is held only for a fraction of a second.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Now that we know what metadata lock manager user wishes to use we
can load it when initializing security driver. This is achieved
by adding new argument to virSecurityManagerNewDriver() and
subsequently to all functions that end up calling it.
The cfg.mk change is needed in order to allow lock_manager.h
inclusion in security driver without 'syntax-check' complaining.
This is safe thing to do as locking APIs will always exist (it's
only backend implementation that changes). However, instead of
allowing the include for all other drivers (like cpu, network,
and so on) allow it only for security driver. This will still
trigger the error if including from other drivers.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This config option allows users to set and enable lock manager
for domain metadata. The lock manager is going to be used by
security drivers to serialize each other when changing a file
ownership or changing the SELinux label. The only supported lock
manager is 'lockd' for now.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
In some cases we might want to not load the lock driver config.
Alter virLockManagerPluginNew() and the lock drivers to cope with
this fact.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Soon there will be a virtlockd client that wants to either lock
all the resources or none (in order to avoid virtlockd killing
the client on connection close). Because on the RPC layer we can
only acquire one resource at a time, we have to perform a
rollback once we hit a resource that can't be acquired.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This is a new type of object that lock drivers can handle.
Currently, it is supported by lockd driver only.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The fact whether domain has or doesn't have RW disks is specific
to VIR_LOCK_MANAGER_OBJECT_TYPE_DOMAIN and therefore should
reside in union specific to it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
We will want virtlockd to lock files on behalf of libvirtd and
not qemu process, because it is libvirtd that needs an exclusive
access not qemu. This requires new lock context.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This flag causes virtlockd to use different offset when locking
the file.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
So far the virLockSpaceAcquireResource() locks the first byte in
the underlying file. But caller might want to lock other range.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The file being present doesn't necessarily mean anything these
days, as it's created independently of whether the kvm module
has been loaded[1]; moreover, we're already gathering all the
information we need through QMP, so poking the filesystem at
all is entirely unnecessary.
[1] https://github.com/systemd/systemd/commit/d35d6249d5a7ed3228
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
This capability is documented as having one meaning (whether
KVM is enabled by default) but is actually assigned two other
meanings over its life: whether the query-kvm QMP command is
available at first, and later on whether KVM is usable / was
used during probing.
Since the query-kvm QMP command was available in 1.5.0, we
can avoid probing for it; additionally, we can simplify the
logic by setting the flag when it applies instead of initially
setting it and then clearing it when it doesn't.
The flag's description is also updated to reflect reality.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
A side effect of recent changes is that we would always try
to regenerate the capabilities cache for non-native QEMU
binaries based on /dev/kvm availability, which is of course
complete nonsense. Make sure that doesn't happen.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
It was already available in 1.5.0.
Moreover, we're not even formatting it on the QEMU command
line, ever: we just use it as part of some logic that decides
whether KVM support should be advertised, and as it turns out
that logic is actually buggy and dropping this capability
fixes it.
https://bugzilla.redhat.com/show_bug.cgi?id=1628469
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Now that committing transactions using pid == -1 means that we're
not fork()-ing to run the transaction in a specific namespace, we
can utilize the transaction processing semantics in order to
start, run a or multiple commands, and then commit the
transaction without being concerned with other interactions or
transactions interrupting the processing. This will eventually
allow us to have a single place where all the paths can be
locked, followed by relabeling and unlocking again.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
It will be desirable to run transactions more often than we
currently do. Even if the domain we're relabeling the paths for
does not run in a namespace. If that's the case, there is no need
to fork() as we are already running in the right namespace. To
differentiate whether transaction code should fork() or not the
@pid argument now accepts -1 (which means do not fork).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
In the future, the transactions are not going to be optional and
they will be run regardless of domain using namespace to collect
list of paths to be relabeled.
To make sure there won't be an API that goes behind transaction
code back update the comment that serves as decision manual
whether an API must be fully implemented or plain #define is
sufficient.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Even though the current use of the functions does not require full
implementation with transactions (none of the callers passes a path
somewhere under /dev), it doesn't hurt either. Moreover, in
future patches the paradigm is going to shift so that any API
that touches a file is required to use transactions.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Even though the current use of the function does not require full
implementation with transactions (none of the callers pass a path
somewhere under /dev), it doesn't hurt either. Moreover, in
future patches the paradigm is going to shift so that any API
that touches a file is required to use transactions.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Functions that deal with virPCIDeviceAddress exclusively
belong to util/virpci.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Currently the libxl driver claims support for Xen >= 4.4, but
Xen 4.4 and 4.5 are no longer supported upstream. Let's increase
the minimum supported Xen version to 4.6 and change the defined
LIBXL_API_VERSION to 0x040500, which is the API version defined
when Xen 4.6 was released.
Since Xen 4.6 contains a pkgconfig file, drop the now unused code
that falls back to using LIBVIRT_CHECK_LIB in the absence of
pkgconfig file. In addition, bumping the LIBXL_API_VERSION
required adjusting the calls to libxl_set_vcpuaffinity to account
for the extra parameter in the 0x040500 version of the API.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
commit b00c9c39 removed the label end_of_netlink_messages and 'return
table' statement, It causes the function virArpTableGet doesn't return
a proper virArpTable pointer.
How to reproduce:
# virsh domiflist sles12sp3
Interface Type Source Model MAC
-------------------------------------------------------
vnet0 network default virtio 52:54:00💿02:e6
# virsh domifaddr sles12sp3 --source arp
error: Failed to query for interfaces addresses
error: An error occurred, but the cause is unknown
It seems that the "if (nh->nlmsg_type == NLMSG_DONE)" statement won't be
meted. So this patch adds 'return table' when the iterations of nlmsghdr
for loop is over.
Signed-off-by: Lin Ma <lma@suse.com>
Reviewed-by: Chen Hanxiao <chenhanxiao@gmail.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
It is not a problem at all if the `tss` user/group does not exist, the code
fallbacks to the `root` user/group. However we report a warning for no reason
on every start-up. Fix this by checking if the user/group actually exists.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Instead of duplicating the code from virGet{User,Group}IDByName(), which are
static anyway, extend those functions to accept NULL pointers for the result and
a boolean for controlling the error reporting.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Otherwise after libvirtd restart we come back to issues fixed by
introducing this flag in [1].
[1] 61a0026a : qemu: Fix xml dump of autogenerated websocket
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
After removing the host CPU model re-computation,
this function is no longer necessary.
This reverts commits:
commit d0498881a0
virQEMUCapsFreeHostCPUModel: Don't always free host cpuData
commit 5276ec712a
testUpdateQEMUCaps: Don't leak host cpuData
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Commit 82327038 moved a couple of checks out of the XML parser
into the domain validation; however, those checks seem to be more
useful as hypervisor specific checks rather than the more general
domain conf checks (nothing in the docs indicate a specific error).
Fortunately only QEMU was processing the memoryBacking, thus
add the changes to qemuDomainDefValidateMemory and change the
code a bit to make usage of the similar deref to def->mem and
the mem->nhugepages filter.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
virDomainDefCollectBootOrder() is called for every item on the list
for each type of device. One of the checks it makes is to gather the
order attributes from the <boot> element of all devices, and assure
that no two devices have been given the same order.
Since (internally to libvirt, *not* in the domain XML) an <interface
type='hostdev'> is on both the list of hostdev devices and the list of
network devices, it will be counted twice, and the code that checks
for multiple devices with the same boot order will give a false
positive.
To remedy this, we make sure to return early for hostdev devices that
have a parent.type != NONE.
This was introduced in commit 5b75a4, which was first in libvirt-4.4.0.
Resolves: https://bugzilla.redhat.com/1601318
Signed-off-by: Laine Stump <laine@laine.org>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We require QEMU 1.5.0 these days, so checking for versions
older than that is pointless.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The capability was introduced in QEMU 1.5.0, which is our
minimum supported QEMU version these days.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The capability was introduced in QEMU 1.3.1 and we require
QEMU 1.5.0 these days.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1613737
When processing the inputvol for encryption, we need to handle
the case where the inputvol is encrypted. This then allows for
the encrypted inputvol to be used either for an output encrypted
volume or an output volume of some XML provided type.
Add tests to show the various conversion options when either input
or output is encrypted. This includes when both are encrypted.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Commit 39cef12a9 altered/fixed the inputvol processing to create
a multistep process when using an inputvol to create an encrypted
output volume; however, it unnecessarily assumed/restricted the
inputvol to be of 'raw' format only.
Modify the processing code to allow the inputvol format to be checked
and used in order to create the encrypted volume.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
There's really no need for it to be there since it's only ever
used inside virStorageBackendCreateQemuImgCmdFromVol
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
In some cases we are checking if the mount namespace is enabled
at two places: one is at the beginning of exported function (e.g.
qemuDomainNamespaceSetupDisk()) and the other is at the beginning
of qemuDomainNamespaceMknodPaths() which is called from the
former function anyway. Then we have some other functions which
rely on the later check solely.
In order to compensate for possibly needless function call,
qemuDomainNamespaceMknodPaths() returns early if @npaths is zero.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This patch simplifies virNetDevBridgeCreate and virNetDevMacVLanCreate
functions by making use of the virNetlinkNewLink helper.
Signed-off-by: Shi Lei <shi_lei@massclouds.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
This patch adds wrapper macros around nla_nest_[start|end] and nla_put,
thus getting rid of some redundancy and making virNetlinkNewLink more
readable.
Signed-off-by: Shi Lei <shi_lei@massclouds.com>
Signed-off-by: Erik Skultety <eskultet@redhat.com>
This patch introduces virNetlinkNewLink helper which wraps the common
libnl/netlink code to create a new link.
Signed-off-by: Shi Lei <shi_lei@massclouds.com>
Signed-off-by: Erik Skultety <eskultet@redhat.com>
It is possible the incoming VM is not fully started when the finish
phase of migration is executed. In libxlDomainMigrationDstFinish,
wait for the thread receiving the VM to complete before executing
finish phase tasks.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
The libxlDomainMigrationDst* functions are a bit flawed in their
handling of modify jobs. A job begins when the destination host
begins receiving the incoming VM and ends after the VM is started.
The finish phase contains another BeginJob/EndJob sequence.
This patch changes the logic to begin a job for the incoming VM
in the prepare phase and end the job in the finish phase.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
The libxlDomainMigrationSrc* functions are a bit flawed in their
handling of modify jobs. A job begins at the start of the begin
phase but ends before the phase completes. No job is running for
the remaining phases of migration on the source host.
Change the logic to keep the job running after a successful begin
phase, and end the job in the confirm phase. The job must also end
in the perform phase in the case of error since confirm phase would
not be executed.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
libxlDoMigrateSrcP2P() performs all phases of the migration
protocol for peer-to-peer migration. Unfortunately the logic
was a bit flawed since it is possible to skip the confirm
phase after a successfull begin and prepare phase. Fix the
logic to always call the confirm phase after a successful begin
and perform. Skip the confirm phase if begin or perform fail.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
If for any reason the restore of a VM fails on the destination host
in a migration operation, the VM is removed (if not persistent) from
the virDomainObjList, meaning it is no longer available for additional
cleanup or processing in the finish phase. Defer removing the VM from
the virDomainObjList until the finish phase, which already contains
logic to remove the VM.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Commit 5b3492fadb moved qemuAgentCheckError calls into
qemuAgentCommand for various reasons; however, subsequent
commit 0977b8aa0 adding a new command made call again
So let's just remove the duplicitous call from
qemuAgentGetInterfaces.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Commit 5b3492fadb moved qemuAgentCheckError calls into
qemuAgentCommand for various reasons; however, subsequent
commit b1aa91e14 restored the call. So let's just remove
the duplicitous call from qemuAgentSetVCPUsCommand.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
When libvirtd is started, initialize domain objects state with its real
state, not only RUNNING/SHUTOFF.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Previous commits removed all capabilities from per-device property
probing for:
pci-assign
kvm-pci-assign
usb-host
scsi-generic
Remove them from the virQEMUCapsDeviceProps list and get rid of the
redundant device-list-properties QMP calls.
Note that 'pci-assign' was already useless, because the QMP version
of the device is called 'kvm-pci-assign', see libvirt commit 7257480
from 2012.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Introduced by QEMU commit 28b77657 in v1.0-rc4~21^2~8.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Introduced by QEMU commit c29029d which was included in 1.5.0
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
At the time of the addition of 'pci-assign' in QEMU commit
v1.3.0-rc0~572^2 the bootindex argument was already supported.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
At the time of the addition of 'pci-assign' in QEMU commit
v1.3.0-rc0~572^2 the configfd argument was already supported.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Added by commit fc66c1603c and not used since.
Also, the device was present in QEMU 1.5.0 so this capability
will not be needed if we ever decide to implement usb-net support.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The qemuSecurityDomainSetPathLabel() function reports perfect
error itself. Do not overwrite it to something less meaningful.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Attempting to use a chardev definition like
<serial type='unix'>
<target type='isa-serial'/>
</serial>
correctly results in an error being reported, since the source
path - a required piece of information - is missing; however,
the very similar
<serial type='unix'>
<target type='pci-serial'/>
</serial>
was happily accepted by libvirt, only to result in libvirtd
crashing as soon as the guest was started.
The issue was caused by checking the chardev's targetType
against whitelisted values from virDomainChrChannelTargetType
without first checking the chardev's deviceType to make sure
it is actually a channel, for which the check makes sense,
rather than a different type of chardev.
The only reason this wasn't spotted earlier is that the
whitelisted values just so happen to correspond to USB and
PCI serial devices and Xen and UML consoles respectively,
all of which are fairly uncommon.
https://bugzilla.redhat.com/show_bug.cgi?id=1609720
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Since its introduction in commit 2e37bf42 the naming of the arguments
between the prototype and the definition does not match.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
To add CMT/MBM feature and let code be consistent in later patches,
renaming variable name from 'controlBuf' to 'childrenBuf', locates
in functions 'virCapabilitiesFormatCaches' and
'virCapabilitiesFormatMemoryBandwidth'.
Signed-off-by: Wang Huaqiang <huaqiang.wang@intel.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
nlmsg_append from the libnl library provides exactly the same
functionality, so we should rely on that instead. This also allows us to
drop the aforementioned function completely.
Signed-off-by: Shi Lei <shi_lei@massclouds.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
@resp is allocated by virNetlinkCommand and the caller is responsible
for freeing the buffer. Since we already converted this module to use
VIR_AUTO{FREE,PTR} macros, let's resolve the problem by using them.
Signed-off-by: Shi Lei <shi_lei@massclouds.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
These two functions (virSecurityDACSetOwnership and
virSecurityDACRestoreFileLabelInternal) do not really change
@src. Make it const.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This function is going call security manager APIs and therefore
it needs pointer to it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This function is going call security manager APIs and therefore
it needs pointer to it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
There is no need to check if @npaths is not zero. Let's
qemuDomainNamespaceUnlinkPaths() handle that.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1608275
Instantiation of an nwfilter binding is only allowed when
the net->filter is defined for the network; however, the
teardown of the binding does not make this check. This
leaves open the possibility that the teardown could be
called during guest shutdown/teardown in session mode
resulting in the following error being logged:
error : nwfilterConnectOpen:383 : internal error: unexpected
nwfilter URI path '/session', try nwfilter:///system
So before going through the teardown processing, let's
be sure the network had a filter and then attempt to
get a connection. For session mode it's not even possible
create an nwfilter binding.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Similar to nwfilterDefineXML, let's be sure the a filter binding
creation is not attempted in session mode and generate the proper
error message.
Failure to open nwfilter in session mode (nwfilterConnectOpen)
fails already, but that doesn't stop the free thinker from using
a different connection in order to attempt to attempt to create
the binding. Although even doing that would result in a failure:
$ virsh nwfilter-binding-create QEMUGuest1-binding.xml
error: Failed to create network filter from QEMUGuest1-binding.xml
error: internal error: Could not get access to ACL tech driver 'ebiptables'
$
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
A virtio device such as
<controller type='scsi' model='virtio-scsi'/>
will be translated to one of four different QEMU devices
based on the address type. This behavior is the same for
all virtio devices, but unfortunately we have separate
ad-hoc code dealing with each and every one of them: not
only this is pointless duplication, but it turns out
that most of that code is not robust against new address
types being introduced and some of it is outright buggy.
Introduce a new function, qemuBuildVirtioDevStr(), which
deals with the issue in a generic fashion, and rewrite
all existing code to use it.
This fixes a bunch of issues such as virtio-serial-pci
being used with virtio-mmio addresses and virtio-gpu
not being usable at all with virtio-mmio addresses.
It also introduces a couple of minor regressions,
namely no longer erroring out when attempting to
use virtio-balloon and virtio-input devices with
virtio-s390 addresses; that said, virtio-s390 has
been superseded by virtio-ccw such a long time ago
that recent QEMU releases have dropped support for
the former entirely, so re-implementing such
device-specific validation is not worth it.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The appropriate time to ensure the required capabilities are
present is validate rather than command line generation: add
a new qemuDomainDeviceDefValidateInput() function and move
all existing checks there.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
So far we've only formatted it for virtio-blk-pci and
virtio-blk-ccw, but other virtio-blk devices also support
the corresponding option; moreover, we've always formatted
it for all virtio-scsi devices.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There are several functions where we pointlessly duplicate
parts of the format string and pass the same arguments:
refactor them so that the common parts are formatted separately
from the variable parts.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1622455
If a domain is configured to use <source type='file'/> under
<memoryBacking/> we have to honour that setting and produce
-mem-path on the command line. We are not doing so if domain has
no guest NUMA nodes nor hugepages.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The function to retrieve the file system info using QEMU-GA is using
some conditionals to retrieve the info. This is wrong because the error
of some conditionals will be raised if VIR_STRDUP return errors and not
if some problem occurred with JSON.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This commit adds support to use the function qemuAgentGetHostname()
to obtain the domain hostname using QEMU-GA command.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This commit implements the function qemuAgentGetHostname() that uses
the QEMU guest agent command 'guest-get-host-name' to retrieve the
guest hostname of virtual machine running the QEMU-GA.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
There's a single user for it which takes an existing
virPCIDeviceAddress, passes its various bits to the
function which in turn constructs a virPCIDevice and
then copies the string representation for the caller
to use: we can use virPCIDeviceAddressAsString()
instead and avoid creating the virPCIDevice in the
first place. Since the function ends up having no
users after the change, we can just drop it.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The struct is called virPCIDeviceAddress and the
functions operating on it should be named accordingly.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
This will help in future expansions of the code when it is be
harder to track if @newName and/or @newLockspace were already
allocated or not and thus whether it is safe to 'return' or we
need to 'goto error'. By using the 'cleanup' label those two
cases merge into a single one.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
If drvNew callback fails, nobody calls drvFree and thus private
data of the driver might leak.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Currently, there are only two types of resource. So effectively
this is a dead code. However, that assumption can change and we
shouldn't just silently ignore the error.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The if() is completely useless since args.path is set to NULL in
the line just above.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
On daemon deinit only fileLockSpaceDir is freed. The other two
(scsiLockSpaceDir and lvmLockSpaceDir) are missing even though
they are allocated in virLockManagerLockDaemonLoadConfig().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
28 bytes in 1 blocks are definitely lost in loss record 26 of 66
at 0x4C2CF0F: malloc (vg_replace_malloc.c:299)
by 0x7A02719: strdup (strdup.c:42)
by 0x197DC1: virStrdup (virstring.c:961)
by 0x12B478: virLockDaemonConfigFilePath (lock_daemon_config.c:44)
by 0x12A759: main (lock_daemon.c:1270)
62 (32 direct, 30 indirect) bytes in 1 blocks are definitely lost in loss record 41 of 66
at 0x4C2EF26: calloc (vg_replace_malloc.c:711)
by 0x151B61: virAlloc (viralloc.c:144)
by 0x12B56C: virLockDaemonConfigNew (lock_daemon_config.c:71)
by 0x12A491: main (lock_daemon.c:1262)
13 bytes in 1 blocks are definitely lost in loss record 21 of 70
at 0x4C2CF0F: malloc (vg_replace_malloc.c:299)
by 0x7A02719: strdup (strdup.c:42)
by 0x197E3F: virStrdup (virstring.c:961)
by 0x12C86B: virLockSpaceProtocolDispatchRegister (lock_daemon_dispatch.c:291)
by 0x12BB73: virLockSpaceProtocolDispatchRegisterHelper (lock_daemon_dispatch_stubs.h:152)
by 0x1336AA: virNetServerProgramDispatchCall (virnetserverprogram.c:437)
by 0x13320D: virNetServerProgramDispatch (virnetserverprogram.c:304)
by 0x139E3E: virNetServerProcessMsg (virnetserver.c:144)
by 0x13A1A2: virNetServerDispatchNewMessage (virnetserver.c:230)
by 0x1350F5: virNetServerClientDispatchMessage (virnetserverclient.c:343)
by 0x137680: virNetServerClientDispatchEvent (virnetserverclient.c:1498)
by 0x147704: virNetSocketEventHandle (virnetsocket.c:2140)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The virSecurityStackAddNested() can fail in which case
virSecurityManagerNewStack() should fail too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
These are simple predicates, which makes bool a more
appropriate return type than int.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function is called on a virDomainDeviceInfo, so it
should be declared along with it.
Moving this function requires moving and making public
virDomainDeviceCCWAddressIsValid() as well, but that's
perfectly fine since the same reasoning above also
applies to it, due to virDomainDeviceCCWAddress being
(correctly) declared in device_conf.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It's used in virDomainDeviceInfo, which makes
domain_conf the wrong place to declare it.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1621910
When users want to update a path to a CDROM they tend to
construct a very minimal XML and feed the API with it. This is
not a good practice as it breaks the assumptions the API is built
on. Most notably, leaving an element out should be treated as a
request for removal of the corresponding setting. Just like
leaving out <bandwidth/> clears out any QoS previously set.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Commit f7b5566 added 'save_error' even though the function
already has 'originalError' used in the 'try_remove' section.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
As advertised in previous commit, there are three APIs that might
run for quite some time (because they read/write data from/to a
volume) and these three are: downloadVol, uploadVol, wipeVol.
Release pool object lock and reacquire it later to allow more
concurrency.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
There are few operations in the storage driver that read/write
data onto volumes. Such operations can take very long time to
finish. During that time the storage pool object is locked which
has bad performance impacts (other threads can't fetch its XML
for instance). This commit prepares the storage driver for
releasing the lock during those operations (downloadVol,
uploadVol, wipeVol).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Just like a few commits earlier, checking for pool source
duplicates and unlocking pools list afterwards is a buggy
pattern. The check must go into virStoragePoolObjAssignDef.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The @conn argument is needed only to do some source matching in
case of iSCSI source. Anyway, it's used just for node device
driver and as such can be replaced with virGetConnectNodeDev().
At the same time, the @conn struct member is dropped from
_virStoragePoolObjFindDuplicateData.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This function is going to be made static in used in
virStoragePoolObjAssignDef(). Therefore move it and all the
static functions it calls a few lines up.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Even though we do some checking it is not as thorough as it
should be. We already have virStoragePoolObjIsDuplicate but the
way we use it is a typical TOCTOU. Imagine two threads trying to
define two pools with the same name but different UUIDs. With the
current code neither of them finds a duplicate and thus proceed
to virStoragePoolObjAssignDef where only names are compared.
Therefore both threads succeed which is obviously wrong.
We should check for duplicates where we care for them.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This function is going to be made static in used in
virStoragePoolObjAssignDef(). Therefore move it a few lines up.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
In two places the passed pool object argument is marked as
ATTRIBUTE_UNUSED even though it's used right away.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1623157
Changing MTU on a running guest is not possible and trying to do
so made us face many problems. That's why we forbid it in
5f44d7e357. However, there is still one possible path where
users can sneak in change: migration XML.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1621910
When introducing this check back in 4ad54a417a my mindset was
that if an element is missing in update XML then user is
requesting for removal of the corresponding setting. For
instance, if <bandwidth/> is not present in update XML any QoS
previously set on <interface/> is cleared out. Well this
assumption is correct but only to some extent.
Turns out, we have some users who when updating path to ISO
image construct very minimalistic disk XML and pass it to device
update API. Such XML is lacking a lot of information, and alias
is one of them. This triggers error in
virDomainDefCompatibleDevice() because we think that user is
requesting to remove the alias. Well, they are not.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The size/capacity stats gathered in qemuDomainBlocksStatsGather when
using -blockdev would be overwritten by assigning/copying the transfered
data statistics at the end. Fix it by moving the assignment prior to
fetching the capacity data.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
S390 is aware of both vfio-pci and vfio-ccw devices, so
on S390 the capability QEMU_CAPS_VFIO_PCI_DISPLAY will be
available. Add an extra check to make sure we only set the
display to off for vfio-pci mediated devices. Otherwise we
add display for vfio-ccw device and this breaks vfio-ccw
device qemu command line.
Fixes: d54e45b6e conf: Introduce new <hostdev> attribute 'display'
Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Commit 6534b3c4 tried to raise an error when there is no numa
nodes by setting access='shared' in the domain config, but added
a helper called from qemuDomainDeviceDefValidate instead of a
helper called from qemuDomainDefValidate for XML:
<memoryBacking>
<hugepages/>
<access mode='shared'/>
</memoryBacking>
Since there are no memory devices in the test XML, there would
be no validation failure, but the test added was still failing.
Investigating that it turns out that unnecessary XML elements
were causing the failure (no need for <video>, <graphics>,
<pm>, usb controller model "piix3-uhci", disk attribute for
"discard='unmap'", <serial>, <console>, <channel> and a
memballoon model). Removing all those before moving the method
caused the test to succeed.
So this patch moves the validation to the right place and
removes all the unnecessary XML pieces that were causing
a false validation failure.
https://bugzilla.redhat.com/show_bug.cgi?id=1448149#c14
Signed-off-by: Luyao Huang <lhuang@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
If @vm has flagged as "to be removed" virDomainObjListFindByNameLocked
returns NULL (although the definition actually exists). Therefore, the
possibility exits that "virHashAddEntry" will raise the error
"Duplicate key" => virDomainObjListAddObjLocked fails =>
virDomainObjEndAPI(&vm) is called and this leads to a freeing of @def
since @def is already assigned to vm->def. But actually this leads to
a double free since the common usage pattern is that the caller of
virDomainObjListAdd(Locked) is responsible for freeing @def in case of
an error.
Let's fix this by setting vm->def to NULL in case of an error.
Backtrace:
➤ bt
#0 virFree (ptrptr=0x7575757575757575)
#1 0x000003ffb5b25b3e in virDomainResourceDefFree
#2 0x000003ffb5b37c34 in virDomainDefFree
#3 0x000003ff9123f734 in qemuDomainDefineXMLFlags
#4 0x000003ff9123f7f4 in qemuDomainDefineXML
#5 0x000003ffb5cd2c84 in virDomainDefineXML
#6 0x000000011745aa82 in remoteDispatchDomainDefineXML
...
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
We would have used virtio for networking anyway, but it's
better to be explicit; for graphics, none of the existing
models work right now but virtio is the only one which
has a non-PCI variant, so it's as good a default as any
Spotted-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
None of the existing models is suitable for use with
RISC-V virt guests, and we don't want information about
the serial console to be missing from the XML.
The name is based on comments in qemu/hw/riscv/virt.c:
RISC-V machine with 16550a UART and VirtIO MMIO
and in qemu/hw/char/serial.c:
QEMU 16550A UART emulation
along with the output of dmesg in the guest:
Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
10000000.uart: ttyS0 at MMIO 0x10000000 (irq = 13,
base_baud= 230400) is a 16550A
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The architecture is new enough that we don't need to
concern ourselves with backwards compatibility in any
capacity.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add a new modifier letter for virJSONValueObjectAddVArgs which will add
a boolean value with our tristate semantics. The value is omitted when
the _ABSENT value is used.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
A generic "failed to parse xml document" message without telling us
which XML file failed is quite unhelpful.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
In preparation for splitting up the CPU map data file, move it into a
dedicated directory of its own.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Get rid of the separate 'error:' label, so all code paths jump straight
to the 'cleanup:' label.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The x86 and ppc impls both duplicate some logic when parsing CPU
features. Change the callback signature so that this duplication can be
pushed up a level to common code.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Two pieces of code accidentally jumped to the wrong label when they
failed causing incorrect cleanup, returning a partially initialized
CPU model struct.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Allow for syntax
<include filename="subdir/fooo.xml"/>
to reference other files in the CPU database directory
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Commit deb057f added a switch without a default case.
Add it and call virReportEnumRangeError for _LAST too.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Commit 6700062 introduced a jump to error which skipped the
initialization of def:
qemu/qemu_parse_command.c:1870:9: error: variable 'def' is
used uninitialized whenever 'if' condition is true
[-Werror,-Wsometimes-uninitialized]
if (!(qemuCaps = virQEMUCapsCacheLookup(capsCache, progargv[0])))
Initialize def to fix this warning and qemuCaps, to prevent
a future error like this.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Historically the argv -> xml convertor wanted the same default machine
as we'd set when parsing xml. The latter has now changed, however, to
use a default defined by libvirt. The former needs fixing to again
honour the default QEMU machine.
This exposed a bug in handling for the aarch64 target, as QEMU does not
define any default machine. Thus we should not having been accepting
argv without a -machine provided.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The virQEMUCapsGetDefaultMachine() method doesn't get QEMU's default
machine any more, instead it gets the historical default that libvirt
prefers for each arch. Rename it, so that the old name can be used for
getting QEMU's default.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We don't honour the QEMU default machine type anymore, always using the
libvirt chosen default instead. The QEMU argv parser, however, will need
to know the exacty QEMU default, so we must record that info.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Turn
virPCIDeviceAddressIsEmpty()
virDeviceInfoPCIAddressIsWanted()
virDeviceInfoPCIAddressIsPresent()
from inline functions to regular functions.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The affected functions are
virDeviceInfoPCIAddressWanted()
virDeviceInfoPCIAddressPresent()
which get renamed to
virDeviceInfoPCIAddressIsWanted()
virDeviceInfoPCIAddressIsPresent()
to comply with the naming convention used for other
predicates.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This fixes virCgroupEnableMissingControllers where virCgroupRemove
was not called in case virCgroupMakeGroup failed.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
The case where we need path of any controller is only for internal use
so move it out to a different function.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
