Hot-adding disks does not parse the full XML to generate apparmor rules.
Instead it uses -f <PATH> to append a generic rule for that file path.

580cdaa7: "virt-aa-helper: locking disk files for qemu 2.10" implemented
the qemu 2.10 requirement to allow locking on disks images that are part of
the domain xml.

But on attach-device a user will still trigger an apparmor deny by going
through virt-aa-helper -f, to fix that add the lock "k" permission to the
append file case of virt-aa-helper.
Signed-off-by: NChristian Ehrhardt <christian.ehrhardt@canonical.com>

When adding CPU usability blockers I forgot to properly free them when
in virDomainCapsCPUModelsDispose.
Reported-by: NMarc Hartmayer <mhartmay@linux.vnet.ibm.com>
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NPavel Hrdina <phrdina@redhat.com>

The assumption so far was an average of 4 disks per guest.
But some architectures, like s390x, still often use plenty of smaller disks.

To include those in the considerations an assumption of an average of 10
disks is more reasonable.
Signed-off-by: NChristian Ehrhardt <christian.ehrhardt@canonical.com>

The initial assumption was ~2 files per guest, but some common setups
like Openstack drive up to 4 files per guest.

E.g. on Arm where the following XML leads to 4 file handles:
    <serial type='file'>
      <source path='/var/lib/nova/instances/7c0dcd78-.../console.log'/>
      <target port='0'/>
      <alias name='serial0'/>
    </serial>
    <console type='file'>
      <source path='/var/lib/nova/instances/7c0dcd78-.../console.log'/>
      <target type='serial' port='0'/>
      <alias name='serial0'/>
    </console>

With that in mind and the target to support 4k guests by default we
should raise the limit to 16k.
Signed-off-by: NChristian Ehrhardt <christian.ehrhardt@canonical.com>

QEMU identified a race condition between the device state serialization
and the end of storage migration. Both QEMU and libvirt needs to be
updated to fix this.

Our migration work flow is modified so that after starting the migration
we to wait for QEMU to enter "pre-switchover", "postcopy-active", or
"completed" state. Once there, we cancel all block jobs as usual. But if
QEMU is in "pre-switchover", we need to resume the migration afterwards
and wait again for the real end (either "postcopy-active" or
"completed" state).

Old QEMU will just enter either "postcopy-active" or "completed"
directly, which is still correctly handled even by new libvirt. The
"pre-switchover" state will only be entered if QEMU supports it and the
pause-before-switchover capability was enabled. Thus all combinations of
libvirt and QEMU will work, but only new QEMU with new libvirt will
avoid the race condition.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

This new capability enables a pause before device state serialization so
that we can finish all block jobs without racing with the end of the
migration. The pause is indicated by "pre-switchover" state. Once we're
done QEMU enters "device" migration state.

This patch just defines the new capability and QEMU migration states and
their mapping to our job states.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

For multipath disks it might be useful to have the same WWN for multiple
disks. It's the users choice to do so. Since we dropped the check that
disallows using duplicate WWNs drop the docs as well.

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

VirutalBox has a IVRDEServerInfo structure available that
gives the effective runtime port that the VM is using when it's
running. This is useful when the "TCP/Ports" VBox property was set to
port range (e.g. via autoport = "yes" or via VBoxManage) in which
case it would be impossible to get the "active" port otherwise.

Originally autoport in vbox driver was setting the port to default value
(3389) which caused multiple VM instances use the same port. Since
libvirt XML does not allow to set port ranges, this patch changes the
"autoport" behavior to set VBox's "TCP/Ports" property to an arbitrary
port range (3389-3689) to avoid that issue.

The VBOX_SESSION_OPEN/CLOSE macros are only called in
_vboxDomainSnapshotRestore and they are unflexible because:

* assume the caller will have variable named "data"
* can only create Write lock type

As per above, it's not that hard to simply use the VBOX API directly.

Commit fdeac7a0 tried to fix the output
of 'virsh domxml-to-native --help' by switching types around. One of the
changes broke the option parser. VSH_OT_ARGV should be used only for
variable argument count, not to make the help generator look pretty.

The correct option type in this case is VSH_OT_STRING as it's not
mandatory now since it can be substituted by using --domain.

This makes --help for this command look incorrect, but the parser works
as it should.

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

