Commit Graph

9240 Commits

Author SHA1 Message Date
Peter Krempa
be1a201533 qemu: capabilities: Lock out incremental backup capability without blockdev
Blockdev is required to do incremental backups properly. Add a helper
function for locking out capabilities and export it to allow re-doing
the processing if a different code path modifies capabilities.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
2019-12-03 15:26:54 +01:00
Peter Krempa
a64265f440 qemu: caps: Don't check capability before clearing it
Checking whether a qemu capability set right before clearing it without
any other logic doesn't make sense.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
2019-12-03 15:26:54 +01:00
Peter Krempa
78c2a8b934 qemu: process: Move handling of qemu capability overrides
Do all post-processing of capabilities in qemuProcessPrepareQEMUCaps.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
2019-12-03 15:26:54 +01:00
Peter Krempa
97c9ece79b qemu: process: Move clearing of QEMU_CAPS_CHARDEV_FD_PASS to qemuProcessPrepareQEMUCaps
Move the post-processing of the QEMU_CAPS_CHARDEV_FD_PASS flag to the
new function.

The clearing of the capability is based on the presence of
VIR_QEMU_PROCESS_START_STANDALONE so we must also pass in the process
start flags.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
2019-12-03 15:26:54 +01:00
Peter Krempa
3a075524d9 qemu: process: Move clearing of the BLOCKDEV capability to qemuProcessPrepareQEMUCaps
Start aggregating all capability post-processing code in one place.

The comment was modified while moving it as it was mentioning floppies
which are no longer clearing the blockdev capability.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-12-03 15:26:54 +01:00
Peter Krempa
dbbc9a3c40 qemu: Move and rename qemuDomainUpdateQEMUCaps
The function is now used only in qemu_process.c so move it there and
name it 'qemuProcessPrepareQEMUCaps' which is more appropriate to what
it's doing.

The reworded comment now mentions that it will also post-process the
caps for VM startup.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-12-03 15:26:54 +01:00
Peter Krempa
530d7a73f4 qemu: process: Don't try to redetect missing qemuCaps on reconnect
The redetection was originally added in 43c01d3838 as a way to recover
from libvirtd upgrade from the time when we didn't persist the qemu
capabilities in the status XML. Also this the oldest supported qemu by
more than two years.

Even if somebody would have a running VM running at least qemu 1.5 with
such an old libvirt we certainly wouldn't do the right thing by
redetecting the capabilities and then trying to communicate with qemu.

For now it will be the best to just stop considering this scenario any
more and error out for such VM.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-12-03 15:26:54 +01:00
Peter Krempa
2504dbeb5d qemu: process: Make it obvious that virDomainDefPostParse is called with NULL opaque
Commit c90fb5a828 added explicit use of the private copy of the qemu
capabilities to various places. The change to qemuProcessInit was bogus
though as at the point where we re-initiate the post parse callbacks
priv->qemuCaps is still NULL as we clear it after shutdown of the VM and
don't initiate it until a later point.

Using the value from priv->qemuCaps might mislead readers of the code
into thinking that something useful is being passed at that point so go
with an explicit NULL instead.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-12-03 15:26:54 +01:00
Peter Krempa
ab163144eb qemu: Always reset @info in qemuDomainGetJobInfo
qemuDomainGetJobInfo didn't always reset the return data in @info.
Thankfully this wouldn't be a problem as the RPC layer does it but we
should do it anyways.

Since we reset the struct we don't have to set the type to
VIR_DOMAIN_JOB_NONE as the value is 0.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
2019-12-03 15:26:54 +01:00
Peter Krempa
2dae916de4 qemu: Implement VIR_DOMAIN_JOB_STATS_KEEP_COMPLETED
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
2019-12-03 15:26:53 +01:00
Daniel P. Berrangé
8aaed287f5 qemu: make 'xz' image compression viable by using -3
For managed save we can choose between various compression
methods. I randomly tested the 'xz' program on a 8 GB guest
and was surprised to have to wait > 50 minutes for it to
finish compressing, with 'xz' burning 100% cpu for the
entire time. Despite the impressive compression, this is
completely useless in the real world as it is far too long
to wait to save the VM.

The 'xz' binary defaults to '-6' optimization level which
aims for high compression, with moderate memory usage,
at the expense of speed.

This change switches it to use the '-3' optimization level
which is documented as being the one that optimizes speed
at expense of compression. Even with this, it will still
outperform all the other options in terms of compression
level. It is a little less than x4 faster than '-6' which
means it starts to be a viable choice to use 'xz' for
people who really want best compression.

