There's only one variable to clean-up, others are just tokens inside that
variable, but it is nicer anyway. Positive returns have not been converted
because the function will change soon and it would not make much sense.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
As soon as a guest using a <tpm> device is launched, libvirt will change
the ownership to 'tss' user and group, with mode 0730, which will cause
RPM verify to then fail.
Reviewed-by: Laine Stump <laine@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Michal Privoznik [Mon, 30 Nov 2020 10:06:14 +0000 (11:06 +0100)]
qemu: Don't prealloc mem for real NVDIMMs
Currently, we configure QEMU to prealloc memory almost by
default. Well, by default for NVDIMMs, hugepages and if user
asked us to (via memoryBacking <allocation mode="immediate"/>).
However, when guest's NVDIMM is backed by real life NVDIMM this
approach is not the best. In this case users should put <pmem/>
into the <memory/> device <source/>, like this:
Instructing QEMU to do prealloc in this case means that each
page of the NVDIMM is "touched" (the first byte is read and
written back - see QEMU commit v2.9.0-rc1~26^2) which cripples
device wear.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1894053 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
which enabled the driver by default. But in meson we are checking
whether the 'driver_vmware' option is enabled without anything
enabling it automagically.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Michal Privoznik [Wed, 21 Aug 2019 11:07:57 +0000 (13:07 +0200)]
domain_conf: Parse full length of some <seclabel/> attributes
In virSecurityLabelDefParseXML() we are parsing the <seclabel/>
element among with its attributes. Some of the attributes are
limited in length (because of virNodeGetSecurityModel()), however
some are not. And for the latter ones we don't need to use
virXMLPropStringLimit() to parse them. Moreover, using
VIR_SECURITY_LABEL_BUFLEN as the limit is wrong - we are not
storing the parsed strings into a static buffer of that size
rather than checking if the string passes string -> enum
conversion.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Wed, 21 Aug 2019 09:47:56 +0000 (11:47 +0200)]
qemu: Obtain @caps only after ACL check in qemuNodeGetSecurityModel
Even though we are getting driver capabilities with
refresh=false (so that it is not expensive), we still should do
ACL check first because there is no point in bothering with the
capabilities if caller doesn't have permissions to call the API.
Also, this way the comment makes more sense.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Fri, 18 Dec 2020 15:09:16 +0000 (16:09 +0100)]
nss: handle leases with infinite expiry time
After v6.3.0-rc1~64 a lease can have infinite expiry time. This
means that the expiration time will appear as a value of zero.
Do the expiration check only if the expiration time is not zero.
Fixes: 97a0aa246799c97d0a9ca9ecd6b4fd932ae4756c Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Michal Privoznik [Fri, 18 Dec 2020 15:09:15 +0000 (16:09 +0100)]
networkGetDHCPLeases: Handle leases with infinite expiry time
After v6.3.0-rc1~64 a lease can have infinite expiry time. This
means that the expiration time will appear as a value of zero.
Do the expiration check only if the expiration time is not zero.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1908053 Fixes: 97a0aa246799c97d0a9ca9ecd6b4fd932ae4756c Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Michal Privoznik [Fri, 18 Dec 2020 15:09:14 +0000 (16:09 +0100)]
network: Rework networkGetDHCPLeases()
Firstly, bring variables that are used only within loops into
their respective loops. Secondly, drop 'error' label which is
redundant since we have @rv which holds the return value.
Thirdly, fix indendation in one case, the rest is indented
properly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Michal Privoznik [Fri, 18 Dec 2020 15:09:12 +0000 (16:09 +0100)]
network: Drop @custom_lease_file_len variable from networkGetDHCPLeases()
We don't need to track the lease file size. Instead, we can
simply check if the file was empty by comparing the buffer the
file was read into with an empty string.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Michal Privoznik [Fri, 18 Dec 2020 15:09:11 +0000 (16:09 +0100)]
virlease: Allow infinite lease expiry time
When adding a new lease by our leaseshelper then virLeaseNew() is
called. Here, we check for DNSMASQ_LEASE_EXPIRES environment
variable which is the expiration time for the lease. For infinite
lease time the value is zero. However, our code is not prepared
for that and adds "expiry-time" into the JSON file only if lease
expiry time is non-zero. This breaks the assumption that the
"expiry-time" attribute is always present (as can be seen in
virLeaseReadCustomLeaseFile() and virLeasePrintLeases()).
Store "expiry-time" always.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Michal Privoznik [Fri, 18 Dec 2020 15:09:10 +0000 (16:09 +0100)]
virlease: Use virTrimSpaces() instead of open coded alternative
In virLeaseNew() we are trying to remove trailing space (per
comment it may happen that older versions of dnsmasq put it into
an env variable). Well, instead of open coding it, we can use
virTrimSpaces().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Michal Privoznik [Fri, 18 Dec 2020 15:09:09 +0000 (16:09 +0100)]
virlease: Rework virLeaseReadCustomLeaseFile()
There are some variables which are used only inside the single
loop the function has. Let's declare them inside the loop body to
make that obvious. Also, fix indendation.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Michal Privoznik [Fri, 18 Dec 2020 15:09:08 +0000 (16:09 +0100)]
leaseshelper: Report errors on failure
If leasehelper fails all that we are left with is a simple error
message produced by dnsmasq:
lease-init script returned exit code 1
This is because the leasehelper did not write any message to
stderr. According to dnsmasq's manpage, whenever it's invoking
leasehelper the stderr is kept open:
All file descriptors are closed except stdin, which is open to
/dev/null, and stdout and stderr which capture output for
logging by dnsmasq.
As debugging leasehelper is not trivial (because dnsmasq invokes
it with plenty of env vars set - that's how data is passed onto
helper), let's print an error into stderr if exiting with an
error. And since we are not calling public APIs, we have to call
virDispatchError() explicitly and since we don't have any
connection open, we have to pass NULL.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Michal Privoznik [Fri, 18 Dec 2020 15:09:07 +0000 (16:09 +0100)]
docs: Document ability to configure lease time
In v6.3.0-rc1~64 we've introduced ability to configure lease
time, but forgot to document the feature. Let's fix that.
Fixes: 97a0aa246799c97d0a9ca9ecd6b4fd932ae4756c
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1908631 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Michal Privoznik [Thu, 10 Dec 2020 11:37:26 +0000 (12:37 +0100)]
qemu: Drop has_ccw_address from _qemuAgentDiskAddress
In recent patches new mambers to _qemuAgentDiskAddress struct
were introduced to keep optional CCW address sent by the guest
agent. These two members are a struct to store CCW address into
and a boolean to keep track whether the CCW address is valid.
Well, we can hold the same information with a pointer - instead
of storing the CCW address structure let's keep just a pointer to
it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Thomas Huth [Wed, 25 Nov 2020 11:06:48 +0000 (12:06 +0100)]
domain_conf: Allow to look up scsi disks when controller uses a CCW address
On s390x, devices are attached to the channel IO subsytem by default,
so we need to look up scsi controllers via their CCW address there
instead of using PCI.
This fixes "virsh domfsinfo" on s390x for virtio-scsi devices (the first
attempt from commit f8333b3b0a7 did it in the wrong way, reporting the
device name on the guest side instead of the target name on the host side).
Fixes: f8333b3b0a ("qemu: Fix domfsinfo for non-PCI device information ...") Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1858771 Signed-off-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Thomas Huth [Wed, 25 Nov 2020 11:06:47 +0000 (12:06 +0100)]
domain_conf: Allow to look up virtio-block devices by their CCW address
On s390x, devices are accessed via the channel subsystem by default,
so we need to look up the devices via their CCW address there instead
of using PCI.
This fixes "virsh domfsinfo" on s390x for virtio-block devices (the first
attempt from commit f8333b3b0a7 did it in the wrong way, reporting the
device name on the guest side instead of the target name on the host side).
Fixes: f8333b3b0a ("qemu: Fix domfsinfo for non-PCI device information ...")
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1858771 Signed-off-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Cornelia Huck <cohuck@redhat.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Thomas Huth [Wed, 25 Nov 2020 11:06:46 +0000 (12:06 +0100)]
qemu: agent: Store CCW address in qemuAgentDiskInfo if provided by the guest
Newer versions of the QEMU guest agent will provide the CCW address
of devices on s390x. Store this information in the qemuAgentDiskInfo
so that we can use this later.
We also map the CSSID 0 from the guest to the value 0xfe on the host,
see https://www.qemu.org/docs/master/system/s390x/css.html for details.
Signed-off-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Tim Wiederhake [Tue, 15 Dec 2020 16:24:53 +0000 (17:24 +0100)]
cpu-gather: Move msr decoding to new script
Fixes the leaking file descriptors. Does not silently ignore errors
(e.g. permission denied on /dev/cpu/0/msr if run as non-root) and
always attempt to read from /dev/kvm if /dev/cpu/0/msr failed.
'gather_msr()' returns a dictionary of values, as a later patch will
add more registers to be interrogated.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Tim Wiederhake [Tue, 15 Dec 2020 16:24:50 +0000 (17:24 +0100)]
cpu-gather: Allow overwriting model name
Some hardware, e.g. exotic platforms or pre-production hardware, may
report wrong or random data for the cpu model name. As the name of
the created files is derived from that name, this may lead to issues.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Tim Wiederhake [Tue, 15 Dec 2020 16:24:48 +0000 (17:24 +0100)]
cpu-gather: Create python wrapper for shell script
This changes the invocation from
./cpu-gather.sh | ./cpu-parse.sh
to
./cpu-gather.py | ./cpu-parse.sh
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Tim Wiederhake [Tue, 15 Dec 2020 12:16:48 +0000 (13:16 +0100)]
schema: Allow counter element in host cpu definition
If the capabilities include a counter element, e.g.
<counter name='tsc' frequency='2591999000' scaling='no'/>
the XML could not be validated:
$ virsh capabilities > cap.xml
$ virsh [hypervisor-]cpu-compare cap.xml --validate
error: Failed to compare hypervisor CPU with cap.txt
error: XML document failed to validate against schema: Unable to validate doc against /usr/share/libvirt/schemas/cpu.rng
Did not expect element counter there
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Tim Wiederhake [Tue, 15 Dec 2020 12:16:47 +0000 (13:16 +0100)]
schemas: Deduplicate cpuTopology in cputypes.rng
The duplicate had the "dies" attribute missing, causing
$ virsh capabilities > cap.xml
$ virsh [hypervisor-]cpu-compare cap.xml --validate
to fail with
error: Failed to compare hypervisor CPU with cap.xml
error: XML document failed to validate against schema: Unable to validate doc against /usr/share/libvirt/schemas/cpu.rng
Invalid attribute dies for element topology
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Tue, 15 Dec 2020 19:06:35 +0000 (20:06 +0100)]
lxd_domain: Require that VIR_LXC_DOMAIN_NAMESPACE_SOURCE_NONE is zero
Our parser code relies on the fact that
VIR_LXC_DOMAIN_NAMESPACE_SOURCE_NONE has value of zero and thus
uses g_new0(). But strictly speaking, this is not mandated by
the enum typedef. Fix that.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Laine Stump <laine@redhat.com>
Michal Privoznik [Wed, 16 Dec 2020 16:52:14 +0000 (17:52 +0100)]
virnetdevopenvswitch: Try to unescape ovs-vsctl reply in one specific case
During testing of my patch v6.10.0-rc1~221 it was found that
'ovs-vsctl get Interface $name name' or
'ovs-vsctl find Interface options:vhost-server-path=$path'
may return a string in double quotes, e.g. "vhost-user1". Later
investigation of openvswitch code showed, that early versions
(like 1.3.0) have somewhat restrictive set of safe characters
(isalpha() || '_' || '-' || '.'), which is then refined with
increasing version. For instance, version 2.11.4 has: isalnum()
|| '_' || '-' || '.'. If the string that ovs-vsctl wants to
output contains any other character it is escaped. You want to be
looking at ovsdb_atom_to_string() which handles outputting of a
single string and calls string_needs_quotes() and possibly
json_serialize_string() in openvswitch code base.
Since the interfaces are usually named "vhost-userN" we are
facing a problem where with one version we get the name in double
quotes and with another we get plain name without funny business.
Because of json involved I thought, let's make ovs-vsctl output
into JSON format and then use our JSON parser, but guess what -
ovs-vsctl ignores --format=json. But with a little help of
g_strdup_printf() it can be turned into JSON.
Fixes: e4c29e2904197472919d050c67acfd59f0144bbc
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1767013 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Laine Stump <laine@redhat.com>
Michal Privoznik [Wed, 16 Dec 2020 17:52:48 +0000 (18:52 +0100)]
virNetDevOpenvswitchGetVhostuserIfname: Actually use @path to lookup interface
In v6.10.0-rc1~221 I wanted to make virNetDevOpenvswitchGetVhostuserIfname()
lookup interface name even for vhostuser interfaces with mode='server'. For
these, we are given a socket path which is then created by QEMU and to which
OpenVSwitch connects to and creates an interface. Because of this, we don't
know the name of the interface upfront (when starting QEMU) and have to use
the path to query OpenVSwitch later (using ovs-vsctl). What I intended to use
was:
Laine Stump [Wed, 16 Dec 2020 19:58:21 +0000 (14:58 -0500)]
lxc: skip the netdev autogenerated name counter past existing devices
the lxc driver uses virNetDevGenerateName() for its veth device names
since patch 2dd0fb492, so it should be using virNetDevReserveName()
during daemon restart/reconnect to skip over the device names that are
in use.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Laine Stump [Tue, 15 Dec 2020 20:14:45 +0000 (15:14 -0500)]
util: minor comment/formatting changes to virNetDevTapCreate()
The comment about auto-generating names was obsoleted by recent
changes, and there was an unnecessary set of braces around a single
line conditional body.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Since commit 282d135ddbb the parser for <interface> has cleared out
any interface name from the input XML that used the macvtap/macvlan
name as a prefix. Along with that, the switch to use the new
virNetDevGenerateName() function for auto-generating macvtap/macvlan
device names (commit 9b5d741a9), has realized two facts:
1) virNetDevGenerateName() can be called with a name already filled
in, and in that case it is an effective NOP.
2) because virNetDevGenerate() will always find an unused name, there
is no need to retry device creation in a loop - if it fails the
first time, it would fail any subsequent time as well.
that, combined with the aforementioned parser change allow us to
simplify virNetDevMacVLanCreateWithVPortProfile() - we no longer need
any extra code to determine if a template "AutoName" was requested,
and don't need a separate code path for creating the device in the
case that a specific name was given in the XML - all we need to do is
log any requested name, and then call exactly the same code as we
would if no name was given.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Laine Stump [Tue, 15 Dec 2020 19:35:35 +0000 (14:35 -0500)]
qemu: remove redundant code that adds "template" netdev name
The lower level function virNetDevGenerateName() now understands that
a blank ifname should be replaced with a generated name based on a
template that it knows about itself - there is no need for the higher
level functions to stuff a template name ("vnet%d") into ifname.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Laine Stump [Tue, 15 Dec 2020 19:32:23 +0000 (14:32 -0500)]
bhyve: remove redundant code that adds "template" netdev name
The FreeBSD version of virNetDevTapCreate() now calls
virNetDevGenerateName(), and virNetDevGenerateName() understands that
a blank ifname should be replaced with a generated name based on a
device-type-specific template - so there is no longer any need for the
higher level functions to stuff a template name ("vnet%d") into
ifname.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Laine Stump [Tue, 15 Dec 2020 20:21:57 +0000 (15:21 -0500)]
util: fix tap device name auto-generation for FreeBSD
The Linux implementation of virNetDevCreate() doesn't require a
template ifname (e.g. "vnet%d") when it is called, but just generates
a new name if ifname is empty. The FreeBSD implementation requires
that the caller actually fill in a template ifname, and will fail if
ifname is empty. Since we want to eliminate all the special code in
callers that is setting the template name, we need to make the
behavior of the FreeBSD virNetDevCreate() match the behavior of the
Linux virNetDevCreate().
The simplest way to do this is to use the new virNetDevGenerateName()
function - if ifname is empty it generates a new name with the proper
prefix, and if it's not empty, it leaves it alone.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Laine Stump [Mon, 14 Dec 2020 21:19:26 +0000 (16:19 -0500)]
lxc: don't try to reserve macvtap name for LXC domains
Commit 729a06c41 added code to the LXC driver (patterned after similar
code in the QEMU driver) that called
virNetDevMacVlanReserveName(net->ifname) for all type='direct'
interfaces during a libvirtd restart, to prevent other domains from
attempting to use a macvtap device name that was already in use by a
domain.
But, unlike a QEMU domain, when an LXC domain creates a macvtap
device, that device is almost immediately moved into the namespace of
the container (and it's then renamed, but that part isn't
important). Because of this, the LXC driver doesn't keep track (in
net->ifname) of the name used to create the device (as the QEMU driver
does).
The result of this is that if libvirtd is restarted while there is an
active LXC domain that has <interface type='direct'>, libvirtd will
segfault (since virNetDevMacVLanReserveName() doesn't check for a NULL
pointer).
The fix is to just not call that function in the case of the LXC
driver, since it is pointless anyway.
Fixes: 729a06c41afaab419a70841b6749646e2f8fad1d Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Erik Skultety [Mon, 14 Dec 2020 15:24:37 +0000 (16:24 +0100)]
ci: containers: Refresh the Dockerfiles
Contains changes utilizing "nosync" and "eatmydata" for speedup as well
as fixes for CentOS-8 repoid regression.
ci-commit: b098ec6631a85880f818f2dd25c437d509e53680
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Erik Skultety <eskultet@redhat.com>
Michal Privoznik [Mon, 14 Dec 2020 11:20:07 +0000 (12:20 +0100)]
qemu: Relax validation for mem->access if guest has no NUMA
In v6.8.0-27-g88957116c9 and friends I've switched the way the
default RAM is specified for QEMU (from plain -m to
memory-backend-*). This means, that even if a guest doesn't have
any NUMA nodes configured we can use memory-backend-* attributes
to translate user config requests. For instance, we can allow
memory to be shared (<access mode='shared'/> under
<memoryBacking/>). But what my original commits are missing is
allowing such configuration in our validator.
Fixes: 88957116c9d3cb4705380c3702c9d4315fb500bb
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1839034#c12 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
projects / libvirt.git / log