https://bugzilla.redhat.com/show_bug.cgi?id=1128751
There's this <driver/> element under <interface/> which can have
several attributes. However, the driver element is currently formated
only if the driver's name or txmode has been specified. This makes
only a little sense as we parse even partial <driver/>, for instance:
<interface type='user'>
<mac address='52:54:00:e5:48:58'/>
<model type='virtio'/>
<driver ioeventfd='on' event_idx='on' queues='5'/>
</interface>
But such XML would never get formatted back.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Commit 4cf53158 tried to set up unique labels per disk in the
example, but ended up choosing strings that don't correspond
to the usual choice of bus types. Tweak the strings once again.
* docs/formatdomain.html.in: Use preferred names.
Signed-off-by: Eric Blake <eblake@redhat.com>
When a network is defined with "<pf dev='xyz'/>", libvirt will query
sysfs to learn the list of all virtual functions (VF) associated with
that Physical Function (PF) then populate the network's interface pool
accordingly. This action was previously done only when the first guest
actually requested an interface from the network. This patch changes
it to populate the pool immediately when the network is started. This
way any problems with the PF or its VFs will become apparent sooner.
Note that we can't remove the old calls to networkCreateInterfacePool
that happen whenever a guest requests an interface - doing so would be
asking for failures on hosts that had libvirt upgraded with a network
that had been started but not yet used.
This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1047818
networkCreateInterfacePool was a bit loose in its error cleanup, which
could result in a network definition with interfaces in the pool that
were NULL. This would in turn lead to a libvirtd crash when a guest
tried to attach an interface using the network with that pool.
In particular this would happen when creating a pool to be used for
macvtap connections. macvtap needs the netdev name of the virtual
function in order to use it, and each VF only has a netdev name if it
is currently bound to a network driver. If one of the VFs of a PF
happened to be bound to the pci-stub or vfio-pci driver (indicating
it's already in use for PCI passthrough), or no driver at all, it
would have no name. In this case networkCreateInterfacePool would
return an error, but would leave the netdef->forward.nifs set to the
total number of VFs in the PF. The interface attach that triggered
calling of networkCreateInterfacePool (it uses a "lazy fill" strategy)
would simply fail, but the very next attempt to attach an interface
using the same network pool would result in a crash.
This patch refactors networkCreateInterfacePool to bring it more in
line with current coding practices (label name, use of a switch with
no default case) as well as providing the following two changes to
behavior:
1) If a VF with no netdev name is encountered, just log a warning and
continue; only fail if exactly 0 devices are found to put in the pool.
2) If the function fails, clean up any partial interface pool and set
netdef->forward.nifs to 0.
This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1111455
Otherwise we fail like
libvirt version: 1.2.7, package: 6 (root 2014-08-08-16:09:22 bogon)
virAuditOpen:62 : Unable to initialize audit layer: Protocol not supported
virFileGetDefaultHugepageSize:2958 : internal error: Unable to parse /proc/meminfo
virStateInitialize:749 : Initialization of QEMU state driver failed: internal error: Unable to parse /proc/meminfo
daemonRunStateInit:922 : Driver state initialization failed
if the data can't be determined.
Reference: http://bugs.debian.org/757609
This fixes compilation on kFreeBSD which otherwise fails like
CC util/libvirt_util_la-virprocess.lo
In file included from /usr/include/sys/cpuset.h:35:0,
from util/virprocess.c:43:
/usr/include/sys/_cpuset.h:49:43: error: 'NBBY' undeclared here (not in
a function)
long __bits[howmany(CPU_SETSIZE, _NCPUBITS)];
^
In file included from util/virprocess.c:43:0:
/usr/include/sys/cpuset.h:215:12: error: unknown type name 'cpusetid_t'
int cpuset(cpusetid_t *);
^
/usr/include/sys/cpuset.h:216:30: error: expected ')' before 'id_t'
int cpuset_setid(cpuwhich_t, id_t, cpusetid_t);
^
/usr/include/sys/cpuset.h:217:42: error: expected ')' before 'id_t'
int cpuset_getid(cpulevel_t, cpuwhich_t, id_t, cpusetid_t *);
^
/usr/include/sys/cpuset.h:218:48: error: expected ')' before 'id_t'
int cpuset_getaffinity(cpulevel_t, cpuwhich_t, id_t, size_t, cpuset_t
*);
^
/usr/include/sys/cpuset.h:219:48: error: expected ')' before 'id_t'
int cpuset_setaffinity(cpulevel_t, cpuwhich_t, id_t, size_t, const
cpuset_t *);
And it's the correct usage as documented in
http://www.freebsd.org/cgi/man.cgi?query=cpuset_setid
Also change the #ifdef HAVE_BSH_CPU_AFFINITY to #if for consistency.
A command to freeze a part of mounted file systems is implemented in
upstream QEMU-guest-agent with a name of 'guest-fsfreeze-freeze-list'.
This fixes the name of the command used to partial fsfreeze in qemu driver
when 'mountpoints' option is specified to virDomainFSFreeze API.
Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com>
The virDomainSetInterfaceParameters implementation in qemu over
VIR_DOMAIN_AFFECT_CONFIG doesn't work as expected. When trying to
clear out the bandwidth settings for an interface, it has no
actual effect:
virsh # domiftune --config $domain $interface
inbound.average: 100
inbound.peak : 0
inbound.burst : 0
outbound.average: 10
outbound.peak : 0
outbound.burst : 0
virsh domiftune --config $domain $interface 0 0
virsh # domiftune --config $domain $interface
inbound.average: 100
inbound.peak : 0
inbound.burst : 0
outbound.average: 10
outbound.peak : 0
outbound.burst : 0
But according to virsh man page:
To clear inbound or outbound settings, use --inbound or
--outbound respectfully with average value of zero.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
introduce function
xenParseXMOS(virConfPtr conf,...........);
which parses the OS config instead
Signed-off-by: Kiarie Kahurani <davidkiarie4@gmail.com>
introduce function
xenParseXMGeneralMeta(virConfPtr conf, .......);
which parses general metadata instead
Signed-off-by: Kiarie Kahurani <davidkiarie4@gmail.com>
introduce function
xenParseXMDisk(virConfPtr conf, ........);
which parses xm disk config instead
Signed-off-by: Kiarie Kahurani <davidkiarie4@gmail.com>
introduce function
xenParseXMCPUFeatures(virConfPtr conf,.........);
which parses CPU features instead
Signed-off-by: Kiarie Kahurani <davidkiarie4@gmail.com>
introduce function
xenParseXMEventActions(virConfPtr conf,........)
which parses events leading to certain actions
Signed-off-by: Kiarie Kahurani <davidkiarie4@gmail.com>
introduce function
xenParseXMTimeOffset(virConfPtr conf,.......);
which parses time offset config instead
Signed-off-by: Kiarie Kahurani <davidkiarie4@gmail.com>
During review of the iSCSI hostdev series, eblake noted that the
prototypes shouldn't have the extranenous space between the "*" and
the function name:
http://www.redhat.com/archives/libvir-list/2014-July/msg01227.html
Since it was more invasive than 1 or 2 lines - I said I'd send a
patch covering this once committed.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Introduce a new structure to handle an iSCSI host device based on the
existing virDomainHostdevSubsysSCSI by adding a "protocol='iscsi'" to
the <source/> element. The existing scsi_host subsystem RNG was modified
to read an optional "protocol='adapter'", although it won't be written
out nor is it documented as an option (by choice).
The new hostdev structure mimics the existing <disk/> element for an
iSCSI device (network) device. New XML is:
<hostdev mode='subsystem' type='scsi' managed='yes'>
<source protocol='iscsi' name='iqn.1992-01.com.example'>
<host name='example.org' port='3260'/>
<auth username='myname'>
<secret type='iscsi' usage='mycluster_myname'/>
</auth>
</source>
<address type='drive' controller='0' bus='0' target='2' unit='5'/>
</hostdev>
The controller element will mimic the existing scsi_host code insomuch
as when 'lsi' and 'virtio-scsi' are used.
In preparation for hostdev support for iSCSI and a virStorageNetHostDefPtr,
split out the network disk storage parsing of the 'host' element into a
separate routine.
Commit febf84c2 tried to delay in-memory modification of the actual
domain disk structure until after the qemu event was received.
However, I missed that the code for block pivot had been temporarily
setting disk->src = disk->mirror prior to the qemu command, in order
to label the backing chain of a reused external blockcopy disk;
and calls into qemu while still in that state before finally undoing
things at the cleanup label. Since the qemu event handler then does:
virStorageSourceFree(disk->src);
disk->src = disk->mirror;
we have the sad race that a fast enough qemu event can cause a leak of
the original disk->src, as well as a use-after-free of the disk->mirror
contents, bad enough to crash libvirtd in some of my test runs, even
though the common case of the qemu event being much later won't trip
the race.
I'll go wear the brown paper bag of shame, for introducing a crasher
in between rc1 and rc2 of the freeze for 1.2.7 :( My only
consolation is that virDomainBlockJobAbort requires the domain:write
ACL, so it is not a CVE.
The valgrind report when the race occurs looks like:
==25612== Invalid read of size 4
==25612== at 0x50E7C90: virStorageSourceGetActualType (virstoragefile.c:1948)
==25612== by 0x209C0B18: qemuDomainDetermineDiskChain (qemu_domain.c:2473)
==25612== by 0x209D7F6A: qemuProcessHandleBlockJob (qemu_process.c:1087)
==25612== by 0x209F40C9: qemuMonitorEmitBlockJob (qemu_monitor.c:1357)
...
==25612== Address 0xe4b5610 is 0 bytes inside a block of size 200 free'd
==25612== at 0x4A07577: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==25612== by 0x50839E9: virFree (viralloc.c:582)
==25612== by 0x50E7E51: virStorageSourceFree (virstoragefile.c:2015)
==25612== by 0x209D7EFF: qemuProcessHandleBlockJob (qemu_process.c:1073)
==25612== by 0x209F40C9: qemuMonitorEmitBlockJob (qemu_monitor.c:1357)
* src/qemu/qemu_driver.c (qemuDomainBlockPivot): Don't corrupt
disk->src, and only label chain for blockcopy.
Signed-off-by: Eric Blake <eblake@redhat.com>
Valgrind caught a memory leak:
==2018== 9 bytes in 1 blocks are definitely lost in loss record 143 of 927
==2018== at 0x4A0645D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==2018== by 0x8C42369: strdup (strdup.c:42)
==2018== by 0x50EACC9: virStrdup (virstring.c:676)
==2018== by 0x50E79E5: virStorageSourceCopy (virstoragefile.c:1845)
==2018== by 0x20A3FAA7: qemuDomainBlockCommit (qemu_driver.c:15620)
==2018== by 0x51DC6B2: virDomainBlockCommit (libvirt.c:20092)
I traced it to the fact that blockcopy and blockcommit end up
reparsing a backing chain on pivot, but the chain parsing code
doesn't gracefully handle the case where the backing file is
already known.
I'm not exactly sure when this was introduced, but suspect that the
refactoring in commit 9944b71 and friends that moved towards probing
in-place rather than into a temporary structure are part of the cause.
* src/util/virstoragefile.c (virStorageFileGetMetadataInternal):
Don't leak any prior value.
Signed-off-by: Eric Blake <eblake@redhat.com>
Jiri Moskovcak reported on IRC that the documentation on valid
<disk> was confusing because it didn't have unique dev='...'
entries.
* docs/formatdomain.html.in: Use unique names.
Signed-off-by: Eric Blake <eblake@redhat.com>
This makes the paragaph about attach-interface more descriptive and
correct, adding in a few bits of information that were previously
missing, e.g. --script is only allowed for bridge interfaces of Xen
domains, target name is regenerated if it starts with vnet, mac
address will be autogenerated if not specified.
(I did this in response to an email asking why a script couldn't be
specified for a bridge interface of a qemu domain, and why an
interface of type='ethernet' couldn't be created with
attach-interface)
Fix a comment in virDomainAuditNetDevice.
Fix a typo in comment of qemuPhysIfaceConnect which is
the caller of virDomainAuditNetDevice.
Signed-off-by: Wang Rui <moon.wangrui@huawei.com>
RNG schema as well as the qemu driver requires absolute paths for memory
and disk snapshot image files but the XML parser was not enforcing it.
Add checks to avoid problems in qemu where the configuration it creates
is invalid.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1126329
Since commit be0782e1 we are parsing /proc/meminfo to find out the
default huge page size. However, if the host we are running at does
not support any huge pages (e.g. CONFIG_HUGETLB_PAGE is turned off),
we will not successfully parse the meminfo file and hence the whole
qemu driver init process fails. Moreover, the default huge page size
is needed if and only if there's at least one hugetlbfs mount point.
So the fix consists of moving the virFileGetDefaultHugepageSize
function call after the first hugetlbfs mount point is found.
With this fix, we fail to start with one or more hugetlbfs mounts and
malformed meminfo file, but that's expected (how can one mount
hugetlbfs without kernel supporting huge pages?). Workaround in that
case is to umount all the hugetlbfs mounts.
Reported-by: Jim Fehlig <jfehlig@suse.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
In a system with Fiber Channel Host Adapters, a query to list all Fibre Channel
HBAs OR Vports currently returns empty list:
$ virsh nodedev-list --cap fc_host
$
Libvirt correctly discovers properties for all HBAs. However, the reporting
fails because of incorrect flag comparison while filtering these types.
This is fixed by removing references to 'VIR_CONNECT_LIST_NODE_DEVICES_CAP_*'
for comparison and replacing those with 'VIR_NODE_DEV_CAP_*'
Introduced by original commit id '652a2ec6'
Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>
Commit 232a31b munged job info to report 'active commit' instead of
'commit' when generating events, but forgot to also munge the polling
variant of the command.
* src/qemu/qemu_driver.c (qemuDomainBlockJobImpl): Adjust type as
needed.
Signed-off-by: Eric Blake <eblake@redhat.com>
Otherwise this beautiful error would be overwritten when
the function is called with a really high rate number:
2014-07-28 12:51:47.920+0000: 2304: error : virCommandWait:2399 :
internal error: Child process (/sbin/tc class add dev vnet0 parent 1:
classid 1:1 htb rate 4294968kbps) unexpected exit status 1: Illegal "rate"
Usage: ... qdisc add ... htb [default N] [r2q N]
default minor id of class to which unclassified packets are sent {0}
r2q DRR quantums are computed as rate in Bps/r2q {10}
debug string of 16 numbers each 0-3 {0}
... class add ... htb rate R1 [burst B1] [mpu B] [overhead O]
[prio P] [slot S] [pslot PS]
[ceil R2] [cburst B2] [mtu MTU] [quantum Q]
rate rate allocated to this class (class can still borrow)
burst max bytes burst which can be accumulated during idle period {computed}
mpu minimum packet size used in rate computations
overhead per-packet size overhead used in rate computations
linklay adapting to a linklayer e.g. atm
ceil definite upper class rate (no borrows) {rate}
cburst burst but for ceil {computed}
mtu max packet size we create rate map for {1600}
prio priority of leaf; lowe
https://bugzilla.redhat.com/show_bug.cgi?id=1043735
We parse the bandwidth rates as unsinged long long,
then try to fit them in VIR_TYPED_PARAM_UINT.
Report an error if they exceed UINT_MAX instead of
quietly using wrong values.
https://bugzilla.redhat.com/show_bug.cgi?id=1043735
https://bugzilla.redhat.com/show_bug.cgi?id=1072653
Upon successful upload of a volume, the target volume and storage pool
were not updated to reflect any changes as a result of the upload. Make
use of the existing stream close callback mechanism to force a backend
pool refresh to occur in a separate thread once the stream closes. The
separate thread should avoid potential deadlocks if the refresh needed
to wait on some event from the event loop which is used to perform
the stream callback.
The variable 'k' in the print_cpu_usage function is not used anywhere
and can fire a warning on some compilers.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Currently, the function follows the usual pattern used in our code:
int ret = -1;
...
ret = 0;
cleanup:
return ret;
However, the function always call exit() on error, so the cleanup
label is never jumped onto. Therefore, it doesn't make any sense to
have the parse_argv function return an integer value, if it
effectively can return only value of zero.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
There are multiple mount points after commit 725a211f, but one comment
wasn't changed to use plurals.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Cygwin has getifaddrs(), but not AF_LINK, leading to:
util/virstats.c: In function 'virNetInterfaceStats':
util/virstats.c:138:41: error: 'AF_LINK' undeclared (first use in this function)
if (ifa->ifa_addr->sa_family != AF_LINK)
...
* src/util/virstats.c (virNetInterfaceStats): Only use getifaddrs
if AF_LINK is present.
Signed-off-by: Eric Blake <eblake@redhat.com>
libvirt previously only touched an interface's disable_ipv6 setting in
sysfs if it needed to be set to 1, assuming that 0 is the
default. Apparently that isn't always the case though (kernel 3.15.7-1
in Arch Linux reportedly defaults a new interface's disable_ipv6
setting to 1) so this patch explicitly sets it to 0 or 1 as
appropriate.
