Fix and unify the naming of external snapshot preparation functions.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Make qemuDomainSnapshotDiskDataCleanup cleanup section friendly by
moving the error preservation code inside it.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

We changed to always saving the status and config XMLs to simplify
code. After a few more refactors it's now possible to move it to the
appropriate place and save the XMLs only on success again.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

When we take a snapshot we must properly remove our locking
infrastructure locks. This was broken by commit 3817fa10 which
attempted to properly track the readonly state for the image as the
locking code was executed after this change. Since we forced the image
which was locked as read-write to read-only prior to unlocking it the
write lock was not dropped.

Fix it by moving the locking code prior to modifying the readonly flag.

https://bugzilla.redhat.com/show_bug.cgi?id=1745618Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

The code preparing data for creating/attaching the target image of block
copy didn't use the correct reference to the existing backing chain in
case when the copy should inherit it. This meant that qemu actually
opened a second copy of the chain and operated on that.

This would de-sync qemu from libvirt's view of node names. Luckily this
is only hypothetical at this point since it happens only when -blockdev
is enabled.

Fix it by passing 'mirrorBacking' which has the proper data as the
backing store when calling
qemuBuildStorageSourceChainAttachPrepareBlockdevTop.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

In some cases we'll need to pass in a backing store which is not
recorded as the backing store of @src. Export backingStore as variable
and fix all callers to pass in the backing store. No semantic changes
for now.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Pass backing store as an argument rather than extracting it locally and
fix the callers.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Extract the loop and supporting infrastructure to the caller as only one
of the two callers actually cares about looping and rename the helper to
qemuBuildStorageSourceChainAttachPrepareBlockdevOne.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Pass in backing store explicitly to qemuBlockStorageSourceGetBlockdevProps
and fix the callers.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Move all bits of the formatting of the 'backing' attribute to a single
condition and make it use a single extracted copy of the backing store.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Since libvirt stores the backing chain into the XML in a nested way it
is the prime possibility to hit libxml2's parsing limit of 256 layers.

Introduce code which will crawl the backing chain and verify that it's
not too deep. The maximum nesting is set to 200 layers so that there's
still some space left for additional properties or nesting into snapshot
XMLs.

The check is applied to all disk use cases (starting, hotplug, media
change) as well as block copy which changes image and snapshots.

We simply report an error and refuse the operation.

Without this check a restart of libvirtd would result in the status XML
failing to be parsed and thus losing the VM.

https://bugzilla.redhat.com/show_bug.cgi?id=1524278Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Use VIR_AUTOUNREF and get rid of the cleanup label.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

With blockdev we must issue the block_set_io_throttle QMP command to
setup disk throttling as we currently can't do it with the 'throttle'
layer.

Unfortunately there's nothing we can do if it fails.

https://bugzilla.redhat.com/show_bug.cgi?id=1733163Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Get rid of the last manually freed var.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Switch to using VIR_AUTOFREE and remove the cleanup label.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

When virtlogd is used to capture QEMU's stdout, qemuDomainObjTaint would
always fail to write the message to the log file when QEMU is already
running (i.e., outside qemuProcessLaunch). This can happen during device
hotplug or by sending a custom QEMU guest agent command:

    warning : qemuDomainObjTaint:8757 : Domain id=9 name='blaf'
        uuid=9cfa4e37-2930-405b-bcb4-faac1829dad8 is tainted:
        custom-ga-command
    error : virLogHandlerDomainOpenLogFile:388 : Cannot open log file:
        '/var/log/libvirt/qemu/blaf.log': Device or resource busy
    error : virNetClientProgramDispatchError:172 : Cannot open log file:
        '/var/log/libvirt/qemu/blaf.log': Device or resource busy

The fix is easy, we just need to use the right API for appending a
message to QEMU log file instead of creating a new log context.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

The offset is unsigned long long thus 'U' must be used.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

The 'u' modifier creates an unsigned int JSON attribute but the disk size
and capacity fields are unsigned long long. If the size of the created
image would be more than 4GiB we'd overflow and create sub-4G image.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

A specially crafted XML which would reference a non-existing disk but
request the mirror to be registered with the blockjob could potentially
make the parser dereference NULL. Fix it by moving the code slightly and
just treat it as a wrong job XML. Found by Coverity.
Reported-by: NJohn Ferlan <jferlan@redhat.com>
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

