Jim Fehlig [Thu, 15 May 2014 18:15:01 +0000 (12:15 -0600)]
security_dac: rework callback parameter passing
Currently, the DAC security driver passes callback data as
void params[2];
params[0] = mgr;
params[1] = def;
Clean this up by defining a structure for passing the callback
data. Moreover, there's no need to pass the whole virDomainDef
in the callback as the only thing needed in the callbacks is
virSecurityLabelDefPtr.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Eric Blake [Fri, 16 May 2014 17:12:20 +0000 (11:12 -0600)]
maint: fix typos related to disk name resolution
In a number of APIs, the text implied that a user might have
<target dev='xvda'/> - but common convention is to use "vda",
not "xvda". For example, virDomainGetDiskErrors was correct,
while virDomainBlockStats was confusing.
Eric Blake [Wed, 14 May 2014 22:40:33 +0000 (16:40 -0600)]
maint: prefer enum over int for virstoragefile structs
For internal structs, we might as well be type-safe and let the
compiler help us with less typing required on our part (getting
rid of casts is always nice). In trying to use enums directly,
I noticed two problems in virstoragefile.h that can't be fixed
without more invasive refactoring: virStorageSource.format is
used as more of a union of multiple enums in storage volume
code (so it has to remain an int), and virStorageSourcePoolDef
refers to pooltype whose enum is declared in src/conf, but where
src/util can't pull in headers from src/conf.
* src/util/virstoragefile.h (virStorageNetHostDef)
(virStorageSourcePoolDef, virStorageSource): Use enums instead of
int for fields of internal types.
* src/qemu/qemu_command.c (qemuParseCommandLine): Cover all values.
* src/conf/domain_conf.c (virDomainDiskSourceParse)
(virDomainDiskSourceFormat): Simplify clients.
* src/qemu/qemu_driver.c
(qemuDomainSnapshotCreateSingleDiskActive)
(qemuDomainSnapshotPrepareDiskExternalBackingInactive)
(qemuDomainSnapshotPrepareDiskExternalOverlayActive)
(qemuDomainSnapshotPrepareDiskInternal): Likewise.
Eric Blake [Wed, 14 May 2014 19:48:15 +0000 (13:48 -0600)]
maint: shorten 'TypeType' function names
The VIR_ENUM_DECL/VIR_ENUM_IMPL helper macros already append 'Type'
to the enum name being converted; it looks silly to have functions
with 'TypeType' in their name. Even though some of our enums have
to have a 'Type' suffix, the corresponding string conversion
functions do not.
Eric Blake [Wed, 14 May 2014 19:36:56 +0000 (13:36 -0600)]
maint: use enum typedef for virstorageencryption.h
Continuing the work of consistent enum cleanups; this time in
virstorageencryption.h.
* src/util/virstorageencryption.h (virStorageEncryptionFormat):
Convert to typedef, renaming to avoid collision with function.
(virStorageEncryptionSecret, virStorageEncryption): Directly use
enums.
One caveat though, qemu-ga is expecting time and returning time
in nanoseconds. With all the buffering and propagation delay, the
time is already wrong once it gets to the qemu-ga, but there's
nothing we can do about it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
These APIs allow users to get or set time in a domain, which may come
handy if the domain has been resumed just recently and NTP is not
configured or hasn't kicked in yet and the guest is running
something time critical. In addition, NTP may refuse to re-set the clock
if the skew is too big.
In addition, new ACL attribute is introduced 'set_time'.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Jiri Denemark [Thu, 15 May 2014 11:11:12 +0000 (13:11 +0200)]
qemu: Avoid leak in qemuDomainCheckRemoveOptionalDisk
Coverity complains about event being leaked in
qemuDomainCheckRemoveOptionalDisk. The best fix for it is to remove the
disk directly since we already know its index.
Li Yang [Wed, 14 May 2014 06:15:37 +0000 (02:15 -0400)]
virsh: reject undefine --wipe-storage without also naming storage
For now, if only '--wipe-storage' is assigned, user can undefine a
domain normally. But actually '--wipe-storage' doesn't do anything,
and this may confuse user. Better is to require that '--wipe-storage'
only works if the user specifies volumes to be removed.
Before:
$ virsh undefine virt-tests-vm1 --wipe-storage
Domain virt-tests-vm1 has been undefined
Julio Faracco [Sun, 11 May 2014 15:08:51 +0000 (12:08 -0300)]
conf: use typedefs for enums in "src/conf/snapshot_conf.h"
In "src/conf/" there are many enumeration (enum) declarations.
Similar to the recent cleanup to "src/util" directory, it's
better to use a typedef for variable types, function types and
other usages. Other enumeration and folders will be changed to
typedef's in the future. Most of the files changed in this
commit are related to snapshot (snapshot_conf) enums.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> Signed-off-by: Eric Blake <eblake@redhat.com>
Julio Faracco [Sun, 11 May 2014 15:08:50 +0000 (12:08 -0300)]
conf: use typedefs for enums in "src/conf/storage_conf.h"
In "src/conf/" there are many enumeration (enum) declarations.
Similar to the recent cleanup to "src/util" directory, it's
better to use a typedef for variable types, function types and
other usages. Other enumeration and folders will be changed to
typedef's in the future. Most of the files changed in this
commit are related to storage (storage_conf) enums.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> Signed-off-by: Eric Blake <eblake@redhat.com>
Julio Faracco [Sun, 11 May 2014 15:08:49 +0000 (12:08 -0300)]
conf: use typedefs for enums in "src/conf/nwfilter_conf.h"
In "src/conf/" there are many enumeration (enum) declarations.
Similar to the recent cleanup to "src/util" directory, it's better
to use a typedef for variable types, function types and other
usages. Other enumeration and folders will be changed to typedef's
in the future. Most of the files changed in this commit are related
to network filter (nwfilter_conf) enums.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> Signed-off-by: Eric Blake <eblake@redhat.com>
Jiri Denemark [Tue, 13 May 2014 12:45:31 +0000 (14:45 +0200)]
qemu: Ignore temporary job errors when checking migration status
When qemu driver is polling for migration to finish (in
qemuMigrationWaitForCompletion), it may happen that another job allowed
during migration is running and if it does not finish within 30 seconds,
migration would be cancelled because of that. However, we can just
ignore the timeout and let the waiting loop try again later.
If an event fired at the end of migration is ever implemented in QEMU,
we can just wait for the event instead of polling for migration status
and libvirt will behave consistently, i.e., migration won't be cancelled
in case another job started during migration takes long time to finish.
For bug https://bugzilla.redhat.com/show_bug.cgi?id=1083238
Peter Krempa [Wed, 14 May 2014 07:43:52 +0000 (09:43 +0200)]
qemu: snapshot: Terminate job when memory compression program isn't found
If the compression program for external snapshot memory image isn't
found we exitted the function without terminating the domain job. This
caused the domain to be unusable.
If qemuDomainBlockResize() is passed a size not on a KiB boundary - that
is passed a size based in bytes (VIR_DOMAIN_BLOCK_RESIZE_BYTES), then
depending on the source format (qcow2 or qed), the value passed must
be on a sector (or 512 byte) boundary. Since other libvirt code quietly
adjusts the capacity values, then do so here as well.
Peter Krempa [Tue, 13 May 2014 09:26:28 +0000 (11:26 +0200)]
virsh: domain: Fix output of the VNC display number for domdisplay
Commit 9976c4b9a665f10ab0d2071954efb7f432d194eb broke the output for VNC
displays as the port number is converted to VNC display number by
subtracting 5900. This yields port 0 for the first display and thus the
output would be skipped.
Before:
$ virsh domdisplay VM
vnc://localhost
After:
$ tools/virsh domdisplay VM
vnc://localhost:0
When a domain was started without registration in sanlock, but libvirt
was restarted after that, most of the operations failed due to
contacting sanlock about that process. E.g. migration could not be
performed because the locks couldn't be released (or inquired before a
release).
Michal Privoznik [Mon, 12 May 2014 12:23:18 +0000 (14:23 +0200)]
apibuild: Disallow 'returns' return description
Our documentation generator is a bit messy, to say the least. For
instance, the description to return values of a function is
searched within C comment. Currently, all lines that start with
'returns' or 'Returns' are viewed as return value description.
However, there are some valid uses where the 'returns' word is in
the middle of a sentence describing function behavior not the
return value. And there are no places where 'returns' is used to
describe return values. For instance:
virDomainDetachDeviceFlags, virConnectNetworkEventRegisterAny and
virDomainGetDiskErrors. This leads to HTML documemtation being
generated incorrectly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Julio Faracco [Sun, 11 May 2014 15:08:48 +0000 (12:08 -0300)]
conf: use typedefs for enums in node_device_conf, nwfilter_params
In "src/conf/" there are many enumeration (enum) declarations. Similar
to the recent cleanup to "src/util" directory, it's better to use a
typedef for variable types, function types and other usages. Other
enumeration and folders will be changed to typedef's in the future.
Most of the files changed in this commit are reltaed to Node and
Network (node_device_conf.h and nwfilter_params.*) enums.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> Signed-off-by: Eric Blake <eblake@redhat.com>
James Shubin [Sun, 11 May 2014 04:24:10 +0000 (00:24 -0400)]
docs: mention vagrant-libvirt in apps.html
Doc patch for apps.html as per: http://libvirt.org/apps.html#add
Disclaimer: I've contributed patches to the project that this commit
adds.
Vagrant-Libvirt is an excellent way to use vagrant with libvirt. This
way you can benefit from the vagrant features, while not loosing access
to the familiar (and useful) tools such as virsh and virt-manager.
Chunyan Liu [Thu, 8 May 2014 06:44:05 +0000 (14:44 +0800)]
update documentation of <interface type='hostdev'>
<interface type='hostdev' managed='yes'> is supported, but
nowhere mentions 'managed' in <interface type='hostdev'> syntax.
Update documentation to cover it.
Chunyan Liu [Thu, 8 May 2014 06:44:04 +0000 (14:44 +0800)]
libxl: fix support for <interface type="hostdev"> syntax
A VIR_DOMAIN_NET_TYPE_HOSTDEV interface device is really a hostdev
device, which is created by the libxl driver in libxlMakePCIList().
There is no need to create a libxl_device_nic for such hostdev
devices, so skip interfaces of type VIR_DOMAIN_NET_TYPE_HOSTDEV in
libxlMakeNicList().
Oleg Strikov [Wed, 7 May 2014 16:38:10 +0000 (20:38 +0400)]
qemu: Implement a stub cpuArchDriver.compare() handler for arm and aarch64
Libvirt calls cpuArchDriver.compare() while doing guest migration.
We don't have any logic to distinguish between different arm and
aarch64 models that's why this patch allows migration to any host.
Since the ESX storage implements VMFS and iSCSI storage backends and
chooses relevant backend dynamically at runtime, there was a segfault
when issuing vol-info on iSCSI volume due to unimplemented
virStorageGetInfo function. This patch implements that function that was
missing in iSCSI backend and returns expected result without a segfault.
Refactoring in commit id '0c2305b3' resulted in the wrong storage
volume object being passed to the new storageVolDeleteInternal().
It should have passed 'voldef' which is the address found in the
pool->volumes.objs[i] array. By passing 'voldef', the DeleteInternal
code will find and remove the voldef from the volumes.objs[] list.
Add functions parallelsIsAlive, parallelsIsEncrypted,
parallelsIsSecure which are very simple to implement, but
may be required by some libvirt users. Almost all other
drivers have these functions.
There is a problem with function parallelsDomainDefineXML. If we
are defining a new domain, then we need to do 2 things: aclually
create a VM in PCS and add new domain to the cached list of domains
_parallelsConn.domains.
This is done in the function parallelsLoadDomains. So call to
virDomainObjListAdd will return a error, because a domain
with the same name and id will already be in the list.
parallels: don't enable VNC when we define a new domain
I added this code year ago, instead of implementing ability
to change VNC configuration, which was not trivial, I added
extra call to prlctl, which sets up VNC with auto port, despite
VNC configuration given by a user.
Let's remove this hack, because, first, it doesn't work on the
latest Parallels Cloud Server release (you have to either specify
--vnc-nopasswd option or password). And also has problem with
error handling. If second call to prlctl fails, VM, created by
first call to prlctl, will not be removed.
virDomainDef.features became an array, so now we can't simply
compare one features variable to another. We need to compare
each each element from the array.
If qemuDomainBlockResize() is passed a size not on a KiB boundary - that
is passed a size based in bytes (VIR_DOMAIN_BLOCK_RESIZE_BYTES), then
depending on the source format (qcow2 or qed), the value passed must
be on a sector (or 512 byte) boundary. Since other libvirt code quietly
adjusts the capacity values, then do so here as well - of course ensuring
that adjustment still fits.
Ján Tomko [Wed, 9 Apr 2014 13:23:45 +0000 (15:23 +0200)]
Add support for timestamping QEMU logs
QEMU commit 5e2ac51 added a boolean '-msg timestamp=[on|off]'
option, which can enable timestamps on errors:
$ qemu-system-x86_64 -msg timestamp=on zghhdorf
2014-04-09T13:25:46.779484Z qemu-system-x86_64: -msg timestamp=on: could
not open disk image zghhdorf: Could not open 'zghhdorf': No such file or
directory
Enable this timestamp if the QEMU binary supports it.
Add a 'log_timestamp' option to qemu.conf for disabling this behavior.
Tomoki Sekiyama [Fri, 2 May 2014 00:06:01 +0000 (20:06 -0400)]
qemu: track quiesced status in qemuDomainSnapshotFSFreeze
Adds 'quiesced' status into qemuDomainObjPrivate that tracks whether
FSFreeze is requested in the domain.
It modifies error code from qemuDomainSnapshotFSFreeze and
qemuDomainSnapshotFSThaw, so that a caller can know whether the command is
actually sent to the guest agent. If the error is caused before sending a
freeze command, a counterpart thaw command shouldn't be sent either, not to
confuse fsfreeze status tracking.
Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com> Signed-off-by: Eric Blake <eblake@redhat.com>
Tomoki Sekiyama [Fri, 2 May 2014 00:05:54 +0000 (20:05 -0400)]
remote: Implement virDomainFSFreeze and virDomainFSThaw
New rules are added in fixup_name in gendispatch.pl to keep the name
FSFreeze and FSThaw. This adds a new ACL permission 'fs_freeze',
which is also applied to VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE flag.
Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com> Acked-by: Daniel P. Berrange <berrange@redhat.com> Signed-off-by: Eric Blake <eblake@redhat.com>
Tomoki Sekiyama [Fri, 2 May 2014 00:05:48 +0000 (20:05 -0400)]
Introduce virDomainFSFreeze() and virDomainFSThaw() public API
These will freeze and thaw filesystems within guest specified by
@mountpoints parameters. The parameters can be NULL and 0, then all
mounted filesystems are frozen or thawed. @flags parameter, which are
currently not used, is for future extensions.
Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com> Signed-off-by: Eric Blake <eblake@redhat.com>
Some CDROM devices are reported by udev to have an ID_TYPE="generic"
thus it is necessary to check if ID_CDROM is present.
As a side effect, treating ID_TYPE="generic" as a missing ID_TYPE will
enable checks for ID_DRIVE_FLASH_SD and ID_DRIVE_FLOPPY and the
udevKludgeStorageType heuristic.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> Signed-off-by: Eric Blake <eblake@redhat.com>
LSN-2014-0003: Don't expand entities when parsing XML
If the XML_PARSE_NOENT flag is passed to libxml2, then any
entities in the input document will be fully expanded. This
allows the user to read arbitrary files on the host machine
by creating an entity pointing to a local file. Removing
the XML_PARSE_NOENT flag means that any entities are left
unchanged by the parser, or expanded to "" by the XPath
APIs.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Laine Stump [Wed, 30 Apr 2014 11:32:19 +0000 (14:32 +0300)]
qemu: specify domain in host-side PCI addresses when needed/supported
This uses the new QEMU_CAPS_HOST_PCI_MULTIDOMAIN capability when
present, for -devivce pci-assign, -device vfio-pci, and -pcidevice.
While creating tests for this new functionality, I noticed that the
xmls for two existing tests had erroneously specified an
until-now-ignored domain="0x0002", so I corrected those two tests, and
also added two failure tests to be sure that we alert users who
attempt to use a non-zero domain with a qemu that doesn't support it.
Laine Stump [Tue, 29 Apr 2014 15:11:45 +0000 (18:11 +0300)]
qemu: add host-pci-multidomain capability
Quite a long time ago, (apparently between qemu 0.12 and 0.13) qemu
quietly began supporting the optional specification of a domain in the
host-side address of all pci passthrough commands (by simply
prepending it to the bus:slot.function format, as
"dddd:bb:ss.f"). Since machines with multiple PCI domains are very
rare, this never came up in practice, so libvirt was never updated to
support it.
This patch takes the first step to supporting specification of a non-0
domain in the host-side address of PCI devices being assigned to a
domain, by adding a capability bit to indicate support
"QEMU_CAPS_HOST_PCI_MULTIDOMAIN", and detect it. Since this support
was added in a version prior to the minimum version required for
QMP-style capabilities detection, the capability is always enabled for
any qemu that uses QMP for capabilities detection. For older qemus,
the only clue that a domain can be specified in the host pci address
is the presence of the string "[seg:]" in the help string for
-pcidevice. (Ironically, libvirt will not be modified to support
specification of domain for -pcidevice, since any qemu new enough for
us to care about also supports "-device pci-assign" or "-device
vfio-pci", which are greatly preferred).
Michal Privoznik [Wed, 16 Apr 2014 13:16:20 +0000 (15:16 +0200)]
storageVolCreateXMLFrom: Allow multiple accesses to origvol
When creating a new volume, it is possible to copy data into it from
another already existing volume (referred to as @origvol). Obviously,
the read-only access to @origvol is required, which is thread safe
(probably not performance-wise though). However, with current code
both @newvol and @origvol are marked as building for the time of
copying data from the @origvol to @newvol. The rationale behind
is to disallow some operations on both @origvol and @newvol, e.g.
vol-wipe, vol-delete, vol-download. While it makes sense to not allow
such operations on partly copied mirror, but it doesn't make sense to
disallow vol-create or vol-download on the source (@origvol).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Ján Tomko [Tue, 6 May 2014 07:14:05 +0000 (09:14 +0200)]
Fix build wihout macvtap or virtualport
Commit 1b14c44 broke the build on FreeBSD by changing
the signature of a few functions without updating the
corresponding stubs that are used when WITH_MACVTAP
or WITH_VIRTUALPORT is not defined.
Julio Faracco [Mon, 28 Apr 2014 00:15:22 +0000 (21:15 -0300)]
conf: use typedefs for enums in "src/conf/{network,interface}_conf.h"
In "src/conf/" there are many enumeration (enum) declarations.
Similar to the recent cleanup to "src/util" directory, it's
better to use a typedef for variable types, function types and
other usages. Other enumeration and folders will be changed to
typedef's in the future. Most of the files changed in this commit
are reltaed to Network (network_conf.* and interface_conf.*) enums.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> Signed-off-by: Eric Blake <eblake@redhat.com>
Julio Faracco [Mon, 28 Apr 2014 00:15:21 +0000 (21:15 -0300)]
conf: use typedefs for enums in "src/conf/cpu_conf.h"
In "src/conf/" there are many enumeration (enum) declarations.
Similar to the recent cleanup to "src/util" directory, it's
better to use a typedef for variable types, function types and
other usages. Other enumeration and folders will be changed to
typedef's in the future. Most of the files changed in this commit
are related to CPU (cpu_conf) enums.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> Signed-off-by: Eric Blake <eblake@redhat.com>
Julio Faracco [Sun, 27 Apr 2014 00:15:22 +0000 (21:15 -0300)]
util: use typedefs for enums in "src/util/" directory
In "src/util/" there are many enumeration (enum) declarations.
Sometimes, it's better using a typedef for variable types,
function types and other usages. Other enumeration will be
changed to typedef's in the future.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> Signed-off-by: Eric Blake <eblake@redhat.com>
Eric Blake [Tue, 29 Apr 2014 03:40:22 +0000 (21:40 -0600)]
conf: drop extra storage probe
All callers of virStorageFileGetMetadataFromBuf were first calling
virStorageFileProbeFormatFromBuf, to learn what format to pass in.
But this function is already wired to do the exact same probe if
the incoming format is VIR_STORAGE_FILE_AUTO, so it's simpler to
just refactor the probing into the central function.
* src/util/virstoragefile.h (virStorageFileGetMetadataFromBuf):
Drop parameter.
(virStorageFileProbeFormatFromBuf): Drop declaration.
* src/util/virstoragefile.c (virStorageFileGetMetadataFromBuf):
Do probe here instead of in callers.
(virStorageFileProbeFormatFromBuf): Make static.
* src/libvirt_private.syms (virstoragefile.h): Drop function.
* src/storage/storage_backend_fs.c (virStorageBackendProbeTarget):
Update caller.
* src/storage/storage_backend_gluster.c
(virStorageBackendGlusterRefreshVol): Likewise.
Add a helper function virBhyveGetDomainTotalCpuStats() to
obtain process CPU time using kvm (kernel memory interface)
and use it to set cpuTime field of the virDomainInfo struct in
bhyveDomainGetInfo().
This resulted in a difference in how 'virsh vol-info --pool <poolName>
<volume>' or 'virsh vol-list vol-list --pool <poolName> --details' outputs
the capacity information for a directory pool with a qcow2 sparse file.
Results in listing a Capacity value. Prior to the commit, the value would
be '1.0 MiB' (1048576 bytes). However, after the commit the output would be
(for example) '192.50 KiB', which for my system was the size of the volume
in my file system (eg 'ls -l TestPool/temp_vol_1' results in '197120' bytes
or 192.50 KiB). While perhaps technically correct, it's not necessarily
what the user expected (certainly virt-test didn't expect it).
This patch restores the code to not update the target capacity for this path
gnutls-3.3.0 and newer leaves 2 FDs open in order to be backwards
compatible when it comes to chrooted binaries [1]. Linking
commandhelper with gnutls then leaves these two FDs open and
commandtest fails thanks to that. This patch does not link
commandhelper with libvirt.la, but rather only the utilities making
the test pass.
Ján Tomko [Fri, 2 May 2014 07:37:34 +0000 (09:37 +0200)]
fix build with older gcc
Older gcc (4.1.2-55.el5, 4.2.1 on FreeBSD) reports bogus warnings:
../../src/conf/nwfilter_conf.c:2111: warning: 'protocol' may be used
uninitialized in this function
../../src/conf/nwfilter_conf.c:2110: warning: 'dataProtocolID' may be
used uninitialized in this function
Initialize them to NULL to make the compiler happy.
Eric Blake [Thu, 1 May 2014 02:17:42 +0000 (20:17 -0600)]
storage: reject negative indices
Commit f22b7899 stumbled across a difference between 32-bit and
64-bit platforms when parsing "-1" as an int. Now that we've
fixed that difference, it's time to fix the testsuite.
* src/util/virstoragefile.c (virStorageFileParseChainIndex):
Require a positive index.
Eric Blake [Thu, 1 May 2014 02:11:09 +0000 (20:11 -0600)]
util: new stricter unsigned int parsing
strtoul() is required to parse negative numbers as their
twos-complement positive counterpart. But sometimes we want
to reject negative numbers. Add new functions to do this.
The 'p' suffix is a mnemonic for 'positive' (technically it
also parses 0, but 'non-negative' doesn't lend itself to a
nice one-letter suffix).
* src/util/virstring.h (virStrToLong_uip, virStrToLong_ulp)
(virStrToLong_ullp): New prototypes.
* src/util/virstring.c (virStrToLong_uip, virStrToLong_ulp)
(virStrToLong_ullp): New functions.
* src/libvirt_private.syms (virstring.h): Export them.
* tests/virstringtest.c (testStringToLong): Test them.
Eric Blake [Wed, 30 Apr 2014 20:46:18 +0000 (14:46 -0600)]
util: fix uint parsing on 64-bit platforms
Commit f22b7899 called to light a long-standing latent bug: the
behavior of virStrToLong_ui was different on 32-bit platforms
than on 64-bit platforms. Curse you, C type promotion and
narrowing rules, and strtoul specification. POSIX says that for
a 32-bit long, strtol handles only 2^32 values [LONG_MIN to
LONG_MAX] while strtoul handles 2^33 - 1 values [-ULONG_MAX to
ULONG_MAX] with twos-complement wraparound for negatives. Thus,
parsing -1 as unsigned long produces ULONG_MAX, rather than a
range error. We WANT[1] this same shortcut for turning -1 into
UINT_MAX when parsing to int; and get it for free with 32-bit
long. But with 64-bit long, ULONG_MAX is outside the range
of int and we were rejecting it as invalid; meanwhile, we were
silently treating -18446744073709551615 as 1 even though it
textually exceeds INT_MIN. Too bad there's not a strtoui() in
libc that does guaranteed parsing to int, regardless of the size
of long.
The bug has been latent since 2007, introduced by Jim Meyering
in commit 5d25419 in the attempt to eradicate unsafe use of
strto[u]l when parsing ints and longs. How embarrassing that we
are only discovering it now - so I'm adding a testsuite to ensure
that it covers all the corner cases we care about.
[1] Ideally, we really want the caller to be able to choose whether
to allow negative numbers to wrap around to their 2s-complement
counterpart, as in strtoul, or to force a stricter input range
of [0 to UINT_MAX] by rejecting negative signs; this will be added
in a later patch for all three int types.
This patch is tested on both 32- and 64-bit; the enhanced
virstringtest passes on both platforms, while virstoragetest now
reliably fails on both platforms instead of just 32-bit platforms.
That test will be fixed later.
* src/util/virstring.c (virStrToLong_ui): Ensure same behavior
regardless of platform long size.
* tests/virstringtest.c (testStringToLong): New function.
(mymain): Comprehensively test string to long parsing.
A couple of places in the QEMU XML -> ARGV conversion code
raised an error but then forgot to return an error status
due to missing gotos. While fixing this also tweak style
of a couple of other error reports
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Laine Stump [Thu, 1 May 2014 08:40:41 +0000 (11:40 +0300)]
qemu: fix crash when removing <filterref> from interface with update-device
If a domain network interface that contains a <filterref> is modified
"live" using "virsh update-device --live", libvirtd would crash. This
was because the code supporting live update of an interface's
filterref was assuming that a filterref might be added or modified,
but didn't account for removing the filterref, resulting in a null
dereference of the filter name.
Introduced with commit 258fb278, which was first in libvirt v1.0.1.
This addresses https://bugzilla.redhat.com/show_bug.cgi?id=1093301
Peter Krempa [Sat, 26 Apr 2014 06:27:58 +0000 (08:27 +0200)]
storage: Clear all data allocated about backing store before reparsing
To avoid memory leak of the "backingStoreRaw" field when reparsing
backing chains a new function is being introduced by this patch that
shall be used to clear backing store information.
Well, libvirt doesn't distinguish between domain poweroff and
hibernation (S4). It's hard to differentiate these two on a real
machine anyway. As a result, any device that is hot(un-)plugged is
lost (appears again) when domain is started again as from our POV
it is a fresh cold boot. Instead of doing anything wise here, we
should just document this as known limitation.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Stefan Berger [Wed, 30 Apr 2014 15:41:18 +0000 (11:41 -0400)]
nwfilter: Validate rule after parsing
An IP or IPv6 rule with port specification but without protocol
specification cannot be instantiated by ebtables. The documentation
points to 'protocol' being required but implementation does not
enforce it to be given.
Implement a rule validation function that checks whether the rule is
valid when it is defined. This for example prevents the definition
of rules like:
<ip dstportstart='53'>
where a protocol attribute would be required for it to be valid and for
ebtables to be able to instantiate it. A valid rule then is:
<ip protocol='udp' dstportstart='53'>
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>