For statically declared arrays one can use G_N_ELEMENTS() instead
of explicit sizeof(array) / sizeof(item). I've noticed couple of
places where the latter was used.
I am not fixing every occurrence because we have some places
which do not use glib (examples and NSS module).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
I've noticed one function inside virpcivpd.c, namely
virPCIVPDParseVPDLargeResourceFields() that declares some
variables at the top level even though they are used only inside
a loop in which they have to be freed explicitly.
Bringing variable declarations into the loop allows us to make
the code nicer.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
I've noticed three functions inside node_device_conf.c, namely:
- virNodeDeviceCapVPDParseCustomFields()
- virNodeDeviceCapVPDParseReadOnlyFields()
- virNodeDeviceCapVPDParseXML()
that have strange attitude towards g_auto* variables. The first
problem is that variables are declared at the top level despite
being used inside a loop. The second problem is use of g_free()
in combination with g_steal_pointer() even though we have
VIR_FREE() which does exactly that.
Bringing variable declarations into their respective loops allows
us to make the code nicer.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
While invalid values need to be ignored when presenting VPD data to the
user, it would be good to attempt to parse a valid portion of the VPD
instead of marking it invalid as a whole.
Based on a mailing list discussion, the set of accepted characters is
extended to the set of printable ASCII characters.
https://listman.redhat.com/archives/libvir-list/2021-October/msg01043.html
The particular example encountered on real hardware was multi-faceted:
* "N/A" strings present in read-only fields. This would not be a useful
valid value for a field (especially if a unique serial number is
expected), however, it was decided to delegate handling of those kinds
of values to higher-level software;
* "4W/1W PCIeG2x4" - looks like some vendors use even more printable
characters in the ASCII range than we currently allow. Since the
PCI/PCIe VPD specs mention alphanumeric characters without specifying
the full character set, it looks like this is ambiguous for vendors
and they tend to use printable ASCII characters;
* 0xFF bytes present in VPD-W field values. Those bytes do not map to
printable ASCII code points and were probably used by the vendor as
placeholders. Ignoring the whole VPD because of that would be too
strict.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Dmitrii Shcherbakov <dmitrii.shcherbakov@canonical.com>
* RV and RW fields must be at the last position in their respective
section (per the conditions in the spec). Therefore, the parser now
stops iterating over fields as soon as it encounters one of those
fields and checks whether the end of the resource has been reached;
* The lack of the RW field is not treated as a parsing error since we
can still extract valid data even though this is a PCI/PCIe VPD spec
violation;
* Individual fields must have a valid length - the parser needs to check
for invalid length values that violate boundary conditions of the
resource.
* A zero-length field may be the last one in the resource, however, the
boundary check is currently too strict to allow that.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Dmitrii Shcherbakov <dmitrii.shcherbakov@canonical.com>
There are a lot of places where we call virInterfaceDefFree()
explicitly. We can define autoptr cleanup macro and annotate
declarations with g_autoptr() and remove plenty of those explicit
free calls.
This also fixes a memory leak in udevInterfaceGetXMLDesc() which
called virInterfaceDefFree() only in successful path.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
Move the code that adds encryption options for the swtpm_setup command
line into its own function.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
QEMU-6.2 added feature flags for enum types. Add support for querying
them into our QMP schema query language.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Starting from QEMU-6.2 enum members are reported as an array of objects
under new name "values" so that extra data can be reported for each
member.
Modify the code so that we prefer 'members' and skip 'values' completely
if we've used 'members'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Move them closer to where they are actually used.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The QMP implementation didn't use any new approach. The command itself
is now only used with legacy qemu versions.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Report an error if the new hotplug is not supported and remove the
alternate code paths.
The modern cpu-hotplug code was introduced in qemu-2.7. We keep the
capability so that proper errors are reported in case a platform doesn't
support hotplug.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Open code virHashAddEntry so that the error code path can be avoided.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The virCapabilitiesAddGuestDomain() function can't fail. It
aborts on OOM. Therefore, there's no need to check for its
return value.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The virCapabilitiesAddGuest() function can't fail. It aborts on
OOM. Therefore, there's no need to check for its return value.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In ch_driver.c there are two forward declarations that are not
needed. Drop them.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
For other interface type, values in tc rules are calculated by
multiply by 8*1000 instead of 8*1024.
Signed-off-by: zhangjl02 <zhangjl02@inspur.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When 'swtpm_setup --print-capabilities' shows the 'tpm12-not-need-root'
flag, then it is possible to create certificates for the TPM 1.2 also
in non-privileged mode since swtpm_setup doesn't need tcsd anymore.
Check for this flag and create the certificates if this flag is found.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
QEMU version 3.1 introduced PV_SEND_IPI CPUID feature bit under
commit 7f710c32bb8 (target-i386: adds PV_SEND_IPI CPUID feature bit).
This patch adds a new KVM feature 'pv-ipi' to disable this feature
(enabled by default). Newer CPU platform (Ex, AMD Zen2) supports
hardware accelation for IPI in guest, to use this feature to get
better performance in some scenarios. Detailed about the discussion:
https://lkml.org/lkml/2021/10/20/423
To disable kvm-pv-ipi and have libvirt add "-cpu host,kvm-pv-ipi=off"
to the QEMU command line, the following XML code needs to be added to the
guest's domain description:
<features>
<kvm>
<pv-ipi state='off'/>
</kvm>
</features>
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This section of code was left unused ever since it was introduced
ten years ago. I think we can safely remove it.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It may happen that qemuProcessStop() is called from "qemu-event"
thread. But this thread doesn't have any virIdentity set
(virIdentity being thread local) and therefore it may be unable
to open connection to secondary drivers. It is unable to do so
in split daemon scenario, because in there opening a connection
is coupled with copying current thread identity onto the
connection. Code-wise, virIdentityGetCurrent() returns NULL which
in turn makes virGetConnectGeneric() fail. This problem does not
occur in monolithic daemon scenario, because no identity copying
is done there.
Long story short, inability to open secondary driver connection
can lead to unwanted results. Therefore, do what
qemuProcessReconnectHelper() does - set the new thread identity
to be the one of the caller.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2013573
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
In some cases the worker func running inside the pool may rely on
virIdentity. While worker func could check for identity and set
one it is not optimal - it may not have access to the identity of
the thread creating the pool and thus would have to call
virIdentityGetSystem(). Allow passing identity when creating the
pool.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This is a typical example of what can go wrong when sending out
an old patch. Back in January, when I was writing
qemuProcessHandleMemoryDeviceSizeChange() events were sent to the
worker pool thread using virThreadPoolSendJob(). Then, in July a
helper was introduced (qemuProcessEventSubmit()) but since my
code was not committed and I did not pay attention my code wasn't
updated. Later, when I merged my code it uses the old approach.
BTW: this also fixes a possible double free which I completely
missed when writing the code ~10 months ago.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Nobody's interested in the return value of any of
struct _qemuMonitorCallbacks callbacks. They are all void, but
domainMemoryDeviceSizeChange. Change it to void.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
While the QCOW2 cluster size is represented in only 4 bits in the QCOW2
header and thus 1 << cluster_size cannot overflow int,
qcow2GetClusterSize is supposed to return unsigned long long so we can
just compute the result as ULL rather than computing it as int and
promoting to unsigned long long.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
One of the paths returned -1 directly without going through the cleanup
section.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
It's only used once and open coding it is at least as clear as using the
macro.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
qemuBuildPMPCIRootHotplugCommandLine() returns 0 unconditionally. There is no
failure scenario at present. So clean up the code by removing integer return
from the function and also remove the failure check conditional from the
function call.
Also fix indentation for the above function call while at it.
Signed-off-by: Ani Sinha <ani@anisinha.ca>
Reviewed-by: Laine Stump <laine@redhat.com>
Error messages must conform to spec as specified here:
https://www.libvirt.org/coding-style.html#error-message-format
This change makes some error messages conform to the spec above.
Fixes: 8eadf82fb5 ("conf: introduce option to enable/disable pci hotplug on pci-root controller")
Signed-off-by: Ani Sinha <ani@anisinha.ca>
Reviewed-by: Laine Stump <laine@redhat.com>
This reverts commit 58ba0f6a3d.
Conflict:
* src/qemu/qemu_capabilities.[ch]
Because other new cap flags had been added since the original
commit, reformatting was necessary to follow the "groups of
five" pattern.
* tests.qemucapabilitiesdata/caps_6.2.0.x86_64.xml
This file was added after the original commit that we
are reverting, so had to be manually edited to remove
the two capabilities.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ani Sinha <ani@anisinha.ca>
This reverts commit 7300ccc9b3.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ani Sinha <ani@anisinha.ca>
This reverts commit bef0f0d8be.
Conflicts:
tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.args
* this file had been renamed from its original, then renamed back,
which understandably confused git. It's being completely removed
here anyway, so the contents don't matter.
tests/qemuxml2argvtest.c
* change in context around removed chunk
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ani Sinha <ani@anisinha.ca>
This reverts commit 7d074c5683.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ani Sinha <ani@anisinha.ca>
This reverts commit bdc3e8f47b.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ani Sinha <ani@anisinha.ca>
This reverts commit 618e8665db.
This is the first in a series of 10 commits that revert (in reverse
order) the changes to add the <acpi-hotplug-bridge state='on|off'/>
switch to libvirt domain XML, which unfortunately needs to be removed
due to QEMU developers discovering a flaw with the design of the QEMU
commandline switch used to implement the libvirt switch that will
likely result in a new and different method of selecting hotplug
modes. Because the libvirt switch has not been in any official
releases of libvirt, we are still able to remove it completely, rather
than deprecating it.
The original commits began with commit
58ba0f6a3d. The other original commit
IDs are documented in each revert commit.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ani Sinha <ani@anisinha.ca>
This commit extends libvirt XML configuration to support luks2 encryption format.
This means that <encryption format="luks2" engine="librbd"> becomes valid.
Currently librbd is the only engine that supports this new format.
Signed-off-by: Or Ozeri <oro@il.ibm.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
rbd encryption is new in qemu 6.1.0.
This commit adds a new encryption engine property which
allows the user to use this new encryption engine.
Signed-off-by: Or Ozeri <oro@il.ibm.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This commit extends libvirt XML configuration to support a custom encryption engine.
This means that <encryption format="luks" engine="qemu"> becomes valid.
The only engine for now is qemu. However, a new engine (librbd) will be added in an upcoming commit.
If no engine is specified, qemu will be used (assuming qemu driver is used).
Signed-off-by: Or Ozeri <oro@il.ibm.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
rbd encryption is new in qemu 6.1.0.
This commit adds capability probing for it.
Signed-off-by: Or Ozeri <oro@il.ibm.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The post parse callback is part of the real (non-test) processing flow.
This commit adds it (for disks) to the qemublocktest flow as well.
Specifically, this will be needed for tests that use luks encryption,
so that the default encryption engine (which is added in an upcoming commit)
will be overridden by qemu.
Signed-off-by: Or Ozeri <oro@il.ibm.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
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: d5ae634ba2
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>
There have been countless reports from users concerned about the following
error reported by libvirtd when qemu domains are shutdown
internal error: End of file from qemu monitor
While the error is harmless, users often mistaken it for real problem with
their deployments. EOF from the monitor can't be entirely ignored since
other threads may be using the monitor and must be able to detect the EOF
condition.
One potential fix is to delay reporting EOF until the monitor is used
after EOF is detected. This patch adds a 'goteof' member to the
qemuMonitor structure, which is set when EOF is detected on the monitor
socket. If another thread later tries to send data on the monitor, the
EOF error is reported.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There are few functions in virnetsocket.c where an object/memory
is freed by explicit call. Use g_autoptr()/g_autofree/VIR_AUTOCLOSE
to do that automatically.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The aim of virNetSocketNewConnectCommand() is to execute passed
command and attach socket pair/pipe to it so that client socket
can be opened (this is used for connections with alternative
transports, e.g. ssh). The virCommand is created in a caller and
then passed to virNetSocketNewConnectCommand() where it is freed
using virCommandFree(). This approach is wrong on two levels:
1) The deallocation happens on a different level than allocation,
2) There's a WIN32 stub that just reports an error and doesn't
free the command.
However, with g_autoptr() trickery the command can be freed in
caller.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Commit ad209e7d adds QEMU_CAPS_VIRTIO_BLK_QUEUE_SIZE capability, but
the following commit 2d6d67e1 missed to use it and uses
QEMU_CAPS_VIRTIO_BLK_NUM_QUEUES instead.
This commit fixes the mistake.
Signed-off-by: Hiroki Narukawa <hnarukaw@yahoo-corp.jp>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The capability reflects whether QEMU is capable of -device
virtio-*,ats=. Since the property was introduced in QEMU commit
v2.9.0-rc0~162^2~32 we can safely assume the property is always
present as the minimal version required is 2.11.0.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The capability reflects whether QEMU is capable of -device
virtio-*,iommu_platform=. Since the property was introduced in
QEMU commit v2.9.0-rc0~162^2~37 we can safely assume the property
is always present as the minimal version required is 2.11.0.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
We did not set priv->migMaxBandwidth if '--bandwidth' was
specified as an option in the 'migrate' virsh command. This
caused in printing the wrong value if virsh command
'migrate-getspeed' was called during the migration. This patch
first sets the value to the given bandwidth (if one was
specified) and restores the previous value after the migration.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1806856
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
QEMU declares the bootindex types as:
bootindexA=<int32>
bootindexB=<int32>
The driveA/driveB parameters were deprecated and removed in qemu-6.0.
We'll keep them for compatibility, but they are not used with -blockdev.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemuBuildFloppyCommandLineControllerOptions was generating config for
both the implicit and explicit fdc. The explicit FDC is using '-device'
and thus will need to be converted to JSON.
Split up the lookup of the floppy drive configs from the actual command
generation.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
QEMU declares the props we control as:
'ccid-card-emulated'
backend=<str>
cert1=<str>
cert2=<str>
cert3=<str>
db=<str>
'ccid-card-passthru'
chardev=<str> - ID of a chardev to use as a backend
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
While this device doesn't have any properties it must be converted to
use qemuBuildDeviceCommandlineFromJSON so that we can validate it in the
future.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
QEMU declares the 'guid' property as:
guid=<str> - UUID (aka GUID) or "auto" for random value (default) (default: "auto")
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Move it into the validator. Note that the placement into the device
validation part is intentional so that it also covers hotplug code
paths.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We have a commonly used helper virDomainControllerAliasFind, which does
the same thing and also reports errors internally.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use 'virXMLPropEnum' to parse it and fix all switch statements which
didn't include the VIR_DOMAIN_SMARTCARD_TYPE_LAST case.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Don't check the type twice, move the chardev validation into the
switch.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Now that the code was converted to use this helper we can remove it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
../../work/libvirt/src/qemu/qemu_hotplug.c: In function ‘qemuDomainAttachFSDevice’:
../../work/libvirt/src/qemu/qemu_hotplug.c:3458:68: error: ordered comparison of pointer with integer zero [-Werror=extra]
3458 | if (qemuBuildVHostUserFsDevProps(fs, vm->def, charAlias, priv) < 0)
| ^
cc1: all warnings being treated as errors
Fixes: b987873034
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Do not depend on passing a logManager. Create a new connection.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
We control only the 'tpmdev' property of TPM devices which is a string.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The codec devices have the following properties we control:
cad=<uint32> - (default: 4294967295)
audiodev=<str> - ID of an audiodev to use as a backend
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The sound devices have only the 'audiodev' property which is a string.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
All virtio devices were converted to the new JSON formatter so we can
remove the old one.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We control the following properties of the devices in question:
'virtio-gpu'
virgl=<bool> - on/off (default: true)
'qxl'
ram_size=<uint32> - (default: 67108864)
vram_size=<uint64> - (default: 67108864)
vram64_size_mb=<uint32> - (default: 4294967295)
vgamem_mb=<uint32> - (default: 16)
max_outputs=<uint16> - (default: 0)
'vhost-user-gpu'
max_outputs=<uint32> - (default: 1)
chardev=<string>
'VGA'
vgamem_mb=<uint32> - (default: 16)
'bochs-display'
vgamem=<size> - (default: 16777216)
common for all devices:
xres=<uint32> - (default: 0)
yres=<uint32> - (default: 0)
The only noticable change is using memory size in bytes for
'bochs-display' instead of kibibytes.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'vhost-user-fs-pci' has following properties we control:
chardev=<str> - ID of a chardev to use as a backend
queue-size=<uint16> - (default: 128)
tag=<str>
bootindex=<int32>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Remove the old-style 'device_add' helpers which parse the commandline
arguments to JSON since we now coverted all usage to use JSON directly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Build the properties of 'vhost-vsock' device via JSON. In comparison to
previous similar refactors this also modifies the hotplug code to attach
the vhost fd handle explicitly rather than using
'qemuMonitorAddDeviceWithFd'.
The properties of vhost-vsock have the following types according to
QEMU:
guest-cid=<uint64> - (default: 0)
vhostfd=<str>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Build the properties of 'vhost-scsi' device via JSON. In comparison to
previous similar refactors this also modifies the hotplug code to attach
the vhost fd handle explicitly rather than using
'qemuMonitorAddDeviceWithFd'.
The 'vhost-scsi' device doesn't have any special (non-string) properties.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Build commandlines for character devices via JSON.
For devices using 'VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL' address
type 'qemuBuildDeviceAddressProps' will now generate the address. The
only special property is 'nr'. QEMU declares it as:
nr=<uint32> - (default: 4294967295)
The test fallout is caused by formatting addresses as decimal numbers
instead of hex as described in the commit which added
'qemuBuildDeviceAddressProps'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The handlers for PCI, SCSI and USB controllers already use JSON
internally. This patch converts 'virtio-serial', 'ccid' and 'sata' to do
the same and passes out the JSON directly so that it can be used in
monitor code to avoid conversion.
From the controllers converted in this patch only 'virtio-serial' has
special properties. QEMU thinks they have the following types:
max_ports=<uint32> - (default: 31)
vectors=<uint32> - (default: 2)
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Internally format the PCI controller properties into JSON, but convert
it back to a string as preparation for upcoming refactors.
The following types are declared for the properties we use by QEMU:
'nec-usb-xhci'
p2=<uint32> - (default: 4)
p3=<uint32> - (default: 4)
'ich9-usb-uhci6'
masterbus=<str>
firstport=<uint32> - (default: 0)
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Move the validation code into a separate function. For now the
validation is still kept in the commandline format step as simply just
moving it to the validator causes failures in the test suite, which will
need to be investigated deeper.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Internally format the PCI controller properties into JSON, but convert
it back to a string so that we for now change just the SCSI controller.
The change in tests is expected as the 'port' field for various PCI
controllers is expected to be a number and thus can't be represented as
a hexadecimal value in JSON.
QEMU expects the following types:
'pci-bridge'
chassis_nr=<uint8> - (default: 0)
'pxb-pcie':
bus_nr=<uint8> - (default: 0)
'pcie-root-port'
port=<uint8> - (default: 0)
chassis=<uint8> - (default: 0)
hotplug=<bool> - (default: true)
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Internally format the SCSI controller properties into JSON, but convert
it back to a string so that we for now change just the SCSI controller.
The change in tests is expected as the 'reg' field for a spapr-vio
address is expected to be a number:
$ qemu-system-ppc64 -device spapr-vscsi,help
spapr-vscsi options:
reg=<uint32> - (default: 4294967295)
The hand-rolled generator used hex representation but that will not be
possible on the monitor via JSON.
The properties of 'virtio-scsi' have following types according to QEMU:
iothread=<link<iothread>>
num_queues=<uint32> - (default: 4294967295)
cmd_per_lun=<uint32> - (default: 128)
max_sectors=<uint32> - (default: 65535)
ioeventfd=<bool> - on/off (default: true)
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Move the code into a new function called qemuBuildControllerPCIDevStr
so that the code is self contained and the original function easier to
follow.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Move the code into a new function called qemuBuildControllerSCSIDevStr
so that the code is self contained and the original function easier to
follow.
This patch also moves the formatting of the properties relevant only for
the 'virtio-scsi' controller to the specific case so it's more clear
where they belong to.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Now that all users were converted to qemuBuildRomProps we can remove the
old code and un-mark qemuBuildRomProps as unused.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Move the bootindex before the address so that the code is simpler.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The types for the special fields of the 'virtio-blk-pci' according to
QEMU are:
iothread=<link<iothread>>
ioeventfd=<bool> - on/off (default: true)
event_idx=<bool> - on/off (default: true)
scsi=<bool> - on/off (default: false)
num-queues=<uint16> - (default: 65535)
queue-size=<uint16> - (default: 256)
For all disks we also use the following properties (based on 'scsi-hd'):
device_id=<str>
share-rw=<bool> - (default: false)
drive=<str> - Node name or ID of a block device to use as a backend
chardev=<str> - ID of a chardev to use as a backend <- vhost-user-blk-pci
bootindex=<int32>
logical_block_size=<size> - A power of two between 512 B and 2 MiB (default: 0)
physical_block_size=<size> - A power of two between 512 B and 2 MiB (default: 0)
wwn=<uint64> - (default: 0)
rotation_rate=<uint16> - (default: 0)
vendor=<str>
product=<str>
removable=<bool> - on/off (default: false)
write-cache=<OnOffAuto> - on/off/auto (default: "auto")
cyls=<uint32> - (default: 0)
heads=<uint32> - (default: 0)
secs=<uint32> - (default: 0)
bios-chs-trans=<BiosAtaTranslation> - Logical CHS translation algorithm, auto/none/lba/large/rechs (default: "auto") <- ide-hd
serial=<str>
werror=<BlockdevOnError> - Error handling policy, report/ignore/enospc/stop/auto (default: "auto")
rerror=<BlockdevOnError> - Error handling policy, report/ignore/enospc/stop/auto (default: "auto")
The 'wwn' field is changed from a hex string to a number since qemu
actually treats it as a number.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Extract the logic to determine the actual settings into
'qemuBuildDiskGetErrorPolicy' so that it can be reused when we'll
convert the disk -device formatter to JSON.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'event_idx' option for virtio devices was introduced by QEMU commit
bcbabae8f which is contained in v0.15.0-rc0 and can't be compiled out,
thus we don't need to conditionally enable it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The support for the 'ioeventfd' knob of virtio devices was introduced by
QEMU commit 25db9ebe15125 contained in v0.14.0-rc0 and it can't be
compiled out. Thus libvirt can assume it's support and remove
conditional code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Since 'qemuBuildDeviceAddressProps' now also builds 'drive' addresses
the generator is way simpler and doesn't use any special fields.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Move the code from 'qemuValidateDomainDeviceDefDiskFrontend' into
'qemuValidateDomainDeviceDefAddressDrive' which is called from
'qemuValidateDomainDeviceDefAddress' so that we have all address
validation code together.
This also allows us to remove the inline validation inside
'qemuBuildSCSIHostdevDevStr'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Introduce infrastructure to format 'drive' addresses via the standard
helper rather than hand-rolled generators used inline.
The code needs to know the disk bus to format the correct address which
is passed in via an internal field in virDomainDeviceDriveAddress.
The field types according to QEMU are as following:
'ide-hd' for VIR_DOMAIN_DISK_BUS_IDE and VIR_DOMAIN_DISK_BUS_SATA
unit=<uint32> - (default: 4294967295)
'floppy' for VIR_DOMAIN_DISK_BUS_FDC
unit=<uint32> - (default: 4294967295)
'scsi-hd' for VIR_DOMAIN_DISK_BUS_SCSI
channel=<uint32> - (default: 0)
scsi-id=<uint32> - (default: 4294967295)
lun=<uint32> - (default: 4294967295)
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
For properties we use these are the QEMU types:
host=<str> - Address (bus/device/function) of the host device, example: 04:10.0
bootindex=<int32>
failover_pair_id=<str>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
For 'usb-mouse'/'usb-tablet'/'usb-kbd' we don't use any special
property.
For 'virtio-input-pci' we only use the 'evdev' argument which is a
string so this conversion doesn't impact anything.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'usb-redir' device has the following types according to QEMU for
properties we control:
chardev=<str> - ID of a chardev to use as a backend
filter=<str>
bootindex=<int32>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'usb-host' device has the following types according to QEMU for
properties we control:
hostdevice=<str>
hostbus=<uint32> - (default: 0)
hostaddr=<uint32> - (default: 0)
bootindex=<int32>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'vfio-pci-nohotplug' device has the following property types
according to QEMU:
display=<OnOffAuto> - on/off/auto (default: "off")
sysfsdev=<str>
ramfb=<bool>
bootindex=<int32>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'virtio-rng' has the following property types according to QEMU:
rng=<link<rng-backend>>
max-bytes=<uint64> - (default: 9223372036854775807)
period=<uint32> - (default: 65536)
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Move the 'deflate-on-oom' and 'free-page-reporting' before the address
to simplify the genrator code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The generated properties have the following types according to QEMU:
deflate-on-oom=<bool> - on/off (default: false)
free-page-reporting=<bool> - on/off (default: false)
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Note that the legacy 'ivshmem' device was already removed upstream, but
it's converted so that the code is identical.
For the two modern devices QEMU considers the properties being of
following types:
'ivshmem-doorbell'
chardev=<str> - ID of a chardev to use as a backend
ioeventfd=<bool> - on/off (default: true)
master=<OnOffAuto> - on/off/auto (default: "off")
vectors=<uint32> - (default: 1)
'ivshmem-plain'
master=<OnOffAuto> - on/off/auto (default: "off")
memdev=<link<memory-backend>>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The watchdog doesn't have any special properties.
Convert the command line generator and hotplug code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Format a JSON object with the device properties and then use
qemuBuildDeviceCommandlineFromJSON to convert it to the standard
commandline for now.
The 'ioport' property of 'pvpanic' is a number in QEMU:
ioport=<uint16> - (default: 1285)
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add a JSON variant of the generator 'rom' properties. For convenience
both the old and new are for now marked as unused, which will be removed
once the conversion is complete.
The formatted properties have following types according to QEMU.
'virtio-blk-pci' was used as an example:
rombar=<uint32> - (default: 1)
romfile=<str>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The helper converts the JSON object to a string and adds it to the
current command as arguments of '-device'. The helper also prepares for
'-device' taking JSON directly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add a JSON variant of the generator of properties for virtio devices.
For convenience both the old and new are for now marked as unused, which
will be removed once the conversion is complete.
The formatted properties have following types according to QEMU.
'virtio-blk-pci' was used as an example:
disable-legacy=<OnOffAuto> - on/off/auto (default: "auto")
disable-modern=<bool> - (default: false)
iommu_platform=<bool> - on/off (default: false)
ats=<bool> - on/off (default: false)
packed=<bool> - on/off (default: false)
Note that <OnOffAuto> is an enum type without alternates in QMP so it
must be represented as a string in JSON.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Move the validation from 'qemuBuildRomStr' into the function which
validates device info. It was originally named
'qemuValidateDomainDeviceDefAddress' but this commit renames it to
'qemuValidateDomainDeviceDefInfo'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Commit ffda44030a added validation of the 'acpiIndex' field in
virDomainDeviceInfo by calling 'virDomainDeviceInfoIterate' from
'qemuValidateDomainDef'. This is overly complicated we have
'qemuValidateDomainDeviceDef' which is already called for every single
device so we can avoid the extra loop.
Restructure the code by calling 'qemuValidateDomainDeviceInfo' directly
from 'qemuValidateDomainDeviceDef' and avoid unnecessary calls to
'virDomainDeviceGetInfo' by calling 'qemuValidateDomainDeviceDefAddress'
from 'qemuValidateDomainDeviceInfo'
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Remove the now unused boot-index related attributes and the code which
is assigning it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Fill in the effective boot index for network devices (or hostdev-backed
network devices via 'qemuProcessPrepareDeviceBootorder'. This patch
doesn't clean up the cruft to make it more obvious what's happening.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Rename it to 'qemuProcessPrepareDeviceBootorder' and call it from
'qemuProcessPrepareDomain' rather than
'qemuProcessPrepareDomainStorage'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'effectiveBootIndex' is a copy of 'bootIndex' if '<boot order=' was
present and left unassigned if not. This allows hypervisor drivers to
reinterpret <os><boot> without being visible in the XML.
QEMU driver had a internal implementation for disks, which is now
replaced. Additionally this will simplify a refactor of network boot
assignment.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'virtio-vga' is a virtio device but we didn't use the virtio formatter
for it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Merge the code from qemuBuildVirtioOptionsStr so that we don't have to
call two separate functions.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The code doesn't need the name as it determines it internally. Remove
the argument and fix all callers. In certain cases it led to
simplification of the logic.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Since we already have code for per-device behaviour we can also populate
the device name and extract virtioOptions in the switch statement so
that callers don't have to pass it in.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add the bus suffix in a separate call. This will make it more obvious
what's happening in the next commit.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function is static and will be needed in the virtio device config
helper.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'virtio' argument was misleadingly implying that it's true for all
virtio devices, but that's not the case. 'virtio-vga(-gl)' is a virtio
device but doesn't accept the usual bus-dependant suffix.
Add a comment for 'qemuDeviceVideoGetModel' and another boolean
'virtioBusSuffix' which carries the above meaning so that the 'virtio'
argument can be fixed (it will be used later).
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Split out the function a bit more to separate the per-device code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Extract the code into 'qemuBuildVirtioDevGetConfig' so that we can
later reuse it when converting individual device code into the more
modern JSON approach as the extracted code will be necessary either way.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
To simplify upcoming refactors change the logic such that we don't
return early for device types which can't be transitional.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Upcoming patches will start converting the formatting of arguments for
-device from a string to JSON so that we can keep proper types around
when using it via QMP.
This means we will need an equivalet for the device address builder
function. 'qemuBuildDeviceAddressProps' provides equal functionality,
but the output differs for fields where a number is expected, where
we've previously formatted a hex value but now end up with a decimal
value per JSON standard.
For given address types I've selected an example device and used
'-device $DEV,help' to obtain the current types recognized by qemu:
Note that 'bus' is not shown below, but it's already a string so we can
keep using it as a string.
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI (virtio-balloon-pci)
acpi-index=<uint32> - (default: 0)
addr=<int32> - Slot and optional function number, example: 06.0 or 06 (default: -1)
multifunction=<bool> - on/off (default: false)
Note that 'addr' is here defined as 'int32' but in fact internally in
qemu is an alternate type between a number and a string so we can keep
using strings here.
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB (usb-tablet)
port=<str>
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO (spapr-vty)
reg=<uint32> - (default: 4294967295)
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW (virtio-blk-cww)
devno=<str> - Identifier of an I/O device in the channel subsystem, example: fe.1.23ab
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA (isa-serial)
iobase=<uint32> - (default: 4294967295)
irq=<uint32> - (default: 4294967295)
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM (pc-dimm)
slot=<int32> - (default: -1)
addr=<uint64> - (default: 0)
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Split up the bus lookup into a function called
'qemuBuildDeviceAddressPCIGetBus'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Per coding guidelines error messages [1] should not be broken into
lines and variables should be separated by apostrophes.
[1] https://libvirt.org/coding-style.html#error-message-format
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The PCI address case grew massive over time. Split it out into a new
function qemuBuildDeviceAddressPCIStr.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Mention the QMP command 'device_add' rather than 'qemuMonitorAddDevice'
and remove the weird formatting.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
For conversion of '-device' we'll try to avoid usage of arrays if
possible, so for now if the array coversion function is not provided the
convertor will error out.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
