Currently, if we want to zero out disk source (e,g, due to
startupPolicy when starting up a domain) we use
virDomainDiskSetSource(disk, NULL). This works well for file
based storage (storage type file, dir, or block). But it doesn't
work at all for other types like volume and network.
So imagine that you have a domain that has a CDROM configured
which source is a volume from an inactive pool. Because it is
startupPolicy='optional', the CDROM is empty when the domain
starts. However, the source element is not cleared out in the
status XML and thus when the daemon restarts and tries to
reconnect to the domain it refreshes the disks (which fails - the
storage pool is still not running) and thus the domain is killed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The virMacMap module is there for dumping [domain, <list of is
MACs>] pairs into a file so that libvirt_guest NSS module can use
it. Whenever a interface is allocated from network (e.g. on
domain<F2> startup or NIC hotplug), network is notified and so is
virMacMap module subsequently. The module update functions
networkMacMgrAdd() and networkMacMgrDel() gracefully handle the
case when there's no module. The problem is, the module is
created if and only if network is freshly started, or if the
daemon restarts and network previously had the module.
This is not very user friendly - if users want to use the NSS
module they need to destroy their network and bring it up again
(and subsequently all the domains using it).
One disadvantage of this approach implemented here is that one
may get just partial results: any already running network does
not record mac maps, thus only newly plugged domains will be
stored in the module. The network restart scenario is not touched
by this of course. But one can argue that older libvirts had
never recorded the mac maps anyway.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
So far our code is full of the following pattern:
dom = virGetDomain(conn, name, uuid)
if (dom)
dom->id = 42;
There is no reasong why it couldn't be just:
dom = virGetDomain(conn, name, uuid, id);
After all, client domain representation consists of tuple (name,
uuid, id).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
There was an unhandled 'open' call which resulted in:
"error: Library function returned error but did not set virError"
Even if this happens during the daemon's start when we still don't have
any set of outputs defined yet, we can safely report an error, since we
automatically fallback to stderr which is fine even for both
running as a daemonized process, since this happens before the daemon
forks into the background, and running as a systemd service, since
systemd re-directs std outputs to journald by default.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1436060
Signed-off-by: Erik Skultety <eskultet@redhat.com>
After the release it's necessary to add a new <release> section for the
upcoming release. Add a template so that it does not have to be
compiled over and over again.
In 9e2465834 a check that denies internal snapshots when pflash
based loader is configured for the domain. However, if there's
none and an user tries to do an internal snapshot they will
witness daemon crash as in that case vm->def->os.loader is NULL
and we dereference it unconditionally.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
CPU features which change their value from disabled to enabled between
two calls to query-cpu-model-expansion (the first with no extra
properties set and the second with 'migratable' property set to false)
can be marked as enabled and non-migratable in qemuMonitorCPUModelInfo.
Since the code consuming qemuMonitorCPUModelInfo currently ignores the
migratable flag, this change is effectively changing the CPU model
advertised in domain capabilities to contain all features (even those
which block migration). And this matches what we do for QEMU older than
2.9.0, when we detect all CPUID bits ourselves without asking QEMU.
As a result of this change
<cpu mode='host-model'>
<feature name='invtsc' policy='require'/>
</cpu>
will work with all QEMU versions. Such CPU definition would be forbidden
with QEMU >= 2.9.0 without this patch.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
If calling query-cpu-model-expansion on the 'host'/'max' CPU model with
'migratable' property set to false succeeds, we know QEMU is able to
tell us which features would disable migration. Thus we can mark all
enabled features as migratable.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
QEMU is able to tell us whether a CPU feature would block migration or
not. This patch adds support for storing such features in
qemuMonitorCPUModelInfo.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
When idx is 0 virStorageFileChainLookup returns the base (bottom) of the
backing chain rather than the top. This is expected by the callers of
qemuDomainGetStorageSourceByDevstr.
Add a special case for idx == 0
Pool types that have the VIR_STORAGE_POOL_SOURCE_NAME flag set
allow omitting the <name> element and instead fill out the pool name
from the <source><name> element.
Relax the schema to make <name> optional for these pools.
Expressing that at least one of these is required is out of scope
of the schema.
One of the problems with our virGetDomain function is that it
copies just domain name and domain UUID. Therefore it's very
easy to forget aboud domain ID. This can cause some bugs, like
virConnectGetAllDomainStats not reporting proper domain IDs.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1434882
Imagine the following scenario:
1) virsh net-start default
2) virsh start myFavouriteDomain
3) virsh net-destroy default
4) virsh destroy myFavouriteDomain
(assuming myFavouriteDomain has an interface from default
network)
Regardless of how unlikely this scenario looks like, we should
not crash. The problem is, on net-destroy in
networkShutdownNetworkVirtual() the virMacMap module is unrefed,
but the stale pointer is kept around. Thus when the domain
destroy procedure comes in, networkReleaseActualDevice() and
subsequently networkMacMgrDel() is called. This function sees the
stale pointer and starts calling the virMacMap module APIs which
work over freed memory.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1398087
Clean up the virsh man page description for --pool-create-as in order
to better describe how the various arguments are used when creating
(or defining) a logical pool.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
off_t is signed and it's size is the same as long only on 64b archs.
Thus it cannot be formatted as %lu.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The value we use internally to represent the lack of a memory
locking limit, VIR_DOMAIN_MEMORY_PARAM_UNLIMITED, doesn't
match the value setrlimit() and prlimit() use for the same
purpose, RLIM_INFINITY, so we have to handle the translation
ourselves.
Partially-resolves: https://bugzilla.redhat.com/1431793
For guests that use <memoryBacking><locked>, our only option
is to remove the memory locking limit altogether.
Partially-resolves: https://bugzilla.redhat.com/1431793
Instead of having a separate function, we can simply return
zero from the existing qemuDomainGetMemLockLimitBytes() to
signal the caller that the memory locking limit doesn't need
to be set for the guest.
Having a single function instead of two makes it less likely
that we will use the wrong value, which is exactly what
happened when we started applying the limit that was meant
for VFIO-using guests to <memoryBacking><locked>-using
guests.
This reverts commit c2e60ad0e5.
Turns out this check is excessively strict: there are ways
other than <memtune><hard_limit> to raise the memory locking
limit for QEMU processes, one prominent example being
tweaking /etc/security/limits.conf.
Partially-resolves: https://bugzilla.redhat.com/1431793
Commits 29f7b5ea6a and 5edf9aaf54 pushed them incorrectly at the end of
the file in the bug fixes section for libvirt 2.5.0.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
When reading release notes, patch summary is not always the best
description of what users can expect in new version. I propose
changing it slightly so that it describes what exactly happens and
when.
However, we do not have to add every single code change to the news
file, that would be ridiculous and unreadable for users. If the patch
subject needs changes like this one, I'm rather tempted to say that
such changes should not be in the news file at all. So that would be
the other way how to fix this.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
The mock, as well as the test, is only available on Linux. So skip
building it everywhere else, especially when it fails on mingw.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
If if_indextoname is not defined, the whole function using it should
not be defined either. Add stub to fix build on mingw.
Caused by 5dd607059d
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Creating a copy of the definition we want to add in a migration cookie
makes the code cleaner and less prone to memory leaks or double free
errors.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1398087
Clean up the virsh man page description for --pool-create-as in order
to better describe how the various arguments are used when creating
(or defining) a logical pool.
Also modify the storage pool XML parsing algorithm to check for the
mismatched "name" and "source-name".
Move the --print-xml to the end of the qualifiers since it's not
properly positionally situated for both --pool-create-as and --pool-define-as
and could be miscontrued as being the 3rd positional argument.
While parsing if the storage source is not present, then a defaultFormat
was not set. This could lead to oddities such as seeing "unknown" format
in output for the "logical" pool even though the only format the pool could
support would be "lvm2".
This does "put a label" on other pool defaults as follows:
File System: FS_AUTO
Network File System: NETFS_AUTO
Disk: UNKNOWN
Each of which is the "0" value for their respective pools and thus
would be no "real" change.
QEMU allows for TSC frequency to be explicitly set to enable migration
with invtsc (migration fails if the destination QEMU cannot set the
exact same frequency used when starting the domain on the source host).
Libvirt already supports setting the TSC frequency in the XML using
<clock>
<timer name='tsc' frequency='1234567890'/>
</clock>
which will be transformed into
-cpu Model,tsc-frequency=1234567890
QEMU command line.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The frequency is documented and formatted as an attribute of the <timer>
element rather than a nested <frequency> element expected by the parser.
Luckily enough, timer frequency has not been used by any driver so far.
And users were not able to set it in the XML either.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