When starting a domain with managed save image, we try to restore it
first. If the image is corrupted, we silently unlink it and just
normally start the domain. At this point the domain has no managed save
image, yet we did not reset the hasManagedSave flag.

https://bugzilla.redhat.com/show_bug.cgi?id=1460962Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

One of the usecases of iohelper is to read from pipe and write
to file with O_DIRECT. As we read from pipe we can have partial
read and then we fail to write this data because output file
is open with O_DIRECT and buffer size is not aligned.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

Instead of enumerating all states which need to be turned into
QEMU_DOMAIN_JOB_STATUS_FAILED (and failing to add all of them), it's
better to mention just the one which needs to be left alone.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Almost every failure in qemuMigrationRun while we are talking to QEMU
monitor results in a jump to exit_monitor label. The only exception is
removed by this patch.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

The "ret" variable is used for storing the return value of a function
and should not be used as a temporary variable.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Merge cancel and cancelPostCopy sections with the generic error section,
where we can easily decide whether canceling the ongoing migration is
required.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Let cleanup only do things common to both failure and success paths and
move error handling code inside the new "error" section.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Some code which was supposed to be executed only when migration
succeeded was buried inside the cleanup code.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

When adding a new job state it's useful to let the compiler complain
about places where we need to think about what to do with the new
state.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

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

This way users can uniquely identify devices at define time.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

We need to format alias even for inactive XMLs since that's the
way how users are going to identify their devices.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

The point of this test is to load live XML and test hotplug. But
even though the XMLs we are parsing are live, the parsing is done
with VIR_DOMAIN_DEF_PARSE_INACTIVE flag.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

The qemuxml2argvtest expects the domain XMLs to be inactive ones.
Therefore we should pass inactive XMLs.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Since we will be allowing users to set device aliases and memory
devices are fragile when it comes to aliases we have to make sure
they won't change during migration. Other devices should be fine.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

They have to be unique within the domain. As usual, backwards
compatibility takes its price. In this particular situation we
have a device that is represented twice in a domain and so is its
alias.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

If driver that is calling the parse supports user supplied
aliases, they can be parsed even for inactive XMLs. However, to
avoid any clashes with aliases that libvirt generates, the user
ones have to have "ua-" prefix.

Note, that some drivers don't have notion of device aliases at
all. Also, in order to support user supplied aliases some extra
checks need to be done (e.g. during hotplug). Therefore we can't
just enable this feature for all the drivers. Thus we need a flag
that drivers set to tell parsing code that they can handle user
supplied device aliases.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

When assigning alias to a device we usually iterate over other
devices of its kind trying to find next index. We do this by
stripping down the prefix and then parsing number at the end,
Usually, if the prefix doesn't match the one we are expecting, we
just continue with next iteration. Except for couple of
functions: qemuGetNextChrDevIndex(),
qemuAssignDeviceRedirdevAlias() and qemuAssignDeviceShmemAlias().
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

To avoid further confusion - rename the array elements to what they are.

Introduced by 6094d6ec.
Found by running libvirt-perl tests.
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>

The function virEventRegisterImpl() checks the attempt to replace the
registered events. But there is a duplicate variable inside the IF statement.
The variable 'removeHandleImpl' was wrongly repeated. One of them needs to be
replaced by 'removeTimeoutImpl'.
Signed-off-by: NJulio Faracco <jcfaracco@gmail.com>
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>

Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

The only remaining user of qemuMonitorGetMigrationCapability is our test
suite. Let's replace qemuMonitorGetMigrationCapability with
qemuMonitorGetMigrationCapabilities there and drop the unused function.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

All calls to qemuMonitorGetMigrationCapability in QEMU driver are
replaced with qemuMigrationCapsGet.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Each time we need to check whether a given migration capability is
supported by QEMU, we call query-migrate-capabilities QMP command and
lookup the capability in the returned list. Asking for the list of
supported capabilities once when we connect to QEMU and storing the
result in a bitmap is much better and we don't need to enter a monitor
just to check whether a migration capability is supported.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

The new function is called qemuProcessInitMonitor and it will enter/exit
the monitor so that the caller doesn't have to deal with this.

The goal of this patch is to simplify the code in qemuConnectMonitor
which would otherwise be a bit hairy after the following patch.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>