Adding virDomainSecDef for general launch security data
and moving virDomainSEVDef as an element for SEV data.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Remember whether the user passed an explicit index when registering the
event so that we can avoid the top level event when it isn't needed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When qos is set or delete, we have to check if the port is an ovs managed
port. If true, call the virNetDevOpenvswitchInterfaceSetQos function when qos
is set, and call the virNetDevOpenvswitchInterfaceClearQos function when
the interface is to be destroyed.
Signed-off-by: Jinsheng Zhang <zhangjl02@inspur.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The DAC security driver has an option to register a callback that
is called instead of chown(). So far QEMU is the only user of
this feature and it's used to set labels on non-local disks (like
gluster), where exists notion of owners but regular chown() can't
be used.
However, this callback (if set) is called always, even for local
disks. And thus the QEMU's implementation duplicated parts of the
DAC driver to deal with chown().
If the DAC driver would call the callback only for non-local
disks then the QEMU's callback can be shorter.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
They were added mostly randomly and we don't really want to keep working
around of false positives.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This reverts commit <1b22dd6dd44202094e0f78f887cbe790c00e9ebc>.
First of all, the reverted commit is incomplete. It only sets
cpuset.mems in the VM root cgroup when the API is used but there is no
code that would do the same when the VM is started.
Libvirt never places any process into the VM root cgroup directly. All
the supporting processes like slirp-helper or dbus-daemon are placed
into the emulator sub-cgroup and all the QEMU threads are distributed
between emulator, vcpu* and iothread* sub-cgroups. The scenario
described in the reverted commit can happen only if someone manually
adds any process there which we should not care about.
If we would like to set the limit in the VM root cgroup we need to
introduce better logic:
- set both (old and new) numa group in the VM root cgroup
- change the numa group in all sub-cgroups to new value
- finally set only the new value in the VM root cgroup
The simplest fix now is to revert the commit.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
QEMU gained support for 'win-dmp' format in it's release of 3.0,
but libvirt doesn't implement it yet. Fortunately, there not much
needed: new value to virDomainCoreDumpFormat public enum, which
unfortunately means that QEMU driver has to be updated in the
same commit, because of VIR_ENUM_IMPL().
Luckily, we don't need any extra QEMU capability - the code
already checks supported formats via
'query-dump-guest-memory-capability' just before issuing
'dump-guest-memory'.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
'block-job-cancel' has one very important semantic difference to
'job-cancel', docummented in qemu as:
Note that if you issue 'block-job-cancel' after 'drive-mirror' has indicated
(via the event BLOCK_JOB_READY) that the source and destination are
synchronized, then the event triggered by this command changes to
BLOCK_JOB_COMPLETED, to indicate that the mirroring has ended and the
destination now has a point-in-time copy tied to the time of the cancellation.
Since libvirt advertises the block copy job as having the synchronous
abort feature we must not use 'job-cancel' here.
Fixes: 4817b5ca1d0
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In certain cases such as when aborting migration we don't really care
for completion of the blockjob. Add 'force' as parameter of
'block-job-cancel'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
qemuDomainBlockCopy needs just the source portion of the disk but uses
the disk parser for it. Since we have a specific function now, refactor
the code to avoid having to deal with the unused virDomainDiskDef.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When querying guest info via virDomainGetGuestInfo() the
'guest-get-disks' agent command is called. It may report disk
serial number which we parse, but never report nor use for
anything else.
As it turns out, it may help management application find matching
disk in their internals.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-By: Tomáš Golembiovský <tgolembi@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Historically, we declared pointer type to our types:
typedef struct _virXXX virXXX;
typedef virXXX *virXXXPtr;
But usefulness of such declaration is questionable, at best.
Unfortunately, we can't drop every such declaration - we have to
carry some over, because they are part of public API (e.g.
virDomainPtr). But for internal types - we can do drop them and
use what every other C project uses 'virXXX *'.
This change was generated by a very ugly shell script that
generated sed script which was then called over each file in the
repository. For the shell script refer to the cover letter:
https://listman.redhat.com/archives/libvir-list/2021-March/msg00537.html
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This API talks to QEMU and changes its internal state. Therefore,
it should acquire QEMU_JOB_MODIFY instead of QEMU_JOB_QUERY.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Recently, a few commits back I've switched bunch of code to
g_steal_pointer() using coccinelle. Problem was that the semantic
patch used was slightly off:
@@
expression a, b;
@@
+ b = g_steal_pointer(&a);
- b = a;
... when != a
- a = NULL;
Problem is that, "... when != a" is supposed to jump over those
lines, which don't contain expression a. My idea was to replace
the following pattern too:
ptrX = ptrY;
if (something(ptrZ) < 0) goto error;
ptrY = NULL;
But what I missed is that the following pattern is also matched
and replaced:
ptrX = ptrY;
if (something(ptrX) < 0) goto error;
ptrY = NULL;
This is not necessarily correct - as demonstrated by our hotplug
code. The real problem is ambiguous memory ownership transfer
(functions which add device to domain def take ownership only on
success), but to not tackle the real issue let's revert those
parts.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The order in which virNetworkUpdate() accepts @section and
@command arguments is not the same as in which it passes them
onto networkUpdate() callback. Until recently, it did not really
matter, because calling the API on client side meant arguments
were encoded in reversed order (compared to the public API), but
then on the server it was fixed again - because the server
decoded RPC (still swapped), called public API (still swapped)
and in turn called the network driver callback (with reversing
the order - so magically fixing the order).
Long story short, if the public API is called even number of
times those swaps cancel each other out. The problem is when the
API is called an odd numbed of times - which happens with split
daemons and the right URI. There's one call in the client (e.g.
virsh net-update), the other in a hypervisor daemon (say
virtqemud) which ends up calling the API in the virnetworkd.
The fix is obvious - fix the order in which arguments are passed
to the callback.
But, to maintain compatibility with older, yet unfixed, daemons
new connection feature is introduced. The feature is detected
just before calling the callback and allows client to pass
arguments in correct order (talking to fixed daemon) or in
reversed order (talking to older daemon).
Unfortunately, older client talking to newer daemon can't be
fixed. Let's hope that it's less frequent scenario.
Fixes: 574b9bc66b6b10cc4cf50f299c3f0ff55f2cbefb
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1870552
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Generated by the following spatch:
@@
expression a, b;
@@
+ b = g_steal_pointer(&a);
- b = a;
... when != a
- a = NULL;
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Extend dirtyrate statistics for domGetStats to display the information
of a domain's memory dirty rate produced by domainStartDirtyRateCalc.
Signed-off-by: Hao Wang <wanghao232@huawei.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When switching to g_autoptr this was incorrectly changed from
'continue;' into 'return -1;' resulting into an error when user tries
to set vcpu_quota of running VM:
error: An error occurred, but the cause is unknown
Fixes: e4a8bbfaf2b4cdd741bb441873bb730f9134b714
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
In short, virXXXPtr type is going away. With big bang. And to
help us rewrite the code with a sed script, it's better if each
variable is declared on its own line.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This patch adds delay time (steal time inside guest) to libvirt
domain per-vcpu stats. Delay time is an important performance metric.
It is a consequence of the overloaded CPU. Knowledge of the delay
time of a virtual machine helps to understand if it is affected and
estimate the impact.
As a result, it is possible to react exactly when needed and
rebalance the load between hosts. This is used by cloud providers
to provide quality of service, especially when the CPU is
oversubscribed.
It's more convenient to work with this metric in a context of a
libvirt domain. Any monitoring software may use this information.
Signed-off-by: Aleksei Zakharov <zaharov@selectel.ru>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Generated using the following spatch:
@@
expression path;
@@
- virFileMakePath(path)
+ g_mkdir_with_parents(path, 0777)
However, 14 occurrences were not replaced, e.g. in
virHostdevManagerNew(). I don't really understand why.
Fixed by hand afterwards.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Now that we enforce the cpu.shares range kernel will no longer silently
change the value that libvirt configures so there is no need to read
the value back to get the actual configuration.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Via coccinelle (not the handbag!)
spatches used:
@ rule1 @
identifier a, b;
symbol NULL;
@@
- b = a;
... when != a
- a = NULL;
+ b = g_steal_pointer(&a);
@@
- *b = a;
... when != a
- a = NULL;
+ *b = g_steal_pointer(&a);
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Commit 76f47889326c4 made qemuNodeDeviceDetachFlags() unusable due to an
'if then else if' chain that will always results in a 'return -1',
regardless of 'driverName' input.
Found by Coverity.
Fixes: 76f47889326c45d2732711bc6dd5751aaf6e5194
Reported-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
libxlNodeDeviceDetachFlags() and qemuNodeDeviceDetachFlags() are mostly
equal, aside from how the virHostdevmanager pointer is retrieved and
the PCI stub driver used.
Now that the PCI stub driver verification is done early in both functions,
we can use the virDomainDriverNodeDeviceDetachFlags() helper to reduce
code duplication between them. 'driverName' is checked inside the helper
to set the appropriate stub driver.
The helper is named with the 'Flags' suffix, even when the helper itself
isn't receiving the flags from the callers, to be compliant with the
ACL function virNodeDeviceDetachFlagsEnsureACL() that is being called
inside it and was called from the original functions. Renaming the helper
would implicate in renaming REMOTE_PROC_NODE_DEVICE_DETACH_FLAGS, and all the
related structs inside remote_protocol.x, to be compliant with the ACL
rules.
This is not being checked at this moment, but we'll fix check-aclrules.py to
verify all the helpers that calls ACL functions in domain_driver.c shortly.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
The validation of 'driverName' does not depend on any other state and can be
done right on the start of the function. We can fail earlier while avoiding
a cleanup jump.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
libxlNodeDeviceReAttach() and qemuNodeDeviceReAttach() are mostly equal,
differing only how the virHostdevManager pointer is retrieved.
Put the common code into virDomainDriverNodeDeviceReAttach() to reduce
code duplication.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
libxlNodeDeviceReset() and qemuNodeDeviceReset() are mostly equal,
differing only how the virHostdevManager pointer is retrieved.
Put the common code into virDomainDriverNodeDeviceReset() to reduce
code duplication.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
When virDomainGetFSInfo() is called over a QEMU/KVM domain it
results into calling of 'guest-get-fsinfo' guest agent command to
which it replies with info on guest (mounted) filesystems. When
filling return structure we also try to do basic lookup and
translate guest agent provided disk address into disk target (as
seen in domain XML). This can of course fail - guest can have
variety of disks not recorded in domain XML (iSCSI, scsi_debug,
NFS to name a few). If that's the case, a debug message is logged
and no disk target is added into the return structure.
However, due to the way our code is written the caller is led to
believe that the target was added into the structure. This may
lead to a situation where the array of disk targets (strings)
contains NULL. But our RPC structure says the array contains only
non-NULL strings. This results in somewhat 'cryptic' (at least to
users) error message:
error: Unable to get filesystem information
error: Unable to encode message payload
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1919783
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
After previous commit, the freeing of @info_ret inside of
virDomainFSInfoFormat() looks like this:
for () {
if (info_ret)
virDomainFSInfoFree(info_ret[i]);
}
It is needless to compare @info_ret against NULL in each
iteration. We can switch the order and do the comparison first
followed by the loop.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When qemuDomainGetFSInfo() is called it calls
qemuDomainGetFSInfoAgent() which executes 'guest-get-fsinfo'
guest agent command, parses returned JSON and returns an array of
qemuAgentFSInfo structures (well, pointers to those structs).
Then it grabs a domain job and tries to do some matching of guest
returned info against domain definition. This matching is done in
virDomainFSInfoFormat() which also frees the array of
qemuAgentFSInfo structures allocated earlier.
But this is not just. If acquiring the domain job fails (or
domain activeness check executed right after that fails) then
virDomainFSInfoFormat() is not called, leaking the array of
structs.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As the very first thing, this function checks whether the number
of items inside @agentinfo array is not negative. This is
redundant as the only caller - qemuDomainGetFSInfo() already
checked for that and would not even call this function if that
was the case.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Our implementation was heavily inspired by the glib version so it's a
drop-in replacement.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Some callers don't need to know the actual lenght of the list but only
care whether the required element is present or the list is non-empty.
Don't calculate the list length in those cases.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The "max" model can be treated the same way as "host" model in general.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Commit <318d807a0bd3372b634d1952b559c5c627ccfa5b> added a fix to skip
most of the block stat code to not log error message for missing storage
sources but forgot to increase the recordnr counter.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Implements QEMU support for vhost-user-blk together with live
hotplug/unplug.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The current virPCIDeviceNew() signature, receiving 4 uints in sequence
(domain, bus, slot, function), is not neat.
We already have a way to represent a PCI address in virPCIDeviceAddress
that is used in the code. Aside from the test files, most of
virPCIDeviceNew() callers have access to a virPCIDeviceAddress reference,
but then we need to retrieve the 4 required uints (addr.domain, addr.bus,
addr.slot, addr.function) to satisfy virPCIDeviceNew(). The result is
that we have extra verbosity/boilerplate to retrieve an information that
is already available in virPCIDeviceAddress.
A better way is presented by virNVMEDeviceNew(), where the caller just
supplies a virPCIDeviceAddress pointer and the function handles the
details internally.
This patch changes virPCIDeviceNew() to receive a virPCIDeviceAddress
pointer instead of 4 uints.
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Instead of receiving 4 uints in order and write domain/bus/slot/function,
receive a virPCIDeviceAddressPtr instead and write into it.
This change will allow us to simplify the API for virPCIDeviceNew()
in the next patch.
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
libxlNodeDeviceGetPCIInfo() and qemuNodeDeviceGetPCIInfo() are equal.
Let's move the logic to a new virDomainDriverNodeDeviceGetPCIInfo()
info to be used by libxl_driver.c and qemu_driver.c.
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
The qemuDomainObjFromDomain() API must be paired with
the virDomainObjEndAPI API. The qemuDomainAuthorizedSSHKeysGet
method simply did 'return -1' leaking a reference and lock
in two paths.
The qemuDomainAuthorizedSSHKeysSet method marked the object
as an autoptr while also have some code paths that will call
virDomainObjEndAPI. As a result the object will be released
but not unlocked in error paths.
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
If QEMU driver fails to initialize for whatever reason (it can be
as trivial as a typo on qemu.conf), the control jumps to error
label in qemuStateInitialize() where qemuStateCleanup() is called
which frees the driver. But the daemon then asks drivers to
prepare for shutdown, which in case of QEMU driver is implemented
in qemuStateShutdownPrepare(). In here, the driver is
dereferenced but since it was freed earlier, the pointer is NULL
which leads to instant crash.
Solution is simple - just check if qemu_driver is not NULL. But
doing so only in qemuStateShutdownPrepare() would push the
problem down to virStateShutdownWait(), well
qemuStateShutdownWait(). Therefore, duplicate the trick there
too.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1895359#c14
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
All callers of this function called virStorageFileParseChainIndex
before. Internalize the logic of that function to prevent multiple calls
and passing around unnecessary temporary variables.
This is achieved by calling virStorageFileParseBackingStoreStr and using
it to fill the values internally.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
