VIR_STORAGE_FILE_AUTO should be used only in xml provided to
libvirt by user, if I understood correctly. Driver should
set storage source format to specific disk format in
*DomainGetXMLDesc.
CDROMs in PCS use raw image format.
Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com>
Some of the nwfilter tests are now failing since --concurrent shows
up in the ebtables command. To avoid this, implement a function
preventing the probing for lock support in the eb/iptables tools
and use it in the tests.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
There is one problem that causes various errors in the daemon. When
domain is waiting for a job, it is unlocked while waiting on the
condition. However, if that domain is for example transient and being
removed in another API (e.g. cancelling incoming migration), it get's
unref'd. If the first call, that was waiting, fails to get the job, it
unref's the domain object, and because it was the last reference, it
causes clearing of the whole domain object. However, when finishing the
call, the domain must be unlocked, but there is no way for the API to
know whether it was cleaned or not (unless there is some ugly temporary
variable, but let's scratch that).
The root cause is that our APIs don't ref the objects they are using and
all use the implicit reference that the object has when it is in the
domain list. That reference can be removed when the API is waiting for
a job. And because each domain doesn't do its ref'ing, it results in
the ugly checking of the return value of virObjectUnref() that we have
everywhere.
This patch changes qemuDomObjFromDomain() to ref the domain (using
virDomainObjListFindByUUIDRef()) and adds qemuDomObjEndAPI() which
should be the only function in which the return value of
virObjectUnref() is checked. This makes all reference counting
deterministic and makes the code a bit clearer.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Commit 1a80b97d, which added the virCgroupHasEmptyTasks() function
forgot that the parameter @cgroup may be NULL and did not check that.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Although QMP returns info about vCPU threads in TCG mode, the
data it returns is mostly lies. Only the first vCPU has a valid
thread_id returned. The thread_id given for the other vCPUs is
in fact the main emulator thread. All vCPUs actually run under
the same thread in TCG mode.
Our vCPU pinning code is not at all able to cope with this
so if you try to set CPU affinity per-vCPU you end up with
wierd errors
error: Failed to start domain instance-00000007
error: cannot set CPU affinity on process 24365: Invalid argument
Since few people will care about the performance of TCG with
strict CPU pinning, lets just disable that for now, so we get
a clear error message
error: Failed to start domain instance-00000007
error: Requested operation is not valid: cpu affinity is not supported
The code assumes that def->vcpus == nvcpupids, so when we setup
fake CPU pids for old QEMU with nvcpupids == 1, we cause the
later code to read off the end of the array. This has fun results
like sche_setaffinity(0, ...) which changes libvirtd's own CPU
affinity, or even better sched_setaffinity($RANDOM, ...) which
changes the affinity of a random OS process.
Libvirt BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1175397
QEMU BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1170093
In qemu there are two interesting arguments:
1) -numa to create a guest NUMA node
2) -object memory-backend-{ram,file} to tell qemu which memory
region on which host's NUMA node it should allocate the guest
memory from.
Combining these two together we can instruct qemu to create a
guest NUMA node that is tied to a host NUMA node. And it works
just fine. However, depending on machine type used, there might
be some issued during migration when OVMF is enabled (see QEMU
BZ). While this truly is a QEMU bug, we can help avoiding it. The
problem lies within the memory backend objects somewhere. Having
said that, fix on our side consists on putting those objects on
the command line if and only if needed. For instance, while
previously we would construct this (in all ways correct) command
line:
-object memory-backend-ram,size=256M,id=ram-node0 \
-numa node,nodeid=0,cpus=0,memdev=ram-node0
now we create just:
-numa node,nodeid=0,cpus=0,mem=256
because the backend object is obviously not tied to any specific
host NUMA node.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
On a system with 160 CPUs the /proc/cpuinfo size grows beyond
the currently set limit of 10KB causing an internal error.
This patch increases the buffer size to 1MB.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Coverity flagged commit 0282ca45 as introducing a memory leak;
in all my refactoring to make capacity probing conditional on
whether the image is non-raw, I missed deleting the unconditional
probe.
* src/qemu/qemu_driver.c (qemuStorageLimitsRefresh): Drop
redundant assignment.
Signed-off-by: Eric Blake <eblake@redhat.com>
A recent lvm change has resulted in a change for the "default" type of
logical volume created when the "--virtualsize" or "--V" is supplied on
the command line (e.g. when the allocation and capacity values of a to
be created volume differ). It seems that at the very least the following
change adjusts the default type:
https://git.fedorahosted.org/cgit/lvm2.git/commit/?id=e0164f21
and the following may also have some impact.
https://git.fedorahosted.org/cgit/lvm2.git/commit/?id=87fc3b71
When using the virsh vol-create-as or vol-create xmlfile commands, the
result is that libvirt will now create a "thin logical volume" and a
"thin logical volume pool" rather than just a "thin snapshot logical
volume". For example the following sequence:
# lvcreate --name test -L 2M -V 5M lvm_test
Rounding up size to full physical extent 4.00 MiB
Rounding up size to full physical extent 8.00 MiB
Logical volume "test" created.
# lvs lvm_test
LV VG Attr LSize Pool Origin Data% Meta% Move Log Cpy%Sync Convert
lvol1 lvm_test twi-a-tz-- 4.00m 0.00 0.98
test lvm_test Vwi-a-tz-- 8.00m lvol1 0.00
compared to the former code which had the following:
LV VG Attr LSize Pool Origin Data% Move Log Cpy%Sync Convert
test LVM_Test swi-a-s--- 4.00m [test_vorigin] 0.00
Since libvirt doesn't know how to parse the thin logical volume
and pool, it will fail to find the newly created volume and pool
even though it exists in the volume group.
It cannot find since the command used to find/parse returns a thin volume
'test' with no associated device, for example the output is:
lvol1##UgUwkp-fTFP-C0rc-ufue-xrYh-dkPr-FGPFPx#lvol1_tdata(0)#thin-pool#1#4194304#4194304#4194304#twi-a-tz--
test##NcaIoH-4YWJ-QKu3-sJc3-EOcS-goff-cThLIL##thin#0#8388608#4194304#8388608#Vwi-a-tz--
as compared to the former which had the following:
test#[test_vorigin]#Dt5Of3-4WE6-buvw-CWJ4-XOiz-ywOU-YULYw6#/dev/sda3(1300)#linear#1#4194304#4194304#4194304#swi-a-s---
While it's possible to generate code to handle the new thin lv and pool, this
patch will add a "--type snapshot" onto the lvcreate command libvirt uses
in order to "for now" be able to continue to utilize the thin snapshots
https://bugzilla.redhat.com/show_bug.cgi?id=1174569
There's nothing we need to do for shared iSCSI devices in
qemuAddSharedHostdev and qemuRemoveSharedHostdev. The iSCSI layer
takes care about that for us.
Signed-off-by: Luyao Huang <lhuang@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Wire up backing chain recursion. For the first time, it is now
possible to get libvirt to expose that qemu tracks read statistics
on backing files, as well as report maximum extent written on a
backing file during a block-commit operation.
For a running domain, where one of the two images has a backing
file, I see the traditional output:
$ virsh domstats --block testvm2
Domain: 'testvm2'
block.count=2
block.0.name=vda
block.0.path=/tmp/wrapper.qcow2
block.0.rd.reqs=1
block.0.rd.bytes=512
block.0.rd.times=28858
block.0.wr.reqs=0
block.0.wr.bytes=0
block.0.wr.times=0
block.0.fl.reqs=0
block.0.fl.times=0
block.0.allocation=0
block.0.capacity=1310720000
block.0.physical=200704
block.1.name=vdb
block.1.path=/dev/sda7
block.1.rd.reqs=0
block.1.rd.bytes=0
block.1.rd.times=0
block.1.wr.reqs=0
block.1.wr.bytes=0
block.1.wr.times=0
block.1.fl.reqs=0
block.1.fl.times=0
block.1.allocation=0
block.1.capacity=1310720000
vs. the new output:
$ virsh domstats --block --backing testvm2
Domain: 'testvm2'
block.count=3
block.0.name=vda
block.0.path=/tmp/wrapper.qcow2
block.0.rd.reqs=1
block.0.rd.bytes=512
block.0.rd.times=28858
block.0.wr.reqs=0
block.0.wr.bytes=0
block.0.wr.times=0
block.0.fl.reqs=0
block.0.fl.times=0
block.0.allocation=0
block.0.capacity=1310720000
block.0.physical=200704
block.1.name=vda
block.1.path=/dev/sda6
block.1.backingIndex=1
block.1.rd.reqs=0
block.1.rd.bytes=0
block.1.rd.times=0
block.1.wr.reqs=0
block.1.wr.bytes=0
block.1.wr.times=0
block.1.fl.reqs=0
block.1.fl.times=0
block.1.allocation=327680
block.1.capacity=786432000
block.2.name=vdb
block.2.path=/dev/sda7
block.2.rd.reqs=0
block.2.rd.bytes=0
block.2.rd.times=0
block.2.wr.reqs=0
block.2.wr.bytes=0
block.2.wr.times=0
block.2.fl.reqs=0
block.2.fl.times=0
block.2.allocation=0
block.2.capacity=1310720000
I may later do a patch that trims the output to avoid 0 stats,
particularly for backing files (which are more likely to have
0 stats, at least for write statistics when no block-commit
is performed). Also, I still plan to expose physical size
information (qemu doesn't expose it yet, so it requires a stat,
and for block devices, a further open/seek operation). But
this patch is good enough without worrying about that yet.
* src/qemu/qemu_driver.c (QEMU_DOMAIN_STATS_BACKING): New internal
enum bit.
(qemuConnectGetAllDomainStats): Recognize new user flag, and pass
details to...
(qemuDomainGetStatsBlock): ...here, where we can do longer recursion.
(qemuDomainGetStatsOneBlock): Output new field.
Signed-off-by: Eric Blake <eblake@redhat.com>
In order to report stats on backing chains, we need to separate
the output of stats for one block from how we traverse blocks.
* src/qemu/qemu_driver.c (qemuDomainGetStatsBlock): Split...
(qemuDomainGetStatsOneBlock): ...into new helper.
Signed-off-by: Eric Blake <eblake@redhat.com>
This patch introduces access to allocation information about
a backing chain of a live domain. While querying storage
volumes for read-only disks could provide some of the details,
we do NOT want to read() a file while qemu is writing it.
Also, there is one case where we have to rely on qemu: when
doing a block commit into a backing file, where that file is
stored in qcow2 format on a host block device, we want to know
the current highest write offset into that image, in order to
know if the disk must be resized larger. qemu-img does not
(currently) show this information, and none of the earlier
block APIs were extensible enough to expose it. But
virDomainListGetStats is perfect for the job!
We don't need a new group of statistics, as the existing block
group is sufficient. On the other hand, as existing libvirt
releases already report 1:1 mapping of block.count to <disk>
devices, changing the array size could confuse older clients;
and even with newer clients, the time and memory taken to
report additional statistics is not always necessary (backing
files are generally read-only except for block-commit, so while
read statistics may change, sizing statistics will not). So
the choice here is to add a new flag that only newer callers
will pass, when they are prepared for the additional information.
This patch introduces the new API, but it will take more
patches to get it implemented for qemu.
* include/libvirt/libvirt-domain.h
(VIR_CONNECT_GET_ALL_DOMAINS_STATS_BACKING): New flag.
* src/libvirt-domain.c (virConnectGetAllDomainStats): Document it,
and add a new field when it is in use.
* tools/virsh-domain-monitor.c (cmdDomstats): Use new flag.
* tools/virsh.pod (domstats): Document it.
Signed-off-by: Eric Blake <eblake@redhat.com>
A coming patch will make it optionally possible to list backing
chain block stats; in this mode of operation, block.counts is no
longer the number of <disks> in the domain, but the number of
blocks in the array being reported. We still want block.count
listed first, but rather than iterate the tree twice (once to
count, and once to list stats), it's easier to just touch things
up after the fact.
* src/qemu/qemu_driver.c (qemuDomainGetStatsBlock): Compute count
after the fact.
Signed-off-by: Eric Blake <eblake@redhat.com>
The prior refactoring can now be put to use. With the same domain
as the earlier commit 7b49926 (one qcow2 disk and an empty
cdrom drive):
$ virsh domstats --block foo
Domain: 'foo'
block.count=2
block.0.name=hda
block.0.path=/var/lib/libvirt/images/foo.qcow2
block.0.allocation=1309614080
block.0.capacity=42949672960
block.0.physical=1309671424
block.1.name=hdc
* src/qemu/qemu_driver.c (qemuDomainGetStatsBlock): Use
qemuStorageLimitsRefresh to report offline statistics.
Signed-off-by: Eric Blake <eblake@redhat.com>
Create a helper function that can be reused for gathering block
info from virDomainListGetStats.
* src/qemu/qemu_driver.c (qemuDomainGetBlockInfo): Split guts...
(qemuStorageLimitsRefresh): ...into new helper function.
Signed-off-by: Eric Blake <eblake@redhat.com>
The documentation for virDomainBlockInfo was confusing: it stated
that 'physical' was the size of the container, then gave an example
of it being the amount of storage used by a sparse file (that is,
for a sparse raw image on a regular file, the wording implied
capacity==physical, while allocation was smaller; but the example
instead claimed physical==allocation). Since we use 'physical' for
the last offset of a block device, we should do likewise for
regular files.
Furthermore, the example claimed that for a qcow2 regular file,
allocation==physical. At the time the code was first written,
this was true (qcow2 files were allocated sequentially, and were
never sparse, so the last sector written happened to also match
the disk space occupied); but modern qemu does much better and
can punch holes for a qcow2 with allocation < physical.
Basically, after this patch, the three fields are now reliably
mapped as:
'capacity' - how much storage the guest can see (equal to
physical for raw images, determined by image metadata otherwise)
'allocation' - how much storage the image occupies (similar to
what 'du' would report)
'physical' - the last offset of the image (similar to what 'ls'
would report)
'capacity' can be larger than 'physical' (such as for a qcow2
image that does not vary much from a backing file) or smaller
(such as for a qcow2 file with lots of internal snapshots).
Likewise, 'allocation' can be (slightly) larger than 'physical'
(such as counting the tail of cluster allocations required to
round a file size up to filesystem granularity) or smaller
(for a sparse file). A block-resize operation changes capacity
(which, for raw images, also changes physical); many non-raw
images automatically grow physical and allocation as necessary
when starting with an allocation smaller than capacity; and even
when capacity and physical stay unchanged, allocation can change
when converting sectors from holes to data or back.
Note that this does not change semantics for qcow2 images stored
on block devices; there, we still rely on qemu to report the
highest written extent for allocation. So using this API to
track when to extend a block device because a qcow2 image is
about to exceed a threshold will not see any changes.
Also, note that virStorageVolInfo is unfortunately limited to
just 'capacity' and 'allocation' (we can't expand it to add
'physical', although we can expand the XML to add it there);
historically, that struct's 'allocation' value has reported
file size for qcow2 files (what this patch terms 'physical'
for a domain block device), but disk usage for raw files (what
this patch terms 'allocation'). So follow-up patches will be
needed to make storage volumes report the same allocation
values and get at physical values, where those differ.
* include/libvirt/libvirt-domain.h (_virDomainBlockInfo): Tweak
documentation to match saner definition.
* src/qemu/qemu_driver.c (qemuDomainGetBlockInfo): For regular
files, physical size is capacity, not allocation.
Signed-off-by: Eric Blake <eblake@redhat.com>
Ultimately, we want to avoid read()ing a file while qemu is running.
We still have to open() block devices to determine their physical
size, but that is safer. This patch rearranges code to group
together all code that reads the image, to make it easier for later
patches to skip the metadata collection when possible.
* src/qemu/qemu_driver.c (qemuDomainGetBlockInfo): Check for empty
disk up front. Place metadata reading next to use.
Signed-off-by: Eric Blake <eblake@redhat.com>
When requested in a later patch, the QMP command results are now
examined recursively. As qemu_driver will eventually have to
read items out of the hash table as stored by this patch, the
computation of backing alias string is done in a shared location.
* src/qemu/qemu_domain.h (qemuDomainStorageAlias): New prototype.
* src/qemu/qemu_domain.c (qemuDomainStorageAlias): Implement it.
* src/qemu/qemu_monitor_json.c
(qemuMonitorJSONGetOneBlockStatsInfo)
(qemuMonitorJSONBlockStatsUpdateCapacityOne): Perform recursion.
(qemuMonitorJSONGetAllBlockStatsInfo)
(qemuMonitorJSONBlockStatsUpdateCapacity): Update callers.
Signed-off-by: Eric Blake <eblake@redhat.com>
A future patch will allow recursion into backing chains when
collecting block stats. This patch should not change behavior,
but merely moves out the common code that will be reused once
recursion is enabled, and adds the parameter that will turn on
recursion.
* src/qemu/qemu_monitor.h (qemuMonitorGetAllBlockStatsInfo)
(qemuMonitorBlockStatsUpdateCapacity): Add recursion parameter,
although it is ignored for now.
* src/qemu/qemu_monitor.h (qemuMonitorGetAllBlockStatsInfo)
(qemuMonitorBlockStatsUpdateCapacity): Likewise.
* src/qemu/qemu_monitor_json.h
(qemuMonitorJSONGetAllBlockStatsInfo)
(qemuMonitorJSONBlockStatsUpdateCapacity): Likewise.
* src/qemu/qemu_monitor_json.c
(qemuMonitorJSONGetAllBlockStatsInfo)
(qemuMonitorJSONBlockStatsUpdateCapacity): Add parameter, and
split...
(qemuMonitorJSONGetOneBlockStatsInfo)
(qemuMonitorJSONBlockStatsUpdateCapacityOne): ...into helpers.
(qemuMonitorJSONGetBlockStatsInfo): Update caller.
* src/qemu/qemu_driver.c (qemuDomainGetStatsBlock): Update caller.
* src/qemu/qemu_migration.c (qemuMigrationCookieAddNBD): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
Right now, grabbing blockinfo always calls stat on the disk, then
opens the image to determine the capacity, using a throw-away
virStorageSourcePtr. This has a couple of drawbacks:
1. We are calling stat and opening a file on every invocation of
the API. However, there are cases where the stats should NOT be
changing between successive calls (if a domain is running, no
one should be changing the physical size of a block device or raw
image behind our backs; capacity of read-only files should not
be changing; and we are the gateway to the block-resize command
to know when the capacity of read-write files should be changing).
True, we still have to use stat in some cases (a sparse raw file
changes allocation if it is read-write and the amount of holes is
changing, and a read-write qcow2 image stored in a file changes
physical size if it was not fully pre-allocated). But for
read-only images, even this should be something we can remember
from the previous time, rather than repeating every call.
2. We want to enhance the power of virDomainListGetStats, by
sharing code. But we already have a virStorageSourcePtr for
each disk, and it would be easier to reuse the common structure
than to have to worry about the one-off virDomainBlockInfoPtr.
While this patch does not optimize reuse of information in point
1, it does get us closer to being able to do so; by updating a
structure that survives between consecutive calls.
* src/util/virstoragefile.h (_virStorageSource): Add physical, to
mirror virDomainBlockInfo; rearrange fields to match public struct.
(virStorageSourceCopy): Copy the new field.
* src/qemu/qemu_driver.c (qemuDomainGetBlockInfo): Store into
storage source, then copy to block info.
Signed-off-by: Eric Blake <eblake@redhat.com>
In order for a future patch to virDomainListGetStats to reuse
some code for determining disk usage of offline domains, we
need to make it easier to pull out part of the guts of grabbing
blockinfo. The current implementation grabs a job fairly late
in the game, while getstats will already own a job; reordering
things so that the job is always grabbed up front in both
functions will make it easier to pull out the common code.
This patch results in grabbing a job in cases where one was not
previously needed, but as it is a query job, it should not be
noticeably slower.
This patch touches the same code as the fix for CVE-2014-6458
(commit b799259); in that patch, we avoided hotplug changing
a disk reference during the time of obtaining a monitor lock
by copying all data we needed and no longer referencing disk;
this patch goes the other way and ensures that by holding the
job, the disk cannot be changed so we no longer need to worry
about the disk being invalidated across the monitor lock.
* src/qemu/qemu_driver.c (qemuDomainGetBlockInfo): Rearrange job
control to be outside of disk information.
Signed-off-by: Eric Blake <eblake@redhat.com>
When any of the functions modified in commit 214c687b took false branch,
the function itself used none of its parameters resulting in "unused
parameter" error. Rewriting these functions to the stubs we use
elsewhere should fix the problem.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Commit e3435caf added cleanup code to qemuDomainSetVcpusFlags() that was
not supposed to reset the error. Usual procedure was done, saving the
error to temporary variable, but it was never free'd, but rather leaked.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Commit af2a1f05 tried clearly separating each condition in
qemuRestoreCgroupState() for the sake of readability, however somehow
one condition body was missing. That means that the body of the next
condition got executed only if both of there were true, which is
impossible, thus resulting in a dead code and a logic error.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
In commit d2632d60 we agreed taht we want the parsed uid to properly
overflow but only to -1, however the value was read into long and then
wrapped into uid_t. That meaned it failed on 32-bit systems.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Currently virStorageFileResize() function uses build conditionals to
choose either the posix_fallocate() or syscall(SYS_fallocate) with no
fallback in order to preallocate the space in the newly resized file.
Since the safezero code has a similar set of conditionals modify the
resize and safezero code in order to allow the resize logic to make use
of safezero to unify the look/feel of the code paths.
Add a new boolean (resize) to safezero() to make the optional decision
whether to try syscall(SYS_fallocate) if the posix_fallocate fails because
HAVE_POSIX_FALLOCATE is not defined (eg, return -1 and errno == 0).
Create a local safezero_sys_fallocate in order to handle the resize
code paths that support that. If not present, the set errno = ENOSYS
in order to allow the caller to handle the failure scenarios.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Currently build conditionals decide which of two safezero() functions
should be built - either the posix_fallocate() or mmap() with a fallback
to a slower safewrite() algorithm in order to preallocate space in a raw file.
This patch will refactor safezero to utilize static functions for either
posix_fallocate or mmap/safewrite. The build conditional still exist, but
are only for shorter sections of code.
The posix_fallocate path will make use of the ret/errno setting to contain
the logic for safezero to decide whether it needs to fallback to other
algorithms. A return of -1 with errno not changed will indicate the conditional
is not present; otherwise, a return of -1 with errno change indicates the
call was made and it failed (no functional difference to current algorithm).
The mmap/safewrite option changes only slightly to handle the ftruncate
failure for mmap. That is, previously if the ftruncate failed, there was
no fallback to the slow safewrite option.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Currently, when there is an API that's blocking with locked domain and
second API that's waiting in virDomainObjListFindByUUID() for the domain
lock (with the domain list locked) no other API can be executed on any
domain on the whole hypervisor because all would wait for the domain
list to be locked. This patch adds new optional approach to this in
which the domain is only ref'd (reference counter is incremented)
instead of being locked and is locked *after* the list itself is
unlocked. We might consider only ref'ing the domain in the future and
leaving locking on particular APIs, but that's no tonight's fairy tale.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Volume and pool formatting functions took different approaches to
unspecified uids/gids. When unknown, it is always parsed as -1, but one
of the functions formatted it as unsigned int (wrong) and one as
int (better). Due to that, our two of our XML files from tests cannot
be parsed on 32-bit machines.
RNG schema needs to be modified as well, but because both
storagepool.rng and storagevol.rng need same schema for permission
element, save some space by moving it to storagecommon.rng.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
When hot-plugging a VCPU into the guest, kvm needs to allocate some data
from the DMA zone, which might be in a memory node that's not allowed in
cpuset.mems. Basically the same problem as there was with starting the
domain and due to which commit 7e72ac7878
exists. This patch just extends it to hotplugging as well.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1161540
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Instead of setting the value of cpuset.mems once when the domain starts
and then re-calculating the value every time we need to change the child
cgroup values, leave the cgroup alone and rather set the child data
every time there is new cgroup created. We don't leave any task in the
parent group anyway. This will ease both current and future code.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
In systemd >= 218, the udev_set_log_fn method has been marked
deprecated and turned into a no-op. Nothing in the udev client
library will print to stderr by default anymore, so we can
just stop installing a logging hook for new enough udev.
For SCSI and SATA devices controller and unit are used
to specify drive address. For IDE devices - bus specifies
IDE bus, becase usually there are 2 IDE buses on IDE
controller.
Parallels SDK allows to set drive position by calling
PrlVmDev_SetStackIndex. Since PCS VMs have only one
controller of each type, for SATA and SCSI devices it
simple means position on bus, for IDE devices -
2 * bus_number + position_on_bus.
This patch fixes mapping from libvirt's disk->info.addr.drive
to parallels's 'StackIndex'.
Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com>
It seems file format is usually specified event for
real block devices. So report that file format is
raw in virDomainGetXMLDesc and add checks for proper
file format to prlsdkAddDisk.
Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com>
NULL value of virDomainVideoAccelDefPtr means default
values for video acceleration, so don't report error in
this case.
Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1174154
When we use attach-device add a hostdev or chr device which have a
iscsi address or others (just like guest agent, subsys iscsi disk...),
we will find there is no basic controller for our new attached device.
Somtimes this will make guest cannot start after we add them (although
they can start at the second time).
Signed-off-by: Luyao Huang <lhuang@redhat.com>
When libvirt is managing a bridge's forwarding database (FDB)
(macTableManager='libvirt'), if we add FDB entries for a new guest
interface even before the qemu process is created, then in the case of
a migration any other guest attached to the "destination" bridge will
have its traffic immediately sent to the destination of the migration
even while the source domain is still running (and the destination, of
course, isn't). To make sure that traffic from other guests on the new
host continues flowing to the old guest until the new one is ready, we
have to wait until the new guest CPUs are started to add the FDB
entries.
Conversely, we need to remove the FDB entries from the bridge any time
the guest CPUs are stopped; among other things, this will assure
proper operation during a post-copy migration (which is just the
opposite of the problem described in the previous paragraph).
We can change vnc password by using virDomainUpdateDeviceFlags API with
live flag. But it can't be changed with config flag. Error is reported as
below.
error: Operation not supported: persistent update of device 'graphics' is not supported
This patch supports the graphics arguments changed with config flag.
Signed-off-by: Wang Rui <moon.wangrui@huawei.com>
It's not supported to change some graphics arguments with '--live'.
Replace some error code VIR_ERR_INTERNAL_ERROR and VIR_ERR_INVALID_ARG
with VIR_ERR_OPERATION_UNSUPPORTED.
Signed-off-by: Wang Rui <moon.wangrui@huawei.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1174096
When both parameter have lockspaces present, virDomainLeaseIndex
always returns -1 even there is a lease the same with the one we
check. This is due to broken logic in 'if-else' statement.
Signed-off-by: Luyao Huang <lhuang@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1173507
It occurred to me that OpenStack uses the following XML when not using
regular huge pages:
<memoryBacking>
<hugepages>
<page size='4' unit='KiB'/>
</hugepages>
</memoryBacking>
However, since we are expecting to see huge pages only, we fail to
startup the domain with following error:
libvirtError: internal error: Unable to find any usable hugetlbfs
mount for 4 KiB
While regular system pages are not huge pages technically, our code is
prepared for that and if it helps OpenStack (or other management
applications) we should cope with that.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1174053
Introduced by commit id '17bddc46f' - fix a libvirtd crash when
matching a network iscsi hostdev with a host iscsi hostdev.
When we use attach-device to coldplug a network iscsi hostdev,
libvirt will check if there is already a device in XML. But if
the 'b' is a host iscsi hostdev and 'a' is a network iscsi hostdev,
then libvirtd will crash in virDomainHostdevMatchSubsysSCSIiSCSI
because 'b' doesn't have a hostname.
Add a check in virDomainHostdevMatchSubsys, if the a's protocol
and b's protocol is not the same.
Following is the backtrace:
0 0x00007f850d6bc307 in virDomainHostdevMatchSubsysSCSIiSCSI at conf/domain_conf.c:10889
1 virDomainHostdevMatchSubsys at conf/domain_conf.c:10911
2 virDomainHostdevMatch at conf/domain_conf.c:10973
3 virDomainHostdevFind at conf/domain_conf.c:10998
4 0x00007f84f6a10560 in qemuDomainAttachDeviceConfig at qemu/qemu_driver.c:7223
5 qemuDomainAttachDeviceFlags at qemu/qemu_driver.c:7554
Signed-off-by: Luyao Huang <lhuang@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1160995
In our config files users are expected to pass several integer values
for different configuration knobs. However, majority of them expect a
nonnegative number and only a few of them accept a negative number too
(notably keepalive_interval in libvirtd.conf).
Therefore, a new type to config value is introduced: VIR_CONF_ULONG
that is set whenever an integer is positive or zero. With this
approach knobs accepting VIR_CONF_LONG should accept VIR_CONF_ULONG
too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
There's no need for condition of the following form:
if (str && STREQ(str, dst))
since we have STREQ_NULLABLE macro that handles NULL cases.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
For historical reasons, only the first <console> element might be of targetType
serial, but we checked for other consoles of targetType serial in our post-parse
callback if and only if we knew the first console was serial, otherwise
the check was skipped.
This patch moves the check one level up, so first
the check for secondary console of type serial is performed and then the
rest of operations continue unchanged.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1170092
We now have a qemuInterfaceStartDevices() which does the final
activation needed for the host-side tap/macvtap devices that are used
for qemu network connections. It will soon make sense to have the
converse qemuInterfaceStopDevices() which will undo whatever was done
during qemuInterfaceStartDevices().
A function to "stop" a single device has also been added, and is
called from the appropriate place in qemuDomainDetachNetDevice(),
although this is currently unnecessary - the device is going to
immediately be deleted anyway, so any extra "deactivation" will be for
naught. The call is included for completeness, though, in anticipation
that in the future there may be some required action that *isn't*
nullified by deleting the device.
This patch is a part of a more complete fix for:
https://bugzilla.redhat.com/show_bug.cgi?id=1081461
The patch that added qemuInterfaceStartDevices() (upstream commit
82977058f5) had an extra conditional to
prevent calling it if the reason for starting the CPUs was
VIR_DOMAIN_RUNNING_UNPAUSED or VIR_DOMAIN_RUNNING_SAVE_CANCELED. This
was put in by the author as the result of a reviewer asking if it was
necessary to ifup the interfaces in *all* occasions (because these
were the two cases where the CPU would have already been started (and
stopped) once, so the interface would already be ifup'ed).
It turns out that, as long as there is no corresponding
qemuInterfaceStopDevices() to ifdown the interfaces anytime the CPUs
are stopped, neglecting to ifup when reason is RUNNING_UNPAUSED or
RUNNING_SAVE_CANCELED doesn't cause any problems (because it just
happens that the interface will have already been ifup'ed by a prior
call when the CPU was previously started for some other reason).
However, it also doesn't *help*, and there will soon be a
qemuInterfaceStopDevices() function which *will* ifdown these
interfaces when the guest CPUs are stopped, and once that is done, the
interfaces will be left down in some cases when they should be up (for
example, if a domain is paused and then unpaused).
So, this patch is removing the condition in favor of always calling
qemuInterfaeStartDevices() when the guest CPUs are started.
This patch (and the aforementioned patch) resolve:
https://bugzilla.redhat.com/show_bug.cgi?id=1081461
When one domain is being undefined and at the same time started, for
example, there is a possibility of a rare problem occuring.
- Thread 1 does virDomainUndefine(), has the lock, checks that the
domain is active and because it's not, calls
virDomainObjListRemove().
- Thread 2 does virDomainCreate() and tries to lock the domain.
- Thread 1 needs to lock domain list in order to remove the domain from
it, but must unlock domain first (proper order is to lock domain list
first and the domain itself second).
- Thread 2 grabs the lock, starts the domain and releases the lock.
- Thread 1 grabs the lock and removes the domain from list.
With this patch:
- The undefining domain gets marked as "to undefine" before it is
unlocked.
- If domain is found in any of the search APIs, it's returned only if
it is not marked as "to undefine". The check is done while the
domain is locked.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1150505
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
When calling virCgroupAllowAllDevices we get these invalid entries
in the device cgroup config.
b -1:-1 rw
c -1:-1 rw
Check for positive values before outputting the major and minor to
avoid that.
For host-passthrough CPU we don't honor the CPU
features specified in the XML, but we allow
outputting them via the UPDATE_CPU flag for dumpxml,
this gives user a rough idea of what features the CPU
might have.
After restoring a managedsave'd domain, the features
might end up in the live status XML (in /var/run) without
the model. This XML cannot be parsed by the daemon after
restart and the domain might disappear.
This fix skips formatting the features for HOST_PASSTHROUGH
when UPDATE_CPU is not specified, so the newly restored domains
and newly created snapshots won't be affected.
Note: this doesn't fix existing snapshots or already restored
running domains.
https://bugzilla.redhat.com/show_bug.cgi?id=1030793https://bugzilla.redhat.com/show_bug.cgi?id=1151885
A logic bug in qemuConnectGetAllDomainStats makes the code mark the
monitor as available when qemuDomainObjBeginJob fails, instead of when
it succeeds, as the correct flow requires.
This patch fixes the check and updates the code documentation
accordingly.
Broken by commit 57023c0a3a.
Signed-off-by: Francesco Romani <fromani@redhat.com>
When using qemuProcessAttach to attach a qemu process,
the DAC label is not filled correctly.
Introduce a new function to get the uid:gid from the system
and fill the label.
This fixes the daemon crash when 'virsh screenshot' is called:
https://bugzilla.redhat.com/show_bug.cgi?id=1161831
It also fixes qemu-attach after the prerequisite of this patch
(commit f8c1fb3) was pushed out of order.
Signed-off-by: Luyao Huang <lhuang@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Currently, MAC registration occurs during device creation, which is
early enough that, during live migration, you end up with duplicate
MAC addresses on still-running source and target devices, even though
the target device isn't actually being used yet.
This patch proposes to defer MAC registration until right before
the guest can actually use the device -- In other words, right
before starting guest CPUs.
Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
Signed-off-by: Laine Stump <laine@laine.org>
Some programs want to change some values for the network interfaces
configuration in /proc/sys/net/ipv[46] folders. Giving RW access on them
allows wicked to work on openSUSE 13.2+.
Reusing the lxcNeedNetworkNamespace function to tell
lxcContainerMountBasicFS if the netns is disabled. When no netns is
set up, then we don't mount the /proc/sys/net/ipv[46] folder RW as
these would provide full access to the host NICs config.
https://bugzilla.redhat.com/show_bug.cgi?id=1172015
The refactoring done as part of commit id '59446096' caused a regression
for the multi initiator IQN commit '6aabcb5b' because the sendtargets was
not done on/for the initiator IQN prior to login (or trying to disable
autologin)
Prior to that commit, the paths were essentially
virStorageBackendISCSIStartPool
virStorageBackendISCSILogin
virStorageBackendISCSIConnection
if initiatoriqn
virStorageBackendCreateIfaceIQN
Issue sendtargets
Perform --login
else
Issue sendtargets
Perform --login
After that commit:
virStorageBackendISCSIStartPool
Issue sendtargets
Call virStorageBackendISCSIConnection
If initiatoriqn
virStorageBackendCreateIfaceIQN
Perform --login
else
Perform --login
So for non initiator IQN paths, nothing changed. For the initiator path,
the --login fails as does any attempts to change autologin via "--op update
--name node.startup --value manual".
In old version of parted like parted-2.1-25, error message is shown in
stdout when printing a disk info without disk label.
Error: /dev/sda: unrecognised disk label
This line has been moved to stderr in newer version of parted. So we
should check both stdout and stderr when locating this message.
This should fix bug:
https://bugzilla.redhat.com/show_bug.cgi?id=1172468
Signed-off-by: Hao Liu <hliu@redhat.com>
When user doesn't have read access on one of the domains he requested,
the for loop could exit abruptly or continue and override pointer which
pointed to locked object.
This patch fixed two issues at once. One is that domflags might have
had QEMU_DOMAIN_STATS_HAVE_JOB even when there was no job started (this
is fixed by doing domflags |= QEMU_DOMAIN_STATS_HAVE_JOB only when the
job was acquired and cleaning domflags on every start of the loop.
Second one is that the domain is kept locked when
virConnectGetAllDomainStatsCheckACL() fails and continues the loop when
it didn't end. Adding a simple virObjectUnlock() and clearing the
pointer ought to do.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
If we want to perform some operation and domain state is not suitable
for that operation, we should report error VIR_ERR_OPERATION_INVALID.
Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com>
When PrlJob_GetRetCode sets second argument to
error value it means sdk function failed and we
must return error from getJobResultHelper.
Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com>
Return error code, returned by parallels SDK from
waitJob and getJobResult, so that caller can handle
different errors.
Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com>
Get cdrom devices list from parallels server in
prlsdkLoadDomains and add ability to define a domain
with cdroms.
Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com>
First, we don't need to call prlsdkApplyConfig after
creating new VM or containers, because it's done in
functions prlsdkCreateVm and prlsdkCreateCt.
No need to check, if domain exists in the list after
prlsdkAddDomain.
Also organize code, so that we can call virObjectUnlock
in one place.
Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com>
This patch replaces code, which creates domains by
running prlctl command.
prlsdkCreateVm/Ct will do prlsdkApplyConfig, because
we send request to the server only once in this case.
But prlsdkApplyConfig will be called also from
parallelsDomainDefineXML function. There is no problem with
it, parallelsDomainDefineXML will be refactored later.
Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com>
Rewrite code, which applies domain configuration given
to virDomainDefineXML function to the VM of container
registered in PCS.
This code first check if there are unsupported parameters
in domain XML and if yes - reports error. Some of such
parameters are not supported by PCS, for some - it's not
obvious, how to convert them into PCS's corresponding params,
so let's put off it, and implement only basic params in
this patch.
Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com>
Change domain state using parallels SDK functions instead of
prlctl command.
We don't need to send events from these functions now, becase
events handler will send them. But we still need to update
virDomainObj in privconn->domains.
Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com>
Subscribe to events from parallels server. It's
needed for 2 things: to update cached domains list
and to send corresponding libvirt events.
Parallels server sends a lot of different events, in
this patch we handle only some of them. In the future
we can handle for example, changes in a host network
configuration or devices states.
Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com>
Move macro parallelsDomNotFoundError to file parallels_utils.h, because
it will be used in parallels_sdk.c.
Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com>
Obtain information about domains using parallels sdk instead of prlctl.
prlsdkLoadDomains functions behaves as former parallelsLoadDomains with
NULL as second parameter (name) - it fills parallelsConn.domains list.
prlsdkLoadDomain is now able to update specified domain by given
virDomainObjPtr.
Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1082521
Support for shared hostdev's was added in a number of commits, initially
starting with 'f2c1d9a80' and most recently commit id 'fd243fc4' to fix
issues with the initial implementation. Missed in all those changes was
the need to mimic the virSELinux{Set|Restore}SecurityDiskLabel code to
handle the "shared" (or shareable) and readonly options when Setting
or Restoring the SELinux labels.
This patch will adjust the virSecuritySELinuxSetSecuritySCSILabel to not
use the virSecuritySELinuxSetSecurityHostdevLabelHelper in order to set
the label. Rather follow what the Disk code does by setting the label
differently based on whether shareable/readonly is set. This patch will
also modify the virSecuritySELinuxRestoreSecuritySCSILabel to follow
the same logic as virSecuritySELinuxRestoreSecurityImageLabelInt and not
restore the label if shared/readonly
https://bugzilla.redhat.com/show_bug.cgi?id=1171582
When we edit a negative controller address number to a device,
some of them will auto generate a controller with invalid index
number. This will make guest disappear after restart libvirtd.
Instead of allowing negative number for controller index, we
should forbid negative number in these place (we did this before,
but after f18c02ec, virStrToLong_ui changed to allow negative
number). Therefore switch to virStrToLong_uip in these places.
Signed-off-by: Luyao Huang <lhuang@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Avoid leaving the domain locked on a failed ACL check in
qemuDomainMigratePerform() and qemuDomainMigrateFinish2().
Introduced in commit abf75aea24 (Add ACL checks into the QEMU driver).
Commit c75425734 introduced a compilation failure:
../../src/access/viraccessdriverpolkit.c: In function 'virAccessDriverPolkitCheck':
../../src/access/viraccessdriverpolkit.c:137:5: error: format '%d' expects argument of type 'int', but argument 9 has type 'pid_t' [-Werror=format=]
VIR_DEBUG("Check action '%s' for process '%d' time %lld uid %d",
^
Since mingw pid_t is 64 bits, it's easier to just follow what we've
done elsewhere and cast to a large enough type when printing pids.
* src/access/viraccessdriverpolkit.c (virAccessDriverPolkitCheck):
Add cast.
Signed-off-by: Eric Blake <eblake@redhat.com>
lxcProcessSetupInterfaces() used to have a special case for
actualType='network' (a network with forward mode of route, nat, or
isolated) to call the libvirt public API to retrieve the bridge being
used by a network. That is no longer necessary - since all network
types that use a bridge and tap device now get the bridge name stored
in the ActualNetDef, we can just always use
virDomainNetGetActualBridgeName() instead.
qemuNetworkIfaceConnect() used to have a special case for
actualType='network' (a network with forward mode of route, nat, or
isolated) to call the libvirt public API to retrieve the bridge being
used by a network. That is no longer necessary - since all network
types that use a bridge and tap device now get the bridge name stored
in the ActualNetDef, we can just always use
virDomainNetGetActualBridgeName() instead.
(an audit of the two callers to qemuNetworkIfaceConnect() confirms
that it is never called for any other type of network, so the dead
code in the else statement (logging an internal error if it is called
for any other type of network) is eliminated in the process.)
When libvirt is managing the MAC table of a Linux host bridge, it must
turn off learning and unicast_flood for each tap device attached to
that bridge, then add a Forwarding Database (fdb) entry for the tap
device using the MAC address from the domain interface config.
Once we have disabled learning and flooding, any packet that has a
destination MAC address not present in the fdb will be dropped by the
bridge. This, along with the opportunistic disabling of promiscuous
mode[*], can result in enhanced network performance. and a potential
slight security improvement.
[*] If there is only one device on the bridge with learning/unicast_flood
enabled, then that device will automatically have promiscuous mode
disabled. If there are *no* devices with learning/unicast_flood
enabled (e.g. for a libvirt "route", "nat", or isolated network that
has no physical device attached), then all non-tap devices will have
promiscuous mode disabled (tap devices always have promiscuous mode
enabled, which may be a bug in the kernel, but in practice has 0
effect).
None of this has any effect for kernels prior to 3.15 (upstream kernel
commit 2796d0c648c940b4796f84384fbcfb0a2399db84 "bridge: Automatically
manage port promiscuous mode"). Even after that, until kernel 3.17
(upstream commit 5be5a2df40f005ea7fb7e280e87bbbcfcf1c2fc0 "bridge: Add
filtering support for default_pvid") traffic will not be properly
forwarded without manually adding vlan table entries. Unfortunately,
although the presence of the first patch is signalled by existence of
the "learning" and "unicast_flood" options in sysfs, there is no
reliable way to query whether or not the system's kernel has the
second of those patches installed, the only thing that can be done is
to try the setting and see if traffic continues to pass.
When the bridge device for a network has macTableManager='libvirt' the
intent is that all kernel management of the bridge's MAC table
(Forwarding Database, or fdb, in the case of a Linux Host Bridge) be
disabled, with libvirt handling updates to the table instead. The
setup required for the bridge itself is:
1) set the "vlan_filtering" property of the bridge device to 1.
2) If the bridge has a "Dummy" tap device used to set a fixed MAC
address on the bridge (which is always the case for a bridge created
by libvirt, and never the case for a bridge created by the host system
network config), turn off learning and unicast_flood on this tap (this
is needed even though this tap is never IFF_UP, because the kernel
ignores the IFF_UP flag of devices when using their settings to
automatically decide whether or not to turn off promiscuous mode for
any attached device).
(1) is done both for libvirt-created/managed bridges, and for bridges
that are created by the host system config, while (2) is done only for
bridges created by libvirt (i.e. for forward modes of nat, routed, and
isolated bridges)
There is no attempt to turn vlan_filtering off when destroying the
network because in the case of a libvirt-created bridge, the bridge is
about to be destroyed anyway, and in the case of a system bridge, if
the other devices attached to the bridge could operate properly before
destroying libvirt's network object, they will continue to operate
properly (this is similar to the way that libvirt will enable
ip_forwarding whenever a routed/natted network is started, but will
never attempt to disable it if they are stopped).
At the time that the network driver allocates a connection to a
network, the tap device that will be used hasn't yet been created -
that will be done later by qemu (or lxc or whoever) - but if the
network has macTableManager='libvirt', then when we do get around to
creating the tap device, we will need to add an entry for it to the
network bridge's fdb (forwarding database) *and* turn off learning and
unicast_flood for that tap device in the bridge's sysfs settings. This
means that qemu needs to know both the bridge name as well as the
setting of macTableManager, so we either need to create a new API to
retrieve that info, or just pass it back in the ActualNetDef that is
created during networkAllocateActualDevice. We choose the latter
method, since it's already done for the bridge device, and it has the
side effect of making the information available in domain status.
(NB: in the future, I think that the tap device should actually be
created by networkAllocateActualDevice(), as that will solve several
other problems, but that is a battle for another day, and this
information will still be useful outside the network driver)
When the actualType of a virDomainNetDef is "network", it means that
we are connecting to a libvirt-managed network (routed, natted, or
isolated) which does use a bridge device (created by libvirt). In the
past we have required drivers such as qemu to call the public API to
retrieve the bridge name in this case (even though it is available in
the NetDef's ActualNetDef if the actualType is "bridge" (i.e., an
externally-created bridge that isn't managed by libvirt). There is no
real reason for this difference, and as a matter of fact it
complicates things for qemu. Also, there is another bridge-related
attribute (macTableManager) that will need to be available in both
cases, so this makes things consistent.
In order to avoid problems when restarting libvirtd after an update
from an older version that *doesn't* store the network's bridgename in
the ActualNetDef, we also need to put it in place during
networkNotifyActualDevice() (this function is run for each interface
of each domain whenever libvirtd is restarted).
Along with making the bridge name available in the internal object, it
is also now reported in the <source> element of the <interface> state
XML (or the <actual> subelement in the internally-stored format).
The one oddity about this change is that usually there is a separate
union for every different "type" in a higher level object (e.g. in the
case of a virDomainNetDef there are separate "network" and "bridge"
members of the union that pivots on the type), but in this case
network and bridge types both have exactly the same attributes, so the
"bridge" member is used for both type==network and type==bridge.
The macTableManager attribute of a network's bridge subelement tells
libvirt how the bridge's MAC address table (used to determine the
egress port for packets) is managed. In the default mode, "kernel",
management is left to the kernel, which usually determines entries in
part by turning on promiscuous mode on all ports of the bridge,
flooding packets to all ports when the correct destination is unknown,
and adding/removing entries to the fdb as it sees incoming traffic
from particular MAC addresses. In "libvirt" mode, libvirt turns off
learning and flooding on all the bridge ports connected to guest
domain interfaces, and adds/removes entries according to the MAC
addresses in the domain interface configurations. A side effect of
turning off learning and unicast_flood on the ports of a bridge is
that (with Linux kernel 3.17 and newer), the kernel can automatically
turn off promiscuous mode on one or more of the bridge's ports
(usually only the one interface that is used to connect the bridge to
the physical network). The result is better performance (because
packets aren't being flooded to all ports, and can be dropped earlier
when they are of no interest) and slightly better security (a guest
can still send out packets with a spoofed source MAC address, but will
only receive traffic intended for the guest interface's configured MAC
address).
The attribute looks like this in the configuration:
<network>
<name>test</name>
<bridge name='br0' macTableManager='libvirt'/>
...
This patch only adds the config knob, documentation, and test
cases. The functionality behind this knob is added in later patches.
These two functions use netlink RTM_NEWNEIGH and RTM_DELNEIGH messages
to add and delete entries from a bridge's fdb. The bridge itself is
not referenced in the arguments to the functions, only the name of the
device that is attached to the bridge (since a device can only be
attached to one bridge at a time, and must be attached for this
function to make sense, the kernel easily infers which bridge's fdb is
being modified by looking at the device name/index).
I'm about to make block stats optionally more complex to cover
backing chains, where block.count will no longer equal the number
of <disks> for a domain. For these reasons, it is nicer if the
statistics output includes the source path (for local files).
This patch doesn't add anything for network disks, although we
may decide to add that later.
With this patch, I now see the following for the same domain as
in the previous patch (one qcow2 file, and an empty cdrom drive):
$ virsh domstats --block foo
Domain: 'foo'
block.count=2
block.0.name=hda
block.0.path=/var/lib/libvirt/images/foo.qcow2
block.1.name=hdc
* src/libvirt-domain.c (virConnectGetAllDomainStats): Document
new field.
* tools/virsh.pod (domstats): Document new field.
* src/qemu/qemu_driver.c (qemuDomainGetStatsBlock): Return the new
stat for local files/block devices.
(QEMU_ADD_NAME_PARAM): Add parameter.
(qemuDomainGetStatsInterface): Update caller.
Signed-off-by: Eric Blake <eblake@redhat.com>
I noticed that for an offline domain, 'virsh domstats --block $dom'
was producing just the domain name, with no stats. But the older
'virsh domblkinfo' works just fine on offline domains. This patch
starts to get us closer, by at least reporting the disk names for
an offline domain.
With this patch, I now see the following for an offline domain
with one qcow2 disk and an empty cdrom drive:
$ virsh domstats --block foo
Domain: 'foo'
block.count=2
block.0.name=hda
block.1.name=hdc
* src/qemu/qemu_driver.c (qemuDomainGetStatsBlock): Don't short-circuit
output of block name.
Signed-off-by: Eric Blake <eblake@redhat.com>
At least with 'virsh domstats --block' on an offline domain, we
currently output no stats even though we recognize the stat
category. Although a later patch will improve this situation,
it is better to document that this is expected behavior.
Also, while the current implementation rejects filtering flags
for virDomainListGetStats, this limitation may be lifted in the
future and we do not enforce it at the API level.
* src/libvirt-domain.c (virConnectGetAllDomainStats): Document
that recognized stats might not be reported.
(virDomainListGetStats): Likewise, and tweak filtering documentation.
Signed-off-by: Eric Blake <eblake@redhat.com>
qemuDomainGetStatsBlock() could leak a stats hash table if it
encountered OOM while populating the virTypedParameters.
Oddly, the fix doesn't even touch qemuDomainGetStatsBlock :)
* src/qemu/qemu_driver.c (QEMU_ADD_COUNT_PARAM)
(QEMU_ADD_NAME_PARAM): Don't return early.
(qemuDomainGetStatsInterface): Adjust caller.
Signed-off-by: Eric Blake <eblake@redhat.com>
Whenever client socket was marked as closed for some reason, it could've
been changed when really closing the connection. With this patch the
proper reason is kept since the first time it's marked as closed.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
When user tries to insert element metadata providing a namespace
declaration as well, currently we insert the element without any validation
check for XML prefix (if provided). The next VM start would then
fail with parse error. This patch fixes this issue by adding a call to
xmlValidateNCName function to check for illegal characters in the
prefix.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1143921
If probing capabilities via QMP fails, we now have a check
that prevents us falling back to -help parsing. Unfortunately
the error message
"Failed to probe capabilities for /usr/bin/qemu-kvm:
unsupported configuration: QEMU 2.1.2 is too new for help parsing"
is proving rather unhelpful to the user. We need to be telling
them why QMP failed (the root cause), rather than they can't
use -help (the side effect).
To do this we should capture stderr during QMP probing, and
if -help parsing then sees a new QEMU version, we know that
QMP should have worked, and so we can show the messages from
stderr. The message thus becomes
"Failed to probe capabilities for /usr/bin/qemu-kvm:
internal error: QEMU / QMP failed: Could not access
KVM kernel module: No such file or directory
failed to initialize KVM: No such file or directory"
When attempting to create internal system checkpoint with a passthrough
device qemu will report the following error:
error: operation failed: Error -22 while writing VM
This patch calls the function to check if migration is possible with
given VM and thus improves the error to:
error: Requested operation is not valid: domain has assigned non-USB host devices
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=874418#c19
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1115292
In one of the previous commits (eafb53fe) we disallowed
network-wide bandwidth to some network types. However, we
forgot about <portgroups/> which can have <bandwidth/> too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Move entering the job into the thread to simplify the program flow. Also
as the code holds a separate reference to the domain object some
conditions can be simplified.
After this patch qemuDomainObjTransferJob is no longer needed so this
patch removes it.
Reboot requires more sophistication and is left as a future work item --
but at least part of the plumbing is in place.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
If someone removes blockcopy storage file when still in mirroring phase
and then requesting blockjob abort using pivot, virsh cmd freezes. This
is not an issue with older qemu versions which did not support
asynchronous jobs (which we prefer by default).
As we have reached the mirroring phase successfully, polling monitor for
blockjob info always returns 1 and the loop never ends.
This fix introduces a check for qemuDomainBlockPivot return code, possibly
skipping the asynchronous waiting completely, if an error occurred and
asynchronous waiting was the preferred method.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1139567
Commit 86a15a25 introduced a new cpu driver API 'getModels'. Public API
allow you to pass NULL for models to get only number of existing models.
However the new code will crash with segfault so we have to count with
the possibility that the user wants only the number.
There is also difference in order of the models gathered by this new API
as the old approach was inserting the elements to the end of the array
so we should use 'VIR_APPEND_ELEMENT'.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reconnect to the VM is a possibly long-running job spawned in a separate
thread. We should reload the snapshot defs and managedsave state prior
to spawning the thread to avoid blocking of the daemon startup which
would serialize on the VM lock.
Also the reloading code would violate the domain job held while
reconnecting as the loader functions don't create jobs.
Coverity pointed out that in other places we always check the return
value from virJSONValueObjectGetNumberLong() but not in the new addition
in leaseshelper. To solve the issue and also be more robust in case
somebody would corrupt the file, skip outputting of the lease entry in
case the expiry time is missing.
https://bugzilla.redhat.com/show_bug.cgi?id=1087104#c5
When trying to use an invalid offset to virStorageVolUpload(), libvirt
fails in virFDStreamOpenFileInternal(), although it seems libvirt does
not check the return in storageVolUpload(), and calls
virFDStreamSetInternalCloseCb() right after. But stream doesn't have a
privateData (is NULL) yet, and the daemon crashes then.
0 0x00007f09429a9c10 in pthread_mutex_lock () from /lib64/libpthread.so.0
1 0x00007f094514dbf5 in virMutexLock (m=<optimized out>) at util/virthread.c:88
2 0x00007f09451cb211 in virFDStreamSetInternalCloseCb at fdstream.c:795
3 0x00007f092ff2c9eb in storageVolUpload at storage/storage_driver.c:2098
4 0x00007f09451f46e0 in virStorageVolUpload at libvirt.c:14000
5 0x00007f0945c78fa1 in remoteDispatchStorageVolUpload at remote_dispatch.h:14339
6 remoteDispatchStorageVolUploadHelper at remote_dispatch.h:14309
7 0x00007f094524a192 in virNetServerProgramDispatchCall at rpc/virnetserverprogram.c:437
Signed-off-by: Luyao Huang <lhuang@redhat.com>
This patch enables the helper program to detect event(s) triggered when
there is a change in lease length or expiry and client-id. This
transfers complete control of leases database to libvirt and obsoletes
use of the lease database file (<network-name>.leases). That file will
not be created, read, or written. This is achieved by adding the option
--leasefile-ro to dnsmasq and passing a custom env var to leaseshelper,
which helps us map events related to leases with their corresponding
network bridges, no matter what the event be.
Also, this requires the addition of a new non-lease entry in our custom
lease database: "server-duid". It is required to identify a DHCPv6
server.
Now that dnsmasq doesn't maintain its own leases database, it relies on
our helper program to tell it about previous leases and server duid.
Thus, this patch makes our leases program honor an extra action: "init",
in which it sends the known info in a particular format to dnsmasq
by printing it to stdout.
The drawback of this change is that upgrade to this new approach does
not transfer the existing leases for the network if the leaseshelper
wasn't already used.
For Intel and PowerPC the implementation is calling a cpu driver
function across driver layers (i.e. from qemu driver directly to cpu
driver).
The correct behavior is to use libvirt API functionality to perform such
a inter-driver call.
This patch introduces a new cpu driver API function getModels() to
retrieve the cpu models. The currect implementation to process the
cpu_map XML content is transferred to the INTEL and PowerPC cpu driver
specific API functions.
Additionally processing the cpu_map XML file is not safe due to the fact
that the cpu map does not exist for all architectures. Therefore it is
better to encapsulate the processing in the architecture specific cpu
drivers.
Signed-off-by: Daniel Hansel <daniel.hansel@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
Based on previous commit, we can now precreate missing volumes. While
digging out the functionality from storage driver would be nicer, if
you've seen the code it's nearly impossible. So I'm going from the
other end:
1) For given disk target, disk path is looked up.
2) For the disk path, storage pool is looked up, a volume XML is
constructed and then passed to virStorageVolCreateXML() which has all
the knowledge how to create raw images, (encrypted) qcow(2) images,
etc.
One of the advantages of this approach is, we don't have to care about
image conversion - qemu does that for us. So for instance, users can
transform qcow2 into raw on migration (if the correct XML is passed to
the migration API).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Up 'til now, users need to precreate non-shared storage on migration
themselves. This is not very friendly requirement and we should do
something about it. In this patch, the migration cookie is extended,
so that <nbd/> section does not only contain NBD port, but info on
disks being migrated. This patch sends a list of pairs of:
<disk target; disk size>
to the destination. The actual storage allocation is left for next
commit.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The function queries the block devices visible to qemu
('query-block') and parses the qemu's output. The info is
returned in a hash table which is expected to be pre-filled by
qemuMonitorJSONGetAllBlockStatsInfo(). However, in the next patch
we are not going to call the latter function at all, so we should
make the former function add devices into the hash table if not
found there.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
While this could be exposed as a public API, it's not done yet as
there's no demand for that yet. Anyway, this is just preparing
the environment for easier volume creation on the destination.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Since virDomainSnapshotFree will call virObjectUnref anyway, let's just use
that directly so as to avoid the possibility that we inadvertently clear out
a pending error message when using the public API.
Since virInterfaceFree will call virObjectUnref anyway, let's just use that
directly so as to avoid the possibility that we inadvertently clear out
a pending error message when using the public API.
Since virNWFilterFree will call virObjectUnref anyway, let's just use that
directly so as to avoid the possibility that we inadvertently clear out
a pending error message when using the public API.
Since virSecretFree will call virObjectUnref anyway, let's just use that
directly so as to avoid the possibility that we inadvertently clear out
a pending error message when using the public API.
Since virStreamFree will call virObjectUnref anyway, let's just use that
directly so as to avoid the possibility that we inadvertently clear out
a pending error message when using the public API.
Since virStoragePoolFree will call virObjectUnref anyway, let's just use that
directly so as to avoid the possibility that we inadvertently clear out
a pending error message when using the public API.
Since virStorageVolFree will call virObjectUnref anyway, let's just use that
directly so as to avoid the possibility that we inadvertently clear out
a pending error message when using the public API.
Since virNodeDeviceFree will call virObjectUnref anyway, let's just use that
directly so as to avoid the possibility that we inadvertently clear out
a pending error message when using the public API.
Since virNetworkFree will call virObjectUnref anyway, let's just use that
directly so as to avoid the possibility that we inadvertently clear out
a pending error message when using the public API.
Since virDomainFree will call virObjectUnref anyway, let's just use that
directly so as to avoid the possibility that we inadvertently clear out
a pending error message when using the public API.
virConnect.privateData is void *, so we can't access
fields of parallelsConn, pointer to which is stored in
virConnect.privateData. So replace all occurences of
conn->privateData->storageState with privconn->storageState.
Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com>
Partially reverts commit 5754dbd.
The code in the specfile adds a MAC address to every <bridge>,
even for <forward mode='bridge'> for which we don't support
changing MAC addresses.
Remove it completely. For new networks, we have been adding
MAC addresses on definition/creation since the commit mentioned above.
For existing networks (pre-0.9.0), the MAC is added by this commit.
https://bugzilla.redhat.com/show_bug.cgi?id=1156367
Since our big split of libvirt.c there are only a few functions
living there. The majority was moved to corresponding subfile,
e.g. domain functions were moved to libvirt-domain.c. However,
the patches for virDomainGetFSInfo() and virDomainFSInfoFree()
introduction were posted prior the big split and merged after.
This resulted in two domain functions landing in wrong file.
Move them to the correct one.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Adding non-existing nwfilter to a network interface device without any
nwfilter specified will crash libvirt daemon with segfault. The reason is
that the nwfilter is not found an libvirt will try to restore old
nwfilter configuration but there is no nwfilter specified.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
The function virNetworkObjListExport() in network_conf.c had a call to
the public API virNetworkFree() which was causing a link error:
CCLD libvirt_driver_vbox_network_impl.la
./.libs/libvirt_conf.a(libvirt_conf_la-network_conf.o): In function `virNetworkObjListExport':
/home/laine/devel/libvirt/src/conf/network_conf.c:4496: undefined reference to `virNetworkFree'
This would happen when I added
#include "network_conf.h"
into domain_conf.h, then attempted to call a new function from that
file (and enum converter, similar to virNetworkForwardTypeToString())
In the end, virNetworkFree() ends up just calling virObjectUnref(obj)
anyway (after clearing all pending errors, which we probably *don't*
want to do in the cleanup of a utility function), so this is likely
more correct than the original code as well.
There is a race condition between the fopen and fscanf calls
in qemuGetProcessInfo. If fopen succeeds, there is a small
possibility that the file no longer exists before reading from it.
Now, if either fopen or fscanf calls fail, the function will behave
just as only fopen had failed.
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1169055
Signed-off-by: Eric Blake <eblake@redhat.com>
Commit id 'cb88d433' refactored the calling sequence to use a thread;
however, in doing so "lost" the check for if virNetSocketAccept returns
failure. Since other code makes that check, Coverity complains. Although
a false positive, adding back the failure check pacifies Coverity
Commit id '0d36a5d05' modified the code slightly, but removed the
return value check thus causing Coverity to complain that this call
was the only one where the return value wasn't checked. Since nothing
was done previously if there was a failure, just use ignore_value here
to pacify Coverity
Coverity complains that many other callers to return err from
virGetLastError() will check if err is not NULL before dereferencing
it. Just do the same here for safety.
Coverity complained that because the cfg->macFilter call checked
net->ifname != NULL before calling ebtablesRemoveForwardAllowIn, then
the virNetDevOpenvswitchRemovePort call should have the same check.
However, if I move the ebtables call prior to the check for TYPE_DIRECT
(where there is a VIR_FREE(net->ifname)), then it seems Coverity is
happy. Since firewall info is tacked on last during setup, removing
it in the opposite order of initialization seems to be natural anyway
https://bugzilla.redhat.com/show_bug.cgi?id=1159180
The virStoragePoolSourceFindDuplicate only checks the incoming definition
against the same type of pool as the def; however, for "scsi_host" and
"fc_host" adapter pools, it's possible that either some pool "scsi_host"
adapter definition is already using the scsi_hostN that the "fc_host"
adapter definition wants to use or some "fc_host" pool adapter definition
is using a vHBA scsi_hostN or parent scsi_hostN that an incoming "scsi_host"
definition is trying to use.
This patch adds the mismatched type checks and adds extraneous comments
to describe what each check is determining.
This patch also modifies the documentation to be describe what scsi_hostN
devices a "scsi_host" source adapter should use and which to avoid. It also
updates the parent definition to specifically call out that for mixed
environments it's better to define which parent to use so that the duplicate
pool checks can be done properly.
https://bugzilla.redhat.com/show_bug.cgi?id=1159180
Move the API from the backend to storage_conf and rename it to
virStoragePoolGetVhbaSCSIHostParent. A future patch will need to
use this functionality from storage_conf
There are some small issue in qemuProcessAttach:
1.Fix virSecurityManagerGetProcessLabel always get pid = 0,
move 'vm->pid = pid' before call virSecurityManagerGetProcessLabel.
2.Use virSecurityManagerGenLabel to get image label.
3.Fix always set selinux label for other security driver label.
Signed-off-by: Luyao Huang <lhuang@redhat.com>
When a block{commit,copy} job was aborted on a domain, block job handler
did not process it correctly, leaving a phantom job in the background.
Any further calls to any blockjob causes "block <jobtype> still active"
error. This patch fixes the blockjob handler so that it checks not only
for VIR_DOMAIN_BLOCK_JOB_FAILED status, but VIR_DOMAIN_BLOCK_JOB_CANCELED
status as well, followed by our existing cleanup routine.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1135169
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
If job is failed in qemuMigrationRun, we expect the jobinfo type as
FAILED. But jobinfo type won't be updated until entering
qemuMigrationWaitForCompletion. We should make it updated in all
conditions. Moreover, we can't use qemuMigrationUpdateJobStatus
here because job may fail in libvirt, so we can't query job status
from QEMU.
Signed-off-by: Wang Rui <moon.wangrui@huawei.com>
The migration job status is traced in qemuMigrationUpdateJobStatus
which is called in qemuMigrationRun. But if migration is cancelled
before the trace such as in qemuMigrationDriveMirror, the jobinfo
type won't be updated to CANCELLED. After this patch, we can get
jobinfo type CANCELLED if migration is cancelled during drive
mirror. Moreover, we can't use qemuMigrationUpdateJobStatus
because from qemu's point of view it's just the drive mirror being
cancelled and the migration hasn't even started yet.
Signed-off-by: Wang Rui <moon.wangrui@huawei.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1160084
As of b6d4dad11b (1.2.5) we are trying to keep the status of FSFreeze
in the guest. Even though I've tried to fixed couple of corner cases
(6ea54769ba), it occurred to me just recently, that the approach is
broken by design. Firstly, there are many other ways to talk to
qemu-ga (even through libvirt) that filesystems can be thawed (e.g.
qemu-agent-command) without libvirt noticing. Moreover, there are
plenty of ways to thaw filesystems without even qemu-ga noticing (yes,
qemu-ga keeps internal track of FSFreeze status). So, instead of
keeping the track ourselves, or asking qemu-ga for stale state, it's
the best to let qemu-ga deal with that (and possibly let guest kernel
propagate an error).
Moreover, there's one bug with the following approach, if fsfreeze
command failed, we've executed fsthaw subsequently. So issuing
domfsfreeze in virsh gave the following result:
virsh # domfsfreeze gentoo
Froze 1 filesystem(s)
virsh # domfsfreeze gentoo
error: Unable to freeze filesystems
error: internal error: unable to execute QEMU agent command 'guest-fsfreeze-freeze': The command guest-fsfreeze-freeze has been disabled for this instance
virsh # domfsfreeze gentoo
Froze 1 filesystem(s)
virsh # domfsfreeze gentoo
error: Unable to freeze filesystems
error: internal error: unable to execute QEMU agent command 'guest-fsfreeze-freeze': The command guest-fsfreeze-freeze has been disabled for this instance
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
virReportSystemError is reserved for reporting system errors, calling it
with VIR_ERR_* error codes produces error messages that do not make any
sense, such as
internal error: guest failed to start: Kernel doesn't support user
namespace: Link has been severed
We should prohibit wrong usage with a syntax-check rule.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
This reverts commit 433b427ff8.
The patch was added in order to overcome a bug in iproute2 and since it
was properly identified as a bug, particularly in openSUSE 13.2, and it
is being worked on [1], the best solution for libvirt seems to be to
keep the old behaviour.
[1] https://bugzilla.novell.com/show_bug.cgi?id=907093
Starting from libvirt-1.2.4, network state XML files moved to another
directory (see commit b9e95491) and libvirt automatically migrates the
network state files to a new location. However, the code used
dirent.d_type which is not supported by all filesystems. Thus, when
libvirt was upgraded on a host which used such filesystem, network state
XMLs were not properly moved and running networks disappeared from
libvirt.
This patch falls back to lstat() whenever dirent.d_type is DT_UNKNOWN to
fix this issue.
https://bugzilla.redhat.com/show_bug.cgi?id=1167145
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Commit 2aa167ca tried to fix the DBus interaction code to allow
callers to use native types instead of 4-byte bools. But in
fixing the issue, I missed the case of an arrayref; Conrad Meyer
shows the following valid complaint issued by clang:
CC util/libvirt_util_la-virdbus.lo
util/virdbus.c:956:13: error: cast from 'bool *' to 'dbus_bool_t *' (aka 'unsigned int *') increases required alignment from 1 to 4 [-Werror,-Wcast-align]
GET_NEXT_VAL(dbus_bool_t, bool_val, bool, "%d");
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
util/virdbus.c:858:17: note: expanded from macro 'GET_NEXT_VAL'
x = (dbustype *)(*xptrptr + (*narrayptr - 1)); \
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1 error generated.
But fixing that points out that we have NEVER supported arrayrefs
of sub-int types (byte, i16, u16, and now bool). Again, while raw
types promote, arrays do not; so the macros HAVE to deal with both
size possibilities rather than assuming that an arrayref uses the
same sizing as the promoted raw type.
Obviously, our testsuite wasn't covering as much as it should have.
* src/util/virdbus.c (GET_NEXT_VAL): Also fix array cases.
(SET_NEXT_VAL): Fix uses of sub-int arrays.
* tests/virdbustest.c (testMessageArray, testMessageArrayRef):
Test it.
Signed-off-by: Eric Blake <eblake@redhat.com>
Commit 6fcddfcd refactored job statistics but missed the jobinfo type updated
in qemuDomainGetJobInfo. After this patch, we can use virDomainGetJobInfo to
get jobinfo type again.
Signed-off-by: Wang Rui <moon.wangrui@huawei.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Commit 'c264eeaa' didn't do the prerequisite 'make syntax-check' before
pushing. There was a <tab> in the whitespace for the comment. Replaced
with spaces and aligned.
pushed as build breaker since Jenkins complained loudly
The typical case where we had a problem is with such a filesystem
definition as created by virt-sandbox-service:
<filesystem type='bind' accessmode='passthrough'>
<source dir='/var/lib/libvirt/filesystems/mysshd/var'/>
<target dir='/var'/>
</filesystem>
In this case, we don't want to unmount the /var subtree or we may
loose the access to the source folder.
Resolving symlinks can fail before mounting any file system if one file
system depends on another being mounted. Symlinks are now resolved in
two passes:
* Before any file system is mounted, but then we are more gentle if
the source path can't be accessed
* Right before mounting a file system, so that we are sure that we
have the resolved path... but then if it can't be accessed we raise
an error.
Due to a change (or bug?) in ip link implementation, the command
'ip link add vnet0...'
is forced into
'ip link add name vnet0...'
The changed command also works on older versions of iproute2, just the
'name' parameter has been made mandatory.
Add attribute to set vgamem_mb parameter of QXL device for QEMU. This
value sets the size of VGA framebuffer for QXL device. Default value in
QEMU is 8MB so reuse it also in libvirt to not break things.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1076098
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
So far we didn't have any option to set video memory size for qemu video
devices. There was only the vram (ram for QXL) attribute but it was valid
only for the QXL video device.
To provide this feature to users QEMU has a dedicated device attribute
called 'vgamem_mb' to set the video memory size. We will use the 'vram'
attribute for setting video memory size for other QEMU video devices.
For the cirrus device we will ignore the vram value because it has
hardcoded video size in QEMU.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1076098
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
QEMU has two different type of QXL display device. The first "qxl-vga"
is for primary video device and second "qxl" is for secondary video
device.
There are also two different ways how to specify those devices on qemu
command line, the first one and obsolete is using "-vga" option and the
current new one is using "-device" option. The "-vga" could be used only
to setup primary video device, so the "-vga qxl" equal to
"-device qxl-vga". Unfortunately the "-vga qxl" doesn't support setting
additional parameters for the device and "-global" option must be used
for this purpose. It's mandatory to use "-global qxl-vga...." to set the
parameters of primary video device previously defined with "-vga qxl".
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1076098
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
The vram attribute was introduced to set the video memory but it is
usable only for few hypervisors excluding QEMU/KVM and the old XEN
driver. Only in case of QEMU the vram was used for QXL.
This patch updates the documentation to reflect current code in libvirt
and also changes the cases when we will set the default vram attribute.
It also fixes existing strange default value for VGA devices 9MB to 16MB
because the video ram should be rounded to power of two.
The change of default value could affect migrations but I found out that
QEMU always round the video ram to power of two internally so it's safe
to change the default value to the next closest power of two and also
silently correct every domain XML definition. And it's also safe because
we don't pass the value to QEMU.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1076098
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
There are two special cases, if the input number is 0 or the number is
larger then 2^31 (for 32bit unsigned int). For the special cases the
return value is 0 because they cannot be rounded.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Get mounted filesystems list, which contains hardware info of disks and its
controllers, from QEMU guest agent 2.2+. Then, convert the hardware info
to corresponding device aliases for the disks.
Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com>
virDomainGetFSInfo returns a list of filesystems information mounted in the
guest, which contains mountpoints, device names, filesystem types, and
device aliases named by libvirt. This will be useful, for example, to
specify mountpoints to fsfreeze when taking snapshot of a part of disks.
Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com>
As qemu is now able to notify us about change of the channel state used
for communication with the guest agent we now can more precisely track
the state of the guest agent.
To allow notifying management apps this patch implements a new event
that will be triggered on changes of the guest agent state.
Improve the monitor function to also retrieve the guest state of
character device (if provided) so that we can refresh the state of
virtio-serial channels and perhaps react to changes in the state in
future patches.
This patch changes the returned data from qemuMonitorGetChardevInfo to
return a structure containing the pty path and the state for all the
character devices.
The change to the testsuite makes sure that the data is parsed
correctly.
This patch contains three domain cleanup improvements in the migration
finish phase, ensuring a domain is properly disposed when a failure is
detected or the migration is cancelled.
The check for virDomainObjIsActive is moved to libxlDomainMigrationFinish,
where cleanup can occur if migration failed and the domain is inactive.
The 'cleanup' label was missplaced in libxlDomainMigrationFinish, causing
a migrated domain to remain in the event of an error or cancelled migration.
In cleanup, the domain was not removed from the driver's list of domains.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
During the perform phase of migration, the domain is started on
the dst host in a running state if VIR_MIGRATE_PAUSED flag is not
specified. In the finish phase, the domain is also unpaused if
VIR_MIGRATE_PAUSED flag is unset. I've noticed this second unpause
fails if the domain was already unpaused following the perform phase.
This patch changes the perform phase to always start the domain
paused, and defers unpausing, if requested, to the finish phase.
Unpausing should occur in the finish phase anyhow, where the domain
can be properly destroyed if the perform phase fails and migration
is cancelled.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Moving data reception of the perform phase of migration to a
thread introduces a race with the finish phase, where checking
if the domain is active races with the thread finishing the
perform phase. The race is easily solved by acquiring a job in
the finish phase, which must wait for the perform phase job to
complete.
While wrapping the finish phase in a job, noticed the virDomainObj
was being unlocked in a callee - libxlDomainMigrationFinish. Move
the unlocking to libxlDomainMigrateFinish3Params, where the lock
is acquired.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
The libxl driver receives migration data within an IO callback invoked
by the event loop, effectively disabling the event loop while migration
occurs.
This patch moves receving of the migration data to a thread. The
incoming connection is still accepted in the IO callback, but control
is immediately returned to the event loop after spawning the thread.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Specifying an explicit path to pygrub (e.g. BINDIR "/pygrub") only works if
Xen and libvirt happen to be installed to the same prefix. A more flexible
approach is to simply specify "pygrub" which will cause libxl to use the
correct path which it knows (since it is built with the same prefix as pygrub).
This is particular problematic in the Debian packaging, since the Debian Xen
package relocates pygrub into a libexec dir, however I think this change makes
sense upstream.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
To be able to express some use cases of the RBD backing with libvirt, we
need to be able to specify a config file for the RBD client to qemu as
that is one of the commonly used options.
Some storage systems have internal support for snapshots. Libvirt should
be able to select a correct snapshot when starting a VM.
This patch adds a XML element to select a storage source snapshot for
the RBD protocol which supports this feature.
As we now have a common function to parse backing store string for RBD
backing store we can reuse it in the backing store walker so that we
don't fail on files backed by RBD storage.
This patch also adds a few tests to verify that the parsing works as
expected.
To allow reuse this non-trivial parser code in the backing store parser
this part of the command line parser needs to be split out into a
separate funciton.
Instead of splitting out various fields, pass the complete structure and
let the function pick various things of it.
As one of the callers isn't using virStorageSourcePtr to store the data,
this patch adds glue code that fills the data into a dummy
virStorageSourcePtr before calling the func.
This change will help when adding new fields that need output processing
in the future.
If there are no hosts for a storage source virStorageSourceCopy and
virStorageSourceNewFromBackingRelative would try to copy them anyways.
As the success of virStorageNetHostDefCopy is determined by returning
a pointer and malloc of 0 elements might return NULL according to the
implementation, the result of the copy function may vary.
Fix this by copying the hosts array only if there are hosts defined.
As we now have a deep copy function for struct virStorageSource add a
notice that extensions of the structure require also appropriate changes
to the virStorageSourceCopy func.
New qemu added a new event that is emitted when a virtio serial channel
is opened in the guest OS. This allows us to update the state of the
port in the output-only XML element.
This patch implements the monitor callbacks and necessary handlers to
update the state in the definition.
To track state of virtio channels this patch adds a new output-only
attribute called 'state' to the <target> element of virtio channels.
This will be later populated with the guest state of the channel.
To unify future additions that require information from "query-chardev"
rename qemuMonitorGetPtyPaths and friends to qemuMonitorGetChardevInfo
and move the allocation of the returned hash into the top level
function.
When creating a disk image snapshot the libvirt code would blindly copy
the parents label to the newly created image. This runs into problems
when you start a VM from an image hosted on NFS (or other storage system
that doesn't support selinux labels) and the snapshot destination is on
a storage system that does support selinux labels. Libvirt's code in
that case generates a different security label for the image hosted on
NFS. This label is valid only for NFS images and doesn't allow access in
case of a locally stored image.
To fix this issue libvirt needs to refrain from copying security
information in cases where the default domain seclabel is a better
choice.
This patch repurposes the now unused @force argument of
virStorageSourceInitChainElement to denote whether a copy of the
security labelling stuff should be attempted or not. This allows to
fine-control the copy operation for cases where we need to keep the
label of the old disk vs. the cases where we need to keep the label
unset to use the default domain imagelabel.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1151718
Commit c0e7022 breaks on a machine that lacks dbus headers:
In file included from util/virdbus.c:24:0:
util/virdbuspriv.h:31:3: error: unknown type name 'dbus_int16_t'
* src/util/virdbuspriv.h (DBusBasicValue): Only provide fallback
when dbus is compiled.
Signed-off-by: Eric Blake <eblake@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1152382
When libvirt create's the vport (VPORT_CREATE) for the vHBA, there isn't
enough "time" between the creation and the running of the following
backend->refreshPool after a backend->startPool in order to find the LU's.
Population of LU's happens asynchronously when udevEventHandleCallback
discovers the "new" vHBA port. Creation of the infrastructure by udev
is an iterative process creating and discovering actual storage devices and
adjusting the environment.
Because of the time it takes to discover and set things up, the backend
refreshPool call done after the startPool call will generally fail to
find any devices. This leaves the newly started pool appear empty when
querying via 'vol-list' after startup. The "workaround" has always been
to run pool-refresh after startup (or any time thereafter) in order to
find the LU's. Depending on how quickly run after startup, this too may
not find any LUs in the pool. Eventually though given enough time and
retries it will find something if LU's exist for the vHBA.
This patch adds a thread to be executed after the VPORT_CREATE which will
attempt to find the LU's without requiring the external run of refresh-pool.
It does this by waiting for 5 seconds and searching for the LU's. If any
are found, then the thread completes; otherwise, it will retry once more
in another 5 seconds. If none are found in that second pass, the thread
gives up.
Things learned while investigating this... No need to try and fill the
pool too quickly or too many times. Over the course of creation, the udev
code may 'add', 'change', and 'delete' the same device. So if the refresh
code runs and finds something, it may display it only to have a subsequent
refresh appear to "lose" the device. The udev processing doesn't seem to
have a way to indicate that it's all done with the creation processing of a
newly found vHBA. Only the Lone Ranger has silver bullets to fix everything.
Fix a problem in the getBlockDevice and processLU where retval initialized
to zero causing some failures to erroneously continue through to the
virStorageBackendSCSINewLun with an attempt to find a path for "/dev/(null)".
This would fail approximately 10 seconds later with debug message:
virStorageBackendSCSINewLun:203 :
No stable path found for '/dev/(null)' in '/dev/disk/by-path'
The root cause of the issue is for many /sys/bus/scsi/devices/<lun path>
there is no "block*" device found for the vHBA's, where "<lun path>" are
the various paths created for the vHBA, such as "17:0:0:0", "17:0:1:0",
"17:0:2:0", "17:0:3:0", etc. If the block device isn't found, then the
directory should just be ignored rather than attempting to process it.
The bug was that in getBlockDevice the assumption was "block" would exist
and either getNewStyleBlockDevice or getOldStyleBlockDevice would fill in
@block_device. However, if 'block*' doesn't exist, then the code returned
NULL for block_device *and* a good (zero) retval value. This caused the
processLU code to attempt the virStorageBackendSCSINewLun which failed
"at some point in time" in the future.
After this change - on test system the refresh-pool didn't have a noticable
pause of about 20 seconds - it completed within a second since no longer
was there an attempt/need to find "/dev/(null)".
Additionally, the virStorageBackendSCSIFindLU's shouldn't be declaring
found unless the processLU actually returns success. This will be
important in the followup patch which relies on whether a LU was found.
Compilation on a RHEL 5 host failed, due to the older dbus headers
present on that machine, and triggered by commit 2aa167ca:
util/virdbus.c: In function 'virDBusMessageIterDecode':
util/virdbus.c:952: error: 'DBusBasicValue' undeclared (first use in this function)
* m4/virt-dbus.m4 (LIBVIRT_CHECK_DBUS): Check for DBusBasicValue.
* src/util/virdbuspriv.h (DBusBasicValue): Provide fallback.
Signed-off-by: Eric Blake <eblake@redhat.com>
getsockopt(sock->fd, SOL_SOCKET, SO_PEERCRED, ...) sets the pid to 0
when the process that opens the connection is in another container.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Commit dc33e6e4 caused older platforms like Fedora 20 to emit
scary log messages at startup:
2014-11-19 23:12:58.800+0000: 28906: error : virCommandWait:2532 : internal error: Child process (/usr/sbin/iptables -w -L -n) unexpected exit status 2: iptables v1.4.19.1: unknown option "-w"
Try `iptables -h' or 'iptables --help' for more information.
Since we are probing and expect to handle the case where -w is not
supported, we should not let virCommand log it as an error.
* src/util/virfirewall.c (virFirewallCheckUpdateLock): Handle
non-zero status ourselves.
Signed-off-by: Eric Blake <eblake@redhat.com>
Oops, I forgot to squash one more instance of the same check in the
previous commit (v1.2.10-144-g52691f9).
https://bugzilla.redhat.com/show_bug.cgi?id=1147331
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Any attempt to start a tunnelled migration with libvirtd that supports
RDMA migration (specifically commit v1.2.8-226-ged22a47) crashes
libvirtd on the destination host.
The crash is inevitable because qemuMigrationPrepareAny is always called
with NULL protocol in case of tunnelled migration.
https://bugzilla.redhat.com/show_bug.cgi?id=1147331
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
As discussed on the upstream list, it's better not to make this
kind of predictions in libvirt. It may happen that qemu learns
how to enable OVMF on other architectures too and we shouldn't
try to chase that.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Currently, we are whitelisting architectures, that we know how to run
OVMF on. So far, only x86_64 was enabled. However, looking at qemu
code, the same commandline can be used to enable OVMF for armv7l and
aarch64.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
I noticed this while working on qemuDomainGetBlockInfo. Assigning
a bool value to an int variable compiles fine, but raises red flags
on the maintenance front as it becomes too easy to assign -1 or 2
or any other non-bool value to the same variable.
* cfg.mk (sc_prohibit_int_assign_bool): New rule.
* src/conf/snapshot_conf.c (virDomainSnapshotRedefinePrep): Fix
offenders.
* src/qemu/qemu_driver.c (qemuDomainGetBlockInfo)
(qemuDomainSnapshotCreateXML): Likewise.
* src/test/test_driver.c (testDomainSnapshotAlignDisks):
Likewise.
* src/util/vircgroup.c (virCgroupSupportsCpuBW): Likewise.
* src/util/virpci.c (virPCIDeviceBindToStub): Likewise.
* src/util/virutil.c (virIsCapableVport): Likewise.
* tools/virsh-domain-monitor.c (cmdDomMemStat): Likewise.
* tools/virsh-domain.c (cmdBlockResize, cmdScreenshot)
(cmdInjectNMI, cmdSendKey, cmdSendProcessSignal)
(cmdDetachInterface): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
Use of an 'int' to represent a 'bool' value is confusing. Just
because dbus made the mistake of cementing their 4-byte wire
format of dbus_bool_t into their API doesn't mean we have to
repeat the mistake. With a little bit of finesse, we can
guarantee that we provide a large-enough value to the DBus
code, while still copying only the relevant one-byte bool
to the client code, and isolate the rest of our code base from
the DBus stupidity.
* src/util/virdbus.c (GET_NEXT_VAL): Add parameter.
(virDBusMessageIterDecode): Adjust all clients.
* src/util/virpolkit.c (virPolkitCheckAuth): Use nicer type.
* tests/virdbustest.c (testMessageSimple, testMessageStruct):
Test new behavior.
Signed-off-by: Eric Blake <eblake@redhat.com>
This function returned non-inactive domains instead of active
domains. This broke virConnectNumOfDefinedDomains() and
virConnectListDefinedDomains() functions.
Ethernet interfaces in libvirt currently do not support bandwidth setting.
For example, following xml file for an interface will not apply these
settings to corresponding qdiscs.
<interface type="ethernet">
<mac address="02:36:1d:18:2a:e4"/>
<model type="virtio"/>
<script path=""/>
<target dev="tap361d182a-e4"/>
<bandwidth>
<inbound average="984" peak="1024" burst="64"/>
<outbound average="2000" peak="2048" burst="128"/>
</bandwidth>
</interface>
Signed-off-by: Anirban Chakraborty <abchak@juniper.net>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
For some reason, commit id '72b4151f' triggered a Coverity uninitialized
'reply' variable check when referenced within the for loop.
It seems Coverity doesn't know that flags will have to be either AFFECT_LIVE
or AFFECT_CONFIG after the virDomainLiveConfigHelperMethod call.
By adding a "sa_assert()" to confirm that fact, Coverity is happy again.
https://bugzilla.redhat.com/show_bug.cgi?id=1164080
After a disk is hotunplugged a subsequent call to qemuDomainGetBlockIoTune
to get the --config settings of that disk will fail because the disk is no
longer found by qemuDiskPathToAlias causing an unexpected failure.
Since only the --live flag needs to have the disk device pointer, move the
fetch inside the (flags & VIR_DOMAIN_AFFECT_LIVE) condition. This will also
affect the results if no flags are provided or the --current flag is provided.
Signed-off-by: Luyao Huang <lhuang@redhat.com>
Seems the 'size_iops_sec' was a late add and the checks for whether
the field was defined, but unsupported and the maximum size of the
field were not being made.
Also, adjust blkdeviotune support error message for grammar, spelling
(paramater), and remove the "(need QEMU 1.7 or superior)". None of
our other similar error messages list which QEMU version is required.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Rule sc_prohibit_newline_at_end_of_diagnostic for syntax-check does
check for passing strings ending with '\n' two lines after known
functions. This is, of course subject to false positives, so for the
sake of future changes, trick that syntax-check by adding one more line
with a comment.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
A previous commit introduced use of locking with invocation
of iptables in the viriptables.c module
commit ba95426d6f
Author: Serge Hallyn <serge.hallyn@ubuntu.com>
Date: Fri Nov 1 12:36:59 2013 -0500
util: use -w flag when calling iptables
This only ever had effect with the virtual network driver,
as it was not wired up into the nwfilter driver. Unfortunately
in the firewall refactoring the use of the -w flag was
accidentally lost.
This patch introduces it to the virfirewall.c module so that
both the virtual network and nwfilter drivers will be using
it. It also ensures that the equivalent --concurrent flag
to ebtables is used.
Since QEMU 1.2.0, we switched to QMP probing instead of parsing -help
(and other commands, such as -cpu ?) output. However, if QMP probing
failed, we still tried starting QEMU with various options and parsing
the output, which was guaranteed to fail because the output changed.
Let's just refuse parsing -help for QEMU >= 1.2.0.
https://bugzilla.redhat.com/show_bug.cgi?id=1160318
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
We used to set migration capabilities only when a user asked for them in
flags. This is fine when migration succeeds since the QEMU process is
killed in the end but in case migration fails or if it's cancelled, some
capabilities may remain turned on with no way to turn them off. To fix
that, migration capabilities have to be turned on if requested but
explicitly turned off in case they were not requested but QEMU supports
them.
https://bugzilla.redhat.com/show_bug.cgi?id=1163953
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Rather than just picking the first CD (or failing that, HDD) we come
across, if the user has picked a boot device ordering with <boot
order=''>, respect that (and just try to boot the lowest-index device).
Adds two sets of tests to bhyve2xmlargv; 'grub-bootorder' shows that we
pick a user-specified device over the first device in the domain;
'grub-bootorder2' shows that we pick the first (lowest index) device.
When user calls setmem on a running LXC machine, we do update its cgroup
entry, however we neither update domain's runtime XML nor
we update our internal structures and this patch fixes it.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1131919
Commit 6e5c79a1 tried to fix deadlock between nwfilter{Define,Undefine}
and starting of guest, but this same deadlock exists for
updating/attaching network device to domain.
The deadlock was introduced by removing global QEMU driver lock because
nwfilter was counting on this lock and ensure that all driver locks are
locked inside of nwfilter{Define,Undefine}.
This patch extends usage of virNWFilterReadLockFilterUpdates to prevent
the deadlock for all possible paths in QEMU driver. LXC and UML drivers
still have global lock.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1143780
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
In one of my previous patches (3a3c3780b) I've tried to fix the
problem of nvram path disappearing on a domain that's been
started and shut down again. I fixed this by explicitly saving
domain's config file. However, I did a bit of clumsy without
realizing we have a transient domains for which we don't save the
config file. Hence, any domain using UEFI became persistent.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1160926
Introduce a 'managed' attribute to allow libvirt to decide whether to
delete a vHBA vport created via external means such as nodedev-create.
The code currently decides whether to delete the vHBA based solely on
whether the parent was provided at creation time. However, that may not
be the desired action, so rather than delete and force someone to create
another vHBA via an additional nodedev-create allow the configuration of
the storage pool to decide the desired action.
During createVport when libvirt does the VPORT_CREATE, set the managed
value to YES if not already set to indicate to the deleteVport code that
it should delete the vHBA when the pool is destroyed.
If libvirtd is restarted all the memory only state was lost, so for a
persistent storage pool, use the virStoragePoolSaveConfig in order to
write out the managed value.
Because we're now saving the current configuration, we need to be sure
to not save the parent in the output XML if it was undefined at start.
Saving the name would cause future starts to always use the same parent
which is not the expected result when not providing a parent. By not
providing a parent, libvirt is expected to find the best available
vHBA port for each subsequent (re)start.
At deleteVport, use the new managed value to decide whether to execute
the VPORT_DELETE. Since we no longer save the parent in memory or in
XML when provided, if it was not provided, then we have to look it up.
https://bugzilla.redhat.com/show_bug.cgi?id=1160926
Passing a copy of the storage pool adapter to a function just changes the
copy of the fields in the particular function and then when returning to
the caller those changes are discarded. While not yet biting us in the
storage clean-up case, it did cause an issue for the fchost storage pool
startup case, createVport. The issue was at startup, if no parent is found
in the XML, the code will search for the 'best available' parent and then
store that in the in memory copy of the adapter. Of course, in this case
it was a copy, so when returning to the virStorageBackendSCSIStartPool that
change was discarded (or lost) from the pool->def->source.adapter which
meant at shutdown (deleteVport), the code assumed no adapter was passed
and skipped the deletion, leaving the vHBA created by libvirt still defined
requiring an additional stop of a nodedev-destroy to remove.
Adjusted the createVport to take virStoragePoolDefPtr instead of the
adapter copy. Then use the virStoragePoolSourceAdapterPtr when processing.
A future patch will need the 'def' anyway, so this just sets up for that.
https://bugzilla.redhat.com/show_bug.cgi?id=1160565
The existing code assumed that the configuration of a 'parent' attribute
was correct for the createVport path. As it turns out, that may not be
the case which leads errors during the deleteVport path because the
wwnn/wwpn isn't associated with the parent.
With this change the following is reported:
error: Failed to start pool fc_pool_host3
error: XML error: Parent attribute 'scsi_host4' does not match parent 'scsi_host3' determined for the 'scsi_host16' wwnn/wwpn lookup.
for XML as follows:
<pool type='scsi'>
<name>fc_pool</name>
<source>
<adapter type='fc_host' parent='scsi_host4' wwnn='5001a4aaf3ca174b' wwpn='5001a4a77192b864'/>
</source>
Where 'nodedev-dumpxml scsi_host16' provides:
<device>
<name>scsi_host16</name>
<path>/sys/devices/pci0000:00/0000:00:04.0/0000:10:00.0/host3/vport-3:0-11/host16</path>
<parent>scsi_host3</parent>
<capability type='scsi_host'>
<host>16</host>
<unique_id>13</unique_id>
<capability type='fc_host'>
<wwnn>5001a4aaf3ca174b</wwnn>
<wwpn>5001a4a77192b864</wwpn>
...
The patch also adjusts the description of the storage pool to describe the
restrictions.
Signed-off-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1160565
If a 'parent' attribute is provided for the fchost, then at startup
time check to ensure it is a vport capable scsi_host. If the parent
is not vport capable, then disallow the startup. The following is the
expected results:
error: Failed to start pool fc_pool
error: XML error: parent 'scsi_host2' specified for vHBA is not vport capable
where the XML for the fc_pool is:
<pool type='scsi'>
<name>fc_pool</name>
<source>
<adapter type='fc_host' parent='scsi_host2' wwnn='5001a4aaf3ca174b' wwpn='5001a4a77192b864'/>
</source>
...
and 'scsi_host2' is not vport capable.
Providing an incorrect parent and a correct wwnn/wwpn could lead to
failures at shutdown (deleteVport) where the assumption is the parent
is for the fchost.
NOTE: If the provided wwnn/wwpn doesn't resolve to an existing scsi_host,
then we will be creating one with code (virManageVport) which
assumes the parent is vport capable.
Signed-off-by: John Ferlan <jferlan@redhat.com>
This enables booting interactive GRUB menus (e.g. install CDs) with
libvirt-bhyve.
Caveat: A terminal other than the '--console' option to 'virsh start'
(e.g. 'cu -l /dev/nmdm0B -s 115200') must be used to connect to
grub-bhyve because the bhyve loader path is synchronous and must occur
before the VM actually starts.
Changing the bhyveProcessStart logic around to accommodate '--console'
for interactive loader use seems like a significant project and probably
not worth it, if UEFI/BIOS support for bhyve is "coming soon."
We still default to bhyveloader(1) if no explicit bootloader
configuration is supplied in the domain.
If the /domain/bootloader looks like grub-bhyve and the user doesn't
supply /domain/bootloader_args, we make an intelligent guess and try
chainloading the first partition on the disk (or a CD if one exists,
under the assumption that for a VM a CD is likely an install source).
Caveat: Assumes the HDD boots from the msdos1 partition. I think this is
a pretty reasonable assumption for a VM. (DrvBhyve with Bhyveload
already assumes that the first disk should be booted.)
I've tested both HDD and CD boot and they seem to work.
Use the device type name if we know it instead of its number,
even if we can't hotplug it:
qemuMonitorJSONAttachCharDevCommand:6094 : operation failed: Unsupported
char device type '10'
virDomainChrSourceDefIsEqual should return 'true' for
identical SPICEVMC chardevs, and those that have no source
specification.
After this change, a failed hotplug no longer leaves a stale
pointer in the domain definition.
https://bugzilla.redhat.com/show_bug.cgi?id=1162097
If the memory mode is specified as 'strict' and with one node, we
get the following error when starting domain.
error: Unable to write to '$cgroup_path/cpuset.mems': Device or resource busy
XML is configured with numatune as follows:
<numatune>
<memory mode='strict' nodeset='0'/>
</numatune>
It's broken by Commit 411cea638f
which moved qemuSetupCgroupForEmulator() before setting cpuset.mems
in qemuSetupCgroupPostInit.
Directory '$cgroup_path/emulator/' is created in qemuSetupCgroupForEmulator.
But '$cgroup_path/emulator/cpuset.mems' it not set and has a default value
(all nodes, such as 0-1). Then we setup '$cgroup_path/cpuset.mems' to the
nodemask (in this case it's '0') in qemuSetupCgroupPostInit. It must fail.
This patch makes '$cgroup_path/emulator/cpuset.mems' is set before
'$cgroup_path/cpuset.mems'. The action is similar with that in
qemuDomainSetNumaParamsLive.
Signed-off-by: Wang Rui <moon.wangrui@huawei.com>
If the memory mode in numatune is not 'strict', we should not setup
cpuset.mems. Before commit 1a7be8c600
we have checked the memory mode in virDomainNumatuneGetNodeset. This
patch adds the check as before.
Signed-off-by: Wang Rui <moon.wangrui@huawei.com>
If the memory mode in numatune is specified as 'preferred' with one node
(such as nodeset='0'), domain's memory is not all in node 0 absolutely.
Assumption that node 0 doesn't have enough memory, memory can be allocated
on node 1 when qemu process startup. Then if we set cpuset.mems to '0',
it may invoke OOM.
Commit 1a7be8c600 changed the former logic of
checking memory mode in virDomainNumatuneGetNodeset. This patch adds the
check as before.
Signed-off-by: Wang Rui <moon.wangrui@huawei.com>
This patch fixes the following issues.
1) When an invalid wwn is introduced, libvirt reports
"Malformed wwn: %s". The template won't be replaced.
2) "target" option for dompmsuspend and "xml" option for
save-image-define are required options and should use
VSH_OT_DATA instead of VSH_OT_STRING as an option type.
3) A typo.
Signed-off-by: Hao Liu <hliu@redhat.com>
Coverity found out that commit cd490086 caused a possible NULL pointer
dereference. This is due to the fact, that phyp_driver is NULL at the
time of closing the socket, instead of connection_data, which kept the
socket before the mentioned commit, could not be NULL.
However, internal_socket is still the local socket that can be
closed, even unconditionally, if we initialize it to -1.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Check the arability of the options with the current qemu binary,
add them in the varable opt if yes, print a message if not.
Signed-off-by: Matthias Gatto <matthias.gatto@outscale.com>
Detect if the the qemu binary currently in use support the bps_max option,
If yes add it to the command, if not, just ignore the option.
We don't print error here, because the check for invalide arguments
has alerady been made in qemu_driver.c
Signed-off-by: Matthias Gatto <matthias.gatto@outscale.com>
Add support for bps_max and friends in the driver part.
In the part checking if a qemu is running, check if the running binary
support bps_max, if not print an error message, if yes add it to
"info" variable
Signed-off-by: Matthias Gatto <matthias.gatto@outscale.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Add the capability to detect if the qemu binary have the capability
to use bps_max and friends
Add a value in the enum virQEMUCapsFlags for the qemu capability.
Set it with virQEMUCapsSet if the binary suport bps_max and they friends.
Signed-off-by: Matthias Gatto <matthias.gatto@outscale.com>
Modify the structure _virDomainBlockIoTuneInfo to support these the new
options.
Change the initialization of the variable expectedInfo in qemumonitorjsontest.c
to avoid compiling problem.
Add documentation about the new xml options
Signed-off-by: Matthias Gatto <matthias.gatto@outscale.com>
nodeSetMemoryParameters() will call nodeSetMemoryParameterValue()
to set parameters. But it just filter the return code '-2' as
failure. Indeed we should report error when rc is negative.
https://bugzilla.redhat.com/show_bug.cgi?id=1161541
Signed-off-by: Jincheng Miao <jmiao@redhat.com>
CPU numa topology implicitly allows memory specification in 'KiB'.
Enabling this to accept the 'unit' in which memory needs to be specified.
This now allows users to specify memory in units of choice, and
lists the same in 'KiB' -- just like other 'memory' elements in XML.
<numa>
<cell cpus='0-3' memory='1024' unit='MiB' />
<cell cpus='4-7' memory='1024' unit='MiB' />
</numa>
Also augment test cases to correctly model NUMA memory specification.
This adds the tag 'unit="KiB"' for memory attribute in NUMA cells.
Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Commit 01b4de2b9f abstracts virDomainParseMemory()
for use by other functions in domain_conf.c
Extend the same for use, for functions outside of this file.
Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Store version numbers in this format
version = 1000000 * major + 1000 * minor + micro
produced by virParseVersionString instead of dedicated enums.
Split the complex esxVI_ProductVersion enum into a simpler
esxVI_ProductLine enum and a product version number.
Relax API and product version number checks to accept everything that
is equal or greater than the supported minimum version. VMware ESX
went through 3 major versions and the vSphere API always stayed
backward compatible. This commit assumes that this will also be true
for future VMware ESX versions.
Also reword error messages in esxConnectTo* to say what was expected
and what was found instead (suggested by Richard W.M. Jones).
As reviewing patches upstream it occurred to me, that we have two
functions doing nearly the same: virDomainParseMemory which
expects XML in the following format:
<memory unit='MiB'>1337</memory>
The other function being virDomainHugepagesParseXML expecting the
following format:
<someElement size='1337' unit='MiB'/>
It wouldn't matter to have two functions handle two different
scenarios like this if we could only not copy code that handles
32bit arches around. So this code merges the common parts into
one by inventing new @units_xpath argument to
virDomainParseMemory which allows overriding the default location
of @unit attribute in XML. With this change both scenarios above
can be parsed with virDomainParseMemory.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
If detect_scsi_host_caps reports errors but keeps libvirtd going on
startup, the user is misled by the error messages. Transforming them
into warning still shows the problems, but indicates this is not fatal.
Introduced by commit c63ef0452b, when nodeset is NULL, validation will
pass in virNumaSetupMemoryPolicy, but virBitmapNextSetBit must ensure
bitmap is not NULL, otherwise that might cause a segmentation fault.
This patch fixes it.
Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
The shared netcf driver is stateful and inside the daemon so
there is no need to use the networkPrivateData field to get the
driver handle. Just access the global driver handle directly.
The shared network driver is stateful and inside the daemon so
there is no need to use the networkPrivateData field to get the
driver handle. Just access the global driver handle directly.
Many places already directly accessed the global driver handle
in any case, so the code could never work without relying on
this.
The shared storage driver is stateful and inside the daemon so
there is no need to use the storagePrivateData field to get the
driver handle. Just access the global driver handle directly.
Since the secondary drivers are only active when the primary
driver is also the Test driver, there is no need to use the
different type specific privateData fields.
Since the secondary drivers are only active when the primary
driver is also the Parallels driver, there is no need to use the
different type specific privateData fields. The object that was
being stored in the storagePrivateData can easily be kept in the
parallelsConn struct instead.
For inexplicable reasons the phyp driver defined two separate
structs for holding its private data. One it keeps in privateData
and the other it keeps in networkPrivateData. It uses them both
from all API driver methods. Merge the two separate structs
into one to remove this horrible abuse.
Since the secondary drivers are only active when the primary
driver is also the Hyper-V driver, there is no need to use the
different type specific privateData fields.
Since the secondary drivers are only active when the primary
driver is also the ESX driver, there is no need to use the
different type specific privateData fields.
Since the secondary drivers are only active when the primary
driver is also the remote driver, there is no need to use the
different type specific privateData fields.
The remote driver has had a long term hack to deal with the fact
that the old Xen driver worked outside libvirtd, but the rest
of the drivers worked inside. So you could have a local hypervisor
driver but everything else go via the remote driver. The Xen driver
long ago moved inside libvirtd, so this hack is no longer needed.
Thus we should open use the remote driver for secondary drivers
if the primary driver is already the remote driver.
IBM Power processors differ uniquely across generations (such as power6,
power7, power8). Each generation signifies a new PowerISA version
that exhibits features unique to that generation.
The higher 16 bits of PVR for IBM Power processors encode the CPU
generation, while the CPU chip (sub)version is encoded in lower 16 bits.
For all practical purposes of launching a VM, we care about the
generation which the vCPU will belong to, and not specifically the chip
version. This patch updates the libvirt PVR check to reflect this
relationship. It allows libvirt to select the right CPU generation
in case the exact match for a a specific CPU is not found.
Hence, there will no longer be a need to add each PowerPC CPU model to
cpu_map.xml; just adding entry for the matching ISA generation will
suffice.
It also contains changes to cpu_map.xml since processor generations
as understood by QEMU compat mode go as "power6", "power7" or "power8"
[Reference : QEMU commit 8dfa3a5e85 ]
Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>
Signed-off-by: Pradipta Kr. Banerjee <bpradip@in.ibm.com>
Signed-off-by: Anton Blanchard <anton@samba.org>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