If a block job reaches failed/cancelled state, or is completed
without pivot then we must remove security driver metadata
associated to the backing chain so that we don't leave any
metadata behind.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1741456Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
ACKed-by: NPeter Krempa <pkrempa@redhat.com>

When a block job is completed, the security image metadata are
moved to the new image. If this fails an warning is printed, but
the message contains only domain name and lacks image paths. Put
them both into the warning message.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
ACKed-by: NPeter Krempa <pkrempa@redhat.com>

Currently, there are only a few lines of code so a separate
function was not necessary, but this will change. So instead of
putting all the new code under 'case
QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT' create a separate function.
Just like every other case has one.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
ACKed-by: NPeter Krempa <pkrempa@redhat.com>

Coverity noted that 'reply' can be NULL after calling
qemuAgentCommand().  Avoid dereferencing reply in
qemuAgentErrorComandUnsupported() in that case.
Signed-off-by: NJonathon Jongsma <jjongsma@redhat.com>

'virQEMUDriverConfigPtr cfg' is declared, initiated, but never
used in virQEMUDriverCreateCapabilities().
Signed-off-by: NDaniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>
Reviewed-by: NChristophe de Dinechin <dinechin@redhat.com>

As qemu documents we should use everything in the 'props' sub-object of
the data returned by query-hotpluggable-cpus. Until now we only used
everything we recognized, but that may break in cases when qemu
introduces new fields.

This change requires a fix to the test data as some fields were
reordered.

https://bugzilla.redhat.com/show_bug.cgi?id=1741658Signed-off-by: NPeter Krempa <pkrempa@redhat.com>

In addition to the data that libvirt needs and extracts internally,
copy and store the whole 'props' JSON sub-object of the data returned by
query-hotpluggable-cpus for future use.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>

On error paths, info_ret could potentially leak. Make sure it's freed.
Signed-off-by: NJonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

When we're collecting guest information, older agents may not support
all agent commands. In the case where the user requested all info
types (i.e. types == 0), ignore unsupported command errors and gather as
much information as possible. If the agent command failed for some other
reason, or if the user explciitly requested a specific info type (i.e.
types != 0), abort on the first error.
Signed-off-by: NJonathon Jongsma <jjongsma@redhat.com>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

Add support to specify a boot order on vfio-ccw passthrough devices.
Reviewed-by: NMarc Hartmayer <mhartmay@linux.ibm.com>
Signed-off-by: NBjoern Walk <bwalk@linux.ibm.com>
Signed-off-by: NBoris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Signed-off-by: NJán Tomko <jtomko@redhat.com>

Refactoring the method signatures in preparation for
checking boot index of the mediated devices.
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NMarc Hartmayer <mhartmay@linux.ibm.com>
Signed-off-by: NBjoern Walk <bwalk@linux.ibm.com>
Signed-off-by: NBoris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: NJán Tomko <jtomko@redhat.com>

Changing the error messages to report the problem encountered.
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NMarc Hartmayer <mhartmay@linux.ibm.com>
Signed-off-by: NBjoern Walk <bwalk@linux.ibm.com>
Signed-off-by: NBoris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: NJán Tomko <jtomko@redhat.com>

Moving the hostdev boot support validation from the command line
generator code into the domain validation code.
Reviewed-by: NMarc Hartmayer <mhartmay@linux.ibm.com>
Signed-off-by: NBjoern Walk <bwalk@linux.ibm.com>
Signed-off-by: NBoris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Signed-off-by: NJán Tomko <jtomko@redhat.com>

Introduced in v3.0.0-rc1~336, the commit message doesn't really
justifies the expensive domain def copy creation. Now, that
vm->def is guarded in this function by job acquirement we can use
vm->def directly.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

These two functions work with vm->def in their critical sections
(i.e. after the job was acquired and before it is released). But
that means, they need QUERY domain job too to prevent vm->def
change.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

All code using LOCALSTATEDIR "/run" is updated to use RUNSTATEDIR
instead. The exception is the remote driver client which still
uses LOCALSTATEDIR "/run". The client needs to connect to remote
machines which may not be using /run, so /var/run is more portable
due to the /var/run -> /run symlink.

Some duplicate paths in the apparmor code are also purged.

There's no functional change by default yet since both expressions
expand to the same value.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Creating various directories using $(runstatedir) instead of
$(localstatedir)/run.

There's no functional change by default yet since both expressions
expand to the same value.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

The recent cleanups allow us to clean up the code a bit.
Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

Now that it's only used once.
Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>