Previous refactors allow us to plainly replace all VIR_FREE by g_free to
finish the modernization of the file.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Restructure the control-flow a bit using an temporary variable to avoid
the need to use VIR_FREE.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use modern allocators, automatic memory feeing, and decrease the scope
of some variables to remove the 'cleanup' label.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use modern allocators, automatic memory feeing, and decrease the scope
of some variables to remove the 'error' and 'cleanup' labels.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use modern allocators, automatic memory feeing, and decrease the scope
of some variables to remove the 'error' label.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use automatic memory freeing to get rid of the 'error' label. Since the
'tmp' variable was used only in one instance, rename it appropriately.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use modern memory handling approach to simplify the code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use modern memory handling approach to simplify the code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Register the the cleanup functions for 'qemuMigrationCookieGraphics',
'qemuMigrationCookieNetwork', 'qemuMigrationCookieNBD', and
'qemuMigrationCookieCaps'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Switch to automatic memory cleaning, use g_new0 for allocation and get
rid of the 'error' label.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Move the code into 'qemuMigrationCookieNBDXMLFormat' and use modern XML
formatting code patterns.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use 'virXMLFormatElement' both for formating the whole <network> element
but also for formatting the <interface> subelements. This alows to
remove the crazy logic which was determining which element was already
formatted.
Additional simplification is achieved by switching to skipping the loop
using 'continue' rather than putting everything in a giant block.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Switch to the two buffer approach to simplify the logic for terminating
the element.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Now it only returns -1 so we can do that directly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Most of our functions report errors so there's no need to mention it
here again.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Make sure that 'virXPathNodeSet' returns '1' as the only expected value
rather than relying on the fact that the previous check for the number
of elements ensures success of the subsequent call.
The error message no longer mentions the number of <domain> elements in
the cookie, but this is a very unlikely internal error anyways.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Don't reuse 'tmp' over and over, but switch to single use automaticaly
freed variables instead.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Move the code into 'qemuMigrationCookieXMLParseMandatoryFeatures' to
simplify 'qemuMigrationCookieXMLParse'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
After recent refactors the function can be refactored to remove the
'cleanup' label by using autoptr for the 'map' variable.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
If we use g_new0 there's no need for the 'cleanup' label as there's
nothing to fail after the allocation.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There are only 3 places using the function. Two can use virBitmapNewCopy
directly. In case of the qemu capabilities code we need to free the old
bitmap first.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virBitmapCopy has a failure condition, which is impossible to meet when
creating a new copy. Copy the contents directly to make it obvious that
virBitmapNewCopy can't fail.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'checkBitmap' helper uses 'virBitmapFormat' internally and also
reports better errors. Use it instead of the open-coded checks.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function will also be reusable in other places of the code by making
the size check optional. For now only test12* is refactored since it
used TEST_MAP directly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The test validates two outputs. Don't reuse 'str' for both.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'test4' was testing three distinct operations on separate instances of a
bitmap. Split it up into 'test4a', 'test4b' and 'test4c' so that the
'bitmap' variable is not reused.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'test12' was testing two distinct operations on two instances of a
bitmap. Split it up into 'test12a' and 'test12b' so that the 'bitmap'
variable is not reused.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We no longer report any errors so all callers can be replaced by
virBitmapNew. Additionally virBitmapNew can't return NULL now so error
handling is not necessary.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We now always return a valid pointer or crash so the return value
doesn't need to be checked.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Modify the condition which would make virBitmapNewQuiet fail to possibly
overallocate by 1 rather than failing.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Test an empty bitmap including it's extension via the self-expanding
APIs and and a "0" and "" strings when converting the string back and
forth.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We now have APIs which automatically expand the bitmap and also API
which allocates a 0 size bitmap. Remove the condition from virBitmapNew.
Effectively reverts ce49cfb48a
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Move scope of variables and get rid of the 'cleanup' section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virBitmapNewEmpty() can create a bitmap with 0 length. With such a
bitmap virBitmapToString will return NULL rather than an empty string.
Initialize the buffer to avoid that.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Clarify which bit is considered most significant in the bitmap and
resulting string. Also be explicit that it's a hex string.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There's only one combination used so we can remove the rest.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Mid-cycle caps resync. Notable change is that virtio-blk enables
multiqueue by default and the addition of
'calc-dirty-rate'/'query-dirty-rate' QMP commands.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use a more descriptive name and move the verb to the end so that the
functions conform with the naming policy.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There is just one caller, inline the code. This also optimizes the code
as we no longer have to calculate length of the output XML as it's
actually stored in the buffer struct.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We need an empty cookie, so use qemuMigrationCookieNew instead of
qemuMigrationEatCookie with NULL/0 arguments.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Allow direct use rather than going through qemuMigrationEatCookie with
NULL/0 arguments.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Move around some code so that we can get rid of the 'cleanup:' label.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use a more descriptive name and move the verb to the end so that the
functions conform with the naming policy.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We didn't actually use this file. Change the disk type to 'file' so that
it works in qemu and add pre and post-blockdev invocations.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Tested-by: Ján Tomko <jtomko@redhat.com>
Remove the overlay if the disk was <transient/>. Note that even if we'd
forbid unplug of such a disk through the API, the disk can still be
ejected from the guest.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Tested-by: Ján Tomko <jtomko@redhat.com>
To implement <transient/> disks we'll need to install an overlay on top
of the original disk image which will be discarded after the VM is
turned off. This was initially implemented by qemu but libvirt never
picked up this option as the overlays were created by qemu without
libvirt involvment which didn't work with SELinux.
With blockdev the qemu feature became unsupported so we need to do this
via the snapshot code anyways.
The helpers introduced in this patch prepare a fake snapshot disk
definition for a disk which is configured as <transient/> and use it to
create a snapshot (without actually modifying metadata or persistent
def).
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Tested-by: Ján Tomko <jtomko@redhat.com>
Later patches will implement support for <transient/> disks in libvirt
by installing an overlay on top of the configured image. This will
require cleanup after the VM will be stopped so that the state is
correctly discarded.
Since the overlay will be installed only during the startup phase of the
VM we need to ensure that qemuProcessStop doesn't delete the original
file on some previous failure. This is solved by adding
'inhibitDiskTransientDelete' VM private data member which is set prior
to any startup step and will be cleared once transient disk overlays are
established.
Based on that we can then delete the overlays for any <transient/> disk.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Tested-by: Ján Tomko <jtomko@redhat.com>
Allow using the function for creating temporary snapshot disk
definitions for creating <transient/> disk overlays.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Tested-by: Ján Tomko <jtomko@redhat.com>
'virTestCompareToFile' automatically fixes newline if it is not present
in the input string but is present in the file. In this case we need to
append the erorr messages with a newline so that
VIR_TEST_REGENERATE_OUTPUT produces files which will pass syntax-check.
Fixes: 9ec77eef2d
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'ndd' tracks the actual number of snapshot disks filled into the
structure and is incremented by the functions filling the context, thus
it must not be set when initializing the context.
Fixes: 8c2ecdf131
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Make it obvious that the snapshot is prepared for the active external
snapshot case.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Extract the code which invokes the monitor and finalizes the snapshot
into a separate function for easier reuse.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add a container struct which holds all data needed to create and clean
up after a (for now external) snapshot. This will aggregate all the
'qemuSnapshotDiskDataPtr' the 'actions' of a transaction QMP command and
everything needed for cleanup at any given point.
This aggregation allows to simplify the arguments of the functions which
prepare the snapshot data and additionally will simplify the code
necessary for creating overlays on top of <transient/> disks.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Be more specific about the role of the function. It's creating the disk
portion of an external active snapshot.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It's no longer needed and is valid only after virDomainSnapshotAlignDisks
is called while holding the lock.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Last step of the algorithm in virDomainSnapshotAlignDisks is to extend
the array of disks to all VM's disk and provide defaults. This was done
by extending the array, adding defaults at the end and then sorting it.
This requires the 'idx' variable and also a separate sorting function.
If we store the pointer to existing snapshot disk definitions in a hash
table and create a new array of snapshot disk definitions, we can fill
the new array directly by either copying the definition from the old
array or adding the default.
This avoids the sorting step and thus even the need to store the index
of the domain disk altogether.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Remove the use of the 'disk_snapshot' temporary variable since accessing
the disk definition now isn't that much longer to write and use explicit
value checks instead of the (non-)zero check to make it more obvious
what the code is doing.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The converted string is used exactly once so we can call the conversion
without storing the result in a variable.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Extract the disk def to a local variable so that it's more obvious
what's happening and it will also allow further simplification.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There are multiple places accessing the domain definition. Extract it to
a local variable so that it's more clear what's happening.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'disk' variable usually refers to a definition of a disk from the
domain definition. Rename it to 'snapdisk' to be clear that we are
talking about the snapshot disk definition especially since this
function also accesses the domain disk definition.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
While this function resides in the snapshot config module, the 'def'
variable is referencing the VM definition in most places. Change the
name to 'snapdef' to avoid ambiguity especially since we are also
dealing with the domain definition in this function.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use automatic pointer for the bitmap and get rid of the 'cleanup' label
and 'ret' variable.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
After virDomainSnapshotAlignDisks is called the definitions of disks in
the snapshot definition and in the domain definition are in the same
order so they can be addressed using the same index.
This frees up 'idx' to be removed later.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add an abort() on the class/object allocation failures so that
virStorageSourceNew() always returns a virStorageSource and remove
checks from all callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
After meson conversion the man pages started to contain the table of
contents.
In autoconf we prevented this by a 'grep -v ::contents' in the command
building the manpages.
A more cultured solution is to strip out the 'contents' docutils element
directly.
Reported-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'readonly' hostdev property is stored separately from the
virStorageSource as some hostdevs are not described by a virStorage
source. We need to propagate the flag to the virStorage source also for
iSCSI backends as it's used to generate the backend properties.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1868856
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We've put the aliases into the backup job definition after the status
XML was already written so they didn't appear in the on-disk state.
Move the code putting them into the private definition earlier, so that
the status XML update done by saving blockjobs already writes them out.
Also add a note notifying that the block job status update writes the
status XML.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1870488
Fixes: 423576679a
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Modify the test case to enable TLS and add private data containing
aliases of objects corresponding to a TLS setup.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Test that we can cope with a long useralias when generating SCSI hostdev
commandline.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
QEMU's blockdev nodenames which are used to back SCSI/iSCSI hostdevs are
limited to 32 characters. If a user passes a very long user alias as
name of the host device it's easy to end up with a too-long nodename.
To prevent this from happening don't base the nodename on the possibly
user-specified alias but on the normal sequential node name generator.
We then store the name in the status XML for further use.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The secret object is used to pass data to the backend so it's better
fitting to base the secret object name on the SCSI host device backend
name.
Since we store the object alias in the status XML this modification is
safe in regards to existing guests.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Allocate the nodename in the setup function rather than in the command
line generator.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function is no longer used once we setup per-hostdev data.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Historically we've prepared secrets for all objects in one place. This
doesn't make much sense and it's semantically more appealing to prepare
everything for a single device type in one place.
Move the setup of the (iSCSI|SCSI) hostdev secrets into a new function
which will be used to setup other things as well in the future.
This is a similar approach we do for disks.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This was a hack when we were locally regenerating the nodename so that
it's not leaked. Now that we use proper virStorageSource with
persistence it's no longer required.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Modify the attach/detach data generators to actually use the
virStorageSourceStructure embedded in the SCSI config data rather than
creating an ad-hoc internal one.
The modification will allow us to properly store the nodename used for
the backend in the status XML rather than re-generating it all the
time.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
For upgrade reasons so that we can modify the used nodename we must
generate the old version for all status XMLs which don't have it stored
explicitly.
The change will be required as using the user-provided alias may result
in too-long nodenames which will be rejected by qemu.
Add code which fills in the appropriate old value and add test cases to
validate that it's added and also that existing nodenames are not
overwritten.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add a local SCSI host device to validate upcoming generated data.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The test case tests other things besides disk secinfos, so we can make
it more universal.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The backend for the SCSI host device is a storage source. While the
definition doesn't look like that it's converted to a storage source
when the VM is running.
Add the storage source to the definition object and also parse/format
its private data which will be used for internal state storage while
the VM is running.
Note that the virStorageSourcePtr may not be allocated all the time so
the private data parser allocates it if there is any private data
present.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa