libxlMakeNic was calling g_strdup(virBufferCurrentContent(&buf)) to
make a copy of the buffer contents, and then later freeing the buffer
without ever using it again. Instead of this extra strdup, just
transfer ownership of the virBuffer's string with
virBufferContentAndReset(), and be done with it.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There are several calls to virBufferFreeAndReset() when functions
encounter an error, but the caller never uses the virBuffer once an
error has been encountered (all callers detect error by looking at the
function return value, not the contents of the virBuffer being
operated on), and now that all virBuffers are auto-freed there is no
reason for the lower level functions like these to spend time freeing
a buffer that is guaranteed to be freed momentarily anyway.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Every other caller of this function checks for an error return and
ends their formatting early if there is an error. This function
happily continues on its way.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The latter function is guaranteed to always clear out the virBuffer
anyway, so this is redundant and could add to extra cargo-cult code if
used as an example.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In most cases this eliminates one or more calls to
virBufferClearAndReset(), but even when it doesn't it's better because:
1) it makes the code more consistent, making it more likely that new
contributors who are "learning by example" will to the right thing.
2) it protects against future modifications that might have otherwise
needed to add a virBufferClearAndReset()
3) Currently some functions don't call virBufferClearAndReset() only
because they're relying on some subordinate function to call it for
them (e.g. bhyveConnectGetSysinfo() in this patch relies on
virSysinfoFormat() to clear out the buffer when there is an
error). I think this is sloppy behavior, and that the toplevel
function that defines and initializes the buffer should be the
function clearing it at the end.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The output of vcpupin and emulatorpin for a domain with vcpu
placement='static' is based on a default bitmap that contains
all possible CPUs in the host, regardless of the CPUs being offline
or not. E.g. for a Linux host with this CPU setup (from lscpu):
On-line CPU(s) list: 0,8,16,24,32,40,(...),184
Off-line CPU(s) list: 1-7,9-15,17-23,25-31,(...),185-191
And a domain with this configuration:
<vcpu placement='static'>1</vcpu>
'virsh vcpupin' will return the following:
$ sudo ./run tools/virsh vcpupin vcpupin_test
VCPU CPU Affinity
----------------------
0 0-191
This is benign by its own, but can make the user believe that all
CPUs from the 0-191 range are eligible for pinning. Which can lead
to situations like this:
$ sudo ./run tools/virsh vcpupin vcpupin_test 0 1
error: Invalid value '1' for 'cpuset.cpus': Invalid argument
This is exarcebated by the fact that 'virsh vcpuinfo' considers only
available host CPUs in the 'CPU Affinity' field:
$ sudo ./run tools/virsh vcpuinfo vcpupin_test
(...)
CPU Affinity: y-------y-------y-------(...)
This patch changes the default bitmap of vcpupin and emulatorpin, in
the case of domains with static vcpu placement, to all available CPUs
instead of all possible CPUs. Aside from making it consistent with
the behavior of 'vcpuinfo', users will now have one less incentive to
try to pin a vcpu in an offline CPU.
https://bugzilla.redhat.com/show_bug.cgi?id=1434276
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The idea is to have a function that calls virHostCPUGetOnlineBitmap()
but, instead of returning NULL if the host does not have CPU
offlining capabilities, fall back to a bitmap containing all
present CPUs.
Next patch will use this helper in two other places.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This function reads the string in sysfspath/cpu/present and
parses it manually to retrieve the number of present CPUs.
virHostCPUGetPresentBitmap() reads and parses the same file,
using a more robust parser via virBitmapParseUnlimited(),
but returns a bitmap. Let's drop all the manual parsing done
here and simply return the size of the resulting bitmap
from virHostCPUGetPresentBitmap().
Given that no more parsing is being done manually in the function,
rename it to virHostCPUCountLinux().
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Since the ESX virtual hardware version 4.0, virtual machines support up
to 10 virtual NICs instead of 4 previously. This changes the limit
accordingly based on the provided `virtualHW.version`.
Signed-off-by: Bastien Orivel <bastien.orivel@diateam.net>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
If the name is 'vcpus', we will get 'vcpussched' instead of 'vcpusched'
in the error message as following:
... 19155 : vcpussched attributes 'vcpus' must not overlap
So we use 'vcpu' to replace 'vcpus'.
Signed-off-by: Liao Pingfang <liao.pingfang@zte.com.cn>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
A few commits ago, in aeecbc87b7, I've implemented command line
generation for ACPI HMAT. For this, we need to know if at least
one guest NUMA node has vCPUs. This is tracked in
@masterInitiator variable, which is initialized to -1, then we
iterate through guest NUMA nodes and break the loop if we find a
node with a vCPU. After the loop, if masterInitiator is still
negative then no NUMA node has a vCPU and we error out. But this
exact check was missing comparison for negativeness.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This capability tracks whether QEMU is capable of defining HMAT
ACPI table for the guest.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
These APIs will be used by QEMU driver when building the command
line.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
There are several restrictions, for instance @initiator and
@target have to refer to existing NUMA nodes (daa), @cache has to
refer to a defined cache level and so on.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
To cite ACPI specification:
Heterogeneous Memory Attribute Table describes the memory
attributes, such as memory side cache attributes and bandwidth
and latency details, related to the System Physical Address
(SPA) Memory Ranges. The software is expected to use this
information as hint for optimization.
According to our upstream discussion [1] this is exposed under
<numa/> as <cache/> under NUMA <cell/> and <latency> or
<bandwidth/> under numa/latencies.
1: https://www.redhat.com/archives/libvir-list/2020-January/msg00422.html
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
QEMU allows creating NUMA nodes that have memory only.
These are somehow important for HMAT.
With check done in qemuValidateDomainDef() for QEMU 2.7 or newer
(checked via QEMU_CAPS_NUMA), we can be sure that the vCPUs are
fully assigned to NUMA nodes in domain XML.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
There is only one caller of virDomainNumaSetNodeCpumask() which
checks for the return value but because the function will return
NULL iff the @cpumask was NULL in the first place. But in that
place @cpumask can't be NULL because it was just allocated by
virBitmapParse().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
The machine can not be NULL at this point -
qemuDomainDefPostParse() makes sure it isn't.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
The function doesn't just build the argument for -numa. Since the
-numa can be repeated multiple times, it also puts -numa onto the
cmd line. Also, the rest of the functions has 'Command' infix.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
There are two functions virDomainNumaDefCPUFormatXML() and
virDomainNumaDefCPUParseXML() which format and parse domain's
<numa/>. There is nothing CPU specific about them. Drop the
infix.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
There is nothing domain specific about the function, thus it
should not have virDomain prefix. Also, the fact that it is a
static function makes it impossible to use from other files.
Move the function to virxml.c and drop the 'Domain' infix.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Commit 82576d8f35 used a string "on" to enable the 'pmem' property.
This is okay for the command line visitor, but the property is declared
as boolean in qemu and thus it will not work when using QMP.
Modify the type to boolean. This changes the command line, but
fortunately the command line visitor in qemu parses both 'yes' and 'on'
as true for the property.
https://bugzilla.redhat.com/show_bug.cgi?id=1854684
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Previous commit removed the last usage of the function. Drop
virQEMUCapsCompareArch as well since virQEMUCapsCacheLookupByArch was
its only caller.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Firstly, SEV is present only on AMD, so we can safely assume x86.
Secondly, the problem with looking up capabilities in the cache by arch
is that it's using virHashSearch with a callback to find the right
capabilities and get the binary name from it as well, but since the
cache is empty, it will return NULL and we won't get the corresponding
binary name out of the lookup either. Then, during the cache validation
we try to create a new cache entry for the emulator, but since we don't
have the binary name, nothing gets created.
Therefore, virQEMUCapsCacheLookupDefault is used to fix this issue,
because it doesn't rely on the capabilities cache to construct the
emulator binary name.
https://bugzilla.redhat.com/show_bug.cgi?id=1852311
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The semantics of the backup operation don't strictly require that all
disks being backed up are part of the same incremental part (when a disk
was checkpointed/backed up separately or in a different VM), or even
they may not have a previous checkpoint at all (e.g. when the disk
was freshly hotplugged to the vm).
In such cases we can still create a common checkpoint for all of them
and backup differences according to configuration.
This patch adds a per-disk configuration of the checkpoint to do the
incremental backup from via the 'incremental' attribute and allows
perform full backups via the 'backupmode' attribute.
Note that no changes to the qemu driver are necessary to take advantage
of this as we already obey the per-disk 'incremental' field.
https://bugzilla.redhat.com/show_bug.cgi?id=1829829
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Use the configured TLS env to setup encryption of the TLS transport.
https://bugzilla.redhat.com/show_bug.cgi?id=1822631
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Allow enabling TLS for the NBD server used to do pull-mode backups. Note
that documentation already mentions 'tls', so this just implements the
schema and XML bits.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
TLS is required to transport backed-up data securely when using
pull-mode backups.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Add fields for storing the aliases necessary to clean up the TLS env for
a backup job after it finishes.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Data is valid only when queried as guest writes may increase the backup
size.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Introduce code which merges the appropriate bitmaps and queries the
final size of the backup, so that we can print the XML with size
information.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Avoid printing '0' size in case when we weren't able to determine the
backup size by adding a flag whether the size is valid and interlock
printing of the field according to the flag.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Merge the bitmaps when finalizing a block pull job so that backups work
properly afterwards.
https://bugzilla.redhat.com/show_bug.cgi?id=1799010
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Users may want to use this to create a full backup or even incremental
if the checkpoints are pre-existing. We still will not allow to create a
checkpoint on a read-only disk as that makes no sense.
https://bugzilla.redhat.com/show_bug.cgi?id=1840053
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Setup the TLS secret when preparing a virStorageSource for use.
https://bugzilla.redhat.com/show_bug.cgi?id=1602328
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Until now libvirt didn't allow using encrypted TLS key for disk clients.
Add fields for configuring the secret and propagate defaults.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Add infrastructure for hot- and cold-plug of the secret object holding
decryption key for the TLS key.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Store the required data in the private data of a storage source and
ensure that the 'alias' of the secret is formatted in the status XML.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The '*_tls_x509_verify' options are relevant only when we are going to
expose a server socket as client sockets always enable verification.
Split up the macro to separate the common bits from the server bits so
that when we'll later extend support of 'nbd' and 'vxhs' disks which are
client only we can reuse the existing macros.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Move the extraction of the config value so that it makes more sense
after upcoming refactors.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
There's no such parameter. Reword the sentence to account for enabling
TLS-encrypted migration using API flags.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Add a helper which will always return the storage source private data
even if it was not allocated before.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The strings allocated in virGetHostnameImpl() are all allocated via
g_strdup(), which will exit on OOM anyway, so the call to
virReportOOMError() is redundant, and removing it allows slight
modification to the code, in particular the cleanup label can be
eliminated.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The error: label in this function just does "return -1", so replace
all the "goto error" in the function with "return -1".
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
hostsfilestr was not being freed. This will be turned into g_autofree
in an upcoming patch converting a lot more of the same file to using
g_auto*, but I wanted to make a separate patch for this first so the
other patch is simpler to review (and to make backporting easier).
The leak was introduced in commit 97a0aa2467
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
AUTOPTR_CLEANUP_FUNC is set to xmlBufferFree() in util/virxml.h (This
is actually new - added accidentally (but fortunately harmlessly!) in
commit 257aba2daf. I had added it along with the hunks in this patch,
then decided to remove it and submit separately, but missed taking out
the hunk in virxml.h)
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Although libvirt itself uses g_malloc0() and friends, which exit when
there isn't enouogh memory, libxml2 uses standard malloc(), which just
returns NULL on OOM - this means we must check for NULL on return from
any libxml2 functions that allocate memory.
xmlBufferCreate(), for example, might return NULL, and we don't always
check for it. This patch adds checks where it isn't already done.
(NB: Although libxml2 has a provision for changing behavior on OOM (by
calling xmlMemSetup() to change what functions are used to
allocating/freeing memory), we can't use that, since parts of libvirt
code end up in libvirt.so, which is linked and called directly by
applications that may themselves use libxml2 (and may have already set
their own alternate malloc()), e.g. drivers like esx which live totally
in the library rather than a separate process.)
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When introducing vcpu.<num>.wait (v1.3.2-rc1~301) and
vcpu.<num>.halted (v2.4.0-rc1~36) the documentation was
not written.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Since feb83c1e71 libvirtd will abort on
startup if run as non-root
2020-07-01 16:30:30.738+0000: 1647444: error : virDirOpenInternal:2869 : cannot open directory '/etc/libvirt/hooks/daemon.d': Permission denied
The root cause flaw is that non-root libvirtd is using /etc/libvirt for
its hooks. Traditionally that has been harmless though since we checked
whether we could access the hook file and degraded gracefully. We need
the same access check for iterating over the hook directory.
Long term we should make it possible to have an unprivileged hook dir
under $HOME.
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
In 076591009a a validation code was added to
virDomainDeviceInfoFormat() which reports an error if zPCI
address entered in was incomplete. But, there are two problems
with this approach.
The first problem is the placement of the code - it doesn't
belong into XML formatter rather than XML validator.
The second one is that at the point of formatting XML the post
parse callback has run and thus filled in required info.
Therefore this check can never do something useful and instead of
moving it into validator, it's removed completely.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
To make the code future proof, the rest of the
qemuValidateDomainDeviceDefAddress() has to be executed (even
though there is nothing there yet) instead of returning directly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Commit 076591009a ("conf: fix zPCI address auto-generation on
s390") is doing a check for virZPCIDeviceAddressIsIncomplete()
prior to checking if the device has a ZPCI address at all. This
results in errors like these when starting libvirt:
error : virDomainDeviceInfoFormat:7527 : internal error:
Missing uid or fid attribute of zPCI address
Fix it by moving virZPCIDeviceAddressIsIncomplete() after the
check done by virZPCIDeviceAddressIsPresent().
Fixes: 076591009a
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The ZPCI device validation is specific to qemu. So, let us move the
ZPCI uid validation out of domain xml parsing into qemu domain device
validation.
Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Let us fix the issues with zPCI address validation and auto-generation
on s390.
Currently, there are two issues with handling the ZPCI address
extension. Firstly, when the uid is to be auto-generated with a
specified fid, .i.e.:
...
<address type='pci'>
<zpci fid='0x0000001f'/>
</address>
...
we expect uid='0x0001' (or the next available uid for the domain).
However, we get a parsing error:
$ virsh define zpci.xml
error: XML error: Invalid PCI address uid='0x0000', must be > 0x0000
and <= 0xffff
Secondly, when the uid is specified explicitly with the invalid
numerical value '0x0000', we actually expect the parsing error above.
However, the domain is being defined and the uid value is silently
changed to a valid value.
The first issue is a bug and the second one is undesired behaviour, and
both issues are related to how we (in-band) signal invalid values for
uid and fid. So let's fix the XML parsing to do validation based on what
is actually specified in the XML.
The first issue is also related to the current code behaviour, which
is, if either uid or fid is specified by the user, it is incorrectly
assumed that both uid and fid are specified. This bug is fixed by
identifying when the user specified ZPCI address is incomplete and
auto-generating the missing ZPCI address.
Signed-off-by: Bjoern Walk <bwalk@linux.ibm.com>
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Refer to the notion of mount propagation instead which describes
the actual behaviour more clearly.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The two sides of a PTY can be referred to as primary and secondary
TTYs.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When querying QEMU we have to iterate over two nested sets
of CPUs. The terms "main vcpu" and "sub vcpu" are a good
representation.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The two sides of a PTY can be referred to as primary and secondary
TTYs.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The term "ignored" is a better choice for the filtering performed
on devices from udev.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This new naming matches the terminology used in the error
messages that the callers report.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When listing CPU models, we need to filter the data based on sets
of permitted and forbidden CPU models.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The term "access control list" better describes the concept involved.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The term "access control list" better describes the concept involved.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Although a ramfb video device is not a PCI device, we don't currently
report an error for ramfb device definitions containing a PCI address.
However, a guest configured with such a device will fail to start:
# virsh start test1
error: Failed to start domain test1
error: internal error: qemu unexpectedly closed the monitor: 2020-06-16T05:23:02.759221Z qemu-kvm: -device ramfb,id=video0,bus=pcie.0,addr=0x1: Device 'ramfb' can't go on PCIE bus
A better approach is to reject any device definitions that contain PCI
addresses. While this is a change in behavior, any existing
configurations were non-functional.
https://bugzilla.redhat.com/show_bug.cgi?id=1847259
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
A few commits back (in v6.4.0-131-gbdb8f2e418) the post parse
function for domain interface was changed so that it doesn't fill
in model for hostdev types of interfaces (including network type
interfaces which would end up hostdevs).
While the idea is sound, the execution can be a bit better:
virDomainNetResolveActualType() which is used to determine
runtime type of given interface is heavy gun - it connects to
network driver, fetches network XML, parses it. This all is
followed by check whether the interface doesn't already have
model set (from domain XML).
If we switch the order of these two checks then the short circuit
evaluation will ensure the expensive check is done only if really
needed.
This commit in fact fixes qemuxml2xmltest which due to lacking
fake network driver tries to connect to network:///session and
start the virtnetworkd. Fortunately, because of
v6.3.0-25-gf28fbb05d3 it fails to do so and
virDomainNetResolveActualType() returns -1. The only reason we
don't see the test failing is because our input XMLs have model
and thus we are saved by the latter (now former) check.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
Until recently, an <interface type='network'> would automatically be
assigned model "rtl8139", which in turn would lead to the device being
assigned a PCI address on a conventional PCI controller (i.e. a
pcie-to-pci-bridge). If the network was a typical Linux host
bridge-based network that used an emulated device, this would be
appropriate, since the guest actually would get an emulated rtl8139
NIC, and that device is a conventional PCI device.
However, if the network being used was a pool of hostdev devices, the
guest would get an actual PCIe network device assigned from the host
via VFIO; while the interface model in that case is irrelevant for the
QEMU commandline to assign the device, the PCI address would have
already been assigned prior to runtime, so the address assignment
would be done based on the model='rtl8139' - a conventional PCI
device. VFIO assignment of a PCIe device to a conventional PCI slot
works, but we would rather have these devices in a PCIe slot.
Since commit bdb8f2e418, if <interface type='network'> points to a
etwork that is a pool of hostdev devices, the interface model will be
_unset_ by default. This patch uses that information when deciding
what type of slot to assign to the device: since all hostdev network
interfaces are SR-IOV VFs, and *all* SR-IOV network cards are PCIe, it
is safe to assume that the VFs are PCIe and we should assign then to a
PCIe slot in the guest.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
All the domain job related APIs were present in `qemu_domain.c`
along with the other domain APIs. In this patch, we move all the
qemu domain job APIs into a separate file.
Also, in this process, `qemuDomainTrackJob()`,
`qemuDomainFreeJob()`, `qemuDomainInitJob()` and
`qemuDomainObjSaveStatus()` were converted to a non-static
funciton and exposed using `qemu_domain.h`.
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In functions `qemuDomainObjInitJob`, `qemuDomainObjResetJob`,
`qemuDomainObjResetAgentJob`, `qemuDomainObjResetAsyncJob`,
`qemuDomainObjFreeJob`, `qemuDomainJobAllowed`,
`qemuDomainNestedJobAllowed` we avoid sending the complete
qemuDomainObjPrivatePtr as parameter and instead just send
qemuDomainJobObjPtr.
This is done in a effort to separating the qemu-job APIs into
a spearate file.
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Some functions or code paths that may fail don't report error
(e.g. when acquiring PID file fails) leading to a silent quit
of the leaseshelper. This makes it super hard for us and users
to debug what is happening. Fortunately, dnsmasq captures both
stdout and stderr so we can write an error message there.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
On a DHCP transaction, dnsmasq runs our leases helper which
updates corresponding JSON files. While one dnsmasq won't run the
leaseshelper in parallel, two dnsmasqs (from two distinct
networks) might. To avoid corrupting JSON file, the leaseshelper
acquires PID file first. Well, the way it's acquiring it is not
ideal - it calls virPidFileAcquirePath(wait = false); which
means, that either it acquires the PID file instantly or returns
an error and does not touch the JSON at all. This in turn means
that there might be a leases record missing. With wait = true,
this won't happen.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1840307
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The "virsh domcapabilities --arch ppc64" command will fail with no
error message set if qemu-system-ppc64 is not currently installed.
This is because virQEMUCapsCacheLookup() does not report any error
message if not capabilities can be obtained from the cache. Almost
all methods calling this expected an error to be set on failure.
Once that's fixed though, we see a further bug which is that
virQEMUCapsCacheLookupDefault() is passing a NULL binary path to
virQEMUCapsCacheLookup(), so we need to catch that too.
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
It's possible to use ramfb as the boot display of an assigned vgpu
device. This was introduced in 4b95738c, but unfortunately the attribute
was not formatted into the xml output for such a device. This patch
fixes that oversight and adds a xml2xml test to verify proper behavior.
https://bugzilla.redhat.com/show_bug.cgi?id=1847791
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The XML format used for QEMU capabilities is not required to be
stable across releases, as we invalidate the cache whenever the
libvirt binary changes.
We none the less always try to parse te entire XML file before
we do any validity checks. Thus if we change the format of any
part of the data, or change permitted values for enums, then
libvirtd logs will be spammed with errors.
These are not in fact errors, but an expected scenario.
This change makes the loading code validate the cache timestamp
against the libvirtd timestamp immediately. If they don't match
then we stop loading the rest of the XML file.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
