Commit Graph

17042 Commits

Author SHA1 Message Date
Peter Krempa
b9f8a2f25e network: bridge: Avoid freeing uninitialized pointer on cleanup path
The cleanup path in networkBuildDhcpDaemonCommandLine could cause a
crash by freeing uninitialized pointer.
2014-06-17 10:03:53 +02:00
Peter Krempa
0657ed2a5c net: leaseshelper: Refactor copying of old entries to avoid double free
When copying entries from the old lease file into the new array the old
code would copy the pointer of the json object into the second array
without removing it from the first. Afterwards when both arrays were
freed this might lead to a crash due to access of already freed memory.

Refactor the code to use the new array item stealing helper added to the
json code so that the entry resides just in one array.
2014-06-17 09:10:08 +02:00
Peter Krempa
45d51681ce net: leaseshelper: Ignore corrupted lease file and rewrite it
Instead of reporting an error and terminating, rewrite the file with
the newly learned info.
2014-06-17 09:02:26 +02:00
Peter Krempa
f1385e222e net: leaseshelper: Don't crash if DNSMASQ doesn't provide lease expiry
The value is provided via environment and causes a crash if not defined.
2014-06-17 08:55:50 +02:00
Peter Krempa
5133b50372 util: json: Add helpers for manipulating json arrays
Add a checker to determine whether a JSON object is an array and a
helper to steal objects from a JSON array.
2014-06-17 08:51:41 +02:00
Peter Krempa
5429a3b648 util: json: Unify function header formatting
Use consistent formatting of function headers:
- two newlines separating functions
- function return type on separate line
- one argument per line
2014-06-17 08:48:44 +02:00
Eric Blake
b50e104923 blockjob: don't remove older-style mirror XML
Commit 7c6fc39 introduced a regression in the XML produced for older
clients.  The argument at the time was that clients shouldn't be
depending on output-only data for something that is only going to
be triggered for a transient guest; but John Ferlan reported that
the automated testsuite was such a client.  It's better to be safe
than sorry by guaranteeing back-compat cruft.  Note that later
patches will be using <mirror> for active block commit, but there
we don't have to worry about back-compat.

* src/conf/domain_conf.c (virDomainDiskDefFormat): Restore old
style output when necessary.
* docs/schemas/domaincommon.rng: Validate back-compat style.
* docs/formatdomain.html.in: Update the documentation.
* tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml:
Update tests.
* tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml: Likewise.

Signed-off-by: Eric Blake <eblake@redhat.com>
2014-06-16 13:48:00 -06:00
Eric Blake
cfe087a211 blockjob: avoid compiler uncertainty in info sizing
We have a policy of avoiding enum types in structs in our public
API, because it is possible for a client to choose compiler options
that can change the in-memory ABI of that struct based on whether
the enum value occupies an int or a minimal size.  But we missed
this for virDomainBlockJobInfo.  We got lucky on little-endian
machines - if the enum fits minimal size (a char), we still end
up padding to the next long before the next field; but on
big-endian, a client interpreting the enum as a char would always
see 0 when the server supplies contents as an int.

* include/libvirt/libvirt.h.in (virDomainBlockJobInfo): Enforce
particular sizing.

Signed-off-by: Eric Blake <eblake@redhat.com>
2014-06-16 11:13:35 -06:00
Eric Blake
3de3294de1 blockjob: document recent job addition
I noticed that the web page lacked documentation on block jobs:
http://libvirt.org/html/libvirt-libvirt.html#virDomainBlockJobType
not only for the recently added active commit, but also for all
the other job types.

* include/libvirt/libvirt.h.in (virDomainBlockJobType): Document
recent addition.

