When attaching a device to live XML we don't care (well,
shouldn't care) that there's already a device in inactive XML
that has the same user alias.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
If we're attaching a device to both inactive and live XML then
@ret is overwritten which may result in incorrect return value.
For instance, if attaching to inactive XML succeeds, @ret is
assigned value of zero and control proceeds to attaching the
device to live XML. Here, if say
virDomainDeviceValidateAliasForHotplug() fails the control jumps
over to 'cleanup' label and zero is returned indicating success.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Our coding style specifies that only negative values are considered as
error. Check for return value of virDomainDiskInsert() properly,
following the style. Not that the function can now return anything other
than 0 or -1, but it just triggers my OCD.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
If the current QEMU guest can't wake up from suspend properly,
and we are able to determine that, avoid suspending the guest
at all. To be able to determine this support, QEMU needs to
implement the 'query-current-machine' QMP call. This is reflected
by the QEMU_CAPS_QUERY_CURRENT_MACHINE cap.
If the cap is enabled, a new function qemuDomainProbeQMPCurrentMachine
is called. This is wrapper for qemuMonitorGetCurrentMachineInfo,
where the 'wakeup-suspend-support' flag is retrieved from
'query-current-machine'. If wakeupSuspendSupport is true,
proceed with the regular flow of qemuDomainPMSuspendForDuration.
The absence of QEMU_CAPS_QUERY_CURRENT_MACHINE indicates that
we're dealing with a QEMU version older than 4.0 (which implements
the required QMP API). In this case, proceed as usual with the
suspend logic of qemuDomainPMSuspendForDuration, since we can't
assume whether the guest has support or not.
Fixes: https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/1759509
Reported-by: Balamuruhan S <bala24@linux.vnet.ibm.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Ports allocated on virtual networks with type=nat|route|open all get
given an actual type of 'network'.
Only ports in networks with type=bridge use an actual type of 'bridge'.
This distinction makes little sense since the virtualization drivers
will treat both actual types in exactly the same way, as they're all
just bridge devices a VM needs to be connected to.
This doesn't affect user visible XML since the "actual" device XML
is internal only, but we need code to convert the data upgrades.
Reviewed-by: Laine Stump <laine@laine.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Firstly, VIR_STRDUP() accepts NULL, so there is no need to check
if the string we want to duplicate is not-NULL. Secondly,
virDomainNetSetModelString() also accepts NULL. Thirdly, we have
VIR_AUTOFREE().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
To ease converting the net->model value to an enum, add
the wrapper functions:
virDomainNetGetModelString
virDomainNetSetModelString
virDomainNetStreqModelString
virDomainNetStrcaseeqModelString
Acked-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
The port allocation APIs are currently called unconditionally for all
types of NIC, but (mostly) only do anything for NICs with type=network.
The exception is the port allocate API which does some validation even
for NICs with type!=network. Relying on this validation is flawed,
however, since the network driver may not even be installed. IOW virt
drivers must not delegate validation to the network driver for NICs
with type != network.
This change allows us to report errors when the virtual network driver
is not registered.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
My earlier commit be46f61326 was incomplete. It removed caching of
microcode version in the CPU driver, which means the capabilities XML
will see the correct microcode version. But it is also cached in the
QEMU capabilities cache where it is used to detect whether we need to
reprobe QEMU. By missing the second place, the original commit
be46f61326 made the situation even worse since libvirt would report
correct microcode version while still using the old host CPU model
(visible in domain capabilities XML).
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Standardize on putting the _LAST enum value on the second line
of VIR_ENUM_IMPL invocations. Later patches that add string labels
to VIR_ENUM_IMPL will push most of these to the second line anyways,
so this saves some noise.
Signed-off-by: Cole Robinson <crobinso@redhat.com>
If a management application wants to use firmware auto selection
feature it can't currently know if the libvirtd it's talking to
support is or not. Moreover, it doesn't know which values that
are accepted for the @firmware attribute of <os/> when parsing
will allow successful start of the domain later, i.e. if the mgmt
application wants to use 'bios' whether there exists a FW
descriptor in the system that describes bios.
This commit then adds 'firmware' enum to <os/> element in
<domainCapabilities/> XML like this:
<enum name='firmware'>
<value>bios</value>
<value>efi</value>
</enum>
We can see both 'bios' and 'efi' listed which means that there
are descriptors for both found in the system (matched with the
machine type and architecture reported in the domain capabilities
earlier and not shown here).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
virutil.(c|h) is a very gross collection of random code. Remove the enum
handlers from there so we can limit the scope where virtutil.h is used.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Since the STOP event handler can use the pausedReason as sent to
qemuProcessStopCPUs, we no longer need to send duplicate suspended
lifecycle events because we know what caused the stop along with extra
details. This processing allows us to also remove the duplicated state
change from qemuProcessStopCPUs.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
qemuMigrationSrcPerform callers expect it to call virDomainObjEndAPI
in any case so on error paths we miss the virDomainObjEndAPI call.
To fix this let's make qemuMigrationSrcPerform callers responsible
for the virDomainObjEndAPI call.
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
'blockdev-snapshot-sync' is present in QEMU since v0.14.0-rc0 and
'transaction' since v1.1.0 (52e7c241ac766406f05fa)
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemu added the 'drive-mirror' command in v1.3.0 (d9b902db3fb71fdc)
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemu added the 'block-commit' command in v1.3.0 (ed61fc10e8c8d2)
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This was detected by the presence of 'block-stream' which is present in
qemu since v1.1 (db58f9c0605fa151b8c4)
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use the common base class virDomainMoment for iterator callbacks
related to snapshots from the qemu code, so that when checkpoint
operations are introduced, they can share the same callbacks.
Simplify the code for qemuDomainSnapshotCurrent by better utilizing
virDomainMoment helpers.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The qemu driver already had a full-blown virDomainMomentObjPtr to
check against, and the test driver ought to have one since we get
better error checking that the user passed in a valid object. Removes
the need for a helper function added in commit commit 4819f54b.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
qemuDomainDetachDeviceLive() is called from two places in
qemu_driver.c, and qemuDomainUpdateDeviceList() is called from the
end of qemuDomainDetachDeviceLive(), which is now in qemu_hotplug.c
This patch replaces the single call to qemuDomainUpdateDeviceList()
with two calls to it immediately after return from
qemuDomainDetachDeviceLive(). This is only done if the return from
that function is exactly 0, in order to exactly preserve previous
behavior.
Removing that one call from qemuDomainDetachDeviceList() will permit
us to call it from the test driver hotplug test, replacing the
separate calls to qemuDomainDetachDeviceDiskLive(),
qemuDomainDetachChrDevice(), qemuDomainDetachShmemDevice() and
qemuDomainDetachWatchdog(). We want to do this so that part of the
common functionality of those three functions (and the rest of the
device-specific Detach functions) can be pulled up into
qemuDomainDetachDeviceLive() without breaking the test. (This is done
in the next patch).
NB: Almost certainly this is "not the best place" to call
qemuDomainUpdateDeviceList() (actually, it is provably the *wrong*
place), since it's purpose is to retrieve an "up to date" list of
aliases for all devices from qemu, and if the guest OS hasn't yet
processed the detach request, the now-being-removed device may still
be on that list. It would arguably be better to instead call
qemuDomainUpdateDevicesList() later during the response to the
DEVICE_DELETED event for the device. But removing the call from the
current point in the detach could have some unforeseen ill effect due
to changed timing, so the change to move it into
qemuDomainRemove*Device() will be done in a separate patch (in order
to make it easily revertible in case it causes a regression).
Signed-off-by: Laine Stump <laine@laine.org>
ACKed-by: Peter Krempa <pkrempa@redhat.com>
This function is going to take on some of the functionality of its
subordinate functions, which all live in qemu_hotplug.c.
qemuDomainDetachDeviceControllerLive() is only called from
qemuDomainDetachDeviceLive() (and will soon be merged into
qemuDomainDetachControllerDevice(), which is in qemu_hotplug.c), so
it is also moved.
Signed-off-by: Laine Stump <laine@laine.org>
ACKed-by: Peter Krempa <pkrempa@redhat.com>
Now that the core of SnapshotObj is agnostic to snapshots and can be
shared with upcoming checkpoint code, it is time to rename the struct
and the functions specific to list operations. A later patch will
shuffle which file holds the common code. This is a fairly mechanical
patch.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Another step towards making the object list reusable for both
snapshots and checkpoints: the list code only ever needs items that
are in the common virDomainMomentDef base type. This undoes a lot of
the churn in accessing common members added in the previous patch, and
the bulk of the patch is mechanical. But there was one spot where I
had to unroll a VIR_STEAL_PTR to work around changed types.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Pull out the common parts of virDomainSnapshotDef that will be reused
for virDomainCheckpointDef into a new base class. Adjust all callers
that use the direct fields (some of it is churn that disappears when
the next patch refactors virDomainSnapshotObj; oh well...).
Someday, I hope to switch this type to be a subclass of virObject, but
that requires a more thorough audit of cleanup paths, and besides
minimal incremental changes are easier to review.
As for the choice of naming:
I promised my teenage daughter Evelyn that I'd give her credit for her
contribution to this commit. I asked her "What would be a good name
for a base class for DomainSnapshot and DomainCheckpoint". After
explaining what a base class was (using the classic OOB Square and
Circle inherit from Shape), she came up with "DomainMoment", which is
way better than my initial thought of "DomainPointInTime" or
"DomainPIT".
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Rather than allowing a leaky abstraction where multiple drivers have
to open-code operations that update the relations in a
virDomainSnapshotObjList, it is better to add accessor functions so
that updates to relations are maintained closer to the internals.
This patch finishes the job started in the previous patch, by getting
rid of all direct access to nchildren, first_child, or sibling outside
of the lowest level functions, making it easier to refactor later on.
The lone new caller to virDomainSnapshotObjListSize() checks for a
return != 0, because it wants to handles errors (-1, only possible if
the hash table wasn't allocated) and existing snapshots (> 0) in the
same manner; we can drop the check for a current snapshot on the
grounds that there shouldn't be one if there are no snapshots.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Rather than allowing a leaky abstraction where multiple drivers have
to open-code operations that update the relations in a
virDomainSnapshotObjList, it is better to add accessor functions so
that updates to relations are maintained closer to the internals.
This patch starts the task with a single new function:
virDomainSnapshotMoveChildren(). The logic might not be immediately
obvious [okay, that's an understatement - the existing code uses black
magic ;-)], so here's an overview: The old code has an implicit for
loop around each call to qemuDomainSnapshotReparentChildren() by using
virDomainSnapshotForEachChild() (you'll need a wider context than
git's default of 3 lines to see that); the new code has a more visible
for loop. Then it helps if you realize that the code is making two
separate changes to each child object: STRDUP of the new parent name
prior to writing XML files (unchanged), and touching up the pointer to
the parent object (refactored); the end result is the same whether a
single pass made both changes (both in driver code), or whether it is
split into two passes making one change each (one in driver code, the
other in the new accessor).
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
It is easier to track the current snapshot as part of the list of
snapshots. In particular, doing so lets us guarantee that the current
snapshot is cleared if that snapshot is removed from the list (rather
than depending on the caller to do so, and risking a use-after-free
problem, such as the one recently patched in 1db9d0efbf). This
requires the addition of several new accessor functions, as well as a
useful return type for virDomainSnapshotObjListRemove(). A few error
handling sites that were previously setting vm->current_snapshot =
NULL can now be dropped, because the previous function call has now
done it already. Also, qemuDomainRevertToSnapshot() was setting the
current vm twice, so keep only the one used on the success path.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Rework the logic in qemuDomainSnapshotLoad() to set
vm->current_snapshot only once at the end of the loop, rather than
repeatedly querying it during the loop, to make it easier for the next
patch to use accessor functions rather than direct manipulation of
vm->current_snapshot. When encountering multiple snapshots claiming
to be current (based on the presence of an <active>1</active> element
in the XML, which libvirt only outputs for internal use and not for
any public API), this changes behavior from warning only once and
running with no current snapshot, to instead warning on each duplicate
and selecting the last one encountered (which is arbitrary based on
readdir() ordering, but actually stands a fair chance of being the
most-recently created snapshot whether by timestamp or by the
propensity of humans to name things in ascending order).
Note that the code in question is only run by libvirtd when it first
starts, reading state from disk from the previous run into memory for
this run. Since the data resides somewhere that only libvirt should be
touching (typically /var/lib/libvirt/qemu/snapshot/*), it should be
clean. So in the common case, the code touched here is unreachable.
But if someone is actually messing with files behind libvirt's back,
they deserve the change in behavior.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The only use for the 'current' member of virDomainSnapshotDef was with
the PARSE/FORMAT_INTERNAL flag for controlling an internal-use
<active> element marking whether a particular snapshot definition was
current, and even then, only by the qemu driver on output, and by qemu
and test driver on input. But this duplicates vm->snapshot_current,
and gets in the way of potential simplifications to have qemu store a
single file for all snapshots rather than one file per snapshot. Get
rid of the member by adding a bool* parameter during parse (ignored if
the PARSE_INTERNAL flag is not set), and by adding a new flag during
format (if FORMAT_INTERNAL is set, the value printed in <active>
depends on the new FORMAT_CURRENT).
Then update the qemu driver accordingly, which involves hoisting
assignments to vm->current_snapshot to occur prior to any point where
a snapshot XML file is written (although qemu kept
vm->current_snapshot and snapshot->def_current in sync by the end of
the function, they were not always identical in the middle of
functions, so the shuffling gets a bit interesting). Later patches
will clean up some of that confusing churn to vm->current_snapshot.
Note: even if later patches refactor qemu to no longer use
FORMAT_INTERNAL for output (by storing bulk snapshot XML instead), we
will always need PARSE_INTERNAL for input (because on upgrade, a new
libvirt still has to parse XML left from a previous libvirt).
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This code originates from:
commit d0aa10fdd6
Author: Daniel P. Berrange <berrange@redhat.com>
Date: Tue Mar 3 12:03:44 2009 +0000
QEMU security driver usage for sVirt support (James Morris, Dan Walsh, Daniel Berrange)
Originally in the qemudDomainGetSecurityLabel function. It doesn't
appear to have done anything useful back then either. The other two
instances look like copy+paste
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
snapshot_conf.h was mixing three separate types: the snapshot
definition, the snapshot object, and the snapshot object list.
Separate out the snapshot object list code into its own file, and
update includes for affected clients.
This is just code motion, but done in preparation of sharing a lot of
the object list code with checkpoints.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Any job which is able to provide statistics that can be queried via
virDomainGetJob{Stats,Info} has to set an appropriate statsType.
Without a proper statsType qemuDomainJobInfoToParams and
qemuDomainJobInfoToInfo have no idea what statistics should be sent to
the API caller.
https://bugzilla.redhat.com/show_bug.cgi?id=1688774
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
snapshot_conf does all the hard work, the qemu driver just has to
accept the new flag.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
virDomainSnapshotDefFormat currently takes two sets of knobs:
an 'unsigned int flags' argument that can currently just be
VIR_DOMAIN_DEF_FORMAT_SECURE, and an 'int internal' argument used as
a bool to determine whether to output an additional element. It
then reuses the 'flags' knob to call into virDomainDefFormatInternal(),
which takes a different set of flags. In fact, prior to commit 0ecd6851
(1.2.12), the 'flags' argument actually took the public
VIR_DOMAIN_XML_SECURE, which was even more confusing. Let's borrow
from the style of that earlier commit, by introducing a function
for translating from the public flags (VIR_DOMAIN_SNAPSHOT_XML_SECURE
was just recently introduced) into a new enum specific to snapshot
formatting, and adjust all callers to use snapshot-specific enum
values when formatting, and where the formatter now uses a new
variable 'domainflags' to make it obvious when we are translating
from snapshot flags back to domain flags. We don't even have to
use the conversion function for drivers that don't accept the
public VIR_DOMAIN_SNAPSHOT_XML_SECURE flag.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Clean up the previous patch which abused switch on virDomainState
while working with a variable containing virDomainSnapshotState, by
converting the two affected switch statements to now use the right
enum.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The existing virDomainSnapshotState is a superset of virDomainState,
adding one more state (disk-snapshot) on top of valid domain states.
But as written, the enum cannot be used for gcc validation that all
enum values are covered in a strongly-typed switch condition, because
the enum does not explicitly include the values it is adding to.
Copy the style used in qemu_blockjob.h of creating new enum names
for every inherited value, and update most clients to use the new
enum names anywhere snapshot state is referenced. The exception is
two switch statements in qemu code, which instead gain a fixme
comment about odd type usage (which will be cleaned up in the next
patch). The rest of the patch is fairly mechanical (I actually did
it by temporarily s/state/xstate/ in snapshot_conf.h to let the
compiler find which spots in the code used the field, did the
obvious search and replace in those functions, then undid the rename).
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The current qemu code rejects the combination of the two flags
VIR_DOMAIN_SNAPSHOT_CREATE_LIVE in tandem with
VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE, but rather late in the cycle
(after the snapshot was already parsed), and with a rather confusing
message (complaining that live snapshots require external storage,
even if the redefined snapshot already declares external storage).
Hoist the rejection message to occur earlier (before parsing any
XML, which also aids upcoming patches that will implement bulk
redefine), and with a more typical error message about mutually
exclusive flags.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1685151
This reverts commit cefb97fb81.
The stateAutoStart callback will be removed in the next commit.
Therefore move autostarting of domains, networks and storage
pools back into stateInitialize callbacks.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The existing qemu snapshot code has a slight bug: if the domain
is currently pmsuspended, you can't use the _REDEFINE flag even
though the current domain state should have no bearing on being
able to recreate metadata state; and conversely, you can use the
_REDEFINE flag to create snapshot metadata claiming to be
pmsuspended as a bypass to the normal restrictions that you can't
create an original qemu snapshot in that state (the restriction
against pmsuspend is specific to qemu, rather than part of the
driver-agnostic snapshot_conf code).
Fix this by checking the snapshot state (when redefining) instead
of the domain state (which is a subset of snapshot states).
Fixes the second problem mentioned in https://bugzilla.redhat.com/1680304
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Upcoming patches plan to introduce virDomainCheckpointPtr as a new
object for use in incremental backups, along with documentation on
how incremental backups differ from snapshots. But first, we need
to rename any existing mention of a 'system checkpoint' to instead
be a 'full system snapshot', so that we aren't overloading
the term checkpoint.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Right now we're reporting errors in virFileWrapperFdFree(),
but that's hardly the appropriate place to do so, as free
functions are supposed to do nothing more than release
allocated resources.
We want to move that code back into virFileWrapperFdClose(),
but before we can do that we need to make sure the function
is actually called every time we're done processing the
wrapped file. The cleanup path is the obvious candidate.
In a couple of cases we can just move the call, but for the
remaining ones we need to duplicate it instead in order not
to alter the existing behavior. We do, however, make sure
that in all cases a failure to properly close the wrapper
results in the overall operation being reported as failed.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use refcounting for priv->monConfig instead of asymmetric freeing.
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Commit f609cb85 (0.9.5) introduced virDomainSnapshotGetXMLDesc()'s use
of @flags as a subset of virDomainXMLFlags, documenting that 2 of the
3 flags defined at the time would never be valid. Later, commit
28f8dfdc (1.0.0) introduced a new flag, VIR_DOMAIN_XML_MIGRATABLE, but
did not adjust the snapshot documentation to declare it as invalid.
However, since the flag is not accepted as valid by any of the
drivers (remote is just passthrough; esx and vbox don't support flags;
qemu, test, and vz only support VIR_DOMAIN_XML_SECURE), and it is
unlikely that the domain state saved off during a snapshot creation
needs to be migration-friendly (as the snapshot is not the source of
a migration), it is easier to just define an explicit set of supported
flags directly related to the snapshot API rather than trying to
borrow from domain API, and risking confusion if even more domain
flags are added later (in fact, I have an upcoming patch that plans to
add a new flag to virDomainGetXMLDesc that makes no sense for
snapshots).
There is no API or ABI impact (since we purposefully used unsigned int
rather than an enum type in public API, and since the new flag name
carries the same value as the reused name).
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Commit d2a929d4 (0.9.4) defined virDomainSaveImageGetXMLDesc()'s use
of @flags as a subset of virDomainXMLFlags, documenting that 2 of the
3 flags defined at the time would never be valid. Later, commit
28f8dfdc (1.0.0) introduced a new flag, VIR_DOMAIN_XML_MIGRATABLE, but
did not adjust the save image documentation to declare it as invalid.
Later, commit a67e3872 (3.7.0) blindly copied and pasted the same text
into virDomainManagedSaveGetXMLDesc.
However, since the flag is not accepted as valid by any of the
drivers (remote is just passthrough; and qemu is the only supporting
driver for either API, with support for just VIR_DOMAIN_XML_SECURE),
it is easier to just define an explicit set of supported flags
directly related to the save image API rather than trying to borrow
from live domain API, and risking confusion if even more domain flags
are added later (in fact, I have an upcoming patch that plans to add
a new flag to virDomainGetXMLDesc that makes no sense for saved
images). We may someday decide that saved images need to support the
_MIGRATABLE flag, as it is possible to load a saved image with a
different version of libvirt than the one that created it, but that
can be a separate patch if it is ever needed. Meanwhile, it DOES make
sense to reuse the same flags for SaveImage and for ManagedSave (since
ManagedSave is really just sugar for creating a normal SaveImage in a
location controlled by libvirt instead of by the user).
There is no API or ABI impact (since we purposefully used unsigned int
rather than an enum type in public API, and since the new flag name
carries the same value as the old reused name).
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Although VIR_DOMAIN_DEF_FORMAT_INACTIVE and VIR_DOMAIN_XML_INACTIVE
happen to have the same value (1<<1), they come from different enums;
and it is nicer to reason about a 'flags' variable if all uses of
that variable are compared against the same enum type. Messed up in
commit 06f75ff2 (3.8.0).
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Many drivers had a comment that they did not validate the incoming
'flags' to virDomainGetXMLDesc() because they were relying on
virDomainDefFormat() to do it instead. This used to be the case
(at least since 461e0f1a and friends in 0.9.4 added unknown flag
checking in general), but regressed in commit 0ecd6851 (1.2.12),
when all of the drivers were changed to pass 'flags' through the
new helper virDomainDefFormatConvertXMLFlags(). Since this helper
silently ignores unknown flags, we need to implement flag checking
in each driver instead.
Annoyingly, this means that any new flag values added will silently
be ignored when targeting an older libvirt, rather than our usual
practice of loudly diagnosing an unsupported flag. Add comments
in domain_conf.[ch] to remind us to be extra vigilant about the
impact when adding flags (a new flag to add data is safe if the
older server omitting the requested data doesn't break things in
the newer client; a new flag to suppress data rather than enhancing
the existing VIR_DOMAIN_XML_SECURE may form a data leak or even a
security hole).
In the qemu driver, there are multiple callers all funnelling to
qemuDomainDefFormatBufInternal(); many of them already validated
flags (and often only a subset of the full set of possible flags),
but for ease of maintenance, we can also check flags at the common
helper function.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
We dropped support in commit 8e91a40 (November 2015), but some
occurrences still remained, even in live code.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reported-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Now that virStorageSource is a subclass of virObject we can use
virObjectUnref and remove virStorageSourceFree which was a thin wrapper.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Since virStorageSource is now a subclass of virObject, we can use
VIR_AUTOUNREF instead.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Add virStorageSourceNew and refactor places allocating that structure to
use the helper.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
The code for creating external snapshots for an offline domain
called out to qemu-img without escaping commas in the manner
that qemu-img expects. This also fixes a typo in the comment.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Let's make use of the auto __cleanup capabilities cleaning up any
now unnecessary goto paths.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This flag tells virDomainMigrateSetMaxSpeed and
virDomainMigrateGetMaxSpeed APIs to work on post-copy migration
bandwidth.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Let's make the code flow easier to follow and get rid of the ugly endjob
label inside if branch.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemuDomainBlockPivot and qemuDomainBlockJobAbort need the job name for
cancelling or pivoting but were generating it locally instead of
accessing the existing copy in the job data structure.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The writing to an image actually starts when the copy job is initiated,
so checking this at the time of the pivot operation is too late.
Move the check to qemuDomainBlockCopyCommon. Note that modern qemu would
have prevented two writers with qcow2 so the slim possibility of a job
started with libvirtd without this patch missing the check is not really
worth worrying about.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
For copy and active commit jobs we record the state of the mirror so
that we can recover. The status XML was not saved in case of
qemuDomainBlockPivot due to an oversight.
Save the XML always when invoking qemuDomainBlockJobAbort even if
the job is not currently tracking any state. This will change later and
also this is not a particularly hot code path.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Missing semicolon at the end of macros can confuse some analyzers
(like cppcheck <filename>), and we have a mix of semicolon and
non-semicolon usage through the code. Let's standardize on using
a semicolon for VIR_ENUM_IMPL calls.
Move the verify() statement to the end of the macro and drop
the semicolon, so the compiler will require callers to add a
semicolon.
While we are touching these call sites, standardize on putting
the closing parenth on its own line, as discussed here:
https://www.redhat.com/archives/libvir-list/2019-January/msg00750.html
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
Missing semicolon at the end of macros can confuse some analyzers
(like cppcheck <filename>), and we have a mix of semicolon and
non-semicolon usage through the code. Let's standardize on using
a semicolon for VIR_ENUM_DECL calls.
Drop the semicolon from the final statement of the macro, so
the compiler will require callers to add a semicolon.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
Some clients poll virDomainGetBlockJobInfo rather than wait for the
VIR_DOMAIN_BLOCK_JOB_READY event. In some cases qemu can get to 100% and
still not reach the synchronised phase. Initiating a pivot in that case
will fail.
Given that computers are interacting here, the error that the job
can't be finalized yet is not handled very well by those specific
implementations.
Our docs now correctly state to use the event. We already do a similar
output adjustment in case when the progress is not available from qemu
as in that case we'd report 0 out of 0, which some apps also incorrectly
considered as 100% complete.
In this case we subtract 1 from the progress if the ready state is not
signalled by qemu if the progress was at 100% otherwise.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Be more sensible when setting labels of the target of a
virDomainBlockCopy operation. Previously we'd relabel everything in case
it's a copy job even if there's no unlabelled backing chain. Since we
are also not sure whether the backing chain is shared we don't relabel
the chain on completion of the blockjob. This certainly won't play nice
with the image permission relabelling feature.
While this does not fix the case where the image is reused and has
backing chain it certainly sanitizes all the other cases. Later on it
will also allow to do the correct thing in cases where only one layer
was introduced.
The change is necessary as in case when -blockdev will be used we will
need to hotplug the backing chain and thus labeling needs to be setup in
advance and not only at the time of pivot. To avoid multiple code paths
move the labeling now.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Previously there weren't any suitable functions which would allow
setting up host side of a full disk chain so we've opted to replace the
'src' in a virDomainDiskDef by the new image source.
That is now no longer necessary so remove the munging.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The same can be achieved by using qemuSecurity[Set|Restore]ImageLabel.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Since the disk is necessary only to get the source modify the functions
to take the source directly and rename them to
qemu[Setup|Teardown]ImageChainCgroup.
Additionally drop a pointless comment containing the old function name.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
When we need to detect a chain for a image which will become the new
source for a disk (e.g. after a disk media change or a blockjob) we'd
need to replace disk->src temporarily to do so.
Move the 'disksrc' temporary variable to an argument and adjust callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This reverts commit 8b035c84d8.
The MTTCG impl in QEMU does allow pinning vCPUs.
When the guest is running we already check if pinning is
possible in the qemuDomainPinVcpuLive method, so this
check was adding no benefit.
When the guest is not running, we cannot know whether the
subsequent launch will use MTTCG or TCG, so we must allow
the pinning request. If the guest does use TCG on the next
launch it will fail, but this is no worse than if the user
had done a virDomainDefineXML with an XML doc specifying
vCPU pinning.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Add documentation that the 'VIR_DOMAIN_BLOCK_COPY_TRANSIENT_JOB' flag
is auto-assumed if the block copy job is started while the VM is
transient and remove the restriction to define the domain when copy
is running.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
disk->mirror would not be cleared while the local pointer was freed in
qemuDomainBlockCommit if qemuDomainObjExitMonitor or qemuBlockJobDiskNew
would return a failure.
Since block job handling is executed in the separate handler which needs
a qemu job, we don't need to pre-set the mirror state prior to starting
the job. Similarly the block copy job does not do that.
Move the setting of the data after starting the job so that we avoid
this problem.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
While this should not be necessary as we clear it in the event handler,
let's be sure and clear it prior to starting the job.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
We have this very handy macro called VIR_STEAL_PTR() which steals
one pointer into the other and sets the other to NULL. The
following coccinelle patch was used to create this commit:
@ rule1 @
identifier a, b;
@@
- b = a;
...
- a = NULL;
+ VIR_STEAL_PTR(b, a);
Some places were clean up afterwards to make syntax-check happy
(e.g. some curly braces were removed where the body become a one
liner).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
If we validate that memballoon is NONE|VIRTIO at parse time,
we can drop similar checks elsewhere in the qemu driver
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
Currently the job name corresponds to the disk the job belongs to. For
jobs which will not correspond to disks we'll need to track the name
separately.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Instead of passing in the disk information, pass in the job and name the
function accordingly.
Few callers needed to be modified to have the job pointer handy.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The job error can be safely accessed in the job structure, so we don't
need to propagate it through qemuBlockJobUpdateDisk.
Drop the propagation and refactor any caller that pased non-NULL error.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add a field tracking the current state of job so that it can be queried
later. Until now the job state e.g. that the job is _READY for
finalizing was tracked only for mirror jobs. Add tracking of state for
all jobs.
Similarly to 'qemuBlockJobType' this maps the existing states of the
blockjob from virConnectDomainEventBlockJobStatus to
'qemuBlockJobState' so that we can track some internal states as well.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Modify qemuBlockJobSyncBeginDisk to operate on qemuBlockt sJobDataPtr and
rename it accordingly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We can properly track the job type when starting the job so that we
don't have to infer it later.
This patch also adds an enum of block job types specific to qemu
(qemuBlockjobType) which mirrors the public block job types
(virDomainBlockJobType) but allows for other types to be added later
which will not be public.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Rather than directly modifying fields in the qemuBlockJobDataPtr
structure add a bunch of fields which allow to do the transitions.
This will help later when adding more complexity to the job handling.
APIs introduced in this patch are:
qemuBlockJobDiskNew - prepare for starting a new blockjob on a disk
qemuBlockJobDiskGetJob - get the block job data structure for a disk
For individual job state manipulation the following APIs are added:
qemuBlockJobStarted - Sets the job as started with qemu. Until that
the job can be cancelled without asking qemu.
qemuBlockJobStartupFinalize - finalize job startup. If the job was
started in qemu already, just releases
reference to the job object. Otherwise
clears everything as if the job was never
started.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The field is used to note the state the job has transitioned to while
handling the blockjob state change event. Rename the field so that it's
obvious that this is the new state and not the general state of the
blockjob.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Now that we reprobe the status of blockjobs when reconnecting in
addition to handling job status events, the status reprobing can be
removed as we always track the correct status internally.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Struct qemuDomainDiskPrivate was holding multiple variables connected to
a disk block job. Consolidate them into a new struct qemuBlockJobData.
This will also allow simpler extensions to the block job mechanisms.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
All the public APIs of the qemu_blockjob module operate on a 'disk'.
Since I'll be adding APIs which operate on a job later let's rename the
existing ones.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Replace use of qemuBlockJobEventProcess with the general helper. A small
tweak is required to pass in the 'type' and 'status' of the job via the
appropriate private data variables.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Introduce a set of bool variables with the 'present' suffix
to track whether the value was actually specified.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This event is emitted on the monitor when a GID table in pvrdma device
is modified and the change needs to be propagate to the backend RDMA
device's GID table.
The control over the RDMA device's GID table is done by updating the
device's Ethernet function addresses.
Usually the first GID entry is determine by the MAC address, the second
by the first IPv6 address and the third by the IPv4 address. Other
entries can be added by adding more IP addresses. The opposite is the
same, i.e. whenever an address is removed, the corresponding GID entry
is removed.
The process is done by the network and RDMA stacks. Whenever an address
is added the ib_core driver is notified and calls the device driver's
add_gid function which in turn update the device.
To support this in pvrdma device we need to hook into the create_bind
and destroy_bind HW commands triggered by pvrdma driver in guest.
Whenever a changed is made to the pvrdma device's GID table a special
QMP messages is sent to be processed by libvirt to update the address of
the backend Ethernet device.
Signed-off-by: Yuval Shaia <yuval.shaia@oracle.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Every time we call all domain stats for inactive domain with
unavailable storage source we get error message in logs [1]. It's a bit noisy.
While it's arguable whether we need such message or not for mandatory
disks we would like not to see messages for optional disks. Let's
filter at least for cases of local files. Fixing other cases would
require passing flag down the stack to .backendInit of storage
which is ugly.
Stats for active domain are fine because we either drop disks
with unavailable sources or clean source which is handled
by virStorageSourceIsEmpty in qemuDomainGetStatsOneBlockFallback.
We have these logs for successful stats since 25aa7035d (version 1.2.15)
which in turn fixes 596a13713 (version 1.2.12 )which added substantial
stats for offline disks.
[1] error message example:
qemuOpenFileAs:3324 : Failed to open file '/path/to/optional/disk': No such file or directory
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
The QEMU command line arguments are very long and currently all written
on a single line to /var/log/libvirt/qemu/$GUEST.log. This introduces
logic to add line breaks after every env variable and "-" optional
argument, and every positional argument. This will create a clearer log
file, which will in turn present better in bug reports when people cut +
paste from the log into a bug comment.
An example log file entry now looks like this:
2018-12-14 12:57:03.677+0000: starting up libvirt version: 5.0.0, qemu version: 3.0.0qemu-3.0.0-1.fc29, kernel: 4.19.5-300.fc29.x86_64, hostname: localhost.localdomain
LC_ALL=C \
PATH=/usr/local/bin:/usr/local/sbin:/usr/bin:/usr/sbin \
HOME=/home/berrange \
USER=berrange \
LOGNAME=berrange \
QEMU_AUDIO_DRV=none \
/usr/bin/qemu-system-ppc64 \
-name guest=guest,debug-threads=on \
-S \
-object secret,id=masterKey0,format=raw,file=/home/berrange/.config/libvirt/qemu/lib/domain-33-guest/master-key.aes \
-machine pseries-2.10,accel=tcg,usb=off,dump-guest-core=off \
-m 1024 \
-realtime mlock=off \
-smp 1,sockets=1,cores=1,threads=1 \
-uuid c8a74977-ab18-41d0-ae3b-4041c7fffbcd \
-display none \
-no-user-config \
-nodefaults \
-chardev socket,id=charmonitor,fd=23,server,nowait \
-mon chardev=charmonitor,id=monitor,mode=control \
-rtc base=utc \
-no-shutdown \
-boot strict=on \
-device qemu-xhci,id=usb,bus=pci.0,addr=0x1 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 \
-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
-msg timestamp=on
2018-12-14 12:57:03.730+0000: shutting down, reason=failed
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
In many files there are header comments that contain an Author:
statement, supposedly reflecting who originally wrote the code.
In a large collaborative project like libvirt, any non-trivial
file will have been modified by a large number of different
contributors. IOW, the Author: comments are quickly out of date,
omitting people who have made significant contribitions.
In some places Author: lines have been added despite the person
merely being responsible for creating the file by moving existing
code out of another file. IOW, the Author: lines give an incorrect
record of authorship.
With this all in mind, the comments are useless as a means to identify
who to talk to about code in a particular file. Contributors will always
be better off using 'git log' and 'git blame' if they need to find the
author of a particular bit of code.
This commit thus deletes all Author: comments from the source and adds
a rule to prevent them reappearing.
The Copyright headers are similarly misleading and inaccurate, however,
we cannot delete these as they have legal meaning, despite being largely
inaccurate. In addition only the copyright holder is permitted to change
their respective copyright statement.
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Disable external snapshot of a readonly disk for domains as
this operation is not very useful. Such a snapshot is not
possible for active domains but the error message from QEMU
is more cryptic:
error: internal error: unable to execute QEMU command 'transaction':
Could not create file: Permission denied
This error at least makes the error more understandable for
active domains and disallows for inactive domains as well.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Commit 212dc9286 made a generic qemuDomainGetIOThreadsMon which
would fail if the QEMU_CAPS_OBJECT_IOTHREAD didn't exist. Then
commit d1eac927 used that helper for the collection of all domain
stats. However, if the capability doesn't exist, then the entire
stats collection fails. Since the IOThread stats were meant to be
if available only, thus rather than failing if the capability
doesn't exist, let's just not collect the stats. Restore the caps
failure logic for qemuDomainGetIOThreadsLive.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
During qemuConnectGetAllDomainStats if qemuDomainGetStats causes
a failure, then when collecting more than one domain's worth of
statistics the loop in virDomainStatsRecordListFree would call
virDomainFree which would call virResetLastError effectively wiping
out the reason we failed leaving the caller with no idea why the
collection failed.
To fix this, let's Preserve the error and Restore it prior to return
so that a caller such as 'virsh domstats' doesn't get the generic
"error: An error occurred, but the cause is unknown".
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There are two functions called from syncNicRxFilterMultiMode:
virNetDevSetRcvAllMulti() and virNetDevSetRcvMulti(). Both of
them return 0 on success and -1 on error. However, currently
their return value is checked for != 0 which conflicts with our
assumptions on retvals: a positive value is still considered
success but with current check it would lead to failure.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Guest network devices can set 'overflow' when there are a number of multicast
ips configured. For virtio_net, the limit is only 64. In this case, the list
of mac addresses is empty and the 'overflow' condition is set. Thus, the guest
will currently receive no multicast traffic in this state.
When 'overflow' is set in the guest, let's turn this into ALLMULTI on the host.
Signed-off-by: Jason Baron <jbaron@akamai.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1545732
Implement the QEMU driver mechanism in order to set the polling
parameters for an IOThread within the bounds specified by the
QEMU qapi parameter passing.
Based heavily on patches originally posted by Pavel Hrdina
<phrdina@redhat.com>, but modified to only handle alterations
for a running guest. For the most part the API names changed,
the typed parameters removed the poll enabled value, and the
capabilities check was moved to just before the live attempt
to set. Since changes are only supported for a running guest,
no guest XML alterations were kept.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Rather than passing an iothread_id, let's pass a qemuMonitorIOThreadInfo
structure so that a subsequent change to modify the iothread info can
just generate and pass one.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
We're about to add a new state "modify" and thus the function
goes from just Add/Del. Use an enum to manage.
Extracted from code originally posted by Pavel Hrdina
<phrdina@redhat.com>, but placed into a separate patch.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Process the IOThreads polling stats if available. Generate the
output params record to be returned to the caller with the three
values - poll-max-ns, poll-grow, and poll-shrink.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Separate out the fetch of the IOThread monitor call into a separate
helper so that a subsequent domain statistics change can fetch the raw
IOThread data and parse it as it sees fit.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Refactoring qemuDomainGetStatsCpu, make it possible to add
more CPU statistics.
Signed-off-by: Wang Huaqiang <huaqiang.wang@intel.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The gotShutdown bool has been redundant since we started setting
VIR_DOMAIN_SHUTDOWN state after receiving SHUTDOWN event from QEMU.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
If qemuDomainSnapshotDiscard() fails for any reason (rare,
but possible with an ill-timed ENOMEM or if
qemuDomainSnapshotForEachQcow2() has problems talking to the
qemu guest monitor), then an attempt to retry the snapshot
deletion API will crash because we didn't undo the effects
of virDomainSnapshotDropParent() temporarily rearranging the
internal list structures, and the second attempt to drop
parents will dereference NULL. Fix it by instead noting that
there are only two callers to qemuDomainSnapshotDiscard(),
and only one of the two callers wants the parent to be updated;
thus we can move the call to virDomainSnapshotDropParent()
into a code path that only gets executed on success.
Signed-off-by: Eric Blake <eblake@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Disk hotplug has slightly different semantics from media changing. Move
the media change code out and add proper initialization of the new
source object and proper cleanups if something fails.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
While the idea was good the implementation not so much as we need to
take into account the old disk data and the new source. The code will be
consolidated later in a different way.
This reverts commit 663b1d55de.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Let's ignore the checking of interface type when we call the function
qemuARPGetInterfaces to get IP from host's arp table.
Signed-off-by: Lin Ma <lma@suse.com>
Reviewed-by: Chen Hanxiao <chenhanxiao@gmail.com>
The only place where VIR_DOMAIN_EVENT_RESUMED should be generated is the
RESUME event handler to make sure we don't generate duplicate events or
state changes. In the worse case the duplicity can revert or cover
changes done by other event handlers.
For example, after QEMU sent RESUME, BLOCK_IO_ERROR, and STOP events
we could happily mark the domain as running and report
VIR_DOMAIN_EVENT_RESUMED to registered clients.
https://bugzilla.redhat.com/show_bug.cgi?id=1612943
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
VIR_DOMAIN_EVENT_RESUMED_FROM_SNAPSHOT was defined but not used anywhere
in our event generation code. This fixes qemuDomainRevertToSnapshot to
properly report why the domain was resumed.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
All of the ones being removed are pulled in by internal.h. The only
exception is sanlock which expects the application to include <stdint.h>
before sanlock's headers, because sanlock prototypes use fixed width
int, but they don't include stdint.h themselves, so we have to leave
that one in place.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Acked-by: Michal Privoznik <mprivozn@redhat.com>
It doesn't really make sense for us to have stdlib.h and string.h but
not stdio.h in the internal.h header.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Acked-by: Michal Privoznik <mprivozn@redhat.com>
When restoring a domain from a compressed image, we launch an
intermediate process for decompressing the saved data. If QEMU fails to
load the data for some reason, we force close the stdin/stdout file
descriptors of the intermediate process and wait for it to die. However,
virCommandWait can report various errors which would overwrite the real
error from QEMU. Thus instead of getting something useful:
internal error: process exited while connecting to monitor:
2018-09-17T15:17:29.998910Z qemu-system-x86_64: can't apply global
Skylake-Client-x86_64-cpu.osxsave=off: Property '.osxsave' not found
we could get an irrelevant error message:
internal error: Child process (lzop -dc --ignore-warn) unexpected
fatal signal 13
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Now that we know what metadata lock manager user wishes to use we
can load it when initializing security driver. This is achieved
by adding new argument to virSecurityManagerNewDriver() and
subsequently to all functions that end up calling it.
The cfg.mk change is needed in order to allow lock_manager.h
inclusion in security driver without 'syntax-check' complaining.
This is safe thing to do as locking APIs will always exist (it's
only backend implementation that changes). However, instead of
allowing the include for all other drivers (like cpu, network,
and so on) allow it only for security driver. This will still
trigger the error if including from other drivers.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Even though the current use of the functions does not require full
implementation with transactions (none of the callers passes a path
somewhere under /dev), it doesn't hurt either. Moreover, in
future patches the paradigm is going to shift so that any API
that touches a file is required to use transactions.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This commit adds support to use the function qemuAgentGetHostname()
to obtain the domain hostname using QEMU-GA command.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The size/capacity stats gathered in qemuDomainBlocksStatsGather when
using -blockdev would be overwritten by assigning/copying the transfered
data statistics at the end. Fix it by moving the assignment prior to
fetching the capacity data.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Historically the argv -> xml convertor wanted the same default machine
as we'd set when parsing xml. The latter has now changed, however, to
use a default defined by libvirt. The former needs fixing to again
honour the default QEMU machine.
This exposed a bug in handling for the aarch64 target, as QEMU does not
define any default machine. Thus we should not having been accepting
argv without a -machine provided.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Add code paths which call into the new functions to gather the data on a
per-node-name basis and tweak the aliases used for extracting the data.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use the 'qdev' instead of the disk alias to lookup the stats and
transfer the capacity from the appropriate node name so that the
function works with -blockdev.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Switch to using the QOM/qdev handles in all calls to
qemuMonitorGetBlockInfo when using -blockdev. The callers also need to
make sure to use the correct handle afterwards to extract the data.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Data relevant for the storage of a backing chain member will need to be
reported separately when switching to blockdev. Prepare a function that
extracts the appropriate data.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When reporting stats for the backing chain some of them make sense only
for the topmost entry as they are actually tied to the frontend device.
We unfortunately can't change that fact, but we can stop reporting all
zero stats for the backing chain members where they don't make any
sense.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
While we report the read and written byte stats for every single layer
of the backing chain, qemu in fact reports them only for the frontend.
Split out the relevant stats into a separate function so that we can
later fix this bug and stop reporting it for backing chain entries where
they don't make sense.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Split out the header so that the loop can be refactored later.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Split out the code which converts the stats gathered in
qemuDomainGetStatsBlock into typed parameters so that it will look
less ugly when extending it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In cases when -blockdev is used we need to use 'query-named-block-nodes'
instead of 'query-block'. This means that we can extract the
write-threshold variable right away.
To keep compatibility with old VMs modify the code which was extracting
the value previously so that it updates the stats structure and a single
code path then can be used to extract the data.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Allow reuse of qemuDomainGetStatsOneBlock to work with nodenames by
removing the code that looks up the stats data to the caller.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Rather than totalling every entry from 'query-block' for stats provided
by qemuDomainBlocksStatsGather total only stats for known disks. This
will allow to return data for nodenames and qdevs in the same hash so
that we can use them with -blockdev.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The string is not modified so it does not make sense to have a copy.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Allow looking up also via QOM id and rename the function accordingly.
Also add documentation of the specifics.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use the nodename to resize the device rather than the drive alias.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
With -blockdev the drive alias can't be used any more so we need to
switch to the QOM name.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use the index stored in virStorageSource struct rather than
recalculating it. Currently we'd report proper numbers but that will
change with blockdev.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'device' field reported by 'query-block' is empty when -blockdev is
used. Add an argument which will allow matching disk by using the qdev
id so we can use this code with -blockdev.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'device' argument matches only the legacy drive alias. For blockdev
we need to set the throttling for a QOM id and thus we'll need to use
the 'id' field.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use qemuDomainAttachDeviceDiskLive to change the media in
qemuDomainChangeDiskLive as the former function already does all the
necessary steps to prepare the new medium.
This also allows us to turn qemuDomainChangeEjectableMedia static.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Remove the pointless "empty path" check and use a better error message
if the disk was not found.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This reverts commit 9cf38263d0.
Jansson cannot parse QEMU's quirky JSON.
Revert back to yajl.
https://bugzilla.redhat.com/show_bug.cgi?id=1614569
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This reverts commit 0f80c71822.
Turns out, our code relies on virCgroupFree(&var) setting
var = NULL.
Conflicts:
src/util/vircgroup.c: context because 94f1855f09 is not
reverted.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Modify virCgroupFree function signature to take a value of type
virCgroupPtr instead of virCgroupPtr * as the parameter.
Change the argument type in all calls to virCgroupFree function
from virCgroupPtr * to virCgroupPtr. This is a step towards
having consistent function signatures for Free helpers so that
they can be used with VIR_AUTOPTR cleanup macro.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1559867
When attaching a device to the domain we need to be sure
to use the correct domain definition (vm->def or vm->newDef)
when calling virDomainDeviceDefParse because the post parse
processing algorithms that may assign an address for the
device will use whatever domain definition was passed in.
Additionally, some devices (SCSI hostdev and SCSI disk) use
algorithms that rely on knowing what already exists of the
other type when generating the new device's address. Using
the wrong VM definition could result in duplicated addresses.
In the case of the bz, two hostdev's with no domain address
provided were added to the running domain's config only.
However, the parsing algorithm used the live domain in
order to figure out the host device address resulting in
the same address being used and a subsequent start failing
due to duplicate address.
Fix this by separating the checks/code into CONFIG and LIVE
processing using the correct definition for each block and
performing cleanup for both options as necessary.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1589115
When doing a memory snapshot qemuOpenFile() is used. This means
that the file where memory is saved is firstly attempted to be
created under root:root (because that's what libvirtd is running
under) and if this fails the second attempt is done under
domain's uid:gid. This does not make much sense - qemu is given
opened FD so it does not need to access the file. Moreover, if
dynamicOwnership is set in qemu.conf and the file lives on a
squashed NFS this is deadly combination and very likely to fail.
The fix consists of using:
qemuOpenFileAs(fallback_uid = cfg->user,
fallback_gid = cfg->group,
dynamicOwnership = false)
In other words, dynamicOwnership is turned off for memory
snapshot (chown() will still be attempted if the file does not
live on NFS) and instead of using domain DAC label, configured
user:group is set as fallback.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Now that we have a saner replacement for checking if the disk source is
the same use it instead of formatting qemu command-line chunks.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In some cases backing chain needs to be cleared prior to re-detection.
Move this step out of qemuDomainDetermineDiskChain as only certain
places need it and the function itself is able to skip to the end of the
chain to perform detection.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Allow updating capacity for the block devices returned by
qemuDomainBlocksStatsGather and replace the open-coded call to
qemuMonitorGetAllBlockStatsInfo by the helper.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
With 'transaction' support we don't need to keep around the multipurpose
code which would create the snapshot if 'transaction' is not supported.
To simplify this add a new helper that just wraps the arguments for
'blockdev-snapshot-sync' operation in 'transaction' and use it instead
of qemuBlockSnapshotAddLegacy.
Additionally this allows to format the arguments prior to creating the
file for simpler cleanup.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Currently we'd audit that we managed to format the data for the
'transaction' command rather than the (un)successful attempt to create
the snapshot.
Move the auditing code so that it can actually audit the result of the
'transaction' command.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In the cleanup path we already checked whether a snapshot needed to be
taken by looking into the collected data. Use the same approach when
creating the snapshot command data and when committing the changes to the
domain definition.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Since we now always do the snapshot via the 'transaction' command we can
drop the code which would enter monitor for individual disk snapshots.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
While qemu supports the 'transaction' command since v1.1.0
(52e7c241ac766406f05fa) and the 'blockdev-snapshot-sync' command since
v0.14.0-rc0 we need to keep the capability bits present since some qemu
downstreams (RHEL/CentOS 7 for example) chose to cripple qemu by
arbitrarily compiling out some stuff which was already present at that
time.
To simplify the crazy code just require both commands to be present at
the beginning of an external snapshot so that we can remove the case when
'transaction' would not be supported.
This also allows to drop any logic connected to the
VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC flag since snapshots are atomic with
the 'transaction' command.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
If a domain is configured to start on boot, it has a symlink to the
domain definition inside the autostart directory. If you rename this
domain, the definition is renamed too. The symlink need to be pointed to
this renamed file. This commit recreates the symlink after renaming the
XML file.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1594985
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Since 2.10 QEMU supports a new display type egl-headless which uses the
drm nodes for OpenGL rendering copying back the rendered bits back to
QEMU into a dma-buf which can be accessed by standard "display" apps
like VNC or SPICE. Although this display type can be used on its own,
for any practical use case it makes sense to pair it with either VNC or
SPICE display. The clear benefit of this display is that VNC gains
OpenGL support, which it natively doesn't have, and SPICE gains remote
OpenGL support (native OpenGL support only works locally through a UNIX
socket, i.e. listen type=socket/none).
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Yajl has not seen much activity upstream recently.
Switch to using Jansson >= 2.5.
All the platforms we target on https://libvirt.org/platforms.html
have a version >= 2.7 listed on the sites below:
https://repology.org/metapackage/jansson/versionshttps://build.opensuse.org/package/show/devel:libraries:c_c++/libjansson
Additionally, Ubuntu 14.04 on Travis-CI has 2.5. Set the requirement
to 2.5 since we don't use anything from newer versions.
Implement virJSONValue{From,To}String using Jansson, delete the yajl
code (and the related virJSONParser structure) and report an error
if someone explicitly specifies --with-yajl.
Also adjust the test data to account for Jansson's different whitespace
usage for empty arrays and tune up the specfile to keep 'make rpm'
working when bisecting.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
QEMU commit bf1e7140e adds reporting of new balloon statistic to QEMU
2.12. Value represents the amount of memory that can be quickly
reclaimed without additional I/O. Let's add that too.
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
When VIR_DOMAIN_SCHEDULER_GLOBAL_PERIOD is matched "cputune.global_period"
should be updated and not "cputune.period".
Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1600427
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
This event is emitted on the monitor if one of pr-managers lost
connection to its pr-helper process. What libvirt needs to do is
restart the pr-helper process iff it corresponds to managed
pr-manager.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This argument is not used anymore. The only function that is
passing non-NULL (qemuDomainSaveMemory) does not actually care
for the value (after 23087cfdb) and every other caller just
passes NULL anyway.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1585108
When updating a live device users might pass different alias than
the one the device has. Currently, this is silently ignored which
goes against our behaviour for other parts of the device where we
explicitly allow only certain changes and error out loudly on
anything else.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This was lost in c57f3fd2f8. But now we are going to
need it again (except the DETACH action where checking for device
compatibility does not make much sense anyway).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
When updating device it's worth parsing live info too as users
might want to update it as well.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Now that the nwfilter driver keeps a list of bindings that it has
created, there is no need for the complex virt driver callbacks. It is
possible to simply iterate of the list of recorded filter bindings.
This means that rebuilding filters no longer has to acquire any locks on
the virDomainObj objects, as they're never touched.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1591628
Attempting to use the FORCE flag for snapshot-revert was resulting
in failures because qemuProcessStart and qemuProcessStartCPUs were
using QEMU_ASYNC_JOB_START after a qemuProcessStop resulting in an
error when entering the monitor:
error: internal error: unexpected async job 6 type expected 0
So create a local @jobType, initialize to QEMU_ASYNC_JOB_START, and
change to QEMU_ASYNC_JOB_NONE if we end up in the --force path
where the qemuProcessStop is run before a Start and StartCPUs.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
If the the snapshot revert involves a forced revert option, then
let's not cause startup to change the genid flag in order to signify
that we're still running the same/previous guest and not some
snapshot reversion.
Related to:
https://bugzilla.redhat.com/show_bug.cgi?id=1149445
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Use and set the @start_flags at the top of the RUNNING and PAUSED
transitions to GEN_VMID | PAUSED.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Promote the @start_flags to the top of the function, a
subsequent patch needs to use it.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
There are two sets of functions here:
1) some functions talk on both monitor and agent monitor,
2) some functions only talk on agent monitor.
For functions from set 1) we need to use
qemuDomainObjBeginJobWithAgent() and for functions from set 2) we
need to use qemuDomainObjBeginAgentJob() only.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Now that we have agent job we can grab it while freezing/thawing
guest file system before/after doing snapshot.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
If measurement retrieval fails we'd forget to call ExitMonitor to unlock
the monitor.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Reported-by: Luyao Huang <lhuang@redhat.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Since it's being called with QEMU_ASYNC_JOB_NONE which is what
qemuDomainObjEnterMonitor is going to use with the internal helper,
let's use that one instead.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.ibm.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1552092
If there's a long running job it might cause us to wait 30
seconds before we give up acquiring the job. This is problematic
to interactive applications that fetch stats repeatedly every few
seconds.
The solution is to introduce
VIR_CONNECT_GET_ALL_DOMAINS_STATS_NOWAIT flag which tries to
acquire job but does not wait if acquiring failed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1591561
For reasons I don't understand my original patch of 75f0fd5112
freed not only the chardev from domain but also the one from
passed virDomainDeviceDefPtr. This caused no troubles until now,
because those two pointers were separate, but after I've
introduced virDomainDetachDeviceAlias() they became the same
resulting in double free on detach.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Free tmp even on failure.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
And replace all calls with virObjectEventStateQueue such that:
qemuDomainEventQueue(driver, event);
becomes:
virObjectEventStateQueue(driver->domainEventState, event);
And remove NULL checking from all callers.
Signed-off-by: Anya Harter <aharter@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
This patch implements the internal driver API for launch event into
qemu driver. When SEV is enabled, execute 'query-sev-launch-measurement'
to get the measurement of memory encrypted through launch sequence.
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Fix the error message to indicate what exactly is failing - that
the controller index provided matches an existing controller.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
On start up of libvirtd the worker pool of the QEMU driver must be
initialized before trying to reconnect to all the running QEMU
instances. Otherwise segmentation faults can occur if there are QEMU
monitor events emitted.
#0 __GI___pthread_mutex_lock
#1 0x000003fffdba9e62 in virMutexLock
#2 0x000003fffdbab2dc in virThreadPoolSendJob
#3 0x000003ffd8343b70 in qemuProcessHandleSerialChanged
#4 0x000003ffd836a776 in qemuMonitorEmitSerialChange
#5 0x000003ffd8378e52 in qemuMonitorJSONHandleSerialChange
#6 0x000003ffd8378930 in qemuMonitorJSONIOProcessEvent
#7 0x000003ffd837edee in qemuMonitorJSONIOProcessLine
#8 0x000003ffd837ef86 in qemuMonitorJSONIOProcess
#9 0x000003ffd836757a in qemuMonitorIOProcess
#10 0x000003ffd836863e in qemuMonitorIO
#11 0x000003fffdb4033a in virEventPollDispatchHandles
#12 0x000003fffdb4055e in virEventPollRunOnce
#13 0x000003fffdb3e782 in virEventRunDefaultImpl
#14 0x000003fffdc89400 in virNetDaemonRun
#15 0x000000010002a816 in main
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Previous patch naively removed all code relevant to disk format
checking. The semantics now dictate that the format check when creating
external snapshots is now impossible as we always fill in the format for
disks in domain definition in the post-parse callback.
Remove the impossible code path.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The option is insecure and it has been long enough for users to migrate
their disk files to use explicit format. Drop the option and related
code.
The config parser still parses it and rejects statup if it's still
present in the config in enabled state.
The augeas lens is also kept so that users can disable it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Replace instances where we previously called virGetLastError just to
either get the code or to check if an error exists with
virGetLastErrorCode to avoid a validity pre-check.
Signed-off-by: Ramy Elkest <ramyelkest@gmail.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
The code that detaches the device from persistent definition copies the
persistent definition first so that it can easily be rolled back. The
actual detaching is then made in the copy which is assigned back on
success (if the live operation succeeded as well).
This is not the case in qemuDomainDetachDeviceAliasLiveAndConfig where
the definition was copied and put back, but the detaching happened from
the other object which was overwritten.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
The function adds the object of a certain type. Change the name so that
we make room for the generic function.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add a new 'vsock' element for the vsock device.
The 'model' attribute is optional.
A <source cid> subelement should be used to specify the guest cid,
or <source auto='yes'/> should be used.
https://bugzilla.redhat.com/show_bug.cgi?id=1291851
Signed-off-by: Ján Tomko <jtomko@redhat.com>
When computing a baseline CPU for a specific hypervisor we have to make
sure to include only CPU features supported by the hypervisor. Otherwise
the computed CPU could not be used for starting a new domain.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Collin Walling <walling@linux.ibm.com>
This is required for virCPUBaseline to accept a list of guest CPU
definitions since they do not have arch set.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Collin Walling <walling@linux.ibm.com>
virConnectGetDomainCapabilities needs to lookup QEMU capabilities
matching a specified binary, architecture, virt type, and machine type
while using default values when any of the parameters are not provided
by the user. Let's extract the lookup code into
virQEMUCapsCacheLookupDefault to make it reusable.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The virDomainDetachDeviceAlias API is designed so that it only
sends detach request to qemu. It's user's responsibility to wait
for DEVICE_DELETED event, not libvirt's. Add @async flag to
qemuDomainDetach*Device() functions so that caller can chose if
detach is semi-synchronous (old virDomainDetachDeviceFlags()) or
fully asynchronous (new virDomainDetachDeviceFlags()).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The fact that we are overwriting @ret multiple times makes it
difficult to see what is actually happening here. Follow our
traditional pattern where @ret is initialized to -1, and set to 0
only in case we know we succeeded.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Funny, we obtain driver caps at the beginning of the function,
but then for unknown reason access driver->caps directly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Before we generate the command line for qemu, if the domain about to
be launched desires to utilize the VM Generation ID functionality, then
handle both the regenerating the GUID value for backup recovery (restore
operation) and the startup after snapshot as both require a new GUID to
be generated to allow the guest operating system to recognize the VM
is re-executing something that has already executed before.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
According to virDomainScreenshot() documentation, screens are
numbered sequentially. e.g. having two graphics cards, both with
four heads, screen ID 5 addresses the second head on the second
card.
But apart from that, there's nothing special happening here.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Commit [1] dropped support for synchronous block job cancel.
This patch erases remnants from comments.
[1] commit 2350d101 "qemu: Remove support for legacy block jobs"
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The pointer to the qemu driver is already included in domain object's
private data, so does not need to be passed as yet another parameter
when the domain object is already passed.
Also removes parameter 'driver' from functions which had it just because of
qemuBlockJobUpdate.
Signed-off-by: Roland Schulz <schullzroll@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Everything besides the top of the chain is readonly. Track this when
parsing the XML and detecting the chain from the disk. Also fix the
state when taking snapshots.
All other cases where the top image is changed already preserve the
readonly state from the original image.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
When adding a new object to the domain object list, there should
have been 2 virObjectRef calls made one for each list into which
the object was placed to match the 2 virObjectUnref calls that
would occur during Remove as part of virHashRemoveEntry when
virObjectFreeHashData is called when the element is removed from
the hash table as set up in virDomainObjListNew.
Some drivers (libxl, lxc, qemu, and vz) handled this inconsistency
by calling virObjectRef upon successful return from virDomainObjListAdd
in order to use virDomainObjEndAPI when done with the returned @vm.
While others (bhyve, openvz, test, and vmware) handled this via only
calling virObjectUnlock upon successful return from virDomainObjListAdd.
This patch will "unify" the approach to use virDomainObjEndAPI
for any @vm successfully returned from virDomainObjListAdd.
Because list removal is so tightly coupled with list addition,
this patch fixes the list removal algorithm to return the object
as entered - "locked and reffed". This way, the callers can then
decide how to uniformly handle add/remove success and failure.
This removes the onus on the caller to "specially handle" the
@vm during removal processing.
The Add/Remove logic allows for some logic simplification such
as in libxl where we can Remove the @vm directly rather than
needing to set a @remove_dom boolean and removing after the
libxlDomainObjEndJob completes as the @vm is locked/reffed.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
The 'domrename' command needs to check if the new domain name contains
the slash character. This character is not accepted by libvirt XML
definition because it is an invalid char (see Cole's commit b1fc6a7b7).
This commit enhace the 'domrename' command adding this check.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1333232
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The virStorageFileSupportsSecurityDriver and
virStorageFileSupportsAccess currently just return a boolean
value. This is ok because they don't have any failure scenarios
but a subsequent patch is going to introduce potential failure
scenario. This changes their return type from a boolean to an
int with values -1, 0, 1.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The virStorageFileGetBackingStoreStr method has overloaded the NULL
return value to indicate both no backing available and a fatal
error dealing with it.
The caller is thus not able to correctly propagate the error
messages.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Rework the code such that virDomainObjListFindByID will always
return a locked/ref counted object so that the callers can
always do the same cleanup logic to call virDomainObjEndAPI.
Makes accessing the objects much more consistent.
NB:
There were 2 callers (lxcDomainLookupByID and qemuDomainLookupByID)
that were already using the ByID name, but not virDomainObjEndAPI -
these were changed as well in this update/patch.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Now that every caller is using virDomainObjListFindByUUIDRef,
let's just remove it and keep the name as virDomainObjListFindByUUID.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
When qemu does not support changing of the backing store string, we'd
reaport that block pull is not supported instead of block commit.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
We store the flags passed to the API which started the migration. Let's
use them instead of a separate bool to check if post-copy migration was
requested.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We store the flags passed to the API which started QEMU_ASYNC_JOB_DUMP
and we can use them to check whether a memory-only dump is running.
There's no need for a specific bool flag.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
migrate_cancel QMP command cannot be used for cancelling memory-only
dumps and priv->job.dump_memory_only is used for reporting an error if
someone calls virDomainAbortJob when memory-only dump job is running.
Since commit 150930e309 the dump_memory_only flag is set only if
dump-guest-memory command was called without the detach parameter. This
would incorrectly allow libvirt to send migrate_cancel while the
detached memory-only dump is running.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When an async job is running, we sometimes need to know how it was
started to distinguish between several types of the job, e.g., post-copy
vs. normal migration. So far we added a specific bool item to
qemuDomainJobObj for such cases, which doesn't scale very well and
storing such bools in status XML would be painful so we didn't do it.
A better approach is to store the flags passed to the API which started
the async job, which can be easily stored in status XML.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The API is renamed as qemuMigrationParamsGetULL and it can be used with
any migration parameter stored as unsigned long long.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Since the monitor code no longer needs to see this enum, we move it
to the place where migration parameters are defined and drop the
"monitor" reference from the name.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use this internal structure only in qemu_migration_params.c and change
other non-test users to use the high level qemuMigrationParams struct.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
By merging qemuMigrationAnyCompressionParse into
qemuMigrationParamsSetCompression we can drop the useless intermediate
qemuMigrationCompression structure and parse compression related typed
parameters and flags directly into qemuMigrationParams.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Since every parameter or capability set in qemuMigrationCompression
structure is now reflected in qemuMigrationParams structure, we can
replace qemuMigrationAnyCompressionDump with a new API which will work
on qemuMigrationParams.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There's no need to call this API explicitly in the migration code. We
can pass the compression parameters to qemuMigrationParamsFromFlags and
it can internally call qemuMigrationParamsSetCompression to apply them
to the qemuMigrationParams structure.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Propagate the calls up the stack to the point where
qemuMigrationParamsFromFlags is called. The end goal achieved in the
following few patches is to merge compression parameters into the
general migration parameters code.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Every migration entry point in qemu_driver is supposed to call
qemuMigrationParamsFromFlags to transform flags and parameters into
qemuMigrationParams structure and pass the result to qemuMigration*
APIs.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Some migration parameters and capabilities are supposed to be set on
both sides of migration while others should only be set on one side. For
example, CPU throttling parameters make no sense on the destination and
they can be used even if the destination is too old to support them.
To make qemuMigrationParamsFromFlags more general and usable on both
sides of migration, we need to tell it what side it's been called on.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Currently migration parameters are stored in a structure which mimics
the QEMU migration parameters handled by query-migrate-parameters and
migrate-set-parameters. The new structure will become a libvirt's
abstraction on top of QEMU migration parameters, capabilities, and
related stuff.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It will get a bit more complicated soon and storing it on a stack with
{0} initializer will no longer work. We need a proper constructor.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Our *Free functions usually do not take a double pointer and the caller
has to make sure it doesn't use the stale pointer after the *Free
function returns.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In the end, this will allow us to have most of the logic around
migration parameters and capabilities done in one place.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function is now called qemuMigrationParamsFromFlags to better
reflect what it is doing: taking migration flags and params and
producing a struct with QEMU migration parameters.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Avoid the need for the drivers to explicitly check for a NULL path by
making sure it is at least the empty string.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Ensuring that we don't call the virDrvConnectOpen method with a NULL URI
means that the drivers can drop various checks for NULL URIs. These were
not needed anymore since the probe functionality was split
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Declare what URI schemes a driver supports in its virConnectDriver
struct. This allows us to skip trying to open the driver entirely
if the URI scheme doesn't match.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Add a localOnly flag to the virConnectDriver struct which allows a
driver to indicate whether it is local-only, or permits remote
connections. Stateful drivers running inside libvirtd are generally
local only. This allows us to remote the check for uri->server != NULL
from most drivers.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Currently the virDrvConnectOpen method is supposed to handle both
opening an explicit URI and auto-probing a driver if no URI is
given. Introduce a dedicated virDrvConnectURIProbe method to enable the
probing functionality to be split from the driver opening functionality.
It is still possible for NULL to be passed to the virDrvConnectOpen
method after this change, because the remote driver needs special
handling to enable probing of the URI against a remote libvirtd daemon.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We require QEMU >= 1.5.0, assume every QEMU supports it.
Sadly that does not let us trivially drop qemuMonitor's
priv->monJSON bool, because of qemuDomainQemuAttach.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Make the function more usable by returning the full disk definition and
fix the only caller for the new semantics. The new name for the function
is virDomainDiskDefParse.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Rather than trying to prevent stealing of the 'actions' virJSONValue
into the monitor command replace the code so that it does the same
thing, since 'actions' was actually not really used after calling the
monitor.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Add typedef for the anonymous enum used for the driver features. This
allows the usage of the type in a switch statement and taking
advantage of the compilers feature to detect uncovered cases.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
introduce VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_ARP to get ip address
of VM from the message of netlink RTM_GETNEIGH
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This time around it's not enough to just pick the latest commit,
because with aed87bb2aa6ed83b49574eb982e3bdd4c36acf17 keycodemapdb
renamed the 'rfb' keycode to 'qnum' and we need to accept the new
name while maintaining backwards compatibility.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Fix this common typo and assign a value rather than implicitly
type-casted comparison result. Introduced by commit b6a264e855.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
This partially reverts 82592551cb.
When migrating a domain, qemuMigrationDstPrepareAny() is called
which eventually calls qemuProcessLaunch(conn = NULL, flags =
VIR_QEMU_PROCESS_START_AUTODESTROY); But the very first thing
that qemuProcessLaunch does is check if AUTODESTROY flag is set
and @conn is not NULL. Well, it is.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
When calling virDomainDefCompatibleDevice to check a new device during
device update, we need to pass the original device which is going to be
updated in addition to the new device. Otherwise, the function can
report false conflicts.
The new argument is currently ignored by virDomainDefCompatibleDevice,
but this will change in the following patch.
https://bugzilla.redhat.com/show_bug.cgi?id=1546971
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
This flag is only used for tests. Let's instead overload bind syscall
in mocks where it is not done yet.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Host tcp4/tcp6 ports is a global resource thus we need to make
port accounting also global or we have issues described in [1] when
port allocator ranges of different instances are overlapped (which
is by default for qemu for example).
Let's have only one global port allocator object that take care
of the entire ports range (0 - 65535) and introduce port range object
for clients to specify desired auto allocation band.
[1] https://www.redhat.com/archives/libvir-list/2017-December/msg00600.html
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Ensure all enum cases are listed in switch statements, or cast away
enum type in places where we don't wish to cover all cases.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
It is very difficult while reading the migration code trying to
understand whether a particular function is being called on the src side
or the dst side, or either. Putting "Src" or "Dst" in the method names will
make this much more obvious. "Any" is used in a few helpers which can be
called from both sides.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
QEMU code does not work well with too big numbers on the JSON monitor so
our monitor code supports sending only numbers up to LLONG_MAX. Avoid a
weird error message by limiting the size of the 'bandwidth' parameter
for block copy.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1532542
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
The qemuMigrationPrecreateStorage method needs a connection
to access the storage driver. Instead of passing it around,
open it at time of use.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
There's a few places in startup code paths which pass around a
virConnectPtr which is no longer required. Specifically, the
qemuProcessStart() method now only requires a non-NULL connection if
autodestroy is requested.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When setting up graphics, we sometimes need to resolve networks,
requiring the caller to pass in a virConnectPtr, except sometimes they
pass in NULL. Use virGetConnectNetwork() to acquire the connection to
the network driver when it is needed.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
During domain startup there are many places where we need to acquire
secrets. Currently code passes around a virConnectPtr, except in the
places where we pass in NULL. So there are a few codepaths where ability
to start guests using secrets will fail. Change to acquire a handle to
the secret driver when needed.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Rather than expecting callers to pass a virConnectPtr into the
virDomainDiskTranslateSourcePool() method, just acquire a connection
to the storage driver when needed.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
There is a long standing hack to pass a virConnectPtr into the
qemuMonitorStartCPUs method, so that when the text monitor prompts
for a disk password, we can lookup virSecretPtr objects. This causes
us to have to pass a virConnectPtr around through countless methods
up the call chain....except some places don't have any virConnectPtr
available so have always just passed NULL. We can finally fix this
disastrous design by using virGetConnectSecret() to open a connection
to the secret driver at time of use.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We forgot to free alloced mem when failed to
dup ifname or macaddr.
Also use VIR_STEAL_PTR to simplify codes.
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The virStorageTranslateDiskSourcePool method modifies a virDomainDiskDef
to resolve any storage pool reference. For some reason this was added
into the storage driver code, despite working entirely in terms of the
public APIs. Move it into the domain conf file and rename it to match the
object it modifies.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Currently the QEMU driver will call directly into the network driver
impl to modify network device bandwidth for interfaces with
type=network. This introduces a callback system to allow us to decouple
the QEMU driver from the network driver.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The QEMU driver loadable module needs to be able to resolve all ELF
symbols it references against libvirt.so. Some of its symbols can only
be resolved against the storage_driver.so loadable module which creates
a hard dependancy between them. By moving the storage file backend
framework into the util directory, this gets included directly in the
libvirt.so library. The actual backend implementations are still done as
loadable modules, so this doesn't re-add deps on gluster libraries.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Since it may be possible that the state is unknown in some cases we
should store it as a tristate so that other code using it can determine
whether the state was updated.
Don't extract the halted state into a separate array, but rater access
the vcpu structures directly. We still need to call the vcpu helper to
retrieve the performance statistics though.
https://bugzilla.redhat.com/show_bug.cgi?id=916061
If the QEMU version running is new enough (based on the DUMP_COMPLETED
event), then we can add a 'detach' boolean to the dump-guest-memory
command in order to tell QEMU to run in a thread. This ensures that we
don't lock out other commands while the potentially long running dump
memory is completed.
This allows the usage of a qemuDumpWaitForCompletion which will wait
for the event while the qemuDomainGetJobInfoDumpStats can be used via
qemuDomainGetJobInfo in order to query QEMU to determine how far along
the job is.
Now that we have a true async job, we'll only set the dump_memory_only
flag only when @detach=false; otherwise, we note that the job is a
for stats dump this allows the opposite end for job info to determine
what to copy.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Add an API to allow fetching the memory only dump statistics
for a job via the qemuDomainGetJobInfo API.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Define the qemuMonitorDumpStats as a new job JobStatsType to handle
being able to get memory dump statistics. For now do nothing with
the new TYPE_MEMDUMP.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Add a TYPE_SAVEDUMP so that when coalescing stats for a save or
dump we don't needlessly try to get the mirror stats for a migration.
Other conditions can still use MIGRATION and SAVEDUMP interchangably
including usage of the @migStats field to fetch/store the data.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Convert the stats field in _qemuDomainJobInfo to be a union. This
will allow for the collection of various different types of stats
in the same field.
When starting the async job that will end up being used for stats,
set the @statsType value appropriately. The @mirrorStats are
special and are used with stats.mig in order to generate the
returned job stats for a migration.
Using the NONE should avoid the possibility that some random
async job would try to return stats for migration even though
a migration is not in progress.
For now a migration and a save job will use the same statsType
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Add and use qemuProcessEventFree for freeing qemuProcessEvents. This
is less error-prone as the compiler can help us make sure that for
every new enumeration value of qemuProcessEventType the
qemuProcessEventFree function has to be adapted.
All process*Event functions are *only* called by
qemuProcessHandleEvent and this function does the freeing by itself
with qemuProcessEventFree. This means that an explicit freeing of
processEvent->data is no longer required in each process*Event
handler.
The effectiveness of this change is also demonstrated by the fact that
it fixes a memory leak of the panic info data in
qemuProcessHandleGuestPanic.
Reported-by: Wang Dong <dongdwdw@linux.vnet.ibm.com>
Signed-off-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Extract out the parts of qemuDomainGetJobStatsInternal that get
the migration stats. We're about to add the ability to get just
dump information.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Use the DEVICE_MISSING error code when helpers fail to find
the requested device. This makes it easier for consumers to
key off the error code rather than the error message.
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
After processing the processEvent->data for a qemuProcessEventHandler
callout, it's expected that the called processEvent->eventType helper
will perform the proper free on the data field. In this case it's
a qemuMonitorEventPanicInfoPtr.
Commit id '162efa1a' added support hotplug a redirdev, but
did not add the hot unplug. This patch will add that support
to allow usage of the detach-device --live on the device.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
A microcode update can cause the CPUID bits to change; an example
from the past was the update that disabled TSX on several Haswell
and Broadwell machines.
Therefore, place microcode version in the virQEMUCaps struct and
XML, and rebuild the cache if the versions do not match.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1522706
If domain is active, but the undefine API was called without the
VIR_DOMAIN_UNDEFINE_KEEP_NVRAM flag set, the following incorrect
error message is produced:
error: Requested operation is not valid: cannot delete inactive domain with nvram
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