The test results on a 1 GB, fairly freshly booted VM are
as follows

  format | save  | restore  size
  =======+=======+=============
  raw    |   05s |    1s  | 428 MB
  lzop   |   05s |    3s  | 160 MB
  gzip   |   29s |    5s  | 118 MB
  bz2    |   54s |   22s  | 114 MB
  xz     | 4m37s |   13s  |  86 MB
  xz -3  | 1m20s |   12s  |  95 MB

Based on this we can say

 * For moderate compression with no noticable loss in speed

       => use lzop

 * For high compression with moderate loss in speed

       => use gzip

 * For best compression with significant loss in speed

       => use xz

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2019-12-03 13:43:45 +00:00
Peter Krempa
6519c1b554 qemu: blockcopy: Fix conditions when virStorageSource should be initialized
Commit 4b58fdf280 which enabled block copy also for network
destinations needed to limit when the 'mirror' storage source is
initialized in cases when we e.g. don't have an appropriate backend.

Limiting it just to virStorageFileSupportsCreate is too restrictive as
for example we can't precreate block devices and thus wouldn't
initialize the 'mirror' but since it's a local source we'd try to
examine it. This would fail since it wouldn't be initialized.

Fix it by introducing a more granular check whether certain operations
are supported and fix the check interlocks.

https://bugzilla.redhat.com/show_bug.cgi?id=1778058

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-12-02 15:44:03 +01:00
Peter Krempa
c958b8eb46 qemu: blockcopy: Report error on image format detection failure
We tolerate image format detection during block copy in very specific
circumstances, but the code didn't error out on failure of the format
detection.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-12-02 15:44:03 +01:00
Peter Krempa
21986f5047 qemu: Fix indexes in statistics of iothreads
In commit 2ccb5335dc I've refactored how we fill the typed parameters
for domain statistics. The commit introduced a regression in the
formating of stats for IOthreads by using the array index to label the
entries as it's common for all other types of statistics rather than
the iothread IDs used for iothreads.

Since only the design of iothread deviates from the common approach used
in all other statistic types this was not caught.

https://bugzilla.redhat.com/show_bug.cgi?id=1778014

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-11-29 14:51:47 +01:00
Peter Krempa
d310b08697 qemu: blockjob: Finish handling job with broken data
Now that we have a separate job type which will not trigger normal code
paths for terminating job we can remove the ad-hoc handling.

This possibly fixes the issue of a broken job inheriting the disk and
then finishing in which case we'd not detach the backing chain.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
2019-11-27 15:59:33 +01:00
Peter Krempa
e67e8c545a qemu: blockjob: Introduce "broken" block job type
To better track jobs we couldn't parse let's introduce a new job type
which will clarify semantics internally in few places.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
2019-11-27 15:59:33 +01:00
Peter Krempa
2624c6d2d2 qemu: blockjob: Separate clearing of per-job data
We will need to clear per-job type data when we will be marking a
blockjob as broken in the new way. Extract the code for future reuse.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
2019-11-27 15:59:33 +01:00
Peter Krempa
0cb3061943 qemu: blockjob: Don't stop processing the finished job early
Both failure to refresh and to dismiss the job are very unlikely but if
they happen there's not much we can do about the blockjob.

The concluded job handlers treat it as if the job failed if we don't
update the state to 'QEMU_BLOCKJOB_STATE_COMPLETED' which is probably
the safest thing to do here.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
2019-11-27 15:59:33 +01:00
Peter Krempa
5b29c7dc27 qemu: blockjob: Mark job with broken data but tracked by qemu as reconnected
Otherwise it would get dropped later on as untracked despite us knowing
about it. Additionally since we cancelled it we must wait to dismiss it
which would not be possible if we unregister it. This also opened a
window for a race condition since the job state change event of the
just-cancelled job might be delivered prior to us unregistering the job
in which case everything would work properly.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
2019-11-27 15:59:33 +01:00
Peter Krempa
f15d4cb1c8 qemu: blockjob: Log blockjobs which are dropped when untracked by qemu
Since we don't know what happened to the job we can't do much about it
but we can at least log that this happened.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
2019-11-27 15:59:33 +01:00
Peter Krempa
8622498f6e qemu: blockjob: Fix deadlock when terminating job with invalid data
We must exit the monitor prior to refusing other work, otherwise the VM
object will become unusable.