Signed-off-by: Eric Blake <eblake@redhat.com>
2014-06-16 11:11:45 -06:00
Shivaprasad G Bhat
eb104e6255 Increase the size of REMOTE_MIGRATE_COOKIE_MAX to REMOTE_STRING_MAX
During guest migration, if the domain xml is bigger than 16384 which is
easily possible for a guest with good number of disks, message encode fails
for xdr_remote_domain_migrate_perform3_ret().
So, Increase the COOKIE_MAX to STRING_MAX value.

Signed-off-by: Shivaprasad G Bhat <shivaprasadbhat@gmail.com>
2014-06-16 18:10:48 +02:00
Michal Privoznik
16ebf10f34 nodedev: Introduce <pci-express/> to PCI devices
This new element is there to represent PCI-Express capabilities
of a PCI devices, like link speed, number of lanes, etc.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2014-06-16 17:40:49 +02:00
Michal Privoznik
a22a7a5ef3 virpci: Introduce virPCIDeviceIsPCIExpress and friends
These functions will handle PCIe devices and their link capabilities
to query some info about it.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2014-06-16 17:40:49 +02:00
Eric Blake
3e3c6ff10f blockcommit: require base below top
The block commit code looks for an explicit base file relative
to the discovered top file; so for a chain of:
  base <- snap1 <- snap2 <- snap3
and a command of:
  virsh blockcommit $dom vda --base snap2 --top snap1
we got a sane message (here from libvirt 1.0.5):
error: invalid argument: could not find base 'snap2' below 'snap1' in chain for 'vda'

Meanwhile, recent refactoring has slightly reduced the quality of the
libvirt error messages, by losing the phrase 'below xyz':
error: invalid argument: could not find image 'snap2' in chain for 'snap3'

But we had a one-off, where we were not excluding the top file
itself in searching for the base; thankfully qemu still reports
the error, but the quality is worse:
  virsh blockcommit $dom vda --base snap2 --top snap2
error: internal error unable to execute QEMU command 'block-commit': Base '/snap2' not found

Fix the one-off in blockcommit by changing the semantics of name
lookup - if a starting point is specified, then the result must
be below that point, rather than including that point.  The only
other call to chain lookup was blockpull code, which was already
forcing the lookup to omit the active layer and only needs a
tweak to use the new semantics.

This also fixes the bug exposed in the testsuite, where when doing
a lookup pinned to an intermediate point in the chain, we were
unable to return the name of the parent also in the chain.

* src/util/virstoragefile.c (virStorageFileChainLookup): Change
semantics for non-NULL startFrom.
* src/qemu/qemu_driver.c (qemuDomainBlockJobImpl): Adjust caller,
to keep existing semantics.
* tests/virstoragetest.c (mymain): Adjust to expose new semantics.

Signed-off-by: Eric Blake <eblake@redhat.com>
2014-06-16 09:33:57 -06:00
Eric Blake
b10a0e9198 storage: better tests of lookup
Add some more tests of what happens when we restrict a lookup
to begin at a point in the middle of a chain.  In particular,
we want to ensure that a parent is not found when starting at
the child.  This commit also demonstrates that we have a slight
difference in behavior on what parent we report when filtering
is in effect; as the determination of the parent affects the
code in block commit, exposing this in the testsuite will help
justify changes in future patches that tweak the semantics of
what lookups are allowed.

* tests/virstoragetest.c (testStorageLookup): Test user input.
(TEST_LOOKUP_TARGET): Add parameter.
(mymain): Add lookup tests.

Signed-off-by: Eric Blake <eblake@redhat.com>
2014-06-16 09:33:57 -06:00
Eric Blake
54597c5698 storage: renumber lookup tests
The next patch will be adding tests, including adding a parameter
for testing more conditions.  For ease of review of that patch, I
want to create common context lines that don't change when the new
tests are added (it's easier to visually review additions than it
is to review an entire chunk of tests rewritten into another
larger chunk of tests).

* tests/virstoragetest.c (mymain): Add a parameter and renumber
the lookup tests.

