To enable the virCapsPtr parameter to the post parse method to be
eliminated, the drivers must fetch the virCapsPtr from their own
driver via the opaque parameter, or use an alternative approach
to validate the parsed data.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

The XML parser currently calls virCapabilitiesDomainDataLookup during
parsing to find the domain capabilities matching the triple

  (virt type, os type, arch)

This is, however, bogus with the QEMU driver as it assumes that there
is an emulator known to the default driver capabilities that matches
this triple. It is entirely possible for the driver to be parsing an
XML file with a custom emulator path specified pointing to a binary
that doesn't exist in the default driver capabilities.  This will,
for example be the case on a RHEL host which only installs the host
native emulator to /usr/bin. The user can have built a custom QEMU
for non-native arches into $HOME and wish to use that.

Aside from validation, this call is also used to fill in a machine type
for the guest if not otherwise specified. Again, this data may be
incorrect for the QEMU driver because it is not taking account of
the emulator binary that is referenced.

To start fixing this, move the validation to the post-parse callbacks
where more intelligent driver specific logic can be applied.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Moving their instance parameter to be the first one, and give consistent
ordering of other parameters across all functions. Ensure that the xml
options are passed into both functions in prep for future work.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Our normal practice is for the object type to be the name prefix, and
the object instance be the first parameter passed in.

Rename these to virDomainObjSave and virDomainDefSave moving their
primary parameter to be the first one. Ensure that the xml options
are passed into both functions in prep for future work.

Finally enforce checking of the return type and mark all parameters
as non-NULL.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Currently the virQEMUCapsPtr objects are just empty. Future patches are
going to expect them to contain real data. Start off by populating the
machine types and arch information.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

As part of a goal to eliminate the need to use virCapsPtr for anything
other than the virConnectGetCapabilies() API impl, cache the host arch
against the QEMU driver struct and use that field directly.

In the tests we move virArchFromHost() globally in testutils.c so that
every test runs with a fixed default architecture reported.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NPavel Hrdina <phrdina@redhat.com>

The functions for converting migration typed parameters to QEMU
migration parameters and back were only implemented for integer types.
This patch adds support for string parameters.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NPavel Hrdina <phrdina@redhat.com>

With blockdev we need to refer to the nodename of the disk source image
as the source argument for the blockdev-mirror operation while still
keeping the old job name. With blockdev we must also persist the job in
qemu.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

Separate out allocation of the virStorageSource corresponding to the
target NBD export of the migration.

As part of the splitout we allocate the export name explicitly as that
one must not change regardless whether blockdev is used or not to
provide compatibility.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

The non-shared-storage migration tracks the storage source used
explicitly in the migration data so we must allow for processing of the
block job which has NULL mirror as the mirror will not be populated.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

Now that the cleanup section does not exist remove the label.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

qemuMigrationSrcNBDCopyCancelOne uses the block job data structure but
generated it's own job name rather than taking it from the block job
data.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

With -blockdev we must use the nodename as the export but we must keep
the name of the export as it was before to ensure compatiblity.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

Declare the variable inside the loop with automatic clearing.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

qemuDomainBlockJobSetSpeed was not converted to get the job name from
the block job data. This means that after enabling blockdev the API call
would fail as we wouldn't use the appropriate name.

https://bugzilla.redhat.com/show_bug.cgi?id=1780497Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

Where appropriate replace the open coded call with the qemu wrapper
which already reports the error.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

Mention the argument used if the disk can't be located.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

With this patch users can cold unplug some sound devices.
use "virsh detach-device vm sound.xml --config" command.
Reviewed-by: NCole Robinson <crobinso@redhat.com>
Signed-off-by: NJidong Xia <xiajidong@cmss.chinamobile.com>

This is a follow-up to patch series posted in

https://www.redhat.com/archives/libvir-list/2019-November/msg01180.html

It implements a suggestion made by Cole in

https://www.redhat.com/archives/libvir-list/2019-November/msg01207.html

and discussed in follow-up messages as there were no objections to the
change.

The aim is to make the code more readable by replacing nested branching
with a flat structure.
Signed-off-by: NPavel Mores <pmores@redhat.com>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NDaniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

In v5.8.0-rc1~122 we've removed the only use of @safename in
qemuMonitorTextLoadSnapshot(). What we are left with is an
declared but not initialized variable that is passed to
VIR_FREE().

Caught by libvirt-php test suite.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

In v5.9.0-370-g8fa0374c I've tried to fix a bug by removing
some stale XATTRs in qemuProcessStop(). However, I forgot to
do nothing when the VIR_QEMU_PROCESS_STOP_NO_RELABEL flag was
specified.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NPeter Krempa <pkrempa@redhat.com>

Until now we only really aborted migration via qemuDomainAbortJob. This
will change with the upcoming addition of the backup job. Additionally
there were a bunch of if statements checking various aspects of the
current job.

To make it more obvious convert qemuDomainAbortJob to use a switch
statement and move the individual conditions to the appropriate job
type.

Every job type has now it's own case despite multiple job types just
plainly cancelling the job for clarity and future extension.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJiri Denemark <jdenemar@redhat.com>

Following patch will refactor qemuDomainAbortJob to use a per-job-type
switch where we will need to abort a migration job in various branches.

Save some code duplication by introducing a helper.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJiri Denemark <jdenemar@redhat.com>

We clear some capabilities here so the lockouts need to be
re-evaluated.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>

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: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>

Checking whether a qemu capability set right before clearing it without
any other logic doesn't make sense.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>

Do all post-processing of capabilities in qemuProcessPrepareQEMUCaps.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>

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: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>

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: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

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: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

The redetection was originally added in 43c01d38 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: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Commit c90fb5a8 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: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

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: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

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: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Commit 4b58fdf2 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=1778058Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

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: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

In commit 2ccb5335 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=1778014Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>