Extend the chardev source XML so that there is a new optional
<log/> element, which is applicable to all character device
backend types. For example, to log output of a TCP backed
serial port
<serial type='tcp'>
<source mode='connect' host='127.0.0.1' service='9999'/>
<protocol type='raw'/>
<log file='/var/log/libvirt/qemu/demo-serial0.log' append='on'/>
<target port='0'/>
</serial>
Not all hypervisors will support use of logfiles.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Not all callers of virLogManagerDomainOpenLogFile will
care about getting the current inode/offset, so we should
allow those parameters to be NULL
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Now that the function was extracted we can get rid of some temp
variables. Additionally formatting of the bitmap string for the event
code should be checked.
Allow pinning for inactive vcpus. The pinning mask will be automatically
applied as we would apply the default mask in case of a cpu hotplug.
Setting the scheduler settings for a vcpu has the same semantics.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1306556
Introduce VIR_DOMAIN_DEF_FEATURE_OFFLINE_VCPUPIN domain feature flag
whcih will allow to skip ignoring of the pinning information for
hypervisor drivers which will want to implement forward-pinning of
vcpus.
Introduce a helper to check supported device and domain config and move
the memory hotplug checks to it.
The advantage of this approach is that by default all new features are
considered unsupported by all hypervisors unless specifically changed
rather than the previous approach where every hypervisor would need to
declare that a given feature is unsupported.
To avoid having to forbid new features added to domain XML in post parse
callbacks for individual hypervisor drivers the feature flag mechanism
will allow to add a central check that will be disabled for the drivers
that will add support.
As a first example flag, the 'hasWideSCSIBus' is converted to the new
bitmap.
The VIR_DOMAIN_EVENT_ID_JOB_COMPLETED event will be triggered once a job
(such as migration) finishes and it will contain statistics for the job
as one would get by calling virDomainGetJobStats. Thanks to this event
it is now possible to get statistics of a completed migration of a
transient domain on the source host.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
We would happily report and free statistics of a completed migration
even before it actually completed (on the source host while migration is
in the Finish phase).
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Computing a total downtime during a migration requires us to store a
time stamp when guest CPUs get stopped. The value (and all other
statistics) is then transferred to the destination to compute the
downtime. Because the stopped time stamp is stored by a STOP event
handler while the statistics which will be sent over to the destination
are copied synchronously within qemuMigrationWaitForCompletion.
Depending on the timing of STOP and MIGRATION events, we may end up
copying (and transferring) statistics without the stopped time stamp
set. Let's make sure we always use the correct time stamp.
https://bugzilla.redhat.com/show_bug.cgi?id=1282744
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
With a very old QEMU which doesn't support events we need to explicitly
call qemuMigrationSetOffline at the end of migration to update our
internal state. On the other hand, if we talk to QEMU using QMP, we
should just wait for the STOP event and let the event handler update the
state and trigger a libvirt event.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
We should not overwrite all migration statistics on the source with the
numbers sent by the destination since the source may have an updated
view in some cases (such as post-copy migration). It's safer to update
just the timing info we need to get from the destination and be prepared
for the future. And we should only do all this after a successful
migration.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Statistics for a completed migration only make sense if the migration
was successful. Let's not store them in priv->job.completed until we
are sure it was a success.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The comment claimed that virPCIDeviceReattach() does not reattach
a device to the host driver; except it actually does, so the
comment is just confusing and we're better off removing it.
Replace the term "loop" with the more generic "step". This allows us
to be more flexible and eg. have a step that consists in a single
function call.
Don't include the number of steps in the first comment of the
function, so that we can add or remove steps without having to worry
about keeping that comment in sync.
For the same reason, remove the summary contained in that comment.
Clean up some weird vertical spacing while we're at it.
If the stars are in the right position and you're building with
VBox >= 4.2.0 it will happen that compiler thinks an array
allocated on the stack may be unbounded:
In file included from vbox/vbox_V4_2.c:13:0:
vbox/vbox_tmpl.c: In function '_virtualboxCreateMachine':
vbox/vbox_tmpl.c:2811:1: error: stack usage might be unbounded [-Werror=stack-usage=]
_virtualboxCreateMachine(vboxGlobalData *data, virDomainDefPtr def, IMachine **machine, char *uuidstr ATTRIBUTE_UNUSED)
^
Well, given how the variable is declared, I had some hard time
seeing it is actually bounded. Surprisingly compiler does not
complain because of -Wframe-larger-than. This is because
variable length arrays do not count into that warning.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The code does not handle renaming of the save state file. In addition to
that the resuming code would need to be tweaked to handle the name
change since the XML is extracted from the save image. The easies option
is to make the rename API even less useful by forbiding this.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1314594
This is an error message I've just seen. Fix it by initializing
@inode.
CC lxc/libvirt_driver_lxc_impl_la-lxc_process.lo
lxc/lxc_process.c: In function 'virLXCProcessMonitorInitNotify':
lxc/lxc_process.c:767:23: error: 'inode' may be used uninitialized in this function [-Werror=maybe-uninitialized]
virDomainAuditInit(vm, initpid, inode);
^
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Original current flag expansion does not filter out non
_CONFIG and _LIVE flags explicitly but they are prohibited
earlier by virCheckFlags.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Flag expansion is the same as in virDomainObjUpdateModificationImpact
which virDomainLiveConfigHelperMethod calls internally. The difference
is merely in implementation. Note that VIR_DOMAIN_MEM_CONFIG is the
same as VIR_DOMAIN_AFFECT_CONFIG. Additionally, the called functions
will properly use flag OR and thus handle the VIR_DOMAIN_MEM_MAXIMUM case.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
While trying to build with -Os couple of compile errors showed
up.
conf/domain_conf.c: In function 'virDomainChrRemove':
conf/domain_conf.c:13666:24: error: 'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized]
virDomainChrDefPtr ret, **arrPtr = NULL;
^
Compiler fails to see that @ret is used only if set in the loop,
but whatever, there's no harm in initializing the variable.
In vboxAttachDrivesNew and _vboxAttachDrivesOld compiler thinks
that @rc may be used uninitialized. Well, not directly, but maybe
after some optimization. Yet again, no harm in initializing a
variable.
In file included from ./util/virthread.h:26:0,
from ./datatypes.h:28,
from vbox/vbox_tmpl.c:43,
from vbox/vbox_V3_1.c:37:
vbox/vbox_tmpl.c: In function '_vboxAttachDrivesOld':
./util/virerror.h:181:5: error: 'rc' may be used uninitialized in this function [-Werror=maybe-uninitialized]
virReportErrorHelper(VIR_FROM_THIS, code, __FILE__, \
^
In file included from vbox/vbox_V3_1.c:37:0:
vbox/vbox_tmpl.c:1041:14: note: 'rc' was declared here
nsresult rc;
^
Yet again, one uninitialized variable:
qemu/qemu_driver.c: In function 'qemuDomainBlockCommit':
qemu/qemu_driver.c:17194:9: error: 'baseSource' may be used uninitialized in this function [-Werror=maybe-uninitialized]
qemuDomainPrepareDiskChainElement(driver, vm, baseSource,
^
And another one:
storage/storage_backend_logical.c: In function 'virStorageBackendLogicalMatchPoolSource.isra.2':
storage/storage_backend_logical.c:618:33: error: 'thisSource' may be used uninitialized in this function [-Werror=maybe-uninitialized]
thisSource->devices[j].path))
^
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
While trying to build with -Os I've encountered some build
failures.
util/vircommand.c: In function 'virCommandAddEnvFormat':
util/vircommand.c:1257:1: error: inlining failed in call to 'virCommandAddEnv': call is unlikely and code size would grow [-Werror=inline]
virCommandAddEnv(virCommandPtr cmd, char *env)
^
util/vircommand.c:1308:5: error: called from here [-Werror=inline]
virCommandAddEnv(cmd, env);
^
This function is big enough for the compiler to be not inlined.
This is the error message I'm seeing:
Then virDomainNumatuneNodeSpecified is exported and called from
other places. It shouldn't be inlined then.
In file included from network/bridge_driver_platform.h:30:0,
from network/bridge_driver_platform.c:26:
network/bridge_driver_linux.c: In function 'networkRemoveRoutingFirewallRules':
./conf/network_conf.h:350:1: error: inlining failed in call to 'virNetworkDefForwardIf.constprop': call is unlikely and code size would grow [-Werror=inline]
virNetworkDefForwardIf(const virNetworkDef *def, size_t n)
^
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
More fallout from changing to using virPolkitAgent and handling error
paths. Needed to clear the 'cmd' once stored and of course add the
virCommandFree(cmd) in the error: label.
Older compilers fail to see that 'close' is not used a function
rather than a variable and produce the following error:
cc1: warnings being treated as errors
../../src/datatypes.c: In function 'virConnectCloseCallbackDataReset':
../../src/datatypes.c:149: error: declaration of 'close' shadows a global declaration [-Wshadow]
Replace all the 'close' occurrences with 'closeData' to resolve
this.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
In virPolkitAgentCreate neglected to initialize agent to NULL. If
there was an error in the pipe, then we jump to error and would have
an issue. Found by coverity.
libxlDomainPinVcpuFlags calls virDomainLiveConfigHelperMethod which will
call virDomainObjUpdateModificationImpact make the same AFFECT_LIVE flags
and !active check, so remove this duplicated check.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: John Ferlan <jferlan@redhat.com>
Prior to commit id '3d021381' virDomainObjUpdateModificationImpact was
part of virDomainLiveConfigHelperMethod and the *flags if condition
VIR_DOMAIN_AFFECT_CONFIG checked the ->persistent boolean and made the
virDomainObjGetPersistentDef call.
Since the functions were split the ->persistent check is all that remained
and thus could be combined into one if statement.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: John Ferlan <jferlan@redhat.com>
When SPICE graphics is configured for a domain but we did not ask the
client to switch to the destination, we should not wait for
SPICE_MIGRATE_COMPLETED event (which will never come).
https://bugzilla.redhat.com/show_bug.cgi?id=1151723
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Migration statistics are not available on the destination host and
starting a query job during incoming migration is not allowed. Trying to
do that would result in
Timed out during operation: cannot acquire state change lock (held
by remoteDispatchDomainMigratePrepare3Params)
error. We should not even try to start the job.
https://bugzilla.redhat.com/show_bug.cgi?id=1278727
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
This parameter represents top level period cgroup
that limits whole domain enforcement period for a quota
Signed-off-by: Alexander Burluka <aburluka@virtuozzo.com>
If connect close is fired then following unregister will fail
as we set callback to NULL and thus callback equality checking
will fail.
Callback is set to NULL to make it fired only one time probabaly.
Instead lets use connection equality to NULL to check if callback
is already fired.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
We have reference to connection object in virConnectCloseCallbackData
object thus we have to refcount it. Obviously we have problems
in dispose and call functions. Let's fix it.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Make register and unregister functions return void because
we can check the state of callback object beforehand via
virConnectCloseCallbackDataGetCallback. This can be done
without race conditions if we use higher level locks for registering
and unregistering. The fact they return void simplifies
task of consistent registering/unregistering.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
qemuProcessSetupEmulator runs at a point in time where there is only
the qemu main thread. Use virCgroupAddTask to put just that one task
into the emulator cgroup. That patch makes virCgroupMoveTask and
virCgroupAddTaskStrController obsolete.
Signed-off-by: Henning Schild <henning.schild@siemens.com>
Move qemuProcessSetupEmulator up under qemuSetupCgroup. That way
we move the one main thread right into the emulator cgroup, instead
of moving multiple threads later on. And we do not actually want any
threads running in the parent cgroups (cpu cpuacct cpuset).
Signed-off-by: Henning Schild <henning.schild@siemens.com>
This attribute is used to extend secondary PCI bar and expose it to the
guest as 64bit memory. It works like this: attribute vram is there to
set size of secondary PCI bar and guest sees it as 32bit memory,
attribute vram64 can extend this secondary PCI bar. If both attributes
are used, guest sees two memory bars, both address the same memory, with
the difference that the 32bit bar can address only the first part of the
whole memory.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1260749
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
We always place primary video device at first place, to make it easier
to create a qemu command or format an xml, but we should also set the
primary boolean for primary video device to 'true'.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Introduce virPolkitAgentCreate and virPolkitAgentDestroy
virPolkitAgentCreate will run the polkit pkttyagent image as an asynchronous
command in order to handle the local agent authentication via stdin/stdout.
The code makes use of the pkttyagent --notify-fd mechanism to let it know
when the agent is successfully registered.
virPolkitAgentDestroy will close the command effectively reaping our
child process
When there isn't a ssh -X type session running and a user has not
been added to the libvirt group, attempts to run 'virsh -c qemu:///system'
commands from an otherwise unprivileged user will fail with rather
generic or opaque error message:
"error: authentication failed: no agent is available to authenticate"
This patch will adjust the error code and message to help reflect the
situation that the problem is the requested mechanism is UNAVAILABLE and
a slightly more descriptive error. The result on a failure then becomes:
"error: authentication unavailable: no polkit agent available to
authenticate action 'org.libvirt.unix.manage'"
A bit more history on this - at one time a failure generated the
following type message when running the 'pkcheck' as a subprocess:
"error: authentication failed: polkit\56retains_authorization_after_challenge=1
Authorization requires authentication but no agent is available."
but, a patch was generated to adjust the error message to help provide
more details about what failed. This was pushed as commit id '96a108c99'.
That patch prepended a "polkit: " to the output. It really didn't solve
the problem, but gave a hint.
After some time it was deemed using DBus API calls directly was a
better way to go (since pkcheck calls them anyway). So, commit id
'1b854c76' (more or less) copied the code from remoteDispatchAuthPolkit
and adjusted it. Then commit id 'c7542573' adjusted the remote.c
code to call the new API (virPolkitCheckAuth). Finally, commit id
'308c0c5a' altered the code to call DBus APIs directly. In doing
so, it reverted the failing error message to the generic message
that would have been received from DBus anyway.
This new API will allocate the secret, assign the def pointer, and
insert the secret onto the passed list. Whether that's the temporary
list in loadSecrets which gets loaded into the driver list or driver
list during secretDefineXML.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Add a temporary helper to search for a specific secret by address
on the list and remove it if it's found. The following patch will
introduce a common allocation and listInsert helper. That means
error paths of the routines calling would need a way to remove the
secret off the list.
Signed-off-by: John Ferlan <jferlan@redhat.com>
This patch removes need for secretBase64Path and secretComputePath. Similar
to the configFile, create an entry for base64File, which will be generated
as the driver->configDir, the UUID value, plus the ".base" suffix. Rather
than generating on the fly, store this in the virSecretObj.
The buildup of the pathname done in loadSecrets where the failure to build
is ignored which is no different than the failure to generate the name
in secretLoadValue which would have been ignored in the failure path
after secretLoad.
This also removes the need for secretComputPath and secretBase64Path.
Signed-off-by: John Ferlan <jferlan@redhat.com>
This patch removes the need for secretXMLPath. Instead save 'path' during
loadSecret as 'configFile'. The secretXMLPath is nothing more than an
open coded virFileBuildPath. All that code did was concantenate the
driver->configDir, the UUID of the secret, and the ".xml" suffix to form
the configFile name which we now will generate and save instead.
Signed-off-by: John Ferlan <jferlan@redhat.com>
The 'secretLoad' was essentially open coding virFileBuildPath.
Adjust the logic to have the caller build the path and pass it. The net
sum of ignoring the virFileBuildPath failure is the same as before where
the failure to virAsprintf the path would have been ignored anyway in
the secretLoad error path.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Remove the need for the local 'secret' in secretConnectListAllSecrets.
A subsequent patch will rename the ObjPtr entry to secret.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Rather than having it interspersed with other changes, do it once.
Remove a couple ^L, 1 argument per line for functions, less than 80 chars
per line, use of spacing between logical groups of code, use of one line
if statements when doing fetch followed by comparison, use direct return
when no cleanup to be done.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Use virCgroupAddTaskController in virCgroupAddTask so we have one
single point where we add tasks to cgroups.
Signed-off-by: Henning Schild <henning.schild@siemens.com>
We honour the placement bitmaps when starting up, so there's no point in
having this check. Additionally the check was buggy since it checked
vm->def all the time even if the user requested to modify the persistent
definition which had different configuration.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1308317
Add Spice graphics gl attribute. qemu 2.6 should have -spice gl=on argument to
enable opengl rendering context (patches on the ML). This is necessary to
actually enable virgl rendering.
Add a qemuxml2argv test for virtio-gpu + spice with virgl.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Per-domain directories were introduced in order to be able to
completely separate security labels for each domain (commit
f1f68ca334). However when the domain
name is long (let's say a ridiculous 110 characters), we cannot
connect to the monitor socket because on length of UNIX socket address
is limited. In order to get around this, let's shorten it in similar
fashion and in order to avoid conflicts, throw in an ID there as well.
Also save that into the status XML and load the old status XMLs
properly (to clean up after older domains). That way we can change it
in the future.
The shortening can be seen in qemuxml2argv tests, for example in the
hugepages-pages2 case.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Found by inspection - after calling virStoragePoolObjAssignDef the
pool is part of the driver->pools.objs list and the failure path
for the virStoragePoolObjSaveDef will use virStoragePoolObjRemove
to remove the pool from the objs list which will unlock and free
the pool pointer (as pools->objs[i] during the loop). Since the call
doesn't clear the pool address from the callee, we need to set it
to NULL; otherwise, the virStoragePoolObjUnlock in the cleanup: code
will fail miserably.
While reviewing how storage driver used ObjListPtr's for reference
in some recent secret driver patches to use the same mechanism, I came
across an instance where the wrong API was called for error paths after
successfully allocating the storage pool pointer and inserting into
the driver pool list.
The path is after virStoragePoolObjAssignDef succeeds - the 'def' passed
in is assigned to pool->def (or newDef) so it shouldn't be the only thing
deleted. The pool is now part of driver->pools.objs, so it would need to
be removed (as happens in the storagePoolCreateXML error paths).
Rather than calling virStoragePoolDefFree to free the def which is now
assigned to the pool, call virStoragePoolObjRemove to ensure the pool
element is removed from the driver list and that anything stored in pool
is properly handled by virStoragePoolObjFree including the call to
virStoragePoolDefFree for the pool->{def|newDef} element.
Commit f1a89a8 allowed parsing configs from /etc/libvirt
without validating the emulator capabilities.
Check for the presence of a machine type in the qemu driver's
post parse function instead of crashing.
https://bugzilla.redhat.com/show_bug.cgi?id=1267256
libxlMakeNic opens a virConnect object and takes a reference on a
virNetwork object, but doesn't drop the references on all error
paths. Rework the function to follow the standard libvirt pattern
of using a local 'ret' variable to hold the function return value,
performing all cleanup and returning 'ret' at a 'cleanup' label.
Generates a false positive for Coverity, but it turns out there's no need
to check ret == -1 since if VIR_APPEND_ELEMENT is successful, the local
vol pointer is cleared anyway.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Found by my Coverity checker - virCheckFlags call could return -1, but
not virCommandFree(destroy_cmd).
Signed-off-by: John Ferlan <jferlan@redhat.com>
When parsing the barrier:limit values, use virStringSplitCount in order
to split the pair and make the approriate checks to get the data.
Signed-off-by: John Ferlan <jferlan@redhat.com>
The virHostdevIsVirtualFunction() was called exactly twice, and in
both cases the return value was saved to a temporary variable before
being checked. This would be okay if it improved readability, but in
this case is pretty pointless.
Get rid of the temporary variable and check the return value
directly; while at it, change the check from '<= 0' to '!= 1' to
align it with the way other similar *IsVirtualFunction() functions
are used thorough the code.
virNetDevIsVirtualFunction() returns 1 if the interface is a
virtual function, 0 if it isn't and -1 on error. This means that,
despite the name suggesting otherwise, using it as a predicate is
not correct.
Fix two callers that were doing so adding an explicit check on
the return value.