Signed-off-by: Eric Blake <eblake@redhat.com>
2014-06-16 09:33:57 -06:00
Eric Blake
47aaceb7cc storage: add alias for less typing
Typing chain->backingStore->backingStore gets old after a while;
introduce some alias variables to make the test more compact.

* tests/virstoragetest.c (mymain): Introduce some shorthand.

Signed-off-by: Eric Blake <eblake@redhat.com>
2014-06-16 09:33:57 -06:00
Peter Krempa
b77d3d9b81 qemu: snapshot: Don't mark all block disks for metadata reuse
For block devices used as snapshot source the new snapshot code would
set the reuse flag. This inhibits to take snapshot without specially
preparing the block image before taking the snapshot.

Fortunately this is not a regression as only the new way of specifying
snapshot source is affected.

For the followin snapshot XML:
 <domainsnapshot>
   <disks>
     <disk name='vda' type='block'>
       <driver type='qcow2'/>
       <source dev="/dev/andariel/testsnap" />
     </disk>
   </disks>
 </domainsnapshot>

You'd get:
error: internal error: unable to execute QEMU command 'transaction': Image is not in qcow2 format

After this patch the snapshot is created successfully.
2014-06-16 17:22:05 +02:00
Pavel Hrdina
647bdf02d8 leaseshelper: fix another crash
We create a 'lease_new' when we are adding new lease entry, then later
in the code we add the 'lease_new' into a 'leases_array_new' which
leads into the crash because we double free the 'lease_new'.

To prevent the double free we set the 'lease_new' to NULL after
successful append into the 'leases_array_new'.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2014-06-16 14:41:19 +02:00
Roman Bogorodskiy
f477f555f0 bhyve: silent destroy command errors on cleanup
When virBhyveProcessStart() fails, it tries to unload
a guest that could have been already loaded using
bhyveload(8) to make sure not to leave it hanging in memory.

However, we could fail before loading a VM into memory,
so 'bhyvectl --destroy' command will fail and print
an error message that looks confusing to users.

So ignore errors when running this in cleanup.
2014-06-14 00:22:35 +04:00
Roman Bogorodskiy
5c1f82efae bhyve: do not cleanup unallocated networks on fail
virBhyveProcessStart() calls bhyveNetCleanup() if it fails. However,
it might fail earlier than networks are allocated, so modify
bhyveNetCleanup() to check if net->ifname is not NULL before
going further with the cleanup.
2014-06-13 20:44:36 +04:00
Roman Bogorodskiy
381a3dbdab bhyve: fix crash in bhyveBuildNetArgStr
bhyveBuildNetArgStr() calls virNetDevTapCreateInBridgePort() and
passes tapfd = NULL, but tapfdSize = 1. That is wrong, because
if virNetDevTapCreateInBridgePort() crashes after successfully
creating a TAP device, it'll jump to 'error' label, that
loops over tapfd and calls VIR_FORCE_CLOSE:

   for (i = 0; i < tapfdSize && tapfd[i] >= 0; i++)

In that case we get a segfault.

As the bhyve code doesn't use tapfd, pass NULL and set tapfdSize to 0.
2014-06-13 20:44:36 +04:00
Giuseppe Scrivano
d6e92bfa38 storage: report VIR_ERR_NO_STORAGE_VOL when the file doesn't exist
Report VIR_ERR_NO_STORAGE_VOL instead of a system error when lstat
fails because the file doesn't exist.

Fixes this problem in virt-install:

  https://bugzilla.redhat.com/show_bug.cgi?id=1108922

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
2014-06-13 10:05:33 -06:00
Roman Bogorodskiy
aad479dc4e bhyve: implement PCI address allocation
Automatically allocate PCI addresses for devices instead
of hardcoding them in the driver code. The current
allocation schema is to dedicate an entire slot for each devices.

