https://bugzilla.redhat.com/show_bug.cgi?id=1034993
SCSI passthrough disks (<disk .. device="lun">) can't be used as backing
for snapshots. Currently with upstream qemu the vm crashes on such
attempt.
This patch adds a early check to catch an attempt to do such a snapshot
and rejects it right away. qemu will fix the issue but this will let us
control the error message.
We shouldn't access the domain definition while we are in the monitor
section as the domain is unlocked. Additionally after we exit from the
monitor we need to check if the VM is still alive. Not doing so resulted
in a crash if qemu exits while attempting to do an external VM snapshot.
spice-server offers an API to disable file transfer messages
on the agent channel between the client and the guest.
This is supported in qemu through the disable-agent-file-xfer option.
This patch exposes this option to libvirt.
Adds a new element 'filetransfer', with one property,
'enable', which accepts a boolean.
Default is enabled, for backward compatibility.
Depends on the capability exported in the first patch of the series.
Signed-off-by: Francesco Romani <fromani@redhat.com>
spice-server offers an API to disable file transfer messages
on the agent channel between the client and the guest.
This is supported in qemu through the disable-agent-file-xfer option.
This patch detects if QEMU supports this option, and add
a capability if does.
Signed-off-by: Francesco Romani <fromani@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1046919
If none (KVM, VFIO) of the supported PCI passthrough methods is known to
work on a host, it's better to fail right away with a nice error message
rather than letting attachment fail with a more cryptic message such as
Failed to bind PCI device '0000:07:05.0' to vfio-pci: No such device
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
With this patch, user can setup throttle blkio cgroup
through virsh for qemu domain.
Signed-off-by: Guan Qiang <hzguanqiang@corp.netease.com>
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
This patch introduces virCgroupSetBlkioDeviceReadIops,
virCgroupSetBlkioDeviceWriteIops,
virCgroupSetBlkioDeviceReadBps and
virCgroupSetBlkioDeviceWriteBps,
we can use these interfaces to set up throttle
blkio cgroup for domain.
This patch also adds the new throttle blkio cgroup
elements to the test xml.
Signed-off-by: Guan Qiang <hzguanqiang@corp.netease.com>
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
While auditing the error reporting, I noticed that migration
had some issues. Some of the static helper functions tried
to call virDispatchError(), even though their caller will also
report the error. Also, if a migration is cancelled early
because a uri was not set, we did not guarantee that the finish
stage would not overwrite the first error message.
* src/qemu/qemu_migration.c (doPeer2PeerMigrate2)
(doPeer2PeerMigrate3): Preserve first error when cancelling.
* src/libvirt.c (virDomainMigrateVersion3Full): Likewise.
(virDomainMigrateVersion1, virDomainMigrateVersion2)
(virDomainMigrateDirect): Avoid redundant error dispatch.
(virDomainMigrateFinish2, virDomainMigrateFinish3)
(virDomainMigrateFinish3Params): Don't report error on cleanup
path.
Signed-off-by: Eric Blake <eblake@redhat.com>
The public virConnectRef and virConnectClose API are just thin
wrappers around virObjectRef/virObjectRef, with added object
validation and an error reset. Within our backend drivers, use
of the object validation is just an inefficiency since we always
pass valid objects. More important to think about is what
happens with the error reset; our uses of virConnectRef happened
to be safe (since we hadn't encountered any earlier errors), but
in several cases the use of virConnectClose could lose a real
error.
Ideally, we should also avoid calling virConnectOpen() from
within backend drivers - but that is a known situation that
needs much more design work.
* src/qemu/qemu_process.c (qemuProcessReconnectHelper)
(qemuProcessReconnect): Avoid nested public API call.
* src/qemu/qemu_driver.c (qemuAutostartDomains)
(qemuStateInitialize, qemuStateStop): Likewise.
* src/qemu/qemu_migration.c (doPeer2PeerMigrate): Likewise.
* src/storage/storage_driver.c (storageDriverAutostart):
Likewise.
* src/uml/uml_driver.c (umlAutostartConfigs): Likewise.
* src/lxc/lxc_process.c (virLXCProcessAutostartAll): Likewise.
(virLXCProcessReboot): Likewise, and avoid leaking conn on error.
Signed-off-by: Eric Blake <eblake@redhat.com>
There is a number of reported issues when we fail starting a domain.
Turns out that, in some scenarios like high load, 3 second timeout is
not enough for qemu to start up to the phase where the socket is
created. Since there is no downside of waiting longer, raise the
timeout right to 30 seconds.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Ever since ACL filtering was added in commit 7639736 (v1.1.1), a
user could still use event registration to obtain access to a
domain that they could not normally access via virDomainLookup*
or virConnectListAllDomains and friends. We already have the
framework in the RPC generator for creating the filter, and
previous cleanup patches got us to the point that we can now
wire the filter through the entire object event stack.
Furthermore, whether or not domain:getattr is honored, use of
global events is a form of obtaining a list of networks, which
is covered by connect:search_domains added in a93cd08 (v1.1.0).
Ideally, we'd have a way to enforce connect:search_domains when
doing global registrations while omitting that check on a
per-domain registration. But this patch just unconditionally
requires connect:search_domains, even when no list could be
obtained, based on the following observations:
1. Administrators are unlikely to grant domain:getattr for one
or all domains while still denying connect:search_domains - a
user that is able to manage domains will want to be able to
manage them efficiently, but efficient management includes being
able to list the domains they can access. The idea of denying
connect:search_domains while still granting access to individual
domains is therefore not adding any real security, but just
serves as a layer of obscurity to annoy the end user.
2. In the current implementation, domain events are filtered
on the client; the server has no idea if a domain filter was
requested, and must therefore assume that all domain event
requests are global. Even if we fix the RPC protocol to
allow for server-side filtering for newer client/server combos,
making the connect:serach_domains ACL check conditional on
whether the domain argument was NULL won't benefit older clients.
Therefore, we choose to document that connect:search_domains
is a pre-requisite to any domain event management.
Network events need the same treatment, with the obvious
change of using connect:search_networks and network:getattr.
* src/access/viraccessperm.h
(VIR_ACCESS_PERM_CONNECT_SEARCH_DOMAINS)
(VIR_ACCESS_PERM_CONNECT_SEARCH_NETWORKS): Document additional
effect of the permission.
* src/conf/domain_event.h (virDomainEventStateRegister)
(virDomainEventStateRegisterID): Add new parameter.
* src/conf/network_event.h (virNetworkEventStateRegisterID):
Likewise.
* src/conf/object_event_private.h (virObjectEventStateRegisterID):
Likewise.
* src/conf/object_event.c (_virObjectEventCallback): Track a filter.
(virObjectEventDispatchMatchCallback): Use filter.
(virObjectEventCallbackListAddID): Register filter.
* src/conf/domain_event.c (virDomainEventFilter): New function.
(virDomainEventStateRegister, virDomainEventStateRegisterID):
Adjust callers.
* src/conf/network_event.c (virNetworkEventFilter): New function.
(virNetworkEventStateRegisterID): Adjust caller.
* src/remote/remote_protocol.x
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER)
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER_ANY)
(REMOTE_PROC_CONNECT_NETWORK_EVENT_REGISTER_ANY): Generate a
filter, and require connect:search_domains instead of weaker
connect:read.
* src/test/test_driver.c (testConnectDomainEventRegister)
(testConnectDomainEventRegisterAny)
(testConnectNetworkEventRegisterAny): Update callers.
* src/remote/remote_driver.c (remoteConnectDomainEventRegister)
(remoteConnectDomainEventRegisterAny): Likewise.
* src/xen/xen_driver.c (xenUnifiedConnectDomainEventRegister)
(xenUnifiedConnectDomainEventRegisterAny): Likewise.
* src/vbox/vbox_tmpl.c (vboxDomainGetXMLDesc): Likewise.
* src/libxl/libxl_driver.c (libxlConnectDomainEventRegister)
(libxlConnectDomainEventRegisterAny): Likewise.
* src/qemu/qemu_driver.c (qemuConnectDomainEventRegister)
(qemuConnectDomainEventRegisterAny): Likewise.
* src/uml/uml_driver.c (umlConnectDomainEventRegister)
(umlConnectDomainEventRegisterAny): Likewise.
* src/network/bridge_driver.c
(networkConnectNetworkEventRegisterAny): Likewise.
* src/lxc/lxc_driver.c (lxcConnectDomainEventRegister)
(lxcConnectDomainEventRegisterAny): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1047659
If a VM dies very early during an attempted connect to the guest agent
while the locks are down the domain monitor object will be freed. The
object is then accessed later as any failure during guest agent startup
isn't considered fatal.
In the current upstream version this doesn't lead to a crash as
virObjectLock called when entering the monitor in
qemuProcessDetectVcpuPIDs checks the pointer before attempting to
dereference (lock) it. The NULL pointer is then caught in the monitor
helper code.
Before the introduction of virObjectLockable - observed on 0.10.2 - the
pointer is locked directly via virMutexLock leading to a crash.
To avoid this problem we need to differentiate between the guest agent
not being present and the VM quitting when the locks were down. The fix
reorganizes the code in qemuConnectAgent to add the check and then adds
special handling to the callers.
CVE-2013-6458
Generally, every API that is going to begin a job should do that before
fetching data from vm->def. However, qemuDomainGetBlockInfo does not
know whether it will have to start a job or not before checking vm->def.
To avoid using disk alias that might have been freed while we were
waiting for a job, we use its copy. In case the disk was removed in the
meantime, we will fail with "cannot find statistics for device '...'"
error message.
CVE-2013-6458
https://bugzilla.redhat.com/show_bug.cgi?id=1043069
When virDomainDetachDeviceFlags is called concurrently to
virDomainBlockStats: libvirtd may crash because qemuDomainBlockStats
finds a disk in vm->def before getting a job on a domain and uses the
disk pointer after getting the job. However, the domain in unlocked
while waiting on a job condition and thus data behind the disk pointer
may disappear. This happens when thread 1 runs
virDomainDetachDeviceFlags and enters monitor to actually remove the
disk. Then another thread starts running virDomainBlockStats, finds the
disk in vm->def, and while it's waiting on the job condition (owned by
the first thread), the first thread finishes the disk removal. When the
second thread gets the job, the memory pointed to be the disk pointer is
already gone.
That said, every API that is going to begin a job should do that before
fetching data from vm->def.
This patch fixes a segmentation fault when creating new virtual machines using QEMU.
The segmentation fault is caused by commit f41830680e
and commit cbb6ec42e2.
In virQEMUCapsProbeQMPMachineTypes, when copying machines to qemuCaps, "none" is skipped.
Therefore, the value of i and "qemuCaps->nmachineTypes - 1" do not always match.
However, defIdx value (used to call virQEMUCapsSetDefaultMachine) is set using the value in i
when the array elements are in qemuCaps->nmachineTypes - 1.
So, when libvirt tries to create virtual machines using the default machine type,
qemuCaps->machineTypes[defIdx] is accessed and since the defIdx is NULL, it results in segmentation fault.
Signed-off-by: Yudai Yamagishi <yummy@sfc.wide.ad.jp>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Currently, the qemuProcessStop tries to open the domain log file
and saves the original error afterwards. Then all the cleanup is
done after which the error is restored back. This has however one
flaw: if opening of the log file fails an error is reported,
which results in previous error being overwritten (the useful
one, e.g. "PCI device XXXX:XXXX could not be found"). Hence, user
sees something like:
error: failed to create logfile /var/log/libvirt/qemu/ovirt_usb.log: No such file or directory
instead of:
error: internal error: Did not find USB device 8644:8003
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reported-by: Zhou Yimin <zhouyimin@huawei.com>
@listenAddress and @cookiein arguments, should be exchanged,
because the order of the caller and the callee does not match.
This results in the listen address being ignored for peer-to-peer
migration and the cookie being ignored for v2 migration.
Introduced by c4ac7ef (v1.1.4-rc1~141).
https://bugzilla.redhat.com/show_bug.cgi?id=1049338
Signed-off-by: Minoru Usui <usui@mxm.nes.nec.co.jp>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
AArch64 qemu has similar behavior as armv7l, like use of mmio etc.
This patch adds similar bypass checks what we have for armv7l to aarch64.
E.g. we are enabling mmio transport for Nicdev.
Making addDefaultUSB and addDefaultMemballoon to false etc.
V3:
- Adding missing domain rng schema for aarcg64 and test case in
testutilsqemu.c which was causing test suite failure
while running make check.
V2:
- Added testcase to qemuxml2argvtest as suggested
during review comments of V1.
V1:
- Initial patch.
Signed-off-by: Anup Patel <anup.patel@linaro.org>
Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
https://bugzilla.redhat.com/show_bug.cgi?id=1047234
Add a range check for supported numa memory placement modes provided by
the user before setting them in the domain definition. Without the check
the user is able to provide a (yet) unknown mode which is then stored in
the domain definition. This potentially causes a NULL dereference when
the defintion is formatted into the XML.
To reproduce run:
virsh numatune DOMNAME --mode 6 --nodeset 0
The XML will then contain:
<numatune>
<memory mode='(null)' nodeset='0'/>
</numatune>
With this fix, the command fails:
error: Unable to change numa parameters
error: invalid argument: unsupported numa_mode: '6'
Add whitespace to separate logical code blocks, reformat error messages
and clean up code flow.
This patch changes error handling in some cases where the the loop would
be continued to jump to cleanup instead and error out rather than modify
the domain any further.
Do not leave the PCI address of the primary video card set
to the legacy default (0000:00:02.0) if we're doing two-pass
allocation.
Since QEMU 1.6 (QEMU_CAPS_VIDEO_PRIMARY) we allow the primary
video card to be on other slots than 0000:00:02.0 (as we use
-device instead of -vga).
However we fail to assign it an address if:
* another device explicitly uses 0000:00:02.0 and
* the primary video device has no address specified
On the first pass, we have set the address to default, then checked
if it's available, leaving it set even if it wasn't. This address
got picked up by the second pass, resulting in a conflict:
XML error: Attempted double use of PCI slot 0000:00:02.0
(may need "multifunction='on'" for device on function 0)
Also fix the test that was supposed to catch this.
This eliminates the misleading error message that was being logged
when a vfio hostdev hotplug failed:
error: unable to set user and group to '107:107' on '/dev/vfio/22':
No such file or directory
as documented in:
https://bugzilla.redhat.com/show_bug.cgi?id=1035490
Commit ee414b5d (pushed as a fix for Bug 1016511 and part of Bug
1025108) replaced the single call to
virSecurityManagerSetHostdevLabel() in qemuDomainAttachHostDevice()
with individual calls to that same function in each
device-type-specific attach function (for PCI, USB, and SCSI). It also
added a corresponding call to virSecurityManagerRestoreHostdevLabel()
in the error handling of the device-type-specific functions, but
forgot to remove the common call to that from
qemuDomainAttachHostDevice() - this resulted in a duplicate call to
virSecurityManagerRestoreHostdevLabel(), with the second occurrence
being after (e.g.) a PCI device has already been re-attached to the
host driver, thus destroying some of the device nodes / links that we
then attempted to re-label (e.f. /dev/vfio/22) and generating an error
log that obscured the original error.
This resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=1035490
virProcessSetMaxMemLock() (which is a wrapper over prlimit(3)) expects
the memory size in bytes, but libvirt's domain definition (which was
being used by qemuDomainAttachHostPciDevice()) stores all memory
tuning parameters in KiB. This was being accounted for when setting
MaxMemLock at domain startup time (so cold-plugged devices would
work), but not for hotplug.
This patch simplifies the few lines that call
virProcessSetMemMaxLock(), and multiply the amount * 1024 so that
we're locking the correct amount of memory.
What remains a mystery to me is why hot-plug of a managed='no' device
would succeed (at least on my system) while managed='yes' would
fail. I guess in one case the memory was coincidentally already
resident and in the other it wasn't.
On a system that is enforcing FIPS, most libraries honor the
current mode by default. Qemu, on the other hand, refused to
honor FIPS mode unless you add the '-enable-fips' command
line option; worse, this option is not discoverable via QMP,
and is only present on binaries built for Linux. So, if we
detect FIPS mode, then we unconditionally ask for FIPS; either
qemu is new enough to have the option and then correctly
cripple insecure VNC passwords, or it is so old that we are
correctly avoiding a FIPS violation by preventing qemu from
starting. Meanwhile, if we don't detect FIPS mode, then
omitting the argument is safe whether the qemu has the option
(but it would do nothing because FIPS is disabled) or whether
qemu lacks the option (including in the case where we are not
running on Linux).
The testsuite was a bit interesting: we don't want our test
to depend on whether it is being run in FIPS mode, so I had
to tweak things to set the capability bit outside of our
normal interaction with capability parsing.
This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1035474
* src/qemu/qemu_capabilities.h (QEMU_CAPS_ENABLE_FIPS): New bit.
* src/qemu/qemu_capabilities.c (virQEMUCapsInitQMP): Conditionally
set capability according to detection of FIPS mode.
* src/qemu/qemu_command.c (qemuBuildCommandLine): Use it.
* tests/qemucapabilitiestest.c (testQemuCaps): Conditionally set
capability to test expected output.
* tests/qemucapabilitiesdata/caps_1.2.2-1.caps: Update list.
* tests/qemucapabilitiesdata/caps_1.6.0-1.caps: Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
The support for <boot rebootTimeout="12345"/> was added before we were
checking for qemu command line options in QMP, so we haven't properly
adapted virQEMUCaps when using it and thus we report unsupported
option with new enough qemu.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1042690
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Recent changes to events (commit 8a29ffcf) resulted in new compile
failures on some targets (such as ARM OMAP5):
conf/domain_event.c: In function 'virDomainEventDispatchDefaultFunc':
conf/domain_event.c:1198:30: error: cast increases required alignment of
target type [-Werror=cast-align]
conf/domain_event.c:1314:34: error: cast increases required alignment of
target type [-Werror=cast-align]
cc1: all warnings being treated as errors
The error is due to alignment; the base class is merely aligned
to the worst of 'int' and 'void*', while the child class must
be aligned to a 'long long'. The solution is to include a
'long long' (and for good measure, a function pointer) in the
base class to ensure correct alignment regardless of what a
child class may add, but to wrap the inclusion in a union so
as to not incur any wasted space. On a typical x86_64 platform,
the base class remains 16 bytes; on i686, the base class remains
12 bytes; and on the impacted ARM platform, the base class grows
from 12 bytes to 16 bytes due to the increase of alignment from
4 to 8 bytes.
Reported by Michele Paolino and others.
* src/util/virobject.h (_virObject): Use a union to ensure that
subclasses never have stricter alignment than the parent.
* src/util/virobject.c (virObjectNew, virObjectUnref)
(virObjectRef): Adjust clients.
* src/libvirt.c (virConnectRef, virDomainRef, virNetworkRef)
(virInterfaceRef, virStoragePoolRef, virStorageVolRef)
(virNodeDeviceRef, virSecretRef, virStreamRef, virNWFilterRef)
(virDomainSnapshotRef): Likewise.
* src/qemu/qemu_monitor.c (qemuMonitorOpenInternal)
(qemuMonitorClose): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
Map the new <panic> device in XML to the '-device pvpanic' command
line of qemu. Clients can then couple the <panic> device and the
<on_crash> directive to control behavior when the guest reports
a panic to qemu.
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1035955
There's a window when starting a qemu process between fork() and exec()
during which we are doing things that may fail but not tunnelling the
error to the daemon. This is basically all within qemuProcessHook().
So whenever we fail in something, e.g. placing a process onto numa node,
users are left with:
error: Child quit during startup handshake: Input/output error
while the original error is thrown into the domain log:
libvirt: error : internal error: NUMA memory tuning in 'preferred'
mode only supports single node
Hence, we should read the log file and search for the error message and
report it to users.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
For dead domains that have no memtune limits, we return 0 instead of
"unlimited", this patch fixes it to return PARAM_UNLIMITED.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
We were unconditionally removing the device from the host list, when it
should only be done on error.
This fixes USB collision detection when hotplugging the same device to
two guests.
If we hit a collision, we free the USB device while it is still part
of our temporary USBDeviceList. When the list is unref'd, the device
is free'd again.
Make the initial device freeing dependent on whether it is present
in the temporary list or not.
Similar to what Jiri did for cgroup setup/teardown in 05e149f94, push
it all into the device handler functions so we can do the necessary prep
work before claiming the device.
This also fixes hotplugging USB devices by product/vendor (virt-manager's
default behavior):
https://bugzilla.redhat.com/show_bug.cgi?id=1016511
https://bugzilla.redhat.com/show_bug.cgi?id=1035108
When attempting to enable more vCPUs in the guest than is currently
enabled in the guest but less than the maximum count for the VM we
currently reported an unhelpful message:
error: internal error: guest agent reports less cpu than requested
This patch changes it to:
error: invalid argument: requested vcpu count is greater than the count
of enabled vcpus in the domain: 3 > 2
When an error occurred in qemuAgentIO, it will be saved in mon->lastError,
but it will not be freed at the end. Present since commit c160ce33;
and compare to commit 9cc8a5af fixing the same problem in qemu_monitor.c.
==22219== 54 bytes in 1 blocks are definitely lost in loss record 982 of 1,379
==22219== at 0x4C26B9B: malloc (vg_replace_malloc.c:263)
==22219== by 0x8520521: strdup (in /lib64/libc-2.11.3.so)
==22219== by 0x52E99CB: virStrdup (virstring.c:554)
==22219== by 0x52B44C4: virCopyError (virerror.c:195)
==22219== by 0x52B5123: virCopyLastError (virerror.c:312)
==22219== by 0x10905877: qemuAgentIO (qemu_agent.c:660)
==22219== by 0x52B6122: virEventPollDispatchHandles (vireventpoll.c:501)
==22219== by 0x52B7AEA: virEventPollRunOnce (vireventpoll.c:647)
==22219== by 0x52B5C1B: virEventRunDefaultImpl (virevent.c:274)
==22219== by 0x54181FD: virNetServerRun (virnetserver.c:1112)
==22219== by 0x11EF4D: main (libvirtd.c:1513)
Signed-off-by: Zhou Yimin <zhouyimin@huawei.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
This patch fixes memory leaks reported by valgrind on running
qemuxml2argvtest; introduced in commit 0df53f04.
Most of them are of the form:
==24777== 15 bytes in 1 blocks are definitely lost in loss record 39 of 129
==24777== at 0x4A0887C: malloc (vg_replace_malloc.c:270)
==24777== by 0x341F485E21: strdup (strdup.c:42)
==24777== by 0x4CADE5F: virStrdup (virstring.c:554)
==24777== by 0x4362B6: qemuBuildDriveStr (qemu_command.c:3848)
==24777== by 0x43EF73: qemuBuildCommandLine (qemu_command.c:8500)
==24777== by 0x426670: testCompareXMLToArgvHelper (qemuxml2argvtest.c:350)
==24777== by 0x427C01: virtTestRun (testutils.c:138)
==24777== by 0x41DDB5: mymain (qemuxml2argvtest.c:658)
==24777== by 0x4282A2: virtTestMain (testutils.c:593)
==24777== by 0x341F421A04: (below main) (libc-start.c:225)
==24777==
Signed-off-by: Eric Blake <eblake@redhat.com>
Ever since the subcpusets(vcpu,emulator) were introduced, the parent
cpuset cannot be modified to remove the nodes that are in use by the
subcpusets.
The fix is to break the memory node modification into three steps:
1. assign new nodes into the parent,
2. change the nodes in the child nodes,
3. remove the old nodes on the parent node.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1009880
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com>
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
This resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=1029732
The BZ asked for the capability to change the number of queues used by
a virtio-net device while the device is in use. Because the number of
queues can only be set at the time the device is created, that isn't
possible. However, libvirt also shouldn't be silently reporting
success when someone tries to change the number of queues. So this
patch flags that as an error (just as attempts to change any of the
other virtio-specific parameters already do).
This resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=888635
(which was already closed as CANTFIX because the qemu "-boot strict"
commandline option wasn't available at the time).
Problem: you couldn't have a domain that used PXE to boot, but also
had an un-bootable disk device *even if that disk wasn't listed in the
boot order*, because if PXE timed out (e.g. due to the bridge
forwarding delay), the BIOS would move on to the next target, which
would be the unbootable disk device (again - even though it wasn't
given a boot order), and get stuck at a "BOOT DISK FAILURE, PRESS ANY
KEY" message until a user intervened.
The solution available since sometime around QEMU 1.5, is to add
"-boot strict=on" to *every* qemu command. When this is done, if any
devices have a boot order specified, then QEMU will *only* attempt to
boot from those devices that have an explicit boot order, ignoring the
rest.
This patch resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=1035188
Commit f094aaac48 changed the PCI device assignment in qemu domains
to default to using VFIO rather than legacy KVM device assignment
(when VFIO is available). It didn't change which driver was used by
default for virNodeDeviceDetachFlags(), though, so that API (and the
virsh nodedev-detach command) was still binding to the pci-stub
driver, used by legacy KVM assignment, by default.
This patch publicizes (only within the qemu module, though, so no
additions to the symbol exports are needed) the functions that check
for presence of KVM and VFIO device assignment, then uses those
functions to decide what to do when no driver is specified for
virNodeDeviceDetachFlags(); if the vfio driver is loaded, the device
will be bound to vfio-pci, or if legacy KVM assignment is supported on
this system, the device will be bound to pci-stub; if neither method
is available, the detach will fail.
Currently the snapshot code did not check if it actually supports
snapshots on various disk backends for domains. To avoid future problems
add checkers that whitelist the supported configurations.
This patch adds function qemuGetDriveSourceString to produce
qemu-compatible disk source strings that will enable to reuse the code
and refactors building of the qemu commandline of disks to use this new
helper.
Automatically assign secret type from the disk source definition and
pull in adding of the comma. Then update callers to keep generated
output the same.
Before this patch, the translation function still needs a second ugly
helper function to actually format the command line for qemu. But if we
do the right stuff in the translation function, we don't have to bother
with the second function any more.
This patch removes the messy qemuBuildVolumeString function and changes
qemuTranslateDiskSourcePool to set stuff up correctly so that the
regular code paths meant for volumes can be used to format the command
line correctly.
For this purpose a new helper "qemuDiskGetActualType()" is introduced to
return the type of the volume in a pool.
As a part of the refactor the qemuTranslateDiskSourcePool function is
fixed to do decisions based on the pool type instead of the volume type.
This allows to separate pool-type-specific stuff more clearly and will
ease addition of other pool types that will require certain other
operations to get the correct pool source.
The previously fixed tests should make sure that we don't break stuff
that was working before.
When doing an internal snapshot on a VM with sheepdog or RBD disks we
would not set a flag to mark the domain is using internal snapshots and
might end up creating a mixed snapshot. Move the setting of the variable
to avoid this problem.
The virsh command 'domxml-to-native' (virConnectDomainXMLToNative())
converts all network devices to "type='ethernet'" in order to make it
more likely that the generated command could be run directly from a
shell (other libvirt network device types end up referencing file
descriptors for tap devices assumed to have been created by libvirt,
which can't be done in this case).
During this conversion, all of the netdev parameters are cleared out,
then specific items are filled in after changing the type. The MAC
address was not one of these preserved items, and the result was that
mac addresses in the generated commandlines were always
00:00:00:00:00:00.
This patch saves the mac address before the conversion, then
repopulates it afterwards, so the proper mac addresses show up in the
commandline.
Signed-off-by: Bing Bu Cao <mars@linux.vnet.ibm.com>
Signed-off-by: Laine Stump <laine@laine.org>
In the 'directory' and 'netfs' storage pools, a user can see
both 'file' and 'dir' storage volume types, to know when they
can descend into a subdirectory. But in a network-based storage
pool, such as the upcoming 'gluster' pool, we use 'network'
instead of 'file', and did not have any counterpart for a
directory until this patch. Adding a new volume type
'network-dir' is better than reusing 'dir', because it makes
it clear that the only way to access 'network' volumes within
that container is through the network mounting (leaving 'dir'
for something accessible in the local file system).
* include/libvirt/libvirt.h.in (virStorageVolType): Expand enum.
* docs/formatstorage.html.in: Document it.
* docs/schemasa/storagevol.rng (vol): Allow new value.
* src/conf/storage_conf.c (virStorageVol): Use new value.
* src/qemu/qemu_command.c (qemuBuildVolumeString): Fix client.
* src/qemu/qemu_conf.c (qemuTranslateDiskSourcePool): Likewise.
* tools/virsh-volume.c (vshVolumeTypeToString): Likewise.
* src/storage/storage_backend_fs.c
(virStorageBackendFileSystemVolDelete): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
The bus type IDE being enum Zero, the bus type on pseries system appears as IDE for all the -hda/-cdrom and for disk drives with if="none" type. Pseries platform needs this to appear as SCSI instead of IDE. The ide being not supported, the explicit requests for ide devices will return an error.
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com>
This nested job is canceled by the first ExitMonitor call (even though
it was not created by the corresponding EnterMonitor call), and
again in qemuMigrationPrepareAny if qemuProcessStart failed.
This can lead to a crash if the vm object was disposed of before calling
qemuDomainRemoveInactive:
0 ..62bc in virClassIsDerivedFrom (klass=0xdeadbeef,
parent=0x7ffce4cdd270) at util/virobject.c:166
1 ..6666 in virObjectIsClass at util/virobject.c:362
2 ..66b4 in virObjectLock at util/virobject.c:314
3 ..477e in virDomainObjListRemove at conf/domain_conf.c:2359
4 ..7a64 in qemuDomainRemoveInactive at qemu/qemu_domain.c:2087
5 ..956c in qemuMigrationPrepareAny at qemu/qemu_migration.c:2469
This was added by commit e4e2822, exposed by 5a4c237 and c7ac251.
https://bugzilla.redhat.com/show_bug.cgi?id=1018267
If a SCSI hostdev is included in an initial domain XML, without a
corresponding controller statement, one is created silently when the
guest is booted.
When hotplugging a SCSI hostdev, a presumption is that the controller
is already present in the domain either from the original XML, or via
an earlier hotplug.
[root@xxxxxxxx ~]# cat disk.xml
<hostdev mode='subsystem' type='scsi'>
<source>
<adapter name='scsi_host0'/>
<address bus='0' target='3' unit='1088438288'/>
</source>
</hostdev>
[root@xxxxxxxx ~]# virsh attach-device guest01 disk.xml
error: Failed to attach device from disk.xml
error: internal error: unable to execute QEMU command 'device_add': Bus 'scsi0.0' not found
Since the infrastructure is in place, we can also create a controller
silently for use by the hotplugged hostdev device.
Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com>
For systems without a PCI bus, attaching a SCSI controller fails:
[root@xxxxxxxx ~]# cat controller.xml
<controller type='scsi' model='virtio-scsi' index='0' />
[root@xxxxxxxx ~]# virsh attach-device guest01 controller.xml
error: Failed to attach device from controller.xml
error: XML error: No PCI buses available
A similar problem occurs with the detach of a controller:
[root@xxxxxxxx ~]# virsh detach-device guest01 controller.xml
error: Failed to detach device from controller.xml
error: operation failed: controller scsi:0 not found
The qemuDomainXXtachPciControllerDevice routines made assumptions
that any caller had a PCI bus. These routines now selectively calls
PCI functions where necessary, and assigns the device information
type to one appropriate for the bus in use.
Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
For attach/detach of controller devices, we rename the functions to
remove 'PCI' from their title. The actual separation of PCI-specific
operations will be handled in the next patch.
Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com>
These changes allow the correct virtio-blk-device and virtio-net-device
devices to be used for the 'virt' machine type for armv7 rather than the
PCI virtio devices.
A test case was added to qemuxml2argvtest for this change.
Signed-off-by: Clark Laughlin <clark.laughlin@linaro.org>
Most of our code base uses space after comma but not before;
fix the remaining uses before adding a syntax check.
* src/qemu/qemu_cgroup.c: Consistently use commas.
* src/qemu/qemu_command.c: Likewise.
* src/qemu/qemu_conf.c: Likewise.
* src/qemu/qemu_driver.c: Likewise.
* src/qemu/qemu_monitor.c: Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
On the domain startup, this function is called to dump some info about
the CPUs. At the beginning of the function we check if we aren't running
older qemu which is not exposing the CPUs via 'qom-list'. However, we
are not checking for even older qemus, which throw 'CommandNotFound'
error.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Report the error in virPortAllocatorAcquire instead
of doing it in every caller.
The error contains the port range name instead of the intended
use for the port, e.g.:
Unable to find an unused port in range 'display' (65534-65535)
instead of:
Unable to find an unused port for SPICE
This also adds error reporting when the QEMU driver could not
find an unused port for VNC, VNC WebSockets or NBD migration.
QEMU 1.6.0 introduced new migration status: setup
Libvirt does not expect such string in QMP and refuses to migrate with error
"unexpected migration status in setup"
This patch fixes it.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1025108
So far qemuSetupHostdevCGroup was called very early during hotplug, even
before we knew the device we were about to hotplug was actually
available. By calling the function later, we make sure QEMU won't be
allowed to access devices used by other domains.
Another important effect of this change is that hopluging USB devices
specified by vendor and product (but not by their USB address) works
again. This was broken since v1.0.5-171-g7d763ac, when the call to
qemuFindHostdevUSBDevice was moved after the call to
qemuSetupHostdevCGroup, which then used an uninitialized USB address.
https://bugzilla.redhat.com/show_bug.cgi?id=1018267
The aim of virObject refing and urefing is to tell where the object is
to be used and when is no longer needed. Hence any object shouldn't be
used after it has been unrefed, as we might be the last to hold the
reference. The better way is to call virObjectUnref() *after* the last
object usage. In this specific case, the monitor EOF handler was called
after the qemuMonitorIO called virObjectUnref. Not only that @mon was
disposed (which is not used in the handler anyway) but the @mon->vm
which is causing a SIGSEGV:
2013-11-15 10:17:54.425+0000: 20110: error : qemuMonitorIO:688 : internal error: early end of file from monitor: possible problem:
qemu-kvm: -incoming tcp:01.01.01.0:49152: Failed to bind socket: Cannot assign requested address
Program received signal SIGSEGV, Segmentation fault.
qemuProcessHandleMonitorEOF (mon=<optimized out>, vm=0x7fb728004170) at qemu/qemu_process.c:299
299 if (priv->beingDestroyed) {
(gdb) p *priv
Cannot access memory at address 0x0
(gdb) p vm
$1 = (virDomainObj *) 0x7fb728004170
(gdb) p *vm
$2 = {parent = {parent = {magic = 3735928559, refs = 0, klass = 0xdeadbeef}, lock = {lock = {__data = {__lock = 2, __count = 0, __owner = 20110, __nusers = 1, __kind = 0, __spins = 0, __list = {__prev = 0x0,
__next = 0x0}}, __size = "\002\000\000\000\000\000\000\000\216N\000\000\001", '\000' <repeats 26 times>, __align = 2}}}, pid = 0, state = {state = 0, reason = 0}, autostart = 0, persistent = 0,
updated = 0, def = 0x0, newDef = 0x0, snapshots = 0x0, current_snapshot = 0x0, hasManagedSave = false, privateData = 0x0, privateDataFreeFunc = 0x0, taint = 304}
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
In the qemuProcessReconnectHelper() a new thread that does all the
interesting work is spawned. The rationale is to not block the daemon
startup process in case of unresponsive qemu. However, the thread
handler is a local variable which gets lost once the control goes out of
scope. Hence the thread gets leaked. We can avoid this if the thread
isn't made joinable.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The QOM path in qemu that contains the CPUID registers of a running VM
may not be present (introduced in QEMU 1.5).
Since commit d94b781771 we have a regression with QEMU that don't
support reporting of the CPUID register state via the monitor as the
process startup code expects the path to exist.
This patch adds code that checks with the monitor if the requested path
already exists and uses it only in this case.
The 'none' machine type is something only intended for use
by libvirt probing capabilities. It isn't something that
is useful for running real VM instances. As such it should
not be exposed to users in the capabilities.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The virQEMUCapsProbeQMPMachineTypes method iterates over machine
types copying them into the qemuCapsPtr object. It only updates
the qemuCaps->nmachinetypes value at the end though. So if OOM
occurs in the middle, the destructor of qemuCapsPtr will not
free the partially initialized machine types.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
If the managedsave image is corrupted, e.g. the XML part is, we fail to
parse it and throw an error, e.g.:
error: Failed to start domain jms8
error: XML error: missing security model when using multiple labels
This is okay, as we can't really start the machine and avoid undefined
qemu behaviour. On the other hand, the error message doesn't give a
clue to users what should they do. The consensus here would be to thrown
a warning to logs saying "Hey, you've got a corrupted file".
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This patch moves some code in the qemuDomainAttachSCSIDisk
function. The check for the existence of a PCI address assigned
to the SCSI controller was moved in order to be executed only
when needed. The PCI address of a controller is not necessary
if QEMU_CAPS_DEVICE is supported.
This fixes issues with the hotplug of SCSI disks on pseries guests.
When adding support for Q35 guests, the code to assign a PCI address
to the primary video card was moved into Q35 and i440fx(PIIX3)
specific functions, but no fallback was kept for other machine types
that might have a video card.
This patch remedies that by assigning a PCI address to the primary
video card if it does not have any kind of address. In particular,
this fixes issues with pseries guests.
Signed-off-by: Vitor de Lima <vitor.lima@eldorado.org.br>
Signed-off-by: Laine Stump <laine@laine.org>
When starting a VM the qemu process may filter out some requested
features of a domain as it's not supported either by the host or by
qemu. Libvirt didn't check if this happened which might end up in
changing of the guest ABI when migrating.
The proof of concept implementation adds the check for the recently
introduced kvm_pv_unhalt cpuid feature bit. This feature depends on both
qemu and host kernel support and thus increase the possibility of guest
ABI breakage.
The linux kernel recently added support for paravirtual spinlock
handling to avoid performance regressions on overcomitted hosts. This
feature needs to be turned in the hypervisor so that the guest OS is
notified about the possible support.
This patch adds a new feature "paravirt-spinlock" to the XML and
supporting code to enable the "kvm_pv_unhalt" pseudo CPU feature in
qemu.
https://bugzilla.redhat.com/show_bug.cgi?id=1008989
Currently we were storing domain feature flags in a bit field as the
they were either enabled or disabled. New features such as paravirtual
spinlocks however can be tri-state as the default option may depend on
hypervisor version.
To allow storing tri-state feature state in the same place instead of
having to declare dedicated variables for each feature this patch
refactors the bit field to an array.
The qemu monitor supports retrieval of actual CPUID bits presented to
the guest using QMP monitor. Add APIs to extract these information and
tests for them.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Since 86d90b3a (yes, my patch; again) we are supporting NBD storage
migration. However, on error recovery path we got the steps reversed.
The correct order is: return NBD port to the virPortAllocator and then
either unlock the vm or remove it from the driver. Not vice versa.
==11192== Invalid write of size 4
==11192== at 0x11488559: qemuMigrationPrepareAny (qemu_migration.c:2459)
==11192== by 0x11488EA6: qemuMigrationPrepareDirect (qemu_migration.c:2652)
==11192== by 0x114D1509: qemuDomainMigratePrepare3Params (qemu_driver.c:10332)
==11192== by 0x519075D: virDomainMigratePrepare3Params (libvirt.c:7290)
==11192== by 0x1502DA: remoteDispatchDomainMigratePrepare3Params (remote.c:4798)
==11192== by 0x12DECA: remoteDispatchDomainMigratePrepare3ParamsHelper (remote_dispatch.h:5741)
==11192== by 0x5212127: virNetServerProgramDispatchCall (virnetserverprogram.c:435)
==11192== by 0x5211C86: virNetServerProgramDispatch (virnetserverprogram.c:305)
==11192== by 0x520A8FD: virNetServerProcessMsg (virnetserver.c:165)
==11192== by 0x520A9E1: virNetServerHandleJob (virnetserver.c:186)
==11192== by 0x50DA78F: virThreadPoolWorker (virthreadpool.c:144)
==11192== by 0x50DA11C: virThreadHelper (virthreadpthread.c:161)
==11192== Address 0x1368baa0 is 576 bytes inside a block of size 688 free'd
==11192== at 0x4A07F5C: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==11192== by 0x5079A2F: virFree (viralloc.c:580)
==11192== by 0x11456C34: qemuDomainObjPrivateFree (qemu_domain.c:267)
==11192== by 0x50F41B4: virDomainObjDispose (domain_conf.c:2034)
==11192== by 0x50C2991: virObjectUnref (virobject.c:262)
==11192== by 0x50F4CFC: virDomainObjListRemove (domain_conf.c:2361)
==11192== by 0x1145C125: qemuDomainRemoveInactive (qemu_domain.c:2087)
==11192== by 0x11488520: qemuMigrationPrepareAny (qemu_migration.c:2456)
==11192== by 0x11488EA6: qemuMigrationPrepareDirect (qemu_migration.c:2652)
==11192== by 0x114D1509: qemuDomainMigratePrepare3Params (qemu_driver.c:10332)
==11192== by 0x519075D: virDomainMigratePrepare3Params (libvirt.c:7290)
==11192== by 0x1502DA: remoteDispatchDomainMigratePrepare3Params (remote.c:4798)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
One of my previous patches (c7ac2519b7) did try to fix the issue when
domain dies too soon during migration. However, this clumsy approach was
missing removal of qemuProcessHandleMonitorDestroy resulting in double
unrefing of mon->vm and hence producing the daemon crash:
==11843== Invalid read of size 4
==11843== at 0x50C28C5: virObjectUnref (virobject.c:255)
==11843== by 0x1148F7DB: qemuMonitorDispose (qemu_monitor.c:258)
==11843== by 0x50C2991: virObjectUnref (virobject.c:262)
==11843== by 0x50C2D13: virObjectFreeCallback (virobject.c:388)
==11843== by 0x509C37B: virEventPollCleanupHandles (vireventpoll.c:583)
==11843== by 0x509C711: virEventPollRunOnce (vireventpoll.c:652)
==11843== by 0x509A620: virEventRunDefaultImpl (virevent.c:274)
==11843== by 0x520D21C: virNetServerRun (virnetserver.c:1112)
==11843== by 0x11F368: main (libvirtd.c:1513)
==11843== Address 0x13b88864 is 4 bytes inside a block of size 136 free'd
==11843== at 0x4A07F5C: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==11843== by 0x5079A2F: virFree (viralloc.c:580)
==11843== by 0x50C29E3: virObjectUnref (virobject.c:270)
==11843== by 0x114770E4: qemuProcessHandleMonitorDestroy (qemu_process.c:1103)
==11843== by 0x1148F7CB: qemuMonitorDispose (qemu_monitor.c:257)
==11843== by 0x50C2991: virObjectUnref (virobject.c:262)
==11843== by 0x50C2D13: virObjectFreeCallback (virobject.c:388)
==11843== by 0x509C37B: virEventPollCleanupHandles (vireventpoll.c:583)
==11843== by 0x509C711: virEventPollRunOnce (vireventpoll.c:652)
==11843== by 0x509A620: virEventRunDefaultImpl (virevent.c:274)
==11843== by 0x520D21C: virNetServerRun (virnetserver.c:1112)
==11843== by 0x11F368: main (libvirtd.c:1513)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
