Peter Krempa [Wed, 3 Sep 2014 13:17:50 +0000 (15:17 +0200)]
qemu: snapshot: Fix job handling when creating snapshots
Creating snapshots modifies the domain state. Currently we wouldn't
enter the job for certain operations although they would modify the
state. Refactor job handling so that everything is covered by an async
job.
For security type='none' libvirt according to the docs should not
generate seclabel be it for selinux or any model. So, skip the
reservation of labels when type is none.
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com>
Eric Blake [Fri, 29 Aug 2014 21:47:28 +0000 (15:47 -0600)]
blockcopy: expose new API in virsh
Expose the new power of virDomainBlockCopy through virsh (well,
all but the finer-grained bandwidth, as that is its own can of
worms for a later patch). Continue to use the older API where
possible, for maximum compatibility.
The command now requires either --dest (with optional --format
and --blockdev), to directly describe the file destination, or
--xml, to name a file that contains an XML description such as:
[well, it may be a while before the qemu driver is actually patched
to act on that particular xml beyond just parsing it, but the virsh
interface won't need changing at that time]
Non-zero option parameters are converted into virTypedParameters,
and if anything requires the new API, the command can synthesize
appropriate XML even if the --dest option was used instead of --xml.
The existing --raw flag remains for back-compat, but the preferred
spelling is now --format=raw, since the new API now allows us
to specify all formats rather than just a boolean raw to suppress
probing.
I hope I did justice in describing the effects of granularity and
buf-size on how they get passed through to qemu.
* tools/virsh-domain.c (cmdBlockCopy): Add new options --xml,
--granularity, --buf-size, --format. Make --raw an alias for
--format=raw. Call new API if new parameters are in use.
* tools/virsh.pod (blockcopy): Document new options.
Eric Blake [Thu, 4 Sep 2014 12:39:54 +0000 (06:39 -0600)]
maint: update to latest gnulib
The usual portability fixes; and this includes a fix that adds
a new syntax check for double semicolons (commit 28de556 fixed
some, but gnulib found a better check).
* .gnulib: Update to latest.
* src/xenconfig/xen_common.c (xenFormatConfigCommon): Fix offender.
Eric Blake [Fri, 29 Aug 2014 21:20:30 +0000 (15:20 -0600)]
blockcopy: split out virsh implementation
I'm about to extend the capabilities of blockcopy. Hiding a few
common lines of implementation gets in the way of the new required
logic, and putting the new logic in the common implementation won't
benefit any of the other blockjob operations. Therefore, it is
simpler to just do the work inline. There should be no semantic
change in this patch.
Resolve a libvirtd crash in virStoragePoolSourceFindDuplicate()
when there is an existing SCSI pool defined with adapter type as
'scsi_host' and defining a new SCSI pool with adapter type as
'fc_host' and parent attribute missing or vice versa.
For example, if there is an existing SCSI pool with adapter type
as 'scsi_host' defined using the following XML
Same is true for the reverse case as well where there exists a SCSI pool
with adapter type as 'fc_host' and another SCSI pool is defined with
adapter type as 'scsi_host'.
This happens because for fc_host 'name' is optional attribute whereas for
scsi_host its mandatory. However the check in libvirt for finding duplicate
storage pools didn't take that into account while comparing
Signed-off-by: Pradipta Kr. Banerjee <bpradip@in.ibm.com>
Eric Blake [Thu, 28 Aug 2014 04:03:04 +0000 (22:03 -0600)]
blockcopy: allow block device destination
To date, anyone performing a block copy and pivot ends up with
the destination being treated as <disk type='file'>. While this
works for data access for a block device, it has at least one
noticeable shortcoming: virDomainGetBlockInfo() reports allocation
differently for block devices visited as files (the size of the
device) than for block devices visited as <disk type='block'>
(the maximum sector used, as reported by qemu); and this difference
is significant when trying to manage qcow2 format on block devices
that can be grown as needed.
Of course, the more powerful virDomainBlockCopy() API can already
express the ability to set the <disk> type. But a new API can't
be backported, while a new flag to an existing API can; and it is
also rather inconvenient to have to resort to the full power of
generating XML when just adding a flag to the older call will do
the trick. So this patch enhances blockcopy to let the user flag
when the resulting XML after the copy must list the device as
type='block'.
* include/libvirt/libvirt.h.in (VIR_DOMAIN_BLOCK_REBASE_COPY_DEV):
New flag.
* src/libvirt.c (virDomainBlockRebase): Document it.
* tools/virsh-domain.c (opts_block_copy, blockJobImpl): Add
--blockdev option.
* tools/virsh.pod (blockcopy): Document it.
* src/qemu/qemu_driver.c (qemuDomainBlockRebase): Allow new flag.
(qemuDomainBlockCopy): Remember the flag, and make sure it is only
used on actual block devices.
* tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml: Test it.
Eric Blake [Thu, 28 Aug 2014 23:39:25 +0000 (17:39 -0600)]
blockjob: add new --bytes flag to virsh blockjob
Expose the new flag just added to virDomainGetBlockJobInfo.
With --raw, the presence or absence of --bytes determines which
flag to use in the single API call. Without --raw, the use of
--bytes forces an error if the server doesn't support it,
otherwise, the code tries to silently fall back to scaling the
MiB/s value.
My goal is to eventually also support --bytes in bandwidth mode;
but that's a bit further down the road (and needs a new API flag
added in libvirt.h first).
This changes the human output, but the previous patch added
raw output precisely so that we can have flexibility with the
human output. For this commit, I used qemu-monitor-command to
force an unusual bandwidth, but the same will be possible once
qemu implements virDomainBlockCopy:
The cache avoids having to repeatedly checking whether the flag
works when talking to an older server, when multiple blockjob
commands are issued during a batch session and the user is
manually polling for job completion.
Eric Blake [Thu, 28 Aug 2014 23:23:49 +0000 (17:23 -0600)]
blockjob: add new --raw flag to virsh blockjob
The current output of 'blockjob [--info]' is a single line
designed for human consumption; it's not very nice for machine
parsing. Furthermore, I have plans to modify the line in
response to the new flag for controlling bandwidth units.
Solve that by adding a --raw parameter, which outputs
information closer to the C struct.
The information is indented, because I'd like for a later patch
to add a mode that iterates over all the vm's disks with status
for each; in that mode, each block name would be listed unindented
before information (if any) about that block.
Now that we have a raw mode, we can guarantee that it won't change
format over time. Any app that cares about parsing the output can
try --raw, and if it fails, know that it was talking to an older
virsh and fall back to parsing the human-readable format which had
not changed until now; meanwhile, when not using --raw, we have
freed future virsh to change the output to whatever makes sense.
My first change to human mode: this command now guarantees a line
is printed on successful use of the API, even when the API did
not find a current block job (consistent with the rest of virsh).
Bonus: https://bugzilla.redhat.com/show_bug.cgi?id=1135441
complained that this message was confusing:
$ virsh blockjob test1 hda --async --bandwidth 10
error: conflict between --abort, --info, and --bandwidth modes
even though the man page already documents that --async implies
abort mode, all because '--abort' wasn't present in the command
line. Since I'm adding another case where options are tied
to or imply a mode, I changed that error to:
error: conflict between abort, info, and bandwidth modes
Eric Blake [Thu, 28 Aug 2014 19:18:22 +0000 (13:18 -0600)]
blockjob: split up virsh blockjob info
I have plans to make future enhancements to the job list mode,
which will be easier to do if the common blockJobImpl function
is not mixing a query command with multiple modify commands.
Besides, it just feels weird that all callers to blockJobImpl
had to supply both a bandwidth input argument (unused for info
mode) and an info output argument (unused for all other modes);
not to mention I just made similar cleanups on the libvirtd
side.
The only reason blockJobImpl returned int was because of info
mode returning -1/0/1 (all other job API are -1/0), so that
can also be cleaned up. No user-visible changes in this commit.
* tools/virsh-domain.c (blockJobImpl): Change signature and return
value. Drop info handling.
(cmdBlockJob): Handle info here.
(cmdBlockCommit, cmdBlockCopy, cmdBlockPull): Adjust callers.
Eric Blake [Wed, 27 Aug 2014 20:04:36 +0000 (14:04 -0600)]
blockjob: allow finer bandwidth tuning for query
While reviewing the new virDomainBlockCopy API, Peter Krempa
pointed out that our existing design of using MiB/s for block
job bandwidth is rather coarse, especially since qemu tracks
it in bytes/s; so virDomainBlockCopy only accepts bytes/s.
But once the new API is implemented for qemu, we will be in
the situation where it is possible to set a value that cannot
be accurately reflected back to the user, because the existing
virDomainGetBlockJobInfo defaults to the coarser units.
Fortunately, we have an escape hatch; and one that has already
served us well in the past: we can use the flags argument to
specify which scale to use (see virDomainBlockResize for prior
art). This patch fixes the query side of the API; made easier
by previous patches that split the query side out from the
modification code. Later patches will address the virsh
interface, as well retrofitting all other blockjob APIs to
also accept a flag for toggling bandwidth units.
Eric Blake [Fri, 5 Sep 2014 11:31:47 +0000 (05:31 -0600)]
blockjob: add new monitor json conversions
The previous patch hoisted some bounds checks to the callers;
but someone that is not aware of the hoisted check could now
try passing an integer between LLONG_MAX and ULLONG_MAX. As a
safety measure, add new json conversion modes that let libvirt
error out early instead of pass bad numbers to qemu, if the
caller ever makes a mistake due to later refactoring.
Convert the various blockjob QMP calls to use the new modes,
and switch some of them to be optional (QMP has always supported
an omitted "speed" the same as "speed":0, for everything except
block-job-set-speed).
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONMakeCommandRaw):
Add 'j'/'y' and 'J'/'Y' to error out on negative input.
(qemuMonitorJSONDriveMirror, qemuMonitorJSONBlockCommit)
(qemuMonitorJSONBlockJob): Use it.
Eric Blake [Fri, 29 Aug 2014 19:58:45 +0000 (13:58 -0600)]
blockjob: hoist bandwidth scaling out of monitor code
qemu treats blockjob bandwidth as a 64-bit number, in the units
of bytes/second. But we stupidly modeled block job bandwidth
after migration bandwidth, which in turn was an 'unsigned long'
and therefore subject to 32-bit vs. 64-bit interpretations, and
with a scale of MiB/s. Our code already has to convert between
the two scales, and report overflow as appropriate; although
this conversion currently lives in the monitor code. In fact,
our conversion code limited things to 63 bits, because we
checked against LLONG_MAX and reject what would be negative
bandwidth if treated as signed.
On the bright side, our use of MiB/s means that even with a
32-bit unsigned long, we still have no problem representing a
bandwidth of 2GiB/s, which is starting to be more feasible as
10-gigabit or even faster interfaces are used. And once you
get past the physical speeds of existing interfaces, any larger
bandwidth number behaves the same - effectively unlimited.
But on the low side, the granularity of 1MiB/s tuning is rather
coarse. So the new virDomainBlockJob API decided to go with
a direct 64-bit bytes/sec number instead of the scaled number
that prior blockjob APIs had used. But there is no point in
rounding this number to MiB/s just to scale it back to bytes/s
for handing to qemu.
In order to make future code sharing possible between the old
virDomainBlockRebase and the new virDomainBlockCopy, this patch
moves the scaling and overflow detection into the driver code.
Several of the block job calls that can set speed are fed
through a common interface, so it was easier to adjust all block
jobs at once, for consistency. This patch is just code motion;
there should be no user-visible change in behavior.
* src/qemu/qemu_monitor.h (qemuMonitorBlockJob)
(qemuMonitorBlockCommit, qemuMonitorDriveMirror): Change
parameter type and scale.
* src/qemu/qemu_monitor.c (qemuMonitorBlockJob)
(qemuMonitorBlockCommit, qemuMonitorDriveMirror): Move scaling
and overflow detection...
* src/qemu/qemu_driver.c (qemuDomainBlockJobImpl)
(qemuDomainBlockRebase, qemuDomainBlockCommit): ...here.
(qemuDomainBlockCopy): Use bytes/sec.
Eric Blake [Wed, 27 Aug 2014 19:29:14 +0000 (13:29 -0600)]
blockjob: split out block info monitor handling
Another layer of overly-multiplexed code that deserves to be
split into obviously separate paths for query vs. modify.
This continues the cleanup started in commit cefe0ba.
In the process, make some tweaks to simplify the logic when
parsing the JSON reply. There should be no user-visible
semantic changes.
* src/qemu/qemu_monitor.h (qemuMonitorBlockJob): Drop parameter.
(qemuMonitorBlockJobInfo): New prototype.
(BLOCK_JOB_INFO): Drop enum.
* src/qemu/qemu_monitor_json.h (qemuMonitorJSONBlockJob)
(qemuMonitorJSONBlockJobInfo): Likewise.
* src/qemu/qemu_monitor.c (qemuMonitorBlockJob): Split...
(qemuMonitorBlockJobInfo): ...into second function.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONBlockJob): Move
block info portions...
(qemuMonitorJSONGetBlockJobInfo): ...here, and rename...
(qemuMonitorJSONBlockJobInfo): ...and export.
(qemuMonitorJSONGetBlockJobInfoOne): Alter return semantics.
* src/qemu/qemu_driver.c (qemuDomainBlockPivot)
(qemuDomainBlockJobImpl, qemuDomainGetBlockJobInfo): Adjust
callers.
* src/qemu/qemu_migration.c (qemuMigrationDriveMirror)
(qemuMigrationCancelDriveMirror): Likewise.
Ján Tomko [Fri, 5 Sep 2014 07:50:36 +0000 (09:50 +0200)]
Don't include non-migratable features in host-model
Commit fba6bc4 introduced support for the 'invtsc' feature,
which blocks migration. We should not include it in the
host-model CPU by default, because it's intended to be used
with migration.
This commit is rather big. Firstly, the in memory config
representation is adjusted like if security_driver was set to "none".
The rest is then just adaptation to the new code that will generate
different seclabels.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
There are two ways in which security model can make it way into
<seclabel/>. One is as the @model attribute, the second one is
via security_driver knob in qemu.conf. Then, while parsing
<seclabel/> several checks and fix ups of old, stale combinations
are performed. However, iff @model is specified. They are not
done in the latter case. So it's still possible to feed libvirt
with senseless combinations (if qemu.conf is adjusted correctly).
One example of a seclabel that needs some adjustment (in case
security_driver=none in qemu.conf) is:
<seclabel type='dynamic' relabel='yes'/>
The fixup code is copied from virSecurityLabelDefParseXML
(covering the former case) into virSecurityLabelDefsParseXML
(which handles the latter case).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Eric Blake [Fri, 29 Aug 2014 19:36:59 +0000 (13:36 -0600)]
blockjob: split out block info driver handling
The qemu implementation for virDomainGetBlockJobInfo() has a
minor bug: it grabs the qemu job with intent to QEMU_JOB_MODIFY,
which means it cannot be run in parallel with any other
domain-modifying command. Among others, virDomainBlockJobAbort()
is such a modifying command, and it defaults to being
synchronous, and can wait as long as several seconds to ensure
that the job has actually finished. Due to the job rules, this
means a user cannot obtain status about the job during that
timeframe, even though we know that some client management code
exists which is using a polling loop on status to see when a job
finishes.
This bug has been present ever since blockpull support was first
introduced (commit b976165, v0.9.4 in Jul 2011), all because we
stupidly tried to cram too much multiplexing through a single
helper routine, but was made worse in 97c59b9 (v1.2.7) when
BlockJobAbort was fixed to wait longer. It's time to disentangle
some of the mess in qemuDomainBlockJobImpl, and in the process
relax block job query to use QEMU_JOB_QUERY, since it can safely
be used in parallel with any long running modify command.
Technically, there is one case where getting block job info can
modify domain XML - we do snooping to see if a 2-phase job has
transitioned into the second phase, for an optimization in the
case of old qemu that lacked an event for the transition. I
claim this optimization is safe (the jobs are all about modifying
qemu state, not necessarily xml state); but if it proves to be
a problem, we could use the difference between the capabilities
QEMU_CAPS_BLOCKJOB_{ASYNC,SYNC} to determine whether we even
need snooping, and only request a modifying job in the case of
older qemu.
* src/qemu/qemu_driver.c (qemuDomainBlockJobImpl): Move info
handling...
(qemuDomainGetBlockJobInfo): ...here, and relax job type.
(qemuDomainBlockJobAbort, qemuDomainBlockJobSetSpeed)
(qemuDomainBlockRebase, qemuDomainBlockPull): Adjust callers.
Eric Blake [Fri, 29 Aug 2014 19:27:18 +0000 (13:27 -0600)]
blockjob: shuffle block rebase code
The existing virDomainBlockRebase code rejected the combination of
_RELATIVE and _COPY flags, but only by accident. It makes sense
to add support for the combination someday, at least for the case
of _SHALLOW and not _REUSE_EXT; but to implement it, libvirt would
have to pre-create the file with a relative backing name, and I'm
not ready to code that in yet.
Meanwhile, the code to forward on to the block copy code is getting
longer, and reorganizing the function to have the block pull done
early makes it easier to add even more block copy prep code.
This patch should have no semantic difference other than the quality
of the error message on the unsupported flag combination. Pre-patch:
error: unsupported flags (0x10) in function qemuDomainBlockCopy
Post-patch:
error: argument unsupported: Relative backing during copy not supported yet
* src/qemu/qemu_driver.c (qemuDomainBlockRebase): Reorder code,
and improve error message of relative copy.
Eric Blake [Wed, 3 Sep 2014 22:18:19 +0000 (16:18 -0600)]
maint: tighten curly brace syntax checking
Now that hanging brace offenders have been fixed, we can automate
the check, and document our style. Done as a separate commit from
code changes, to make it easier to just backport code changes, if
that is ever needed.
Eric Blake [Wed, 3 Sep 2014 22:24:43 +0000 (16:24 -0600)]
maint: use hanging curly braces
Our style overwhelmingly uses hanging braces (the open brace
hangs at the end of the compound condition, rather than on
its own line), with the primary exception of the top level function
body. Fix the few remaining outliers, before adding a syntax
check in a later patch.
Wang Rui [Mon, 1 Sep 2014 12:08:08 +0000 (20:08 +0800)]
lxc_container: Resolve Coverity RESOURCE_LEAK
Memory is allocated for 'mnt_src' by VIR_STRDUP in the loop. Next
loop it will be allocated again. So we need to free 'mnt_src'
before continue the loop.
Chunyan Liu [Wed, 3 Sep 2014 06:18:07 +0000 (14:18 +0800)]
qemu: ensure sane umask for qemu process
Add umask to _virCommand, allow user to set umask to command.
Set umask(002) to qemu process to overwrite the default umask
of 022 set by many distros, so that unix sockets created for
virtio-serial has expected permissions.
Fix problem reported here:
https://sourceware.org/bugzilla/show_bug.cgi?id=13078#c11
https://bugzilla.novell.com/show_bug.cgi?id=888166
To use virtio-serial device, unix socket created for chardev with
default umask(022) has insufficient permissions.
e.g.:
-device virtio-serial \
-chardev socket,path=/tmp/foo,server,nowait,id=foo \
-device virtserialport,chardev=foo,name=org.fedoraproject.port.0
Matthew Rosato [Wed, 27 Aug 2014 14:34:13 +0000 (10:34 -0400)]
util: Introduce flags field for macvtap creation
Currently, there is one flag passed in during macvtap creation
(withTap) -- Let's convert this field to an unsigned int flag
field for future expansion.
Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Eric Blake [Sun, 31 Aug 2014 04:02:19 +0000 (22:02 -0600)]
blockcopy: allow larger buf-size
While qemu definitely caps granularity to 64 MiB, it places no
limits on buf-size. On a machine beefy enough for lots of
memory, a buf-size larger than 2 GiB is feasible, so we should
pass a 64-bit parameter.
The cleanup in commit cf976d9d used secdef->label to label the tap
FDs, but that is not possible since it's process-only label (svirt_t)
and not a object label (e.g. svirt_image_t). Starting a domain failed
with EPERM, but simply using secdef->imagelabel instead of
secdef->label fixes it.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Fix connection to already running session libvirtd
Since 1b807f92, connecting with virsh to an already running session
libvirtd fails with:
$ virsh list --all
error: failed to connect to the hypervisor
error: no valid connection
error: Failed to connect socket to
'/run/user/1000/libvirt/libvirt-sock': Transport endpoint is already
connected
This is caused by a logic error in virNetSocketNewConnectUnix: even if
the connection to the daemon socket succeeded, we still try to spawn the
daemon and then connect to it.
This commit changes the logic to not try to spawn libvirtd if we
successfully connected to its socket.
Most of this commit is whitespace changes, use of -w is recommended to
look at it.
Currently, after calling commands to create a new volumes,
virStorageBackendZFSCreateVol calls virStorageBackendZFSFindVols that
calls virStorageBackendZFSParseVol.
virStorageBackendZFSParseVol checks if a volume already exists by
trying to get it using virStorageVolDefFindByName.
For a just created volume it returns NULL, so volume is reported as
new and appended to pool->volumes. This causes a volume to be listed
twice as storageVolCreateXML appends this new volume to the list as
well.
Fix that by passing a new volume definition to
virStorageBackendZFSParseVol so it could determine if it needs to add
this volume to the list.
(3) Event ptr_arith:
Performing pointer arithmetic on "cur_fd" in expression "cur_fd++".
130 return virNetServerServiceNewFD(*cur_fd++,
The complaint is that pointer arithmetic taking place instead of the
expected auto increment of the variable... Adding some well placed
parentheses ensures our order of operation.
John Ferlan [Mon, 25 Aug 2014 19:59:32 +0000 (15:59 -0400)]
qemu: Allow use of iothreads for disk definitions
For virtio-blk-pci disks with the disk iothread attribute that are
running the correct emulator, add the "iothread=iothread#" to the
-device command line in order to enable iothreads for the disk as
long as the command is available, the disk iothread value provided is
valid, and is supported for the disk device being added
John Ferlan [Mon, 25 Aug 2014 12:43:17 +0000 (08:43 -0400)]
domain_conf: Add support for iothreads in disk definition
Add a new disk "driver" attribute "iothread" to be parsed as the thread
number for the disk to use. In order to more easily facilitate the usage
and configuration of the iothread, a "zero" for the attribute indicates
iothreads are not supported for the device and a positive value indicates
the specific thread to try and use.
John Ferlan [Fri, 22 Aug 2014 22:15:30 +0000 (18:15 -0400)]
qemu: Add support for iothreads
Add a new capability to ensure the iothreads feature exists for the qemu
emulator being run - requires the "query-iothreads" QMP command. Using the
domain XML add correspoding command argument in order to generate the
threads. The iothreads will use a name space "iothread#" where, the
future patch to add support for using an iothread to a disk definition to
merely define which of the available threads to use.
Add tests to ensure the xml/argv processing is correct. Note that no
change was made to qemuargv2xmltest.c as processing the -object element
would require knowing more than just iothreads.
John Ferlan [Fri, 22 Aug 2014 14:15:51 +0000 (10:15 -0400)]
domain_conf: Introduce iothreads XML
Introduce XML to allowing adding iothreads to the domain. These can be
used by virtio-blk-pci devices in order to assign a specific thread to
handle the workload for the device. The iothreads are the official
implementation of the virtio-blk Data Plane that's been in tech preview
for QEMU.
John Ferlan [Thu, 28 Aug 2014 18:56:33 +0000 (14:56 -0400)]
libxl_migration: Resolve Coverity NULL_RETURNS
Coverity noted that all callers to libxlDomainEventQueue() could ensure
the second parameter (event) was true before calling except this case.
As I look at the code and how events are used - it seems that prior to
generating an event for the dom == NULL condition, the resume/suspend
event should be queue'd after the virDomainSaveStatus() call which will
goto cleanup and queue the saved event anyway.
Peter Krempa [Mon, 25 Aug 2014 16:54:49 +0000 (18:54 +0200)]
qemu: Implement bulk stats API and one of the stats groups to return
Implement the API function for virDomainListGetStats and
virConnectGetAllDomainStats in a modular way and implement the
VIR_DOMAIN_STATS_STATE group of statistics.
Although it may look like the function looks universal I'd rather not
expose it to other drivers as the coming stats groups are likely to do
qemu specific stuff to obtain the stats.
John Ferlan [Wed, 27 Aug 2014 20:33:12 +0000 (16:33 -0400)]
qemu_command: Resolve Coverity DEADCODE
One useless warning, but the other one rather pertinent. On entry
the 'trans' variable is initialized to VIR_DOMAIN_DISK_TRANS_DEFAULT.
When the "trans" was found in the parsing loop it def->geometry.trans
was assigned to the return from virDomainDiskGeometryTransTypeFromString
and then 'trans' was used to do the comparison to see if it was valid.
So remove 'trans' and use def->geometry.trans properly
John Ferlan [Wed, 27 Aug 2014 19:22:33 +0000 (15:22 -0400)]
libxl_migration: Resolve Coverity RESOURCE_LEAK
In libxlDomainMigrationPrepare() if the uri_in is false, then
'hostname' is allocated and used "generically" in the routine,
but not freed. Conversely, if uri_in is true, then a uri is
allocated and hostname is set to the uri->hostname value and
likewise generically used.
At function exit, hostname wasn't free'd in the !uri_in path,
so that was added. To just make it clearer on usage the else
path became the call to virURIFree() although I suppose technically
it didn't have to since it would be a call using (NULL)
John Ferlan [Wed, 27 Aug 2014 18:57:08 +0000 (14:57 -0400)]
qemu_capabilities: Resolve Coverity RESOURCE_LEAK
Coverity determined that on error path that 'mach' wouldn't be free'd
Since virCapabilitiesFreeGuestMachine() isn't globally available, we'll
insert first and then if the VIR_STRDUP's fail they it will eventually
cause the 'mach' to be freed in the error path
John Ferlan [Wed, 27 Aug 2014 18:42:41 +0000 (14:42 -0400)]
qemu_agent: Resolve Coverity RESOURCE_LEAK
Coverity found that on error paths, the 'arg' value wasn't be cleaned
up. Followed the example in qemuAgentSetVCPUs() where upon successful call
to qemuAgentCommand() the 'cpus' is set to NULL; otherwise, when cleanup
occurs the free the memory for 'arg'
John Ferlan [Wed, 27 Aug 2014 17:38:29 +0000 (13:38 -0400)]
daemon: Resolve Coverity NEGATIVE_RETURNS
In each of these cases, Coverity complains that the result count returned
on error paths would be -1 disregarding that the count and the corresponding
are "linked" together (it doesn't know that). Simple enough to check and
remove the warning
Peter Krempa [Tue, 26 Aug 2014 13:55:14 +0000 (15:55 +0200)]
virsh: Implement command to excercise the bulk stats APIs
Add "domstats" command that excercises both of the new APIs depending if
you specify a domain list or not. The output is printed as a key=value
list of the returned parameters.
Wang Rui [Thu, 28 Aug 2014 10:20:58 +0000 (18:20 +0800)]
qemu_capabilities: Resolve Coverity RESOURCE_LEAK
In function virQEMUCapsParseMachineTypesStr, VIR_STRNDUP allocates
memory for 'name' in {do,while} loop. If 'name' isn't freed before
'continue', its memory will be allocated again in the next loop.
In this case the memory allocated for 'name' in privious loop is
useless and not freed. Free it before continue this loop to fix that.
John Ferlan [Wed, 27 Aug 2014 12:51:15 +0000 (08:51 -0400)]
domain_conf: Resolve Coverity REVERSE_INULL
Coverity complains that checking for domain->def being non NULL in the
if (live) path of virDomainObjAssignDef() would be unnecessary or a
NULL deref since the call to virDomainObjIsActive() would already
dereference domain->def when checking if the def->id field was != -1.
Checked all callers to virDomainObjAssignDef() and each at some point
dereferences (vm)->def->{field} prior to calling when live is true.
John Ferlan [Wed, 27 Aug 2014 12:35:08 +0000 (08:35 -0400)]
qemu_command: Resolve Coverity REVERSE_INULL
In qemuNetworkIfaceConnect() a call to virNetDevBandwidthSet() is
made where the function prototype requires the first parameter
(net->ifname) to be non NULL. Coverity complains that the subsequent
non NULL check for net->ifname prior to the next call gets flagged as
an unnecessary check. Resolve by removing the extra check
John Ferlan [Wed, 27 Aug 2014 12:01:44 +0000 (08:01 -0400)]
domain_conf: Resolve Coverity REVERSE_INULL
In virDomainActualNetDefFormat() a call to virDomainNetGetActualType(def)
was made before a check for (!def) a few lines later. This triggered
Coverity to note the possible NULL deref. Just moving the initialization
to after the !def checks resolves the issue
John Ferlan [Wed, 27 Aug 2014 11:56:00 +0000 (07:56 -0400)]
storage_driver: Resolve Coverity REVERSE_INULL
There were two occurrances of attempting to initialize actualType by
calling virStorageSourceGetActualType(src) prior to a check if (!src)
resulting in Coverity complaining about the possible NULL dereference
in virStorageSourceGetActualType() of src.
Resolve by moving the actualType setting until after checking !src
John Ferlan [Wed, 27 Aug 2014 11:48:37 +0000 (07:48 -0400)]
xen_xm: Resolve Coverity USE_AFTER_FREE
If virDomainDiskDefFree(disk) is called in 'skipdisk:', then it's possible
to either return to skipdisk without reallocating a new disk (via the if
condition just prior) or to end the loop having deleted the disk. Since
virDomainDiskDefFree() does not pass by reference, disk isn't changed in
this context, thus the possible issue.
John Ferlan [Wed, 27 Aug 2014 11:40:02 +0000 (07:40 -0400)]
xen_common: Resolve Coverity USE_AFTER_FREE
There were two warnings in this module
If the VIR_ALLOC_N(def->serials, 1) fails, then a virDomainChrDefFree(chr)
is called and we jump to cleanup which makes the same call. Just remove
the one after VIR_ALLOC_N()
In the label "skipnic:" a virDomainNetDefFree(net) is made; however, if
in going back to the top of the loop we jump back down to skipnic for any
reason, the call will attempt to free an already freed structure since
"net" was not passed by reference to virDomainNetDefFree(). Just set
net = NULL in skipnic: to resolve the issue.
John Ferlan [Wed, 27 Aug 2014 11:32:11 +0000 (07:32 -0400)]
parallels: Resolve Coverity USE_AFTER_FREE
Coverity complains that calling virNetworkDefFree(def), then jumping
to the cleanup: label which calls virNetworkDefFree(def) could result
in a double_free. Just remove the call from the if statement.
Michal Privoznik [Fri, 22 Aug 2014 09:37:52 +0000 (11:37 +0200)]
vbox: Register per partes
Since times when vbox moved to the daemon (due to some licensing
issue) the subdrivers that vbox implements were registered, but not
opened since our generic subdrivers took priority. I've tried to fix
this in 65b7d553f39ff9 but it was not correct. Apparently moving
vbox driver registration upfront changes the default connection URI
which makes some users sad. So, this commit breaks vbox into pieces
and register vbox's network and storage drivers first, and vbox driver
then at the end. This way, the vbox driver is registered in the order
it always was, but its subdrivers are registered prior the generic
ones.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Michal Privoznik [Fri, 22 Aug 2014 09:37:51 +0000 (11:37 +0200)]
virDriverLoadModule: Honor libvirt func name tranlsation
There's this unwritten rule in libvirt that vir_function is translated
into virFunction when needed (e.g. in remote protocol definition,
python, ...). Up till now we ignored such translation in driver module
loading and did fine. Well, we didn't have any module with an
underscore in its name. But this will change in next commit. The
problem is, once an a module is dlopen()-ed, we derive register
function name from its name. So instead of "driver_subdriverRegister"
do some magic to turn that into "driverSubdriverRegister".
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Fri, 22 Aug 2014 09:37:50 +0000 (11:37 +0200)]
virdrivermoduletest: Test all the modules
Even though we kept adding new and new modules (e.g. vbox or bhyve)
the test wasn't updated. Do that now. Moreover, while it's not
crucial, it's nice to reorder test cases to match the order in which
the daemon loads the modules.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
projects / libvirt.git / log