Also, allow having arbitrary number of devices.
2014-06-13 19:25:27 +04:00
Michal Privoznik
90ba5ef1bb virNetDevGetLinkInfo: Don't report link speed if NIC's not up
The kernel's more broken than one would think. Various drivers report
various (usually spurious) values if the interface is in other state
than 'up' . While on some we experience -EINVAL when read()-ing the
speed sysfs file, with other drivers we might get anything from 0 to
UINT_MAX. If that's the case it's better to not report link speed.
Well, the interface is not up anyway.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2014-06-13 11:29:48 +02:00
Peter Krempa
51c439056b tests: virstoragetest: Fix output when hitting errors
When the test is failing but the debug output isn't enabled the
resulting line would look ugly like and would not contain the actual
difference.

TEST: virstoragetest
      .................chain member 1!chain member 1!chain member 1!

Store the member index in the actual checked string to hide this problem
2014-06-13 10:57:43 +02:00
Eric Blake
278c51af3a blockcommit: update error messages related to block jobs
A future patch will add two-phase block commit jobs; as the
mechanism for managing them is similar to managing a block copy
job, existing errors should be made generic enough to occur
for either job type.

* src/conf/domain_conf.c (virDomainHasDiskMirror): Update
comment.
* src/qemu/qemu_driver.c (qemuDomainDefineXML)
(qemuDomainSnapshotCreateXML, qemuDomainRevertToSnapshot)
(qemuDomainBlockJobImpl, qemuDomainBlockCopy): Update error
message.
* src/qemu/qemu_hotplug.c (qemuDomainDetachDiskDevice): Likewise.

Signed-off-by: Eric Blake <eblake@redhat.com>
2014-06-12 20:54:32 -06:00
Eric Blake
1784037991 virsh: improve blockcopy UI
Peter's review of an early version of my addition of active block
commit pointed out some issues that I was copying from the block
copy code; fix them up now before perpetuating them.

For virsh commands that manage a single API call, it's nice to have
a 1:1 mapping of options to flags, so that we can test that
lower-layer software handles flag combinations correctly.  But where
virsh is introducing syntactic sugar to combine multiple API calls
into a single user interface, we might as well make that interface
compact.  That is, we should allow the shorter command-line of
'blockcopy $dom $disk --pivot' without having to explicitly specify
--wait, because this isn't directly a flag passed to a single
underlying API call.

Also, my use of embedded ?: ternaries bordered on unreadable.

* tools/virsh-domain.c (cmdBlockCopy): Make --pivot, --finish,
and --timeout imply --wait. Drop excess ?: operators.
* tools/virsh.pod (blockcopy): Update documentation.

Signed-off-by: Eric Blake <eblake@redhat.com>
2014-06-12 14:41:40 -06:00
Michal Privoznik
bab4558547 virNodeDevCapPCIDevParseXML: Initialize numa_node variable
With one of my recent patches (1c70277) libvirt's capable of
reporting NUMA node locality for PCI devices. The node ID is
stored in pci_dev.numa_node variable. However, since zero is
valid NUMA node ID, the default is -1 as it is in kernel too.
So, if the PCI device is not tied to any specific NUMA node, the
default is then NOT printed into XML. Therefore, when parsing
node device XML, the <node/> element is optional. But currently,
if it's not there, we must set sane default, otherwise after
parsing in the memory representation doesn't match the XML. We
are already doing this in other place: udevProcessPCI().

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2014-06-12 17:18:29 +02:00
Eric Blake
519181d932 blockcommit: fix regression with explicit top argument
Commit f586965 accidentally changed the semantics of the
virDomainBlockCommit command; where it previously looked for
an explicit top argument from the top of the chain, it now
starts from the backing file of the top.  Of course, until
we allow active commits, the only difference it makes is in
the quality of the error message, but with code for active
commit coming soon, we need to support an explicit mention
of the active layer.

* src/qemu/qemu_driver.c (qemuDomainBlockCommit): Start looking
from top of chain.