This bug was introduced in commit v5.5.0-244-gc412383796 but thankfully
the code path was not excercised without QEMU_CAPS_BLOCKDEV.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
2019-11-27 15:59:33 +01:00
Peter Krempa
ccde9ca1f4 qemu: process: Move block job refresh after async job recovery
Block jobs may be members of async jobs so it makes more sense to
refresh block job state after we do steps for async job recovery.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
2019-11-27 15:59:33 +01:00
Peter Krempa
a887da529c qemu: blockjob: Properly propagate cancellation of blockjobs
qemu returns an error message in the job statistics even if the job was
cancelled to emphasize it was not successful. Libvirt didn't properly
transform it into QEMU_BLOCKJOB_STATE_CANCELLED though.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
2019-11-27 15:59:33 +01:00
Peter Krempa
01270a9a5f qemu: monitor: Finish implementation of infrastructure for 'query-jobs'
Commit ed56851f1b didn't wire up fetching of the statistics for the
job which are reported by 'query-jobs'.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
2019-11-27 15:59:33 +01:00
Peter Krempa
a10eb61340 Revert "qemu: Don't repeat virDomainObjEndAPI in qemuDomainBlockPull"
This reverts commit 421c9550f5

qemuDomainBlockPullCommon calls virDomainObjEndAPI internally so the
original commit made us shed two references of @vm instead of one
getting us into a premature free of @vm.

This is not a straight revert as qemuDomainBlockPull was modified
meanwhile. I've also added a warning comment that @vm is consumed.

https://bugzilla.redhat.com/show_bug.cgi?id=1777230

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-11-27 14:19:50 +01:00
Laine Stump
b03d9e9593 conf: add hypervisor agnostic, domain start-time, validation function for NetDef
<interface> devices (virDomainNetDef) are a bit different from other
types of devices in that their actual type may come from a network (in
the form of a port connection), and that doesn't happen until the
domain is started. This means that any validation of an <interface> at
parse time needs to be a bit liberal in what it accepts - when
type='network', you could think that something is/isn't allowed, but
once the domain is started and a port is created by the configured
network, the opposite might be true.

To solve this problem hypervisor drivers need to do an extra
validation step when the domain is being started. I recently (commit
3cff23f7, libvirt 5.7.0) added a function to peform such validation
for all interfaces to the QEMU driver -
qemuDomainValidateActualNetDef() - but while that function is a good
single point to call for the multiple places that need to "start" an
interface (domain startup, device hotplug, device update), it can't be
called by the other hypervisor drivers, since 1) it's in the QEMU
driver, and 2) it contains some checks specific to QEMU. For
validation that applies to network devices on *all* hypervisors, we
need yet another interface validation function that can be called by
any hypervisor driver (not just QEMU) right after its network port has
been created during domain startup or hotplug. This patch adds that
function - virDomainActualNetDefValidate(), in the conf directory,
and calls it in appropriate places in the QEMU, lxc, and libxl
drivers.

This new function is the place to put all network device validation
that 1) is hypervisor agnostic, and 2) can't be done until we know the
"actual type" of an interface.

There is no framework for validation at domain startup as there is for
post-parse validation, but I don't want to create a whole elaborate
system that will only be used by one type of device. For that reason,
I just made a single function that should be called directly from the
hypervisors, when they are initializing interfaces to start a domain,
right after conditionally allocating the network port (and regardless
of whether or not that was actually needed). In the case of the QEMU
driver, qemuDomainValidateActualNetDef() is already called in all the
appropriate places, so we can just call the new function from
there. In the case of the other hypervisors, we search for
virDomainNetAllocateActualDevice() (which is the hypervisor-agnostic
function that calls virNetworkPortCreateXML()), and add the call to our
new function right after that.

The new function itself could be plunked down into many places in the
code, but we already have 3 validation functions for network devices
in 2 different places (not counting any basic validation done in
virDomainNetDefParseXML() itself):

1) post-parse hypervisor-agnostic
   (virDomainNetDefValidate() - domain_conf.c:6145)
2) post-parse hypervisor-specific
   (qemuDomainDeviceDefValidateNetwork() - qemu_domain.c:5498)
3) domain-start hypervisor-specific
   (qemuDomainValidateActualNetDef() - qemu_domain.c:5390)

