As described in the previous commit, the units for 'burst' are
kibibytes and not kilobytes, i.e. multiples of 1024 not 1000.
Therefore, when constructing ovs-vsctl command the burst value
must be multiplied by 1024 and not just 1000. And because ovs
expects this size in bits the value has to be multiplied again by
8.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1510237#c26
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The burst attribute for bandwidth specifies how much bytes can be
transmitted in a single burst. Therefore, the unit is in
multiples of 1024 (thus kibibytes) not SI-like 1000. It has
always been like that.
The 'tc' output is still confusing though, for instance:
# tc class add dev $DEV parent 1: classid 1:1 htb rate 1000kbps burst 2097152
# tc class show dev vnet2
class htb 1:1 root rate 8Mbit ceil 8Mbit burst 2Mb cburst 1600b
Please note that 2097152 = 2*1024*1024. Even the man page is
confusing. From tc(8):
kb or k Kilobytes
mb or m Megabytes
But I guess this is because 'tc' predates IEC standardisation of
binary multiples and thus can't change without breaking scripts
parsing its output.
And while at it, adjust _virNetDevBandwidthRate struct member
description, to make it obvious which members use SI/IEC units.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
There are few places in virDomainDefFormatFeatures() which can
use virXMLFormatElement() or virXMLFormatElementEmpty() instead
of writing directly into the output buffer.
After this, there are still a lot of places left, but that is
much bigger task.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
According to ioctl_ficlonerange(2)
These ioctl operations [FICLONE and FICLONERANGE] first
appeared in Linux 4.5. They were previously known as
BTRFS_IOC_CLONE and BTRFS_IOC_CLONE_RANGE, and were private
to Btrfs.
We no longer target any distro that comes with a kernel older
than 4.5, so we can stop looking for the btrfs and xfs
specific versions of the constant and just use the generic
version directly.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The virt_socket_lib is built from virnetsocket.c (among others).
But this file includes virprobe.h which includes libvirt_probes.h
which is a generated file. But this dependency is not recorded in
meson which may lead to a failed build.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The virDomainTPMDefFormat() function can't fail really. There's
no point in it returning an integer then. Make it return void and
fix both places which check for its retval.
And while at it, turn @def into a const pointer to make it
obvious the function does not modify passed struct.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The <tpm/> element formatting is handled in
virDomainTPMDefFormat() which uses the "old style" - appending
strings directly into the output buffer. With this, it's easy to
get conditions that tell when an element has ended wrong. In this
particular case, if both <encryption/> and <active_pcr_banks/>
are to be formatted the current code puts a stray '>' into the
output buffer, resulting in invalid XML.
Rewrite the function to use virXMLFormatElement() which is more
clever.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2016599#c15
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Changing <active_pcr_banks/> means changing the guest ABI and as
such must be prevented on both restoring from a file or
migration.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2035888
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The new helper replaces the 'value' part of the key-value tuple in an
object. The advantage of this new helper is that it preserves the
ordering of the key in the object when compared to a combination of
stealing the old key and adding a new value. This will be needed for a
new test/helper for validating and modifying qemu capabilities data.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We need libparted to be available at build time otherwise we
can't link against it; we don't, however, need the parted
command to be present until runtime and, just as is the case
for other commands, we already perform a lookup through the
virCommand API so making sure it's available at build time
is unnecessary.
This doesn't make any difference for platform such as Fedora
and CentOS, where both the library and the command are in the
same package, but others like Debian, Ubuntu and openSUSE
have separate packages for the two components and this change
means that we can install one less package at build time.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
As encryption norms get more strict it's easy to fall on the
insecure side. For instance, so far we are generating 2048 bits
long prime for Diffie-Hellman keys. Some systems consider this
not long enough. While we may just keep increasing the value
passed to the corresponding gnutls_* function, that is not well
maintainable. Instead, we may do what's recommended in the
gnutls_* manpage. From gnutls_dh_params_generate2(3):
It is recommended not to set the number of bits directly, but
use gnutls_sec_param_to_pk_bits() instead.
Looking into the gnutls_sec_param_to_pk_bits() then [1], 2048
bits corresponds to parameter MEDIUM.
1: https://www.gnutls.org/manual/gnutls.html#tab_003akey_002dsizes
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ani Sinha <ani@anisinha.ca>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
According to the gnutls_dh_set_prime_bits() manpage:
The function has no effect in server side.
Therefore, don't call it when creating server side context.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ani Sinha <ani@anisinha.ca>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
If needed 'virJSONValueIsNull' can be easily replaced by
'virJSONValueGetType(obj) == VIR_JSON_TYPE_NULL'.
'virJSONValueObjectIsNull' has confusing name because it checks that a
virJSONValue of OBJECT type has a key which is NULL, not that the object
itself is NULL. This can be replaced according to the needs e.g. by
virJSONValueObjectHasKey or the above check.
Both are unused.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
Replace the function by a call to virJSONValueNewString, when we copy
the string using g_strndup. Remove the unused helper.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
With 'g_strdup' not needing error handling we can ask callers to pass a
copy of the string which will be adopted by the JSON value.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
In two instances we've created a string virJSONValue just to append it
to the array. Replace it by use of the virJSONValueArrayAppendString
helper.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
The auth mode array is static, parse it from a JSON string.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
In cases where the qcow2 image is using subclusters/extended_l2 entries
we should propagate them to the new images which are based on such
images.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In order to be able to propagate image configuration to newly formatted
images we need to be able to query it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Allow creating the qcow2 with the new subcluster format if required.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add machinery for probing the incompatible feature flags field and
specifically extract whether the extended l2 feature (1 << 4) is
present.
For now we don't care abot the other features.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Prepare for extraction of features from the 'incompatible features'
group.
This is done by moving the extraction loop into a new function called
qcow2GetFeaturesProcessGroup. The new function also allows to ingore
features we don't care about by passing VIR_STORAGE_FILE_FEATURE_LAST as
the target flag.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
QCOW2 images now support 'extended_l2' which splits the default clusters
into 32 subcluster allocation units. This allows the allocation units to
be smaller without increasing the size of L2 table too much and thus also
the cache requirements for holding the full L2 table in memory.
Unfortunately it's incompatible with qemu versions older than 5.2 thus
can't be used as default.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function was formatted weirdly which prompted additions to conform
to the unusual style.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
After previous commit, it's no longer possible to change nodeset
for strict numatune. Therefore, we can start generating
host-nodes onto command line again.
This partially reverts d73265af6e.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Let's imagine a guest that's configured with strict numatune:
<numatune>
<memory mode='strict' nodeset='0'/>
</numatune>
For guests with NUMA:
Depending on machine type used (see commit v6.4.0-rc1~75) we
generate either:
1) -object '{"qom-type":"memory-backend-ram","id":"ram-node0",\
"size":20971520,"host-nodes":[0],"policy":"preferred"}' \
-numa node,nodeid=0,cpus=0,memdev=ram-node0
or
2) -numa node,nodeid=0,cpus=0,mem=20480
Later, when QEMU boots up and cpuset CGroup controller is
available we further restrict QEMU there too. But there's a
behaviour difference hidden: while in case 1) QEMU is restricted
from beginning, in case 2) it is not and thus it may happen that
it will allocate memory from different NUMA node and even though
CGroup will try to migrate it, it may fail to do so (e.g. because
memory is locked). Therefore, one can argue that case 2) is
broken. NB, case 2) is exactly what mode 'restrictive' is for.
However, in case 1) we are unable to update QEMU with new
host-nodes, simply because it's lacking a command to do so.
For guests without NUMA:
It's very close to case 2) from above. We have commit
v7.10.0-rc1~163 that prevents us from outputting host-nodes when
generating memory-backend-* for system memory, but that simply
allows QEMU to allocate memory anywhere and then relies on
CGroups to move it to desired location.
Due to all of this, there is no reliable way to change nodeset
for mode 'strict'. Let's forbid it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The whole idea of VIR_DOMAIN_NUMATUNE_MEM_RESTRICTIVE is that the
memory location is restricted only via CGroups and thus can be
changed on the fly (which is exactly what
qemuDomainSetNumaParamsLive() does. Allow this mode there then.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Set the kernel-hashes property on the sev-guest object if the config
asked for it explicitly. While QEMU machine types currently default to
having this setting off, it is not guaranteed to remain this way.
We can't assume that the QEMU capabilities were generated on an AMD host
with SEV, so we must force set the QEMU_CAPS_SEV_GUEST. This also means
that the 'sev' info in the qemuCaps struct might be NULL, but this is
harmless from POV of testing the CLI generator.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This sev-guest object property indicates whether QEMU should
expose the kernel, ramdisk, cmdline hashes to the firmware
for measurement.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Normally the SEV measurement only covers the firmware
loader contents. When doing a direct kernel boot, however,
with new enough OVMF it is possible to ask for the
measurement to cover the kernel, ramdisk and command line.
It can't be done automatically as that would break existing
guests using direct kernel boot with old firmware, so there
is a new XML setting allowing this behaviour to be toggled.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The VNC password authentication scheme is quite horrendous in that it
takes the user password and directly uses it as a DES case. DES is a
byte 8 keyed cipher, so the VNC password can never be more than 8
characters long. Anything over that length will be silently dropped.
We should validate this length restriction when accepting user XML
configs and report an error. For the global VNC password we don't
really want to break daemon startup by reporting an error, but
logging a warning is worthwhile.
https://bugzilla.redhat.com/show_bug.cgi?id=1506689
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Over time, the code using them got split into other files.
(Mostly qemu_interface.c and qemu_process.c)
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ani Sinha <ani@anisinha.ca>
'virDomainDiskInsert' orders the inserted disks by target. If the target
is not provided though it would try to parse it anyways. This lead to a
crash when parsing a definition where there are multiple disks and of
two disks sharing the bus at least one also misses the target.
Since we want to actually use the parser for stuff which doesn't
necessarily need the disk target, we make virDomainDiskInsert tolerant
of missing target instead. The definition will be rejected by the
validator regardless of the order the disks were inserted in.
Fixes: 61fd7174
Closes: https://gitlab.com/libvirt/libvirt/-/issues/257
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Commit 52521de8332c2323bd ("qemu: Use qemuDomainSaveStatus") replaced a call
to virDomainObjSave() with qemuDomainSaveStatus() as a part of cleanup. Since
qemuDomainSaveStatus() does not indicate any failure through its return code,
the error handling cleanup code got eliminated in the process. Thus upon
failure, we will no longer killing the started qemu process. This commit fixes
this by reverting the change that was introduced with the above commit.
Fixes: 52521de8332c2323bd ("qemu: Use qemuDomainSaveStatus")
Signed-off-by: Ani Sinha <ani@anisinha.ca>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Now that we only check whether the dnsmasq version is new enough,
there is no need for the caps field.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
Since dnsmasq supports --ra-param for a long time, this code is now
unused.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
All the capabilities should be supported in 2.67.
Make this the minimum version, since even the oldest
distros we support have moved on:
Debian 8: 2.72
CentOS 7: 2.76
Ubuntu 18.04: 2.79
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
The admin module is very closely tied to RPC. If we are
building without RPC support there's not much use for the
admin module, in fact it fails to build.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The logging manager is very closely tied to RPC. If we are
building without RPC support there's not much use for the
manager, in fact it fails to build.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Our RPC layer is as tied to XDR as possible. Therefore, if we
haven't detected and XDR library there's not much sense in trying
to build RPC layer.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There's nothing RPC specific about virnettlscontext.c or
virnetsocket.c. We use TLS for other things than just RPC
encryption (e.g. for generating random numbers) and sockets can
be used even without RPC.
Move these two sources into a static library (virt_socket) so
that other areas can use it even when RPC is disabled.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When implementing sparse streams, one of improvements I did was
to increase client buffer size for sending/receiving stream data
(commit v1.3.5-rc1~502). Previously, we were using 64KiB buffer
while packets on RPC are 256KiB (usable data is slightly less
because of the header). This meant that it took multiple calls of
virStreamRecv()/virStreamSend() to serve a single packet of data.
In my fix, I've included the virnetprotocol.h file which provides
VIR_NET_MESSAGE_LEGACY_PAYLOAD_MAX macro which is the exact size
of data in a single packet. However, including the file from
libvirt-stream.c which implements public APIs is not right. If
RPC module is not built then the file doesn't exists.
Redefine the macro and drop the include. The size can never
change anyways.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
And its callers. The parameter is no longer used since virDomainObjSave
was replaced with qemuDomainSaveStatus wrapper.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
It is a nice wrapper around virDomainObjSave which logs a warning, but
otherwise ignores the error. Let's use it where appropriate.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When return-path is enabled, QEMU on the source host won't report
completed migration until the destination QEMU sends a confirmation it
successfully loaded all data. Libvirt would detect such situation in the
Finish phase and report the error read from QEMU's stderr back to the
source, but using return-path could give use a bit better error
reporting with an earlier restart of vCPUs on the source.
The capability is only enabled when the connection between QEMU
processes on the source and destination hosts is bidirectional. In other
words, only when VIR_MIGRATE_TUNNELLED is not set, because our tunnel
only allows one-way communication from the source to the destination.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
So far we were enabling specific migration capabilities when a
corresponding API flag is set. We need to generalize our code to be able
to enable some migration capabilities unless a particular API flag is
used.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Different CPU generations have different limits on the number
of SEV/SEV-ES guests that can be run. Since both limits come
from the same overall set, there is typically also BIOS config
to set the tradeoff betweeen SEV and SEV-ES guest limits.
This is important information to expose for a mgmt application
scheduling guests to hosts.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This will be needed directly in the QEMU driver in a later patch.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
There are limits on the number of SEV/SEV-ES guests that can
be run on machines, which may be influenced by firmware
settings. This is important to expose to users.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Report extra info about the SEV setup, returning those fields
that are required to calculate the expected launch measurement
HMAC(0x04 || API_MAJOR || API_MINOR || BUILD ||
GCTX.POLICY || GCTX.LD || MNONCE; GCTX.TIK)
specified in section 6.5.1 of AMD Secure Encrypted
Virtualization API.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We're only returning the set of fields needed to perform an
attestation, per the SEV API docs.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Querying launch params on a inactive guest currently triggers
a warning about the monitor being NULL.
https://bugzilla.redhat.com/show_bug.cgi?id=2030437
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Since commit 46783e6307, the 'virsh dominfo' command calls
virDomainGetMessages to report any messages from the domain.
Hypervisors not implementing the API now get the following
libvirtd log message when clients invoke 'virsh dominfo'
this function is not supported by the connection driver: virDomainGetMessages
Although libxl currently does not support any tainting or
deprecation messages, provide an implementation to squelch
the previously unseen error message when collecting dominfo.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Currently, this attribute may either have a value of "custom", or be absent
(which defaults to "custom"), for backwards compatibility.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use two variables with automatic cleanup instead of reusing one.
Remove the pointless cleanup label.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_auto and get rid of the cleanup label, as well as the ret
variable.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_auto where possible, reduce scope of some variables and remove
pointless ret and rc variables.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
On QEMU command line it's represented by the dirty-ring-size
attribute of KVM accelerator.
Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Dirty ring feature was introduced in qemu-6.1.0, this patch
add the corresponding feature named 'dirty-ring', which enable
dirty ring feature when starting VM.
To enable the feature, the following XML needs to be added to
the guest's domain description:
<features>
<kvm>
<dirty-ring state='on' size='xxx'>
</kvm>
</features>
If property "state=on", property "size" must be specified, which
should be power of 2 and range in [1024, 65526].
Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In future commits we will need to store not just an array of
VIR_TRISTATE_SWITCH_* but also an additional integer. Follow the
example of TCG and introduce a structure where both the array an
integer can live.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There is no longer anything to initialize at binary startup time.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Since the currentBackend (direct vs. firewalld) setting is no longer
used for anything, we don't need to set it (either explicitly from
tests, or implicitly during init), and can completely remove it.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
It's unclear exactly why this check exists; possibly a parallel to a
long-removed check for the firewall-cmd binary (added to viriptables.c
with the initial support for firewalld in commit bf156385a0 in 2012,
and long since removed), or possibly because virFirewallOnceInit() was
intended to be called at daemon startup, and it seemed like a good
idea to just log this error once when trying to determine whether to
use firewalld, or direct iptables commands, and then not waste time
building commands that could never be executed. The odd thing is that
it would sometimes result in logging an error when it couldn't find a
binary that wasn't needed anyway (e.g., if all the rules were iptables
rules, but ebtables and/or ip6tables weren't also installed).
If we just remove this check, then virCommandRun() will end up logging
an error and failing if the needed binary isn't found when we try to
execute it, which seems like it should just as good (or at least good
enough, especially since we eventually want to get rid of iptables
completely).
So let's remove it!
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This function doesn't have anything to do with manipulating
virFirewall objects, but rather should be called in response to dbus
events about the firewalld service. Move this function into
virfirewalld.c, and rename it to virFirewallDSynchronize().
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This function doesn't need to check for a backend - synchronization
with firewalld should always be done whenever firewalld is registered
and available, not just when the firewalld backend is selected.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Since commit b19863640 both useful cases of the switch statement in
this function have made the same call (and the other/default case is
just an error that can never happen). Eliminate the switch to help
eliminate use of currentBackend.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Rather than calling these "ADD" and "REMOVE", which could be confused
with some other random items with the same names, make them more
specific by prepending "VIR_NETFILTER_" (because they will also be
used by the nftables backend) and rename them to match the
iptables/nftables operators they signify, i.e. INSERT and DELETE, just
to eliminate confusion (in particular, in case someone ever decides
that we need to also use the nftables "add" operator, which appends a
rule to a chain rather than inserting it at the beginning of the
chain).
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This function formats an address + prefix as, e.g. 192.168.122.0/24,
which is useful in places other than iptables. Move it to
virsocketaddr.c and make it public so that others can use it. While
moving, the bit that masks off the host bits of the address is made
optional, so that the function is more generally useful.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The network driver has put all its rules into private chains (created
by libvirt) since commit 7431b3eb9a, which was included in
libvirt-5.1.0. When the conversion was made, code was included that
would attempt to delete existing rules in the default chains, to make
it possible to upgrade libvirt without restarting the host OS.
Almost 3 years has passed, and it is doubtful that anyone will be
attempting to upgrade directly from a pre-5.1.0 libvirt to something
as new as 8.0.0 (possibly with the exception of upgrading the entire
OS to a new release, which would include also rebooting), so it is now
safe to remove this code.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_auto for virCommand and char * and drop the cleanup label.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use automatic cleanup for virCommand, steal it on success
and remove the error label.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_auto and remove the ret variable, as well as the cleanup label.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_auto and remove the 'ret' variable, as well as the out label.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_auto and remove the 'ret' variable, as well as the cleanup label.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_auto and remove the 'ret' variable, as well as the cleanup label.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Generating command line is pretty easy - just put tb-size=XXX
onto -accel tcg part. Note, that QEMU expects the size in MiB.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/229
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Kashyap Chamarthy <kchamart@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
After previous commit it's possible for domains to fine tune TCG
features (well, just one - tb-cache). Check that domain has TCG
enabled, otherwise the feature makes no sense.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
It may come handy to be able to tweak TCG options, in this
specific case the size of translation block cache size (tb-size).
Since we can expect more knobs to tweak let's put them under
common element, like this:
<domain>
<features>
<tcg>
<tb-cache unit='MiB'>128</tb-cache>
</tcg>
</features>
</domain>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Kashyap Chamarthy <kchamart@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When using the monolithic daemon the driver for virStream is
always virFDStreamDrv and thus calling virStreamInData() results
in calling virFDStreamInData().
But things are different with split daemon, especially when a
client connects to one of hypervisor daemons (e.g. virtqemud) and
then lets the daemon connect to the storage daemon for
vol-upload/vol-download. Here, the hypervisor daemon acts like
both client and server. This is reflected by stream->driver
pointing to remoteStreamDrv, which doesn't have streamInData
callback implemented and thus vol-upload/vol-download with sparse
flag fails.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2026537
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The aim of this function is to look at a virNetClientStream and
tell whether the incoming packet (if there's one) contains data
(type VIR_NET_STREAM) or a hole (type VIR_NET_STREAM_HOLE) and
how big the section is. This function will be called from the
remote driver in one of future commits.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
So far, virStreamInData() is effectively a wrapper over
virFDStreamInData() which means it deals with files which can be
rewound (lseek()-ed) to whatever position we need. And in fact,
that's what virFDStreamInData() does - it makes sure that the FD
is left unchanged in terms of position in the file. Skipping the
hole happens soon after - in daemonStreamHandleRead() when
virStreamSendHole() is called.
But this is about to change. Soon we will have another implementation
where we won't be dealing with FDs but virNetMessage queue and it will
be handy to pop message at the beginning of the queue. Implement and
document this new behavior.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
In the QEMU driver, we allocate private source data unconditionally
for every chardev and the rest of the function just assumes it's there.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
After recent cleanups, there are some pointless cleanup sections.
Clean them up.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Return the desired values directly and clean up the redundant
else branches.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Convert all the functions that generate virCaps to use g_auto
and g_steal_pointer.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Convert all the users who unref their virCaps object unconditionally.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When adding the ACL check and caps getter, we assumed that
the default return value is -1, not 0 as usual.
Fix the return value on error by assigning them explicitly.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
As of ff024b60cc we are opening chardevs before starting QEMU.
However, we are also doing that before domain private directories
are created. This leaves us unable to create guest agent socket
which lives under priv->channelTargetDir.
While creating the dirs can be moved just before
qemuProcessPrepareHostBackendChardev() it's better to do it as
the very first step so that this kind of error is prevented in
future.
Fixes: ff024b60cc
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Specifically, include non-option argument 'GUEST-XML-FILE'
in usage summary.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Specifically:
* include non-option argument 'URI' in usage summary;
* mention that it's an internal tool not meant to be
called directly;
* exit earlier if required arguments are absent.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It's a getopt interface and we're not using getopt, at least
directly, so even though it works relying on it feels wrong.
GOption takes care of removing any trace of the arguments it
consumes from argc and argv, leaving behind only non-option
arguments, so we can just use those standard variables.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Userfaultfd is by default allowed only for privileged processes. Since
libvirt runs QEMU unprivileged, we need to enable unprivileged access to
userfaultfd to enable post-copy migration.
https://bugzilla.redhat.com/show_bug.cgi?id=1945420
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Since the backend of the TPM is a chardev we can use the common helper
to instantiate it.
This commit also ensures proper ordering so that the backend chardev is
formatted before it's being referenced.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add handling to qemuDomainDeviceBackendChardevForeachOne and callbacks
so that we can later use 'qemuBuildChardevCommand' for TPM devices.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Now that the API for qemuBuildChrChardevCommand is sane enough, we can
use it to centralize formatting of '-chardev' generally.
The 'virDomainVideoDef' doesn't use 'virDomainChrSourceDef' internally so
we create it for this occasion manually.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Now that the API for qemuBuildChrChardevCommand is sane enough, we can
use it to centralize formatting of '-chardev' generally.
The 'virDomainFSDef' doesn't use 'virDomainChrSourceDef' internally so
we create it for this occasion manually.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Now that the API for qemuBuildChrChardevCommand is sane enough, we can
use it to centralize formatting of '-chardev' generally.
For virtiofs we don't have a centrally stored chardev source so we
allocate one inline for temporary use.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add handling to qemuDomainDeviceBackendChardevForeachOne and callbacks
so that we can later use 'qemuBuildChardevCommand' for vhost-user disks
instead of a custom formatter.
Since we don't pass the FD for the vhost-user connection to qemu all of
the setup can be skipped.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Now that the parameter is unused we can remove it as well as from each
caller that doesn't need it any more.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When setting up TLS options from config in qemuDomainPrepareChardevSourceOne
we can also extract the x509 certificate path and default tlsVerify
setting so that 'qemuBuildChardevCommand' doesn't need to access the
config object any more.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Completely seprate the creation of the commandline string from the setup
of other objects instantiated on the commandline.
'qemuBuildChardevCommand' will aggregate the setup of individual
parameters such as -add-fd and setup of TLS and the -chardev parameter
itself while the code formatting the commandline will be moved into
qemuBuildChardevStr.
'fdset' names are then stored in qemuDomainChrSourcePrivate.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Make the callers construct the alias for the chardev so that the
function can be used also for code paths which use a different
convention.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'qemuBuildChrChardevStr' used a hybrid approach where some arguments
were directly added to '@cmd' while the commandline itself was returned
as a string.
This patch renames qemuBuildChrChardevStr to qemuBuildChardevCommand
and adds the argument directly to @cmd inside the function.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Unify the cases for SCLP/SCLPLM/VIRTIO consoles as the code is
identical.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We have just one case when we wish to wait for incomming connections for
a listening socket and that is for vhost-user network devices.
Passing this via a flag to qemuBuildChrChardevStr is unwieldy. Add a
field to qemuDomainChrSourcePrivate and populate it for our special
case inside of qemuDomainPrepareChardevSourceOne.
Since we wait for incomming connections only on startup of a new VM we
also need to pass in a flag whether qemuDomainPrepareChardevSourceOne
is called on a new start or on hotplug.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use the qemuDomainDeviceBackendChardevForeach helper to iterate all
eligible structs and convert the setup of the TLS defaults from the
config.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'qemuBuildChrChardevStr' doesn't use these flags any more. Stop passing
them.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The opening of files for FD passing for a chardev backend was
historically done in the function which is formatting the commandline.
This has multiple problems. Firstly the function takes a lot of
parameters which need to be passed through the commandline formatters.
This made the 'qemuBuildChrChardevStr' extremely unappealing to the
extent that we have multiple other custom formatters in places which
didn't really want to use the function.
Additionally the function is also creating files in the host in certain
configurations which is wrong for a commandline formatter to do. This
meant that e.g. not all chardev test cases can be converted to use
DO_TEST_CAPS_LATEST as we attempt to use such code path and attempt to
create files outside of the test directory.
This patch moves the opening of the filedescriptors from
'qemuBuildChrChardevFileStr' into a new helper
'qemuProcessPrepareHostBackendChardevOne' which is called using
'qemuDomainDeviceBackendChardevForeach'.
To preserve test behaviour we also have another instance
'testPrepareHostBackendChardevOne' which is populating mock
filedescriptors.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Introduce qemuDomainDeviceBackendChardevForeach(One) which calls the
callback if either given device has a chardev backend or for all chardev
backends of all devices.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The bitmap is allocated just above the explicit clear, so it's already
empty.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use automatic memory clearing for virBitmap and remove a reuse of a
temporary string.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There were two separate instances of string->virBitmap code:
virBitmapParseInternal and virBitmapParseUnlimited.
By adding a flag to switch to expanding APIs we can merge the two
implementations into one.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In order to prepare for reuse of the function, move the allocation of
the bitmap to the caller.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Since the feature is not needed remove it and remove the function to
virBitmapParseInternal.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function can't fail at this point. Remove the return value.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function can't fail at this point. Remove the return value.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function can't fail at this point. Remove the return value.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There's nothing that can fail in the function. Remove the return value
and adjust callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function isn't used besides tests. Since the separator parsing
capability is trivial we can keep it in place and just unexport it for
now.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use automatic memory freeing for the temporary bitmap and remove the
pointless 'cleanup' section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use automatic memory freeing for the temporary bitmap and remove the
pointless 'cleanup' section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use automatic memory freeing for the temporary bitmap and remove the
pointless 'cleanup' section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Automatically free 'cmd' and 'created' by moving them to the appropriate
scopes.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use automatic memory freeing for the temporary bitmap and remove the
pointless 'cleanup' section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use automatic memory freeing for the temporary bitmap and remove the
pointless 'cleanup' section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use automatic memory freeing for the temporary bitmap and remove the
pointless 'cleanup' section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use automatic memory freeing for the 'ret' bitmap and remove the
pointless 'cleanup' section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Automatically free the 'slotmap' bitmap and get rid of the cleanup
section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use g_autoptr for the temp bitmap. To achieve this the variable must be
moved down to the appropriate scope.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use automatic memory clearing for the temporary strings and bitmap and
remove the cleanup section. There are multiple temporary strings added
so that we don't reuse one.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Automatically free the 'ret' temporary bitmap and get rid of the cleanup
section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use automatic freeing where possible and use g_clear_pointer instead of
manual NULL-ing.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Move the 'path' and 'type' variables down to the appropriate block and
use automatic freeing for them as well as the temporary virBitmap.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Unfortunately, this fix breakes machinectl in a very nasty way,
for instance 'machinectl shell' drops into the host shell. It's
worse than being unable to start a container with CGroupsV1.
Revert until a proper fix is figured out.
This reverts commit 1b9ce05ce2.
References: https://gitlab.com/libvirt/libvirt/-/issues/182
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
We need to make sure the URI scheme is present before passing
it to strchr(), otherwise we're going to get
$ virt-ssh-helper foo
Segmentation fault (core dumped)
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This reverts commit 69977ff105.
After previous commit it's no longer possible that QEMU driver is
not initialized in qemuStateShutdownPrepare() nor
qemuStateShutdownWait().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The initialization of drivers happens in a separate thread.
However, the main thread continues initialization and sets
shutdown callbacks (virStateShutdownPrepare() and
virStateShutdownWait()) even though the driver init thread is
still running. This is dangerous because if the daemon decides to
quit early (e.g. because SIGINT was delivered) the
shutdownPrepare and shutdownWait callback are called over
partially init drivers.
Set callbacks only after all drivers were initialized.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/218
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2027400
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Use the modern style and fix all offenders since new functions were
already using the contemporary style.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The error is a hard error, so the part about fallback doesn't make
sense. Spell the attribute the same way as it's in XML.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
For x86, we invalidate qemu caps cache if the host CPUID changed.
However other cpu drivers do not have the 'getHostData' function
implemented.
Skip the comparison if we do not have host CPUData available,
since virCPUDataIsIdentical always returns an error in that case.
https://bugzilla.redhat.com/show_bug.cgi?id=2030119
Fixes: 3bc6f46d30
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Use it to enable the 'write-blocking' mode of 'blockdev-mirror'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Non-shared storage migration of guests which are disk I/O intensive and
have fast local storage may actually never converge if the guest happens
to dirty the disk faster than it can be copied.
This patch introduces a new flag
'VIR_MIGRATE_NON_SHARED_SYNCHRONOUS_WRITES' which will instruct
hypervisors to synchronize local I/O writes with the writes to remote
storage used for migration so that the guest can't overwhelm the
migration. This comes at a cost of decreased local I/O performance for
guests which behave well on average.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Make the macro useful also for cases when one of multiple flags is
required.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Wire up the flag to enable the 'write-blocking' 'copy-mode' of
'blockdev-mirror'.
It's not supported by all qemu versions but it is with those which we
use -blockdev with so we can use that instead of adding another custom
capability as we use blockdev for some time now.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In cases when the destination storage is slower than the normal VM
storage and the VM does intensive I/O to the disk a block copy job may
never converge.
Switching it to synchronous mode will ensure that all writes done by the
guest are propagated to the destination at the cost of slowing down I/O
of the guest to the synchronous speed.
This patch adds the new API flag and implements virsh support.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Forces the data to be written synchronously to both the original and the
mirrored images which ensures that the job will reach synchronized
phase.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Move the construction of the command from the variable declaration so
that it doesn't exceed the line length and we can also move the logic of
determining the protocol outside of the command construction.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The migration API takes specific flags which are then converted to
boolean parameters for the command. Extract the flag into helper
variables rather than using ternary operators while constructing the
command itself.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Instead of a ternary operator we can use the existing helper.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Remove the cleanup sections where not needed after we've converted to
automatic freeing of virJSONValue.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Convert the code to use g_autoptr for the few cases sill using explicit
cleanup.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Refactor the control flow so we can remove the cleanup label.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Don't use 'goto' for looping. Extract the monitor interaction code into
a new function and restructure the logic to avoid jumping back in the
code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Don't use 'goto' for looping. Extract the sync sending code into a new
function and restructure the logic to avoid jumping back in the code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Move the unlinking of the state file right after reading it so that we
can get rid of the cleanup section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Move the unlinking of the state file earlier and get rid of the cleanup
label.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Convert two temp strings and one virJSONValue to g_auto(free|ptr).
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use virJSONValueObjectAdd instead of step-by-step construction of the
object. This also removes a handful impossible to reach errors with
translatable messages.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We use this approach for other APIs which take a virJSONValue as
argument and the logic is also simpler.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There are a few uses which still explicitly free JSON objects, fix them.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Whenever virPCIGetNetName() is called, it is either called with
physPortID = NULL, or with it set by the caller calling
virNetDevGetPhysPortID() soon before virPCIGetNetName(). The
physPortID is then used *only* in virPCIGetNetName().
Rather than replicating that same call to virNetDevGetPhysPortID() in
all the callers of virPCIGetNetName(), lets just have all those
callers send the NetDevName whose physPortID they want down to
virPCIGetNetName(), and let virPCIGetNetName() call
virNetDevGetPhysPortID().
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Commit 795e9e05c3 (libvirt-7.7.0) refactored the code in virpci.c and
virnetdev.c that gathered lists of the Virtual Functions (VF) of an
SRIOV Physical Function (PF) to simplify the code.
Unfortunately the simplification made the assumption, in the new
function virPCIGetVirtualFunctionsFull(), that a VF's netdev
interface name should only be retrieved if the PF had a valid
phys_port_id. That is an incorrect assumption - only a small handful
of (now previous-generation) Mellanox SRIOV cards actually use
phys_port_id (this is for an odd design where there are multiple
physical network ports on a single PCI address); all other SRIOV cards
(including new Mellanox cards) have a file in sysfs called
phys_port_id, but it can't be read, and so the pfPhysPortID string is
NULL.
The result of this logic error is that virtual networks that are a
pool of VFs to be used for macvtap connections will be unable to
start, giving an errror like this:
VF 0 of SRIOV PF enp130s0f0 couldn't be added to the interface pool because it isn't bound to a network driver - possibly in use elsewhere
This error message is misinformed - the caller of
virNetDevGetVirtualFunctionsFull() only *thinks* that the VF isn't
bound to a network driver because it doesn't see a netdev name for the
VF in the list. But that's only because
virNetDevGetVirtualFunctionsFull() didn't even try to get the names!
We do need a way for virPCIGetVirtualFunctionsFull() to sometimes
retrieve the netdev names and sometimes not. One way of doing that
would be to send down the netdev name of the PF whenever we also want
to know the netdev names of the VFs, but send a NULL when we
don't. This can conveniently be done by just *replacing* pfPhysPortID
in the arglist with pfNetDevName - pfPhysPortID is determined by
simply calling virNetDevGetPhysPortID(pfNetDevName) so we can just
make that call down in virPCIGetVirtualFunctionsFull() (when needed).
This solves the regression introduced by commit 795e9e05c3, and also
nicely sets us up to (in a subsequent commit) move the call to
virNetDevGetPhysPortID() down one layer further to virPCIGetNetName(),
where it really belongs!
Resolves: https://bugzilla.redhat.com/2025432
Fixes: 795e9e05c3
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
After previous cleanups some labels became needless because they
contain just a return statement. There's no point in having such
labels.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Instead of calling virDomainDefFree() explicitly, we can annotate
variables with g_autoptr().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Instead of calling VIR_FREE() explicitly, we can annotate
variables with g_autofree.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Commit <f33ce12e9cd9cab7e6022e91d3765c33d99bf777> dropped unused code
but missed one variable.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The virNetDaemonQuit(dmn) command in virLXCControllerSignalChildIO triggers an
early close of all clients of lxc_controller. Here, libvirtd itself is a client
of this controller, and the client connection is used to notify libvirtd if a
reboot of the container is required. However, the client connection was closed
before such a status could be sent to libvirtd. To fix this bug, we will
immediately send the reboot or shutdown status of the container to libvirtd,
and only after client disconnect will we trigger virNetDaemonQuit.
Fixes: https://gitlab.com/libvirt/libvirt/-/issues/237
Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=991773
Signed-off-by: Joachim Falk <joachim.falk@gmx.de>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The hash table of log file objects in libxlLogger is not protected against
concurrent access. It is possible for one thread to remove an entry while
another is updating it. Add a mutex to the libxlLogger object and lock it
when accessing the files hash table.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
libxl can deliver events and invoke callbacks on any application thread
calling into libxl. This can cause deadlock in the libvirt libxl driver
Thread 19 (Thread 0x7f31411ec700 (LWP 14068) "libvirtd"):
#0 0x00007f318520cc7d in __lll_lock_wait () from /lib64/libpthread.so.0
#1 0x00007f3185205ed5 in pthread_mutex_lock () from /lib64/libpthread.so.0
#2 0x00007f3189488015 in virMutexLock (m=<optimized out>) at ../../src/util/virthread.c:79
#3 0x00007f3189463f3b in virObjectLock (anyobj=<optimized out>) at ../../src/util/virobject.c:433
#4 0x00007f31894f2f41 in virDomainObjListSearchID (payload=0x7f317400a6d0, name=<optimized out>, data=0x7f31411eaeac) at ../../src/conf/virdomainobjlist.c:105
#5 0x00007f3189437ac5 in virHashSearch (ctable=0x7f3124025a30, iter=iter@entry=0x7f31894f2f30 <virDomainObjListSearchID>, data=data@entry=0x7f31411eaeac, name=name@entry=0x0) at ../../src/util/virhash.c:745
#6 0x00007f31894f3919 in virDomainObjListFindByID (doms=0x7f3124025430, id=<optimized out>) at ../../src/conf/virdomainobjlist.c:121
#7 0x00007f3152f292e5 in libxlDomainEventHandler (data=0x7f3124023d80, event=0x7f310c010ae0) at ../../src/libxl/libxl_domain.c:660
#8 0x00007f3152c6ff5d in egc_run_callbacks (egc=egc@entry=0x7f31411eaf50) at libxl_event.c:1427
#9 0x00007f3152c718bd in libxl__egc_cleanup (egc=0x7f31411eaf50) at libxl_event.c:1458
#10 libxl__ao_inprogress (ao=ao@entry=0x7f310c00b8a0, file=file@entry=0x7f3152cce987 "libxl_domain.c", line=line@entry=730, func=func@entry=0x7f3152ccf750 <__func__.22238> "libxl_domain_unpause") at libxl_event.c:2047
#11 0x00007f3152c8c5b8 in libxl_domain_unpause (ctx=0x7f3124015a40, domid=<optimized out>, ao_how=ao_how@entry=0x0) at libxl_domain.c:730
#12 0x00007f3152f2a584 in libxl_domain_unpause_0x041200 (domid=<optimized out>, ctx=<optimized out>) at /usr/include/libxl.h:1756
#13 libxlDomainStart (driver=driver@entry=0x7f3124023d80, vm=vm@entry=0x7f317400a6d0, start_paused=start_paused@entry=false, restore_fd=restore_fd@entry=-1, restore_ver=<optimized out>, restore_ver@entry=2) at ../../src/libxl/libxl_domain.c:1482
#14 0x00007f3152f2a6e3 in libxlDomainStartNew (driver=driver@entry=0x7f3124023d80, vm=vm@entry=0x7f317400a6d0, start_paused=start_paused@entry=false) at ../../src/libxl/libxl_domain.c:1545
#15 0x00007f3152f2a789 in libxlDomainShutdownHandleRestart (driver=0x7f3124023d80, vm=0x7f317400a6d0) at ../../src/libxl/libxl_domain.c:464
#16 0x00007f3152f2a9e4 in libxlDomainShutdownThread (opaque=<optimized out>) at ../../src/libxl/libxl_domain.c:559
#17 0x00007f3189487ee2 in virThreadHelper (data=<optimized out>) at ../../src/util/virthread.c:196
#18 0x00007f3185203539 in start_thread () from /lib64/libpthread.so.0
#19 0x00007f3184f3becf in clone () from /lib64/libc.so.6
Frame 16 runs a thread created to handle domain shutdown processing for
domid 28712. In this case the event contained the reboot reason, so the
old domain is destroyed and a new one is created by libxlDomainStart new.
After starting the domain, it is unpaused by calling libxl_domain_unpause
in frame 12. While the thread is running within libxl, libxl takes the
opportunity to deliver a pending domain shutdown event for unrelated domid
28710. While searching for the associated virDomainObj by ID, a deadlock is
encountered when attempting to lock the virDomainObj for domid 28712, which
is already locked since this thread is processing its shutdown event.
The deadlock can be avoided by moving the search for a virDomainObj
associated with the event domid to the shutdown thread. The same is done
for the death thread.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Similar to domain shutdown events, processing domain death events can be a
lengthy process and we don't want to block the event handler while the
operation completes. Move the death handling function to a thread.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