Signed-off-by: Eric Blake <eblake@redhat.com>
2014-06-12 07:01:18 -06:00
Jincheng Miao
e430410480 virsh: forbid negative vcpu argument to vcpupin
The vcpupin command allowed specifying a negative number for the --vcpu
argument. This would the overflow when the underlying virDomainPinVcpu
API was called.

 $ virsh vcpupin r7 -1 0
 error: numerical overflow: input too large: 4294967295

Switch the vCPU variable to a unsigned int and parse it using the
corresponding function.

Also improve the vcpupin test to cover all the defects.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1101059

Signed-off-by: Jincheng Miao <jmiao@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
2014-06-12 14:06:21 +02:00
Peter Krempa
c62125395b virsh: Reject negative numbers in vshCommandOptULongLong
To follow the new semantics of the vshCommandOptToU* functions convert
this one to reject negative numbers too. To allow using -1 for "maximum"
semantics for the vol-*load two bandwidth functions that use this helper
introduce vshCommandOptULongLongWrap.
2014-06-12 14:06:21 +02:00
Peter Krempa
0e2d73051a virsh: Reject negative numbers in vshCommandOptUL
To follow the new semantics of the vshCommandOptToU* functions convert
this one to reject negative numbers too. To allow using -1 for "maximum"
semantics for the two bandwidth functions that use this helper introduce
vshCommandOptULWrap. Although currently the migrate-setspeed function
for the qemu driver will reject -1 as maximum.
2014-06-12 14:06:20 +02:00
Peter Krempa
37e663adb6 virsh: Reject negative numbers in vshCommandOptUInt
Use virStrToLong_uip instead of virStrToLong_ui to reject negative
numbers in the helper. None of the callers expects the wraparound
"feature" for negative numbers.

Also add a function that allows wrapping of negative numbers as it might
be used in the future and be explicit about the new semantics in the
function docs.
2014-06-12 14:06:20 +02:00
Ján Tomko
7eb0ee175b Fix crash when saving a domain with type none dac label
qemuDomainGetImageIds did not check if there was a label
in the seclabel, thus crashing on
<seclabel type='none' model='dac'/>