I placed (3) right next to (2) when I added it, specifically to avoid
spreading validation all over the code. For the same reason, I decided
to put this new function right next to (1) - this way if someone needs
to add validation specific to qemu, they go to one location, and if
they need to add validation applying to everyone, they go to the
other. It looks a bit strange to have a public function in between a
bunch of statics, but I think it's better than the alternative of
further fragmentation. (I'm open to other ideas though, of course.)

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
2019-11-25 15:30:05 -05:00
Laine Stump
fdcd273be2 conf: return a const from virDomainNetGetActualVirtPortProfile
This also isn't required (due to the vportprofile being stored in the
NetDef as a pointer rather than being directly contained), but it
seemed dishonest to not mark it as const (and thus permit users to
modify its contents)

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
2019-11-25 15:29:56 -05:00
Laine Stump
583ac17f5d conf: make virDomainNetGetActualBandwidth arg/return value const
In this case, the virNetDevBandwidthPtr that is returned is not to a
region within the virDomainNetDef arg, but points elsewhere (the
NetDef has the pointer, not the entire object), so technically it's
not necessary to make the return value a const, but it's a bit
disingenuous to *not* do it.

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
2019-11-25 15:29:51 -05:00
Laine Stump
1b029a929d qemu: add mac address to error messages in qemuDomainValidateActualNetDef
This makes it easier to understand which interface's config caused the
error.

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
2019-11-25 15:29:41 -05:00
Michal Privoznik
9b1d53d417 qemu_capabilities: Use proper free function for caps->cpuModels
The cpuModels member of _virQEMUCapsAccel struct is not a
virObject but regular struct with a free function defined:
qemuMonitorCPUDefsFree(). Use that when clearing parent structure
instead of virObjectUnref() to avoid a memleak:

==212322== 57,275 (48 direct, 57,227 indirect) bytes in 3 blocks are definitely lost in loss record 623 of 627
==212322==    at 0x4838B86: calloc (vg_replace_malloc.c:762)
==212322==    by 0x554A158: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.6000.6)
==212322==    by 0x17B14BF5: qemuMonitorCPUDefsNew (qemu_monitor.c:3587)
==212322==    by 0x17B27BA7: qemuMonitorJSONGetCPUDefinitions (qemu_monitor_json.c:5616)
==212322==    by 0x17B14B0B: qemuMonitorGetCPUDefinitions (qemu_monitor.c:3559)
==212322==    by 0x17A6AFBB: virQEMUCapsFetchCPUDefinitions (qemu_capabilities.c:2571)
==212322==    by 0x17A6B2CC: virQEMUCapsProbeQMPCPUDefinitions (qemu_capabilities.c:2629)
==212322==    by 0x17A70C00: virQEMUCapsInitQMPMonitorTCG (qemu_capabilities.c:4769)
==212322==    by 0x17A70DDF: virQEMUCapsInitQMPSingle (qemu_capabilities.c:4820)
==212322==    by 0x17A70E99: virQEMUCapsInitQMP (qemu_capabilities.c:4848)
==212322==    by 0x17A71044: virQEMUCapsNewForBinaryInternal (qemu_capabilities.c:4891)
==212322==    by 0x17A7119C: virQEMUCapsNewData (qemu_capabilities.c:4923)

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2019-11-25 17:13:05 +01:00
Jiri Denemark
24d8202294 qemu: Use host-model CPU on s390 by default
On s390 machines host-passthrough and host-model CPUs result in the same
guest ABI (with QEMU new enough to be able to tell us what "host" CPU is
expanded to, which was implemented around 2.9.0). So instead of using
host-passthrough CPU when there's no CPU specified in a domain XML we
can safely use host-model and benefit from CPU compatibility checks
during migration, snapshot restore and similar operations.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-11-25 15:29:19 +01:00
Pavel Mores
33a9757852 qemu: the actual change of default video devide type selection algorithm
If a graphics device was added to XML that had no video device, libvirt
automatically added a video device which was always of type 'cirrus' on
x86_64, even if the underlying qemu didn't support cirrus.

This patch refines a bit the decision about the type of the video device.
Based on QEMU capabilities, cirrus is still preferred but only added if
QEMU supports it, otherwise VGA is used if supported by QEMU.  There is now
no fallback as libvirt only aspires to generate a basic working config and
leaves anything more specific up to higher-level management tools.

Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Pavel Mores <pmores@redhat.com>
2019-11-25 08:47:08 -05:00
Pavel Mores
b648d96289 qemu: default video device type selection algoritm moved into its own function
The default video device type selection algorithm we're about to deploy will
increase the amount of code dedicated to the task by amount enough to warrant
factoring the whole thing into its own function so as not to pollute the
caller qemuDomainDeviceVideoDefPostParse().  Do it now so that the actual
algorithm change later on is in a clean commit by itself and easy to review.

Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Pavel Mores <pmores@redhat.com>
2019-11-25 08:47:08 -05:00
Daniel P. Berrangé
bc7e72914a util: consolidate on one free callback for hash data
This previous commit introduced a simpler free callback for
hash data with only 1 arg, the value to free:

  commit 49288fac96
  Author: Peter Krempa <pkrempa@redhat.com>
  Date:   Wed Oct 9 15:26:37 2019 +0200

    util: hash: Add possibility to use simpler data free function in virHash

