After previous cleanups, some 'error' labels were rendered
needless - they contain nothing more than a return statement.
Well, those labels can be dropped and 'goto error' can be
replaced with return statement directly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
After previous cleanups, some 'cleanup' labels were rendered
needless - they contain nothing more than a return statement.
Well, those labels can be dropped and 'goto cleanup' can be
replaced with return statement directly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
Let's replace VIR_FREE() calls with g_autofree. Not all calls can
be replaced though - the legitimate ones are kept (e.g. those
which free array, or which free a struct for which we don't have
g_autoptr() yet, and so on).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
A lot of explicit free calls can be saved when virJSONValue
variables are declared with g_autoptr(). There's one caveat:
there was a slight deviation from our usual pattern such that
@cmd variable was not initialized to NULL but as the very first
step it was assigned a value using qemuMonitorJSONMakeCommand().
While this works in theory it upset my GCC-11.2 (but only when
building with -O2). So I had to initialize the variable in such
case too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
The reason why @entry variable in qemuMonitorJSONExtractPRManagerInfo()
was declared at the top most level was that the variable is used under
the cleanup label. However, if declared using g_autofree then the
variable can be declared inside the loop it is used in.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
There's one place (specifically qemuMonitorJSONParseCPUModel())
where we can avoid explicit free call for qemuMonitorCPUModelInfo
struct.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
We have g_autoptr() for virCPUData struct defined already. Let's
use it in qemu_monitor_json.c and drop explicit free calls.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
The qemuMonitorJSONQueryRxFilterParse() function is called to
parse the output of 'query-rx-filter' and store results into
passed virNetDevRxFilter structure. However, it is doing so in a
bit clumsy way - the return pointer is set in all cases (i.e.
even in case of error) and thus the cleanup label is more
complicated than it needs to be. With a help of g_autoptr() and
g_steal_pointer() the return pointer can be set only in case of
success - which is what callers expect anyway.
The same applies to qemuMonitorJSONQueryRxFilter().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
In the qemuMonitorJSONGetMigrationStats() there's a code under
cleanup label that's clearing returned @stats if the function
returns with an error. However, transitively there's just one
caller - qemuMigrationAnyFetchStats() - and it doesn't care for
this behaviour. Drop the code to simplify the cleanup label.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
All callers of qemuMonitorJSONHumanCommand() pass a non-NULL pointer
as @reply_str therefore there's no need to check whether it is NULL.
NB, the sister function (qemuMonitorJSONArbitraryCommand()) doesn't
check for NULL either.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
In qemuMonitorJSONCommandWithFd() given command (represented by
virJSONValue struct) is translated to string (represented by
virBuffer). The ownership of the string is then transferred to
the message which is then sent. The downside of this approach is
we have to have an explicit call to free the string from the
message. But if the message just "borrowed" the string (which it
can safely do because it is just reading from the string) then
automatic free of the buffer takes care of freeing the string.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
The qemuMonitorNextCommandID() function can never fail. There's
no need to check for its retval then. Moreover, the temporary
variable used to hold the retval can be declared in the inner
most block.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
In a few places we declare a variable (which is optionally
followed by a code not touching it) then set the variable to a
value and return the variable immediately. It's obvious that the
variable is needless and the value can be returned directly
instead.
This patch was generated using this semantic patch:
@@
type T;
identifier ret;
expression E;
@@
- T ret;
... when != ret
when strict
- ret = E;
- return ret;
+ return E;
After that I fixed couple of formatting issues because coccinelle
formatted some lines differently than our coding style.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The qemuMonitorJSONGetMigrationCapabilities() command executes
'query-migrate-capabilities' command and returns early if QEMU
doesn't know the command. Well, the command was introduced in
QEMU release 1.2 (specifically in commit v1.2.0-rc0~29^2~11) and
since the minimum required version is 2.11.0 we can be sure that
command will always exist.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The qemuMonitorJSONGetMemoryDeviceInfo() command executes
'query-memory-devices' command and returns early if QEMU
doesn't know the command. Well, the command was introduced in
QEMU release 2.1 (specifically in commit v2.1.0-rc0~41^2~9) and
since the minimum required version is 2.11.0 we can be sure that
command will always exist.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The qemuMonitorJSONGetKVMState() command executes 'query-kvm'
command and returns early if QEMU doesn't know the command. Well,
the command was introduced in QEMU release 0.14 and since the
minimum required version is 2.11.0 we can be sure that command
will always exist.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The qemuMonitorJSONGetDumpGuestMemoryCapability() command
executes 'query-dump-guest-memory-capability' command and returns
early if QEMU doesn't know the command. Well, the command was
introduced in QEMU release 2.0 (specifically in commit
v2.0.0-rc0~43^2~16) and since the minimum required version is
2.11.0 we can be sure that command will always exist.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The qemuMonitorJSONGetMigrationParams() function executes
'query-migrate-parameters' command and returns early if QEMU
doesn't know the command. Well, the command was introduced in
QEMU release 2.4 (specifically in commit v2.4.0-rc0~147^2~3) and
since the minimum required version is 2.11.0 we can be sure that
the command will always exist.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
* XML serialization and deserialization of PCI VPD;
* PCI VPD capability flags added and used in relevant places;
* XML to XML tests for the added capability.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Dmitrii Shcherbakov <dmitrii.shcherbakov@canonical.com>
Add helper functions to virpci to provide means of checking for a VPD
file presence and for VPD resource retrieval using the PCI VPD parser.
The added test assesses the basic functionality of VPD retrieval while
the full parser is tested by virpcivpdtest.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Dmitrii Shcherbakov <dmitrii.shcherbakov@canonical.com>
Add support for deserializing the binary PCI/PCIe VPD format and storing
results in memory.
The VPD format is specified in "I.3. VPD Definitions" in PCI specs
(2.2+) and "6.28.1 VPD Format" PCIe 4.0. As section 6.28 in PCIe 4.0
notes, the PCI Local Bus and PCIe VPD formats are binary compatible
and PCIe 4.0 merely started incorporating what was already present in
PCI specs.
Linux kernel exposes a binary blob in the VPD format via sysfs since
v2.6.26 (commit 94e6108803469a37ee1e3c92dafdd1d59298602f) which requires
a parser to interpret.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Dmitrii Shcherbakov <dmitrii.shcherbakov@canonical.com>
This will make it possible to limit changes to a single spot
later on, and is also just an overall nicer way to create and
destroy objects.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There doesn't seem to be a reason for IOMMUs not to be handled
by this function.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This involves a bit of a hack, but is overall preferable to
forcing callers to pass non-const devdata as argument.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This commit is related to 5de203f879 which I pushed a few days
ago. While that commit prioritized closing clients socket over
the rest of I/O process, this one goes one step further and
temporarily suspends processing new connection requests.
A brief recapitulation of the problem:
1) assume that libvirt is at the top of RLIMIT_NOFILE (that is no
new FDs can be opened).
2) we have a client trying to connect to a UNIX/TCP socket
Because of 2) our event loop sees POLLIN on the socket and thus
calls virNetServerServiceAccept(). But since no new FDs can be
opened (because of 1)) the request is not handled and we will get
the same event on next iteration. The poll() will exit
immediately because there is an event on the socket. Thus we end
up in an endless loop.
To break the loop and stop burning CPU cycles we can stop
listening for events on the socket and set up a timer tho enable
listening again after some time (I chose 5 seconds because of no
obvious reason).
There's another area where we play with temporarily suspending
accept() of new clients - when a client disconnects and we check
max_clients against number of current clients. Problem here is
that max_clients can be orders of magnitude larger than
RLIMIT_NOFILE but more importantly, what this code considers
client disconnect is not equal to closing client's FD.
A client disconnecting means that the corresponding client
structure is removed from the internal list of clients. Closing
of the client's FD is done from event loop - asynchronously.
To avoid this part stepping on the toes of my fix, let's make the
code NOP if socket timer (as described above) is active.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Some guest features that map to the -cpu arg are still added using
implicit syntax "feature" which is a deprecated shorthand for
"feature=on".
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
With the previous refactorings, there's no real benefit from the
qemuBuildCpuFeature helper method. Only one of the callers really
needs the CPU feature name re-writing logic, the others can just
use the right name directly.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The -cpu arg gained support for feature=on|off syntax for the x86
emulator in 2.4.0
commit 38e5c119c2925812bd441450ab9e5e00fc79e662
Author: Eduardo Habkost <ehabkost@redhat.com>
Date: Mon Mar 23 17:29:32 2015 -0300
target-i386: Register QOM properties for feature flags
Most other targets gained this syntax even earlier in 1.4.1
commit 1590bbcb02921dfe8e3cf66e3a3aafd31193babf
Author: Andreas Färber <afaerber@suse.de>
Date: Mon Mar 3 23:33:51 2014 +0100
cpu: Implement CPUClass::parse_features() for the rest of CPUs
CPUs who do not provide their own implementation of feature parsing
will treat each option as a QOM property and set it to the supplied
value.
There appears no reason to keep supporting "+|-feature" syntax,
given the current minimum QEMU version.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
QEMU switched from using underscores in x86 CPU features to hyphens
in the 2.8.0 series with two commits
commit fc7dfd205f3287893c436d932a167bffa30579c8 (HEAD, refs/bisect/bad)
Author: Eduardo Habkost <ehabkost@redhat.com>
Date: Fri Sep 30 15:49:40 2016 -0300
target-i386: Remove underscores from feat_names arrays
commit 54b8dc7c19cd781e96f1e9b001ca6001d804eb19
Author: Eduardo Habkost <ehabkost@redhat.com>
Date: Fri Sep 30 15:49:38 2016 -0300
target-i386: Register aliases for feature names with underscores
Libvirt names use underscores so we conditionally tranlate the
names when talking to new QEMU. Since the min QEMU was raised to
version 2.11.0, all QEMU versions we talk to expect hypens, so
the translation can be done unconditionally.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
QEMU switched from using underscores in x86 CPU features to hyphens
in the 2.8.0 series with two commits
commit fc7dfd205f3287893c436d932a167bffa30579c8 (HEAD, refs/bisect/bad)
Author: Eduardo Habkost <ehabkost@redhat.com>
Date: Fri Sep 30 15:49:40 2016 -0300
target-i386: Remove underscores from feat_names arrays
commit 54b8dc7c19cd781e96f1e9b001ca6001d804eb19
Author: Eduardo Habkost <ehabkost@redhat.com>
Date: Fri Sep 30 15:49:38 2016 -0300
target-i386: Register aliases for feature names with underscores
Libvirt names use underscores so we conditionally tranlate the
names when talking to new QEMU. Since the min QEMU was raised to
version 2.11.0, all QEMU versions we talk to expect hypens, so
the translation can be done unconditionally.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Always fetch the stats for all backing chain members. Callers from
qemu_driver.c already always passed 'true' and the caller from the
migration code won't mind when we fetch all stats.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
All (proper) callers pass true so we can remove the argument.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use automatic memory clearing and remove the cleanup label.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use automatic memory clearing for the temporary variables and remove the
cleanup section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use automatic memory clearing for the temporary variable and remove the
cleanup section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use automatic memory clearing for the temporary variable and remove the
cleanup section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Similarly to the fix to 'qemuDomainBlocksStatsGather' we should be
always fetching the full backing chain so that we can avoid any
automatic filter notes which would prevent us from fetching the stats
for the correct nodename.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In certain cases such as when running a backup blockjob qemu installs a
filter node between the frontend and the top node of the backend of the
disk. The stats gathering code didn't instruct the monitor code to fetch
the stats for all the layers, so since the top layer now doesn't have
stats we were reporting wrong stats such as allocation.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2015281
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Using swtpm v0.7.0 we can run swtpm_setup to create default config files
for swtpm_setup and swtpm-localca in session mode. Now a user can start
a VM with an attached TPM without having to run this program on the
command line before. This program needs to run once.
This patch addresses the issue raised in
https://bugzilla.redhat.com/show_bug.cgi?id=2010649
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Libvirt will put the pid file of virtiofsd to per-domain directory.
However, the ownership of the per-domain directory is the user to run
the QEMU process and the user has the write permission of the directory.
If VM escape occurs, the attacker can
1. write arbitrary content to the pid file (if running QEMU using root),
then the attacker can kill any process by writing appropriate pid to
the pid file;
2. spoof the pid file (if running QEMU using a regular user), then the
virtiofsd process will never be cleared even if the VM is destroyed.
So, move the pid file of virtiofsd from per-domain directory to
stateDir.
Signed-off-by: Peng Liang <liangpeng10@huawei.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Libvirt will put the pid file of pr-helper to per-domain directory.
However, the ownership of the per-domain directory is the user to run
the QEMU process and the user has the write permission of the directory.
If VM escape occurs, the attacker can
1. write arbitrary content to the pid file (if running QEMU using root),
then the attacker can kill any process by writing appropriate pid to
the pid file;
2. spoof the pid file (if running QEMU using a regular user), then the
pr-helper process will never be cleared even if the VM is destroyed.
So, move the pid file of pr-helper from per-domain directory to
stateDir.
Signed-off-by: Peng Liang <liangpeng10@huawei.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When parsing of the node device XML fails we'd still call the post-parse
and validation callbacks which makes no sense. Additionally the
callbacks were expecting a non-NULL pointer which leads to a crash.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2014139
Fixes: d5ae634ba28
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Starting with QEMU-6.2 started accepting a JSON object as argument for
'-device' which will also become the only syntax considered stable by
qemu in the future.
Since libvirt was recently converted to generate the properties via JSON
to begin wit we can start using it on the commandline as well, by simply
enabling the QEMU_CAPS_DEVICE_JSON capability, which we do by probing
for the 'json-cli' feature flag of 'device_add'.
Normally a change which changes a commandline output should be happening
only after the impacted real-caps test files are forked in the version
preceding the change, but in this case it's not necessary as the logic
for generating the device properties stays identical and we just change
the output format (avoid conversion). Additionally we still have a lot
of tests validating the conversion to the old commandline options.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There are two variables (@vm and @domflags) in qemuConnectGetAllDomainStats()
that are used only within the for() loop but declared for entire function.
Bring them into the loop to make it obvious they are not used outside of it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
query-dirty-rate command is used for virsh domstats by default, but this
is available only on qemu >=5.2.0.
By this commit, qemu domain stats will check capabilities requirements before issuing actual query.
Signed-off-by: Hiroki Narukawa <hnarukaw@yahoo-corp.jp>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
One of qemuDomainGetStatsWorkers requires capabilities to run.
This commit adds capability information to qemuDomainGetStatsWorkers.
Signed-off-by: Hiroki Narukawa <hnarukaw@yahoo-corp.jp>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
query-dirty-rate command is used for virsh domstats by default, but this
is available only on qemu >=5.2.0.
In this commit, add capability flag for query-dirty-rate first.
Signed-off-by: Hiroki Narukawa <hnarukaw@yahoo-corp.jp>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