https://bugzilla.redhat.com/show_bug.cgi?id=1108590
2014-06-12 12:01:35 +02:00
John Ferlan
fdfdbc0d12 vbox_snapshot_conf: Resolve Coverity warnings
Clean up some Coverity warnings from commit id '4dc5d8f1'
2014-06-12 05:44:47 -04:00
John Ferlan
70bd552e67 vbox_temp: Resolve Coverity warnings
Clean up code to resolve Coverity RESOURCE_LEAK's from commit id's
'632b9600' and 'b739f807'.
2014-06-12 05:36:37 -04:00
Peter Krempa
cc6484d486 security: Don't skip labelling for network disks
A network disk might actually be backed by local storage. Also the path
iterator actually handles networked disks well now so remove the code
that skips the labelling in dac and selinux security driver.
2014-06-12 10:32:12 +02:00
Peter Krempa
f332ffc0dc storage: volume: Rework lookup of volume objects
Add a helper to do all the lookup steps and remove a ton of duplicated
code.
2014-06-12 10:26:56 +02:00
Peter Krempa
30d99eb534 storage: Clean up unlocking of storage pool objects
Most of the APIs now don't reach the cleanup section when the pool
object wasn't found and thus don't need to check before unlocking it.
2014-06-12 09:55:17 +02:00
Peter Krempa
efd892761e storage: pool: Fix handling of errors on pool lookup failure
Rework internal pool lookup code to avoid printing the raw UUID buffer
in the case a storage pool can't be found:

 $ virsh pool-name e012ace0-0460-5810-39ef-1bce5fa5a4dd
 error: failed to get pool 'e012ace0-0460-5810-39ef-1bce5fa5a4dd'
 error: Storage pool not found: no storage pool with matching uuid à¬à`X9ï_¥¤Ý

The rework is mostly done by switching the lookup code to the newly
introduced helper virStoragePoolObjFromStoragePool

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1104993
2014-06-12 09:55:08 +02:00
Ján Tomko
046a441f3d build: prefer -fstack-protector-strong to -all
Try -fstack-protector-strong first on Linux. If that fails,
fall back to -fstack-protector-all.
2014-06-12 08:16:03 +02:00
Ján Tomko
5567baa973 build: remove ssp-buffer-size
This option only makes sense for -fstack-protector.
With -fstack-protector-all or -fstack-protector-strong,
functions are protected regardless of buffer size.

https://bugzilla.redhat.com/show_bug.cgi?id=1105456
2014-06-12 08:16:03 +02:00
Ján Tomko
fa3519cbe5 build: remove duplicit warning suppression
These warnings have already been added to $dontwarn.
2014-06-12 08:15:54 +02:00
John Ferlan
5ca7c95549 libxl: Resolve Coverity warnings
Resolve two Coverity issues introduced by commit id '9b8d6e1e'

Signed-off-by: John Ferlan <jferlan@redhat.com>
2014-06-11 14:58:20 -04:00
Roman Bogorodskiy
aec0c6d3d5 virsh: include bhyve in virsh -V output
Add 'Bhyve' in hypervisor list reported by 'virsh -V'
if it's compiled it.
2014-06-11 22:16:09 +04:00
Pavel Hrdina
a93504cca4 leaseshelper: fix crash
Commit baafe668 introduced new leaseshelper with a crash of freeing
env string. Calling 'getenv()' inside 'virGetEnvAllowSUID()' may
return a static string and we definitely should not free it.

The author probably want to free the copy of that string.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2014-06-11 17:21:11 +02:00
Eric Blake
de43582bf5 maint: exempt graphic binaries from syntax check
Roman Bogorodskiy reported a syntax-check failure when using
FreeBSD; complaining that:

prohibit_empty_first_line
tools/libvirt_win_icon_16x16.ico:1:
tools/libvirt_win_icon_32x32.ico:1:
tools/libvirt_win_icon_48x48.ico:1:
tools/libvirt_win_icon_64x64.ico:1:
maint.mk: Prohibited empty first line

In reality, the first 'line' of that file is NOT empty; but since
it is a binary file, awk is not required to handle it gracefully.
The simplest solution is to exempt all image files from syntax
checks in the first place - after all, we only store them in git
because they are inconvenient to regenerate, but they are not our
preferred format for making modifications, and syntax check should
only cover files that we are likely to modify.

* cfg.mk (VC_LIST_ALWAYS_EXCLUDE_REGEX): Exempt images.
(exclude_file_name_regexp--sc_prohibit_empty_lines_at_EOF): Simplify.
(exclude_file_name_regexp--sc_trailing_blank): Likewise.

Signed-off-by: Eric Blake <eblake@redhat.com>
2014-06-11 08:41:37 -06:00
Shivaprasad G Bhat
edc80e2344 qemu: Properly label FDs when restoring domain with static label
When saving domain with relabel=no, the file that gets created must have the
context set anyway.  That way restore can be successful without the need of
relabelling the file.

Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com>
2014-06-11 16:33:51 +02:00
Jean-Baptiste Rouault
775bb9b15f vmware: make version parsing more robust
Since commit d69415d4, vmware version is parsed from both stdout and
stderr. This patch makes version parsing work even if there is garbage
(libvirt debug messages for example) in the command output.

Add test data for this case.
2014-06-11 14:56:18 +02:00
Michal Privoznik
3c43b6a7b2 virnetdev: Use ifname in virNetDevGetLinkInfo
If we're compiling on non-Linux platform, the virNetDevGetLinkInfo()
is a dummy function which barely logs debug message that getting link
info is not supported. However, while the debug message was prepared
for printing the interface name too, I actually forgot to pass the
variable which resulted in build error on platforms like mingw or
FreeBSD.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2014-06-11 13:05:17 +02:00