It missed two functions in the hash table code which need
to call the alternate data free function, virHashRemoveEntry
and virHashRemoveSet.

After the previous patch though, there is no code that
makes functional use of the 2nd key arg in the data
free function. There is merely one log message that can
be dropped.

We can thus purge the current virHashDataFree callback
entirely, and rename virHashDataFreeSimple to replace
it.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2019-11-22 14:21:28 +00:00
Peter Krempa
c6a9e54ce3 qemu: enable blockdev support
Now that all pieces are in place (hopefully) let's enable -blockdev.

We base the capability on presence of the fix for 'auto-read-only' on
files so that blockdev works properly, mandate that qemu supports
explicit SCSI id strings to avoid ABI regression and that the fix for
'savevm' is present so that internal snapshots work.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2019-11-22 12:51:27 +01:00
Peter Krempa
5b4b503be6 qemu: capabilities: Add detection of the 'savevm' fix for -blockdev
The 'savevm' HMP command didn't work properly with blockdev as it tried
to do snapshot of everything including the protocol nodes accessing
files which are not snapshottable. Qemu fixed this bug so now we need to
detect it to allow enabling blockdev.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2019-11-22 12:51:27 +01:00
Peter Krempa
e0300f92fc qemu: qapi: Add support for command features
The top level commands now can have 'feature' flags for fixes so add
support for querying those as well.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2019-11-22 12:51:27 +01:00
Peter Krempa
3460fef5a0 qemu: caps: Add capability for dynamic 'auto-read-only' support for files
Initial implementation of 'auto-read-only' didn't reopen the backing
files when needed. For '-blockdev' to work we need to be able to tel
qemu to open a file read-only and change it during blockjobs as we label
backing chains with a sVirt label which does not allow writing. The
dynamic auto-read-only supports this as it reopens files when writing
is demanded.

Add a capability to detect that the posix file based backends support
the dynamic part.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2019-11-22 12:51:27 +01:00
Peter Krempa
48e57cd632 qemu: caps: Base support of 'backingStoreInput' domain feature on QEMU_CAPS_BLOCKDEV
The qemu driver will obey <backingStore> when we support blockdev.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2019-11-22 12:51:27 +01:00
Michal Privoznik
8fa0374c5b qemuProcessStop: Remove image metadata for running mirror jobs
If user starts a blockcommit or a blockcopy then we modify access
for qemu on both images and leave it like that until the job
terminates.  So far so good. Problem is, if user instead of
terminating the job (where we would modify the access again so
that the state before the job is restored) calls destroy on the
domain or if qemu dies whilst executing the block job.  In this
case we don't ever clear the access we granted at the beginning.
To fix this, maybe a bit harsh approach is used, but it works:
after all labels were restored (that is after
qemuSecurityRestoreAllLabel() was called), we iterate over each
disk in the domain and remove XATTRs from the whole backing chain
and also from any file the disk is being mirrored to.

This would have been done at the time of pivot, but it isn't
because user decided to kill the domain instead. If we don't do
this and leave some XATTRs behind the domain might be unable to
start.

Also, secdriver can't do this because it doesn't know if there is
any job running. It's outside of its scope - the hypervisor
driver is responsible for calling secdriver's APIs.

Moreover, this is safe to call because we don't remember labels
for any member of a backing chain except of the top layer. But
that one was restored in qemuSecurityRestoreAllLabel() call done
earlier. Therefore, not only we don't remember labels (and thus
this is basically a NOP for other images in the backing chain) it
is also safe to call this when no blockjob was started in the
first place, or if some parts of the backing chain are shared
with some other domains - this is NOP, unless a block job is
active at the time of domain destroy.

