This serves the same purpose as VIR_ERR_NO_xxx where xxx is any object
that API can be called upon. Only this particular one is used for
daemon's servers.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Since servers know their name, there is no need to supply such
information twice. Also defeats inconsistencies.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
At first I did not want to do this, but after trying to implement some
newer feaures in the admin API I realized we need that to make our lives
easier. On the other hand they are not saved redundantly and the
virNetServer objects are still kept in a hash table.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Function virAdmConnectListServers() forgot to check for flags at all,
virAdmConnectOpen() on the other hand checked them but did no dispatch
the error. virCheckFlags() should be used only when there should be no
other thing done after erroring out and since they are used on different
places then just public API, they cannot dispatch errors. So let's use
virCheckFlagsGoto instead.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Add new function to manage adding the network device options to the
command line removing that task from the mainline qemuBuildCommandLine.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Add new function to manage adding the -fsdev options to the
command line removing that task from the mainline qemuBuildCommandLine.
Alter the code slightly to perform the !caps and fsdev failure check
up front.
Since both qemuBuildFSStr and qemuBuildFSDevStr are local, make them
static and fix their prototypes to use the const virDomainDef as well.
Make some minor formatting changes for long lines.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Add new function to manage adding the disk -drive options to the
command line removing that task from the mainline qemuBuildCommandLine.
Also since using const virDomainDef in new function, that means other
functions called needed to change their usage.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Add new function to manage adding the hub -device options to the
command line removing that task from the mainline qemuBuildCommandLine.
Also make qemuBuildHubDevStr static to the module since it's only
used here.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Add new function to manage adding the controller -device options to the
command line removing that task from the mainline qemuBuildCommandLine.
Also adjust to using const virDomainDef instead of virDomainDefPtr.
This causes collateral damage in order to modify called APIs to use
the const virDomainDef instead as well.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Add new function to manage adding the -global controller options to
the command line removing that task from the mainline qemuBuildCommandLine.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Add new function to manage adding the -boot options to the command
line removing that task from the mainline qemuBuildCommandLine.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Add new function to manage adding the power management options to the
command line removing that task from the mainline qemuBuildCommandLine.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Add new function to manage adding the '-clock' options to the command
line removing that task from the mainline qemuBuildCommandLine.
Also includes some minor formatting cleanups.
Signed-off-by: John Ferlan <jferlan@redhat.com>
When debug-threads is enabled, individual threads are given a separate
name (on Linux)
Fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=1140121
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
QEMU (somewhere around 2.0) added a new sub-option to the -name flag
-name debug-threads=on.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Currently the file based character devices let QEMU write
directly to a file on disk. This allows a malicious QEMU
to inflict a denial of service by consuming all free space.
Switch QEMU to use a pipe to virtlogd, which will enforce
file rollover.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
If use of virtlogd is enabled, then use it for backing the
character device log files too. This avoids the possibility
of a guest denial of service by writing too much data to
the log file.
The virtlogd daemon currently opens all files for append, but
in some cases the user may wish to discard existing data. Define
a new flag to indicate that logfiles should be truncated when
opening.
The functions for handling FD passing when building command line
arguments need to be used by many different bits of code, so need
to be at the start of the source file
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The act of formatting a chardev backend value may need to
append command line arguments for passing FDs. If we append
the -chardev arg before formatting the value, then the
resulting arguments will end up interspersed
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Honour the <log file='...'/> element in chardevs to output
data to a file. This requires QEMU >= 2.6
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
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.
Introduce support for domainInterfaceStats API call for querying
network interface statistics. Consequently it also enables the use of
`virsh domifstat <dom> <interface name>` command plus seeing the
interfaces names instead of "-" when doing `virsh domiflist <dom>`.
After successful guest creation we fill the network interfaces names
based on domain, device id and append suffix if it's emulated in the
following form: vif<domid>.<devid>[-emu]. We extract the network
interfaces info from the libxl_domain_config object in
libxlDomainCreateIfaceNames() to generate ifname. On domain cleanup we
also clear ifname, in case it was set by libvirt (i.e. being prefixed
with "vif"). We also skip these two steps in case the name of the
interface was manually inserted by the administrator. Since the
introduction of netprefix (commit a040ba9), ifnames with a registered
prefix will be freed on virDomain{Obj,Def}Format*, thus eliminating
the migration issues observed with the reverted commit d2e5538 whereas
source and destination would have the same ifname.
For getting the interface statistics we resort to virNetInterfaceStats
and let libvirt handle the platform specific nits. Note that the
latter is not yet supported in FreeBSD.
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
%zu is not always synonymous with uint64_t; on 32-bit machines,
size_t is only 32 bits. Prefer "%lld"/'unsigned long long' when
the variable is under our control, and "%"PRIu64 when we are
stuck with 'uint64_t' from RBD.
Fixes errors such as:
../../src/storage/storage_backend_rbd.c: In function 'virStorageBackendRBDVolWipe':
../../src/storage/storage_backend_rbd.c:1281:15: error: format '%zu' expects argument of type 'size_t', but argument 8 has type 'uint64_t {aka long long unsigned int}' [-Werror=format=]
VIR_DEBUG("Need to wipe %zu bytes from RBD image %s/%s",
^
../../src/util/virlog.h:90:73: note: in definition of macro 'VIR_DEBUG_INT'
virLogMessage(src, VIR_LOG_DEBUG, filename, linenr, funcname, NULL, __VA_ARGS__)
^
../../src/storage/storage_backend_rbd.c:1281:5: note: in expansion of macro 'VIR_DEBUG'
VIR_DEBUG("Need to wipe %zu bytes from RBD image %s/%s",
^
Signed-off-by: Eric Blake <eblake@redhat.com>
There's this check when building command line that whenever
domain has no graphics card configured we put -nographics onto
qemu command line. The check is 'if (!def->graphics)'. This
makes coverity think that def->graphics can be NULL, which is
true. But later in the code every access to def->graphics is
guarded by check for def->ngraphics, so no crash occurs. But this
is something that coverity fails to deduct.
In order to shut coverity up lets change the condition to
'if (!def->ngraphics)'.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
After 6604a3dd9f in which new helper function has been
introduced, the code calls virStringReplace and dereference the
result immediately. The string function can, however, return NULL
so this would SIGSEGV right away. Check for the return value of
the string function.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
After 457ff97fa there are two defects in our code. In both of
them we use a signed variable to hold up a number of snapshots
that domain has. We use a helper function to count the number.
However, the helper function may fail in which case it returns
a negative one and control jumps to cleanup label where an
unsigned variable is used to iterate over array of snapshots. The
loop condition thus compare signed and unsigned variables which
in this specific case ends up badly for us.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
xl/libxl already supports qemu's network-based block backends
such as nbd and rbd. libvirt has supported configuring such
<disk>s for long time too. This patch adds support for rbd
disks in the libxl driver by generating a rbd device URL from
the virDomainDiskDef object. The URL is passed to libxl via the
pdev_path field of libxl_device_disk struct. libxl then passes
the URL to qemu for cosumption by the rbd backend.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
The target= setting in xl disk configuration can be used to encode
meta info that is meaningful to a backend. Leverage this fact to
support qdisk network disk types such as rbd. E.g. <disk> config
such as
<disk type='network' device='disk'>
<driver name='qemu' type='raw'/>
<source protocol='rbd' name='pool/image'>
<host name='mon1.example.org' port='6321'/>
<host name='mon2.example.org' port='6322'/>
<host name='mon3.example.org' port='6322'/>
</source>
<target dev='hdb' bus='ide'/>
<address type='drive' controller='0' bus='0' target='0' unit='1'/>
</disk>
can be converted to the following xl config (and vice versa)
disk = [ "format=raw,vdev=hdb,access=rw,backendtype=qdisk,
target=rbd:pool/image:auth_supported=none:mon_host=mon1.example.org\\:6321\\;mon2.example.org\\:6322\\;mon3.example.org\\:6322"
]
Note that in xl disk config, a literal backslash in target= must
be escaped with a backslash. Conversion of <auth> config is not
handled in this patch, but can be done in a follow-up patch.
Also add a test for the conversions.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
The most formal form of xl disk configuration uses key=value
syntax to define each configuration item, e.g.
format=raw, vdev=xvda, access=rw, backendtype=phy, target=disksrc
Change the xl disk formatter to produce this syntax, which allows
target= to contain meta info needed to setup a network-based
disksrc (e.g. rbd, nbd, iscsi). For details on xl disk config
format, see $xen-src/docs/misc/xl-disk-configuration.txt
Update the disk config in the tests to use the formal syntax.
But add tests to ensure disks specified with the positional
parameter syntax are correctly converted to <disk> XML.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
It may be useful in some cases to call TristateSwitch helper with TristateBool.
Document that enum values equivalency in the code.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
In case you will specify graphics like this:
<graphics type='spice' port='-1'/>
or
<graphics type='spice' port='-1' tlsPort='6000'/>
libvirt will automatically add autoport='no'. This leads to an issue
that in qemuProcessStop() we don't release that port because we are
releasing both port if autoport=yes or only port marked as reserved.
If autoport=no but we request to generate port via '-1' we need to mark
that port as reserved in order to release it.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1299696
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Checking whether x > 0 before looping over [0..x] items doesn't make
sense and multi-line body must have curly brackets around it.
Best viewed with '-w'.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
This does nothing more than adding the new device and capability.
The device is present since QEMU 2.6.0.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
There's a check if a domain definition has any graphics card and
if so, we iterate over each one of them. This makes no sense,
because even if it has none we can still iterate over.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
GIC v2 is the default, but checking against that specific version when
we want to know whether the default has been selected is potentially
error prone; using an alias instead makes it safer.
Similarly to VM startup always set the legacy affinity. Additionally we
don't need to report an explicit error since virProcessSetAffinity
reports them themselves.
Similarly to VM startup always set the legacy affinity. Additionally we
don't need to report an explicit error since virProcessSetAffinity
reports them themselves.
PostParse handles it for us now.
This causes some test suite churn; qemu's custom PostParse could is
now invoked before the generic AddImplicitControllers, so PCI
controllers end up sequentially in the XML before the generically
added IDE controllers. So it's just some XML reordering
Seems like the natural fit, since we are already adding other XML bits
in the PostParse routine.
Previously AddImplicitControllers was only called at the end of XML
parsing, meaning code that builds a DomainDef by hand had to manually
call it. Now those PostParse callers get it for free.
There's some test churn here; xen xm and sexpr test suite bits weren't
calling this before, but now they are, so you'll see new IDE controllers.
I don't think this will cause problems in practice, since the code already
needs to handle these implicit controllers like in the case when a user
defines their own XML.
virDomainObjWait is designed to be called in a loop. Make sure we break
the loop in case the domain dies to avoid waiting for an event which
will never happen.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