https://bugzilla.redhat.com/show_bug.cgi?id=1741456#c19

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2019-11-22 10:48:14 +01:00
Michal Privoznik
1c12b86185 qemu: Separate image metadata removal into a function
There are four places where we remove image XATTRs and in all of
them we have the same for() loop with the same body. Move it into
a separate function because I'm about to introduce fifth place
where the same needs to be done.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2019-11-22 10:48:04 +01:00
Peter Krempa
86085c9a2f qemu: Instantiate pflash via -machine when using blockdev
Install the convertor function which enables the internals that will use
-blockdev to make qemu open the firmware image and stop using -drive.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2019-11-22 08:32:25 +01:00
Peter Krempa
c8eb99eebf qemu: command: Build the 'pflash' drives via -machine
The old way to instantiate a pflash device via -drive was a hack since
it's a platform device.

The modern approach calls for configuring it via -machine and takes the
node name as an argument.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2019-11-22 08:32:25 +01:00
Peter Krempa
7d2f942af9 qemu: command: Build -blockdev-s for backing of pflash
As a first step we will build the blockdevs which will be supposed to
back the pflash drives when moving away from -drive.

This code is similar to the way we build the blockdevs for the disk, but
skips the copy-on-read layer and doesn't implement any legacy approach.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2019-11-22 08:32:25 +01:00
Peter Krempa
11d13ad8cf qemu: domain: Introduce helper to convert <loader> into virStorageSource
Add a helper which will covert the PFLASH code file and variable file
into the virStorageSource objects stored in private data so that we can
use them with -blockdev while keeping the infrastructure to determine
the path to the loaders intact.

This is a temporary solution until we will want to do snapshots of the
pflash where we will be forced do track the full backing chain in the
XML.

In the meanwhile just convert it partially so that we can stop using
-drive.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2019-11-22 08:32:25 +01:00
Peter Krempa
07675b0100 qemu: domain: Store virStorageSources representing pflash backing
To allow converting the pflash drives to blockdev we will need a
virStorageSource to allow using our helpers. Temporarily prior to
coverting loader data to a virStorageSoruce add private data which will
house this.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2019-11-22 08:32:25 +01:00
Peter Krempa
316223b6ad qemu: command: Extract formatting of -drive for pflash
Extract the old way to instantiate pflash devices to hold the firmware
via -drive to a separate function so that it can later be conditionally
disabled when -blockdev will be used.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-11-22 08:32:23 +01:00
Peter Krempa
c78fadb57c domcaps: Remove function initializing domain caps as unsupported
Commit 5751a0b6b1 added a helper function
called virDomainCapsFeaturesInitUnsupported which initialized all domain
capability features as unsupported.

When adding a new feature this would initialize it as unsupported also
for hypervisor drivers which the original author possibly didn't intend
to modify. To prevent accidental wrong value being reported in such case
revert back to initializing individual features in the hypervisor
drivers themselves.

This is not a straight revert as additonal patches modified how we store
the capabilities.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2019-11-22 08:16:51 +01:00
Jiri Denemark
5e939cea89 qemu: Store default CPU in domain XML
When starting a domain without a CPU model specified in the domain XML,
QEMU will choose a default one. Which is fine unless the domain gets
migrated to another host because libvirt doesn't perform any CPU ABI
checks and the virtual CPU provided by QEMU on the destination host can
differ from the one on the source host.

With QEMU 4.2.0 we can probe for the default CPU model used by QEMU for
a particular machine type and store it in the domain XML. This way the
chosen CPU model is more visible to users and libvirt will make sure
the guest will see the exact same CPU after migration.

Architecture specific notes
- aarch64: We only set the default CPU for TCG domains as KVM requires
  explicit "-cpu host" to work.

- ppc64: The default CPU for KVM is "host" thanks to some hacks in QEMU,
  we will translate the default model to the model corresponding to the
  host CPU ("POWER8" on a Power8 host, "POWER9" on Power9 host, etc.).
  This is not a problem as the corresponding CPU model is in fact an
  alias for "host". This is probably not ideal, but it's not wrong and
  the default virtual CPU configured by libvirt is the same QEMU would
  use. TCG uses various CPU models depending on machine type and its
  version.

- s390x: The default CPU for KVM is "host" while TCG defaults to "qemu".

- x86_64: The default CPU model (qemu64) is not runnable on any host
  with KVM, but QEMU just disables unavailable features and starts
  happily.

https://bugzilla.redhat.com/show_bug.cgi?id=1598151
https://bugzilla.redhat.com/show_bug.cgi?id=1598162

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-11-20 17:22:07 +01:00
Jiri Denemark
ac89b0549e qemu: Use g_autoptr in qemuDomainDefPostParse
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-11-20 17:22:06 +01:00