Now the a dmi-to-pci-bridge is automatically added just as it's needed
(when a pci-bridge is being added), we no longer have any need to
force-add one to every single Q35 domain.
Previously libvirt would only add pci-bridge devices automatically
when an address was requested for a device that required a legacy PCI
slot and none was available. This patch expands that support to
dmi-to-pci-bridge (which is needed in order to add a pci-bridge on a
machine with a pcie-root), and pcie-root-port (which is needed to add
a hotpluggable PCIe device). It does *not* automatically add
pcie-switch-upstream-ports or pcie-switch-downstream-ports (and
currently there are no plans for that).
Given the existing code to auto-add pci-bridge devices, automatically
adding pcie-root-ports is fairly straightforward. The
dmi-to-pci-bridge support is a bit tricky though, for a few reasons:
1) Although the only reason to add a dmi-to-pci-bridge is so that
there is a reasonable place to plug in a pci-bridge controller,
most of the time it's not the presence of a pci-bridge *in the
config* that triggers the requirement to add a dmi-to-pci-bridge.
Rather, it is the presence of a legacy-PCI device in the config,
which triggers auto-add of a pci-bridge, which triggers auto-add of
a dmi-to-pci-bridge (this is handled in
virDomainPCIAddressSetGrow() - if there's a request to add a
pci-bridge we'll check if there is a suitable bus to plug it into;
if not, we first add a dmi-to-pci-bridge).
2) Once there is already a single dmi-to-pci-bridge on the system,
there won't be a need for any more, even if it's full, as long as
there is a pci-bridge with an open slot - you can also plug
pci-bridges into existing pci-bridges. So we have to make sure we
don't add a dmi-to-pci-bridge unless there aren't any
dmi-to-pci-bridges *or* any pci-bridges.
3) Although it is strongly discouraged, it is legal for a pci-bridge
to be directly plugged into pcie-root, and we don't want to
auto-add a dmi-to-pci-bridge if there is already a pci-bridge
that's been forced directly into pcie-root.
Although libvirt will now automatically create a dmi-to-pci-bridge
when it's needed, the code still remains for now that forces a
dmi-to-pci-bridge on all domains with pcie-root (in
qemuDomainDefAddDefaultDevices()). That will be removed in a future
patch.
For now, the pcie-root-ports are added one to a slot, which is a bit
wasteful and means it will fail after 31 total PCIe devices (30 if
there are also some PCI devices), but helps keep the changeset down
for this patch. A future patch will have 8 pcie-root-ports sharing the
functions on a single slot.
Andrea had the right idea when he disabled the "reserve an extra
unused slot" bit for aarch64/virt. For *any* PCI Express-based
machine, it is pointless since 1) an extra legacy-PCI slot can't be
used for hotplug, since hotplug into legacy PCI slots doesn't work on
PCI Express machinetypes, and 2) even for "coldplug" expansion,
everybody will want to expand using Express controllers, not legacy
PCI.
This patch eliminates the extra slot reserve unless the system has a
pci-root (i.e. legacy PCI)
The nec-usb-xhci device (which is a USB3 controller) has always
presented itself as a PCI device when plugged into a legacy PCI slot,
and a PCIe device when plugged into a PCIe slot, but libvirt has
always auto-assigned it to a legacy PCI slot.
This patch changes that behavior to auto-assign to a PCIe slot on
systems that have pcie-root (e.g. Q35 and aarch64/virt).
Since we don't yet auto-create pcie-*-port controllers on demand, this
means a config with an nec-xhci USB controller that has no PCI address
assigned will also need to have an otherwise-unused pcie-*-port
controller specified:
<controller type='pci' model='pcie-root-port'/>
<controller type='usb' model='nec-xhci'/>
(this assumes there is an otherwise-unused slot on pcie-root to accept
the pcie-root-port)
The e1000e is an emulated network device based on the Intel 82574,
present in qemu 2.7.0 and later. Among other differences from the
e1000, it presents itself as a PCIe device rather than legacy PCI. In
order to get it assigned to a PCIe controller, this patch updates the
flags setting for network devices when the model name is "e1000e".
(Note that for some reason libvirt has never validated the network
device model names other than to check that there are no dangerous
characters in them. That should probably change, but is the subject of
another patch.)
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1343094
libvirt previously assigned nearly all devices to a "hotpluggable"
legacy PCI slot even on machines with a PCIe root bus (and even though
most such machines don't even support hotplug on legacy PCI slots!)
Forcing all devices onto legacy PCI slots means that the domain will
need a dmi-to-pci-bridge (to convert from PCIe to legacy PCI) and a
pci-bridge (to provide hotpluggable legacy PCI slots which, again,
usually aren't hotpluggable anyway).
To help reduce the need for these legacy controllers, this patch tries
to assign virtio-1.0-capable devices to PCIe slots whenever possible,
by setting appropriate connectFlags in
virDomainCalculateDevicePCIConnectFlags(). Happily, when that function
was written (just a few commits ago) it was created with a
"virtioFlags" argument, set by both of its callers, which is the
proper connectFlags to set for any virtio-*-pci device - depending on
the arch/machinetype of the domain, and whether or not the qemu binary
supports virtio-1.0, that flag will have either been set to PCI or
PCIe. This patch merely enables the functionality by setting the flags
for the device to whatever is in virtioFlags if the device is a
virtio-*-pci device.
NB: the first virtio video device will be placed directly on bus 0
slot 1 rather than on a pcie-root-port due to the override for primary
video devices in qemuDomainValidateDevicePCISlotsQ35(). Whether or not
to change that is a topic of discussion, but this patch doesn't change
that particular behavior.
NB2: since the slot must be hotpluggable, and pcie-root (the PCIe root
complex) does *not* support hotplug, this means that suitable
controllers must also be in the config (i.e. either pcie-root-port, or
pcie-downstream-port). For now, libvirt doesn't add those
automatically, so if you put virtio devices in a config for a qemu
that has PCIe-capable virtio devices, you'll need to add extra
pcie-root-ports yourself. That requirement will be eliminated in a
future patch, but for now, it's simple to do this:
<controller type='pci' model='pcie-root-port'/>
<controller type='pci' model='pcie-root-port'/>
<controller type='pci' model='pcie-root-port'/>
...
Partially Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1330024
This patch cleans up the connect flags for certain types/models of
devices that aren't PCI to return 0. In the future that may be used as
an indicator to the caller about whether or not a device needs a PCI
address. For now it's just ignored, except for in
virDomainPCIAddressEnsureAddr() - called during device hotplug - (and
in some cases actually needs to be re-set to PCI|HOTPLUGGABLE just in
case someone (in some old config) has manually set a PCI address for a
device that isn't PCI.
Before now, all the qemu hotplug functions assumed that all devices to
be hotplugged were legacy PCI endpoint devices
(VIR_PCI_CONNECT_TYPE_PCI_DEVICE). This worked out "okay", because all
devices *are* legacy PCI endpoint devices on x86/440fx machinetypes,
and hotplug didn't work properly on machinetypes using PCIe anyway
(hotplugging onto a legacy PCI slot doesn't work, and until commit
b87703cf any attempt to manually specify a PCIe address for a
hotplugged device would be erroneously rejected).
This patch makes all qemu hotplug operations honor the pciConnectFlags
set by the single all-knowing function
qemuDomainDeviceCalculatePCIConnectFlags(). This is done in 3 steps,
but in a single commit since we would have to touch the other points
at each step anyway:
1) add a flags argument to the hypervisor-agnostic
virDomainPCIAddressEnsureAddr() (previously it hardcoded
..._PCI_DEVICE)
2) add a new qemu-specific function qemuDomainEnsurePCIAddress() which
gets the correct pciConnectFlags for the device from
qemuDomainDeviceConnectFlags(), then calls
virDomainPCIAddressEnsureAddr().
3) in qemu_hotplug.c replace all calls to
virDomainPCIAddressEnsureAddr() with calls to
qemuDomainEnsurePCIAddress()
So in effect, we're putting a "shim" on top of all calls to
virDomainPCIAddressEnsureAddr() that sets the right pciConnectFlags.
Set pciConnectFlags in each device's DeviceInfo and then use those
flags later when validating existing addresses in
qemuDomainCollectPCIAddress() and when assigning new addresses with
qemuDomainPCIAddressReserveNextAddr() (rather than scattering the
logic about which devices need which type of slot all over the place).
Note that the exact flags set by
qemuDomainDeviceCalculatePCIConnectFlags() are different from the
flags previously set manually in qemuDomainCollectPCIAddress(), but
this doesn't matter because all validation of addresses in that case
ignores the setting of the HOTPLUGGABLE flag, and treats PCIE_DEVICE
and PCI_DEVICE the same (this lax checking was done on purpose,
because there are some things that we want to allow the user to
specify manually, e.g. assigning a PCIe device to a PCI slot, that we
*don't* ever want libvirt to do automatically. The flag settings that
we *really* want to match are 1) the old flag settings in
qemuDomainAssignDevicePCISlots() (which is HOTPLUGGABLE | PCI_DEVICE
for everything except PCI controllers) and 2) the new flag settings
done by qemuDomainDeviceCalculatePCIConnectFlags() (which are
currently exactly that - HOTPLUGGABLE | PCI_DEVICE for everything
except PCI controllers).
The lowest level function of this trio
(qemuDomainDeviceCalculatePCIConnectFlags()) aims to be the single
authority for the virDomainPCIConnectFlags to use for any given device
using a particular arch/machinetype/qemu-binary.
qemuDomainFillDevicePCIConnectFlags() sets info->pciConnectFlags in a
single device (unless it has no virDomainDeviceInfo, in which case
it's a NOP).
qemuDomainFillAllPCIConnectFlags() sets info->pciConnectFlags in all
devices that have a virDomainDeviceInfo
The latter two functions aren't called anywhere yet. This commit is
just making them available. Later patches will replace all the current
hodge-podge of flag settings with calls to this single authority.
dom xml generated on begin step should be passed
to perform step in VIR_MIGRATE_PARAM_DEST_XML parameter.
Otherwise 'XML error: failed to parse xml document' is
raised on destination host as dom xml is NULL.
Signed-off-by: Pavel Glushchak <pglushchak@virtuozzo.com>
Coverity identified that this variable might be leaked. And it's
right. If an error occurred and we have to roll back the control
jumps to try_remove label where we save the current error (see
0e82fa4c34 for more info). However, inside the code a jump onto
other label is possible thus leaking the error object.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Use the newly introduced close callback helpers to make the code look just a
bit cleaner and more importantly, to fix the following memleak regarding a
dangling virAdmConnect object reference caused by assigning NULL to the close
callback data once the catch-disconnect routine used the callback followed
by a comparison of NULL to the originally defined close callback (which at that
moment had already been NULL'd by remoteAdminClientCloseFunc) in
virAdmConnectCloseCallbackUnregister.
717 (88 direct, 629 indirect) bytes in 1 blocks are definitely lost record
110 of 141
at 0x4C2A988: calloc (vg_replace_malloc.c:711)
by 0x530696F: virAllocVar (viralloc.c:560)
by 0x53689E6: virObjectNew (virobject.c:193)
by 0x5368B5E: virObjectLockableNew (virobject.c:219)
by 0x4E3E7EE: virAdmConnectNew (datatypes.c:900)
by 0x4E398BB: virAdmConnectOpen (libvirt-admin.c:220)
by 0x10D3E3: vshAdmConnect (virt-admin.c:161)
by 0x10D624: vshAdmReconnect (virt-admin.c:215)
by 0x10DB0A: cmdConnect (virt-admin.c:353)
by 0x11288F: vshCommandRun (vsh.c:1313)
by 0x10FDB6: main (virt-admin.c:1439)
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1357358
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Well, there were three different spots where closeCallback->freeCallback was
called, not looking the same --> potential for bugs - and there indeed is a bug
with refcounting of the @conn object. So this patch partially follows the path
set by commit 24dbb69f by introducing some close callback helpers both to
replace all the spots where we call clean the close callback data with a
dedicated function and to be able to fix the refcounting bug causing a memleak.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
The only place we change the @conn object is actually virAdmConnectOpen
routine, thus at the moment we don't really need to lock it, given the fact that
what we're trying to do here is to change the closeCallback object which is a
lockable object itself, so that should be enough to avoid races.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
As was suggested in an earlier review comment[1], we can
catch some additional code points by cleaning up how we use the
hostdev subsystem type in some switch statements.
[1] End of https://www.redhat.com/archives/libvir-list/2016-September/msg00399.html
Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com>
Signed-off-by: John Ferlan <jferlan@redhat.com>
The memory device alias needs to be treated as machine ABI as qemu is
using it in the migration stream for section labels. To simplify this
generate the alias from the slot number unless an existing broken
configuration is detected.
With this patch the aliases are predictable and even certain
configurations which would not be migratable previously are fixed.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1359135
As with other devices assign the slot number right away when adding the
device. This will make the slot numbers static as we do with other
addressing elements and it will ultimately simplify allocation of the
alias in a static way which does not break with qemu.
Detect on reconnect to a running qemu VM whether the alias of a
hotpluggable memory device (dimm) does not match the dimm slot number
where it's connected to. This is necessary as qemu is actually
considering the alias as machine ABI used to connect the backend object
to the dimm device.
This will require us to keep them consistent so that we can reliably
restore them on migration. In some situations it was currently possible
to create a mismatched configuration and qemu would refuse to restore
the migration stream.
To avoid breaking existing VMs we'll need to keep the old algorithm
though.
Simplify handling of the 'dimm' address element by allowing to specify
the slot number only. This will allow libvirt to allocate slot numbers
before starting qemu.
We dropped support for RHEL-5 vintage Xen a while ago,
but forgot to remove some of the hacks for it.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1386976
We have everything ready. Actually the only limitation was our
check that denied hotplug of vhost-user.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
If there is an error hotpluging a net device (for whatever
reason) a rollback operation is performed. However, whilst doing
so various helper functions that are called report errors on
their own. This results in the original error to be overwritten
and thus misleading the user.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Even though using /dev/shm/asdf as the backend, we still need to make
the mapping shared. The original patch forgot to add that parameter.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1392031
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
../../src/qemu/qemu_capabilities.c:3757: error: declaration of
'basename' shadows a global declaration [-Wshadow]
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Function qemuDomainAttachShmemDevice() steals the device data if the
hotplug was successful, but the condition checked for unsuccessful
execution otherwise.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Propagate the selected or default level to qemu if it's supported.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1376009
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
This helps in selecting log level of the gluster gfapi, output to stderr.
The option is 'gluster_debug_level', can be tuned by editing
'/etc/libvirt/qemu.conf'
Debug levels ranges 0-9, with 9 being the most verbose, and 0
representing no debugging output. The default is the same as it was
before, which is a level of 4. The current logging levels defined in
the gluster gfapi are:
0 - None
1 - Emergency
2 - Alert
3 - Critical
4 - Error
5 - Warning
6 - Notice
7 - Info
8 - Debug
9 - Trace
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Allow detecting capabilities according to the qemu QMP schema. This is
necessary as sometimes the availability of certain options depends on
the presence of a field in the schema.
This patch adds support for loading the QMP schema when detecting qemu
capabilities and adds a very simple query language to allow traversing
the schema and selecting a certain element from it.
The infrastructure in this patch uses a query path to set a specific
capability flag according to the availability of the given element in
the schema.
Some operations like reboot, save, coreDump, blockStats,
ifaceStats make sense iff domain is running. While it is
technically possible for our test driver to return success
regardless of domain state, we should copy constraints from
other drivers and thus deny these operations over inactive
domains.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1379196
Add check in qemuCheckDiskConfig for an invalid combination
of using the 'scsi' bus for a block 'lun' device and any disk
source format other than 'raw'.
Fixes the behavior when destroying a domain more than once.
VIR_ERR_OPERATION_INVALID should be raised when destroying an
already destroyed domain.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Commit c29e6d4805 cause build failure on RHEL-6:
../../src/qemu/qemu_capabilities.c: In function 'virQEMUCapsIsValid':
../../src/qemu/qemu_capabilities.c:4085: error: declaration of 'ctime'
shadows a global declaration [-Wshadow]
/usr/include/time.h:258: error: shadowed declaration is here [-Wshadow]
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Let's keep all run time validation of cached QEMU capabilities in
virQEMUCapsIsValid and call it whenever we access the cache.
virQEMUCapsInitCached should keep only the checks which do not make
sense once the cache is loaded in memory.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
virQEMUCapsLoadCache loads QEMU capabilities from a file, but strangely
enough it returns the loaded QEMU binary ctime in qemuctime parameter
instead of storing it in qemuCaps.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
This is needed in order to migrate a domain with shmem devices as that
is not allowed to migrate.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
QEMU added support for ivshmem-plain and ivshmem-doorbell. Those are
reworked varians of legacy ivshmem that are compatible from the guest
POV, but not from host's POV and have sane specification and handling.
Details about the newer device type can be found in qemu's commit
5400c02b90bb:
http://git.qemu.org/?p=qemu.git;a=commit;h=5400c02b90bb
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
We're keeping some things at default and that's not something we want to
do intentionally. Let's save some sensible defaults upfront in order to
avoid having problems later. The details for the defaults (of the newer
implementation) can be found in qemu's commit 5400c02b90bb:
http://git.qemu.org/?p=qemu.git;a=commit;h=5400c02b90bb
Since we are merely saving the defaults it will not change the guest ABI
and thanks to the fact that we're doing it in the PostParse callback it
will not break the ABI stability checks.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
The old ivshmem is deprecated in QEMU, so let's use the better
ivshmem-{plain,doorbell} variants instead.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Unlike other migration capabilities, post-copy is also set on the
destination host which means it doesn't disappear once domain is
migrated. As a result of that other functionality which internally uses
migration to a file (virDomainManagedSave, virDomainSave,
virDomainCoreDump) may fail after migration because the post-copy
capability is still set.
https://bugzilla.redhat.com/show_bug.cgi?id=1374718
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
commit 9065cfaa added the ability to disable DNS services for a
libvirt virtual network. If neither DNS nor DHCP is needed for a
network, then we don't need to start dnsmasq, so code was added to
check for this.
Unfortunately, it was written with a great lack of attention to detail
(I can say that, because I was the author), and the loop that checked
if DHCP is needed for the network would never end if the network had
multiple IP addresses and the first <ip> had no <dhcp> subelement
(which would have contained a <range> or <host> subelement, thus
requiring DHCP services).
This patch rewrites the check to be more compact and (more
importantly) finite.
This bug was present in release 2.2.0 and 2.3.0, so will need to be
backported to any relevant maintainence branches.
Reported here:
https://www.redhat.com/archives/libvirt-users/2016-October/msg00032.htmlhttps://www.redhat.com/archives/libvirt-users/2016-October/msg00045.html
If we failed to unlink old dom cfg file, we goto rollback.
But inside rollback, we fogot to unlink the new dom cfg file.
This patch fixes this issue.
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Whilst working on another issue, I've noticed that in some
functions we have a local @driver variable among with access to
global @qemu_driver variable. This makes no sense.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Rather than waiting until we've free'd up all the resources, cause the
'workerPool' thread pool to flush as soon as possible during stateCleanup.
Otherwise, it's possible something waiting to run will SEGV such as is the
case during race conditions of simultaneous exiting libvirtd and qemu process.
Resolves the following crash:
[1] crash backtrace: (bt is shortened a bit):
0 0x00007ffff7282f2b in virClassIsDerivedFrom
(klass=0xdeadbeef, parent=0x55555581d650) at util/virobject.c:169
1 0x00007ffff72835fd in virObjectIsClass
(anyobj=0x7fffd024f580, klass=0x55555581d650) at util/virobject.c:365
2 0x00007ffff7283498 in virObjectLock
(anyobj=0x7fffd024f580) at util/virobject.c:317
3 0x00007ffff722f0a3 in virCloseCallbacksUnset
(closeCallbacks=0x7fffd024f580, vm=0x7fffd0194db0,
cb=0x7fffdf1af765 <qemuProcessAutoDestroy>)
at util/virclosecallbacks.c:164
4 0x00007fffdf1afa7b in qemuProcessAutoDestroyRemove
(driver=0x7fffd00f3a60, vm=0x7fffd0194db0) at qemu/qemu_process.c:6365
5 0x00007fffdf1adff1 in qemuProcessStop
(driver=0x7fffd00f3a60, vm=0x7fffd0194db0, reason=VIR_DOMAIN_SHUTOFF_CRASHED,
asyncJob=QEMU_ASYNC_JOB_NONE, flags=0)
at qemu/qemu_process.c:5877
6 0x00007fffdf1f711c in processMonitorEOFEvent
(driver=0x7fffd00f3a60, vm=0x7fffd0194db0) at qemu/qemu_driver.c:4545
7 0x00007fffdf1f7313 in qemuProcessEventHandler
(data=0x555555832710, opaque=0x7fffd00f3a60) at qemu/qemu_driver.c:4589
8 0x00007ffff72a84c4 in virThreadPoolWorker
(opaque=0x555555805da0) at util/virthreadpool.c:167
Thread 1 (Thread 0x7ffff7fb1880 (LWP 494472)):
1 0x00007ffff72a7898 in virCondWait
(c=0x7fffd01c21f8, m=0x7fffd01c21a0) at util/virthread.c:154
2 0x00007ffff72a8a22 in virThreadPoolFree
(pool=0x7fffd01c2160) at util/virthreadpool.c:290
3 0x00007fffdf1edd44 in qemuStateCleanup ()
at qemu/qemu_driver.c:1102
4 0x00007ffff736570a in virStateCleanup ()
at libvirt.c:807
5 0x000055555556f991 in main (argc=1, argv=0x7fffffffe458) at libvirtd.c:1660
Recently, libprlsdk got a separate flag PNA_BRIDGE corresponding to
type=bridge libvirt network interfaces. Let's use it and get rid of
all workarounds previously added to support it.
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>
We don't support cpu pinning for TCG domains because QEMU runs them in
one thread only. But vcpupin command was able to set them, which
resulted in a failed startup, so make sure that doesn't happen.
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
When starting a new domain, we allocate the USB addresses and keep
an address cache in the domain object's private data.
However this data is lost on libvirtd restart.
Also generate the address cache if all the addresses have been
specified, so that devices hotplugged after libvirtd restart
also get theirs assigned.
https://bugzilla.redhat.com/show_bug.cgi?id=1387666
Return 0 instead of 1, so that qemuDomainAttachChrDevice does not
assume the address neeeds to be released on error.
No functional change, since qemuDomainReleaseDeviceAddress has been a noop
for virtio serial addresses since the address cache was removed
in commit 19a148b.
This time do not require an address cache as a parameter.
Simplify qemuDomainAttachChrDeviceAssignAddr to not generate
the virtio serial address cache for devices of other types.
Partially reverts commit 925fa4b.
Commit 19a148b dropped the cache from QEMU's private domain object.
Assume the callers do not have the cache by default and use
a longer name for the internal ones that do.
This makes the shorter 'virDomainVirtioSerialAddrAutoAssign'
name availabe for a function that will not require the cache.
When user tries to resume already running domain (Qemu or LXC)
VIR_ERR_OPERATION_INVALID error should be raised with message that
domain is already running.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1009008
Support for virtio disks was added in commit id 'fceeeda', but not for
SCSI drives. Add the secret for the server when hotplugging a SCSI drive.
No need to make any adjustments for unplug since that's handled during
the qemuDomainDetachDiskDevice call to qemuDomainRemoveDiskDevice in
the qemuDomainDetachDeviceDiskLive switch.
Added a test to/for the command line processing to show the command line
options when adding a SCSI drive for the guest.
https://bugzilla.redhat.com/show_bug.cgi?id=1300776
Complete the implementation of support for TLS encryption on
chardev TCP transports by adding the hotplug ability of a secret
to generate the passwordid for the TLS object for chrdev, RNG,
and redirdev.
Fix up the order of object removal on failure to be the inverse
of the attempted attach (for redirdev, chr, rng) - for each the
tls object was being removed before the chardev backend.
Likewise, add the ability to hot unplug that secret object as well
and be sure the order of unplug matches that inverse order of plug.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Add the secret object so the 'passwordid=' can be added if the command line
if there's a secret defined in/on the host for TCP chardev TLS objects.
Preparation for the secret involves adding the secinfo to the char source
device prior to command line processing. There are multiple possibilities
for TCP chardev source backend usage.
Add test for at least a serial chardev as an example.
Need to remove the drive first, then the secobj and/or encobj if they exist.
This is because the drive has a dependency on secobj (or the secret for
the networked storage server) and/or the encobj (or the secret for the
LUKS encrypted volume). Deleting either object first leaves an drive
without it's respective objects.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Add in the block I/O throttling length/duration parameter to the command
line if supported. If not supported, fail command creation.
Add the xml2argvtest for testing.
Add support for a duration/length for the bps/iops and friends.
Modify the API in order to add the "blkdeviotune." specific definitions
for the iotune throttling duration/length options
total_bytes_sec_max_length
write_bytes_sec_max_length
read_bytes_sec_max_length
total_iops_sec_max_length
write_iops_sec_max_length
read_iops_sec_max_length
Create a helper to set the bytes/iops iotune default values based on
the current qemu setting for both the live and persistent definitions.
NB: This also fixes an unreported bug where the persistent values for
*_max and size_iops_sec would be set back to 0 if unrelated persistent
values were set.
This patch will also adjust the qemuMonitorJSONSetBlockIoThrottle error
procession so that rather than returning/displaying:
"error: internal error: Unexpected error"
Fetch the actual error message from qemu and display that
Create a macros to hide all the comparisons for each of the fields.
Add a 'continue;' for a compiler hint that we only need to find one
this should be similar enough to the if - elseif - elseif logic.
Commit id '2c32237' added the TLS object removal to the DetachChrDevice
all when it should have been added to the RemoveChrDevice since that's
the norm for similar processing (e.g. disk)
Signed-off-by: John Ferlan <jferlan@redhat.com>
After succesfully reading an outdated caps cache from disk,
calling virQEMUCapsReset did not properly clear out the calculated
host CPU model. This lead to a memory leak when the host CPU model
pointer was overwritten later in virQEMUCapsNewForBinaryInternal.
Introduced by commit 68c70118.
Although the migration port is immediately released in the
finish phase of migration, it was never set in the domain
private object when allocated in the prepare phase. So
libxlDomainMigrationFinish() always released a 0-initialized
migrationPort, leaking any allocated port. After enough
migrations to exhaust the migration port pool, migration would
fail with
error: internal error: Unable to find an unused port in range
'migration' (49152-49216)
Fix it by setting libxlDomainObjPrivate->migrationPort to the
port allocated in the prepare phase. While at it, also fix
leaking an allocated port if the prepare phase fails.
Extended qemuDomainGetStatsVcpu to include the per vcpu halted
indicator if reported by QEMU. The key for new boolean value
has the format "vcpu.<n>.halted".
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Adding a field to the domain's private vcpu object to hold the halted
state information.
Adding two functions in support of the halted state:
- qemuDomainGetVcpuHalted: retrieve the halted state from a
private vcpu object
- qemuDomainRefreshVcpuHalted: obtain the per-vcpu halted states
via qemu monitor and store the results in the private vcpu objects
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Reviewed-by: Hao QingFeng <haoqf@linux.vnet.ibm.com>
Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Extended the qemuMonitorCPUInfo with a halted flag. Extract the halted
flag for both text and JSON monitor.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
An upcoming commit will remove the "flag" argument from all the calls
to reserve the next available address|slot, but I don't want to change
the arguments in the hypervisor-agnostic
virDomainPCIAddressReserveNext*() functions, so this patch places a
simple qemu-specific wrapper around those functions - the new
functions don't take a flags arg, but grab it from the device's
info->pciConnectFlags.
There is an existing virDomainPCIAddressReserveNextSlot() which will
reserve all functions of the next available PCI slot. One place in the
qemu PCI address assignment code requires reserving a *single*
function of the next available PCI slot. This patch modifies and
renames virDomainPCIAddressReserveNextSlot() so that it can fulfill
both the original purpose and the need to reserve a single function.
(This is being done so that the abovementioned code in qemu can have
its "kind of open coded" solution replaced with a call to this new
function).
Since TLS was introduced hostwide for libvirt 2.3.0 and a domain
configurable haveTLS was implemented for libvirt 2.4.0, we have to
modify the migratable XML for specific case where the 'tls' attribute
is based on setting from qemu.conf.
The "tlsFromConfig" is libvirt internal attribute and is stored only in
status XML to ensure that when libvirtd is restarted this internal flag
is not lost by the restart.
That flag is used to decide whether we should put *tls* attribute to
migratable XML or not.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Add an optional "tls='yes|no'" attribute for a TCP chardev.
For QEMU, this will allow for disabling the host config setting of the
'chardev_tls' for a domain chardev channel by setting the value to "no" or
to attempt to use a host TLS environment when setting the value to "yes"
when the host config 'chardev_tls' setting is disabled, but a TLS environment
is configured via either the host config 'chardev_tls_x509_cert_dir' or
'default_tls_x509_cert_dir'
Signed-off-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Currently the union has only one member so remove that union. If there
is a need to add a new type of source for new bus in the future this
will force the author to add a union and properly check bus type before
any access to union member.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Commit id '2c322378' missed the nuance that the rng backend could be
using a TCP chardev and if TLS is enabled on the host, thus will need
to have the TLS object added.
Commit id '2c322378' missed the nuance that the redirdev backend could
be using a TCP chardev and if TLS is enabled on the host, thus will need
to have the TLS object added.
Rather than VIR_ALLOC() the data, use virDomainChrSourceDefNew in order
to get the private data if necessary.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Use a pointer and the virDomainChrSourceDefNew() function in order to
allocate the structure for _virDomainRedirdevDef.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Use a pointer and the virDomainChrSourceDefNew() function in order to
allocate the structure for _virDomainSmartcardDef.
Signed-off-by: John Ferlan <jferlan@redhat.com>
instead of:
virBufferAdd(buf, "arg1,");
virBufferAdd(buf, "arg2");
lets have:
virBufferAdd(buf, "arg1");
virBufferAdd(buf, ",arg2");
Because it's better. Consider we want to add conditionally arg3.
With this change, it's simple:
if (cond)
virBufferAdd(buf, ",arg3");
with current code there might be a comma hanging at EOL.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
from virDomainDefPtr to virDomainObjPtr so that the function has
access to the other parts of the virDomainObjPtr. Take advantage of
this by removing the "priv" arg and retrieving it from the
virDomainObjPtr instead.
No functional change.
For some reason the values of memballoon model are set using an
anonymous enum, making it impossible to perform nice tricks like
demanding there are cases for all possible values in a switch. This
patch turns the anonymous enum into virDomainMemballoonModel.
More occurences of repeatedly dereferencing the same pointer stored in
an array are replaced with the definition of a temporary pointer that
is then used directly. No functional change.
Commit id '5f2a132786' should have placed the data in the host source
def structure since that's also used by smartcard, redirdev, and rng in
order to provide a backend tcp channel. The data in the private structure
will be necessary in order to provide the secret properly.
This also renames the previous names from "Chardev" to "ChrSource" for
the private data structures and API's
Change the virDomainChrDef to use a pointer to 'source' and allocate
that pointer during virDomainChrDefNew.
This has tremendous "fallout" in the rest of the code which mainly
has to change source.$field to source->$field.
Signed-off-by: John Ferlan <jferlan@redhat.com>
When hotplugging networks with ancient QEMUs not supporting
QEMU_CAPS_NETDEV, we use space instead of a comma as the separator
between the network type and other options.
Except for "user", all the network types pass other options
and use up the first separator by the time we get to the section
that adds the alias (or vlan for QEMUs without CAPS_NETDEV).
Since the alias/vlan is mandatory, convert all preceding code to add
the separator at the end, removing the need to rewrite type_sep for
all types but NET_TYPE_USER.
Absent driver name attribute is invalid xml. Which in turn makes
unusable 'virsh edit' for example. The value does not make
much sense and ignored on input so nobody will hurt.
vz sdk supports setting serial number only for disk devices.
Getting serial upon cdrom(for example) is error however
setting is just ignored. Let's check for disk device
explicitly for clarity in both cases.
Setting serial number for other devices is ignored
with an info note just as before.
We need usual conversion from "" to NULL in direction
vz sdk -> libvirt, because "" is not valid for libvirt
and "" means unspecifiend in vz sdk which is NULL for libvirt.
New line character in name of network is now forbidden because it
mess virsh output and can be confusing for users. Validation of
name is done in network driver, after parsing XML to avoid
problems with disappeared network which was already created with
new-line char in name.
Closes-Bug: https://bugzilla.redhat.com/show_bug.cgi?id=818064
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
New util function virXMLCheckIllegalChars is now used to test if
parsed network contains illegal char '/' in it's name.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This new function can be used to check if e.g. name of XML
node don't contains forbidden chars like "/" or "\n".
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Modeled after the qemuDomainHostdevPrivatePtr (commit id '27726d8c'),
create a privateData pointer in the _virDomainChardevDef to allow storage
of private data for a hypervisor in order to at least temporarily store
secret data for usage during qemuBuildCommandLine.
NB: Since the qemu_parse_command (qemuParseCommandLine) code is not
expecting to restore the secret data, there's no need to add code
code to handle this new structure there.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Add a new qemu.conf variables to store the UUID for the secret that could
be used to present credentials to access the TLS chardev. Since this will
be a server level and it's possible to use some sort of default, introduce
both the default and chardev logic at the same time making the setting of
the chardev check for it's own value, then if not present checking whether
the default value had been set.
Signed-off-by: John Ferlan <jferlan@redhat.com>
When converting a domain xml containing a CDROM device without
any attached source, don't add a target=(null) to the libxl config
disk definition: xen doesn't like it at all and would fail to start
the domain.
There was inconsistency between alias used to create tls-creds-x509
object and alias used to link that object to chardev while hotpluging.
Hotplug ends with this error:
error: Failed to detach device from channel-tcp.xml
error: internal error: unable to execute QEMU command 'chardev-add':
No TLS credentials with id 'objcharchannel3_tls0'
In XML we have for example alias "serial0", but on qemu command line we
generate "charserial0".
The issue was that code, that creates QMP command to hotplug chardev
devices uses only the second alias "charserial0" and that alias is also
used to link the tls-creds-x509 object.
This patch unifies the aliases for tls-creds-x509 to be always generated
from "charserial0".
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Instead of typing the prefix every time we want to append parameters
to qemu command line use a variable that contains prefixed alias.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
We need to make sure that the chardev is TCP. Without this check we
may access different part of union and corrupt pointers.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
The code is entirely correct, but it still managed to trip me
up when I first ran into it because I did not realize right away
that VIR_PCI_CONNECT_TYPES_ENDPOINT was not a single flag, but
rather a mask including both VIR_PCI_CONNECT_TYPE_PCI_DEVICE and
VIR_PCI_CONNECT_TYPE_PCIE_DEVICE.
In order to save the next distracted traveler in PCI Address Land
some time, document this fact with a comment. Add a test case for
the behavior as well.
A pci-bridge has *almost* the same rules as a legacy PCI endpoint
device for where it can be automatically connected, and until now both
had been considered identical. There is one pairing that is okay when
specifically requested by the user (i.e. manual assignment), but we
want to avoid it when auto-assigning addresses - plugging a pci-bridge
directly into pcie-root (it is cleaner to plug in a dmi-to-pci-bridge,
then plug the pci-bridge into that).
In order to allow that difference, this patch makes a separate
CONNECT_TYPE for pci-bridge, and uses it to restrict auto-assigned
addresses for pci-bridges to be only on pci-root, pci-expander-bus,
dmi-to-pci-bridge, or on another pci-bridge.
NB: As with other discouraged-but-seem-to-work configurations
(e.g. plugging a legacy PCI device into a pcie-root-port) if someone
*really* wants to, they can still force a pci-bridge to be plugged
into pcie-root (by manually specifying its PCI address.)
https://bugzilla.redhat.com/show_bug.cgi?id=1357416
Rather than return a 0 or -1 and the *result string, return just the result
string to the caller. Alter all the callers to handle the different return.
As a side effect or result of this, it's much clearer that we cannot just
assign the returned string into the scsi_host wwnn, wwpn, and fabric_wwn
fields - rather we should fetch a temporary string, then as long as our
fetch was good, VIR_FREE what may have been there, and STEAL what we just got.
This fixes a memory leak in the virNodeDeviceCreateXML code path through
find_new_device and nodeDeviceLookupSCSIHostByWWN which will continually
call nodeDeviceSysfsGetSCSIHostCaps until the expected wwnn/wwpn is found
in the device object capabilities.
Signed-off-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1366108
There are couple of things that needs to be done in order to
allow vhost-user hotplug. Firstly, vhost-user requires a chardev
which is connected to vhost-user bridge and through which qemu
communicates with the bridge (no acutal guest traffic is sent
through there, just some metadata). In order to generate proper
chardev alias, we must assign device alias way sooner.
Then, because we are plugging the chardev first, we need to do
the proper undo if something fails - that is remove netdev too.
We don't want anything to be left over in case attach fails at
some point.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1366505
So far, this function lacked support for
VIR_DOMAIN_NET_TYPE_VHOSTUSER leaving callers to hack around the
problem by constructing the command line on their own. This is
not ideal as it blocks hot plug support.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Currently, what we do for vhost-user network is generate the
following part of command line:
-netdev type=vhost-user,id=hostnet0,chardev=charnet0
There's no need for 'type=' it is the default. Drop it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
There's no need to reinvent the wheel here. We already have a
function to format virDomainChrSourceDefPtr. It's called
qemuBuildChrChardevStr(). Use that instead of some dummy
virBufferAsprintf().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This alone makes not much sense. But the aim is to reuse this
function in qemuBuildVhostuserCommandLine() where 'nowait' is not
supported for vhost-user devices.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
We tend to prevent using 'default' in switches. And it is for a
good reason - control may end up in paths we wouldn't want for
new values. In this specific case, if qemuBuildHostNetStr is
called over VIR_DOMAIN_NET_TYPE_VHOSTUSER it would produce
meaningless output. Fortunately, there no such call yet.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Instead of blindly claim support for hot-plugging of every
interface type out there we should copy approach we have for
device types: white listing supported types and explicitly error
out on unsupported ones.
For instance, trying to hotplug vhostuser interface results in
nothing usable from guest currently. vhostuser typed interfaces
require additional work on our side.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The idea is to have function that does some checking at its
beginning and then have one big switch for all the interface
types it supports.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The idea is to have function that does some checking of the
arguments at its beginning and then have one big switch for all
the interface types it supports. Each one of them generating the
corresponding part of the command line.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The idea is to have function that does some checking of the
arguments at its beginning and then have one big switch for all
the interface types it supports. Each one of them generating the
corresponding part of the command line.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This function for some weird reason returns integer instead of
virDomainNetType type. It is important to return the correct type
so that we know what values we can expect.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Not every system out there has syslog, that's why we check for it
in our configure script. However, in 640b58abdf while fixing
another issue, some variables and functions are called that are
defined only when syslog.h is present. But these function
calls/variables were not guarded by #ifdef-s.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
qemuBuildSmbiosBiosStr and qemuBuildSmbiosSystemStr return NULL if
there's nothing to format on the commandline. Reporting errors from
buffer creation doesn't make sense since it would be ignored.
This initially started as a fix of some debug printing in
virCgroupDetect. However it turned out that other places suffer
from the similar problem. While dealing with pids, esp. in cases
where we cannot use pid_t for ABI stability reasons, we often
chose an unsigned integer type. This makes no sense as pid_t is
signed.
Also, new syntax-check rule is introduced so we won't repeat this
mistake.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
There are two video devices with models without VGA compatibility mode.
They are primary used as secondary video devices, but in some cases it
is required to use them also as primary video devices.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
This improves commit 706b5b6277 in a way that we check qemu capabilities
instead of what architecture we are running on to detect whether we can
use *virtio-vga* model or not. This is not a case only for arm/aarch64.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Commit 21373feb added support for primary virtio-vga device but it was
checking for virtio-gpu. Let's check for existence of virtio-vga if we
want to use it.
Virtio video device is currently represented by three different models
*virtio-gpu-device*, *virtio-gpu-pci* and *virtio-vga*. The first two
models are tied together and if virtio video devices is compiled in they
both exist. However, the *virtio-vga* model doesn't have to exist on
some architectures even if the first two models exist. So we cannot
group all three together.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Before this patch we've checked qemu capabilities for video devices
only while constructing qemu command line using "-device" option.
Since we support qemu only if "-device" option is present we can use
the same capabilities to check also video devices while using "-vga"
option to construct qemu command line.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
All definition validation that doesn't depend on qemu capabilities
and was allowed previously as valid definition should be placed into
qemuDomainDefValidate.
The check whether video type is supported or not was based on an enum
that translates type into model. Use switch to ensure that if new
video type is added, it will be properly handled.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
We generally uses QEMU_CAPS_DEVICE_$NAME to probe for existence of some
device and QEMU_CAPS_$NAME_$PROP to probe for existence of some property
of that device.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
If QEMU in question supports QMP, this capability is set if
QEMU_CAPS_DEVICE_QXL was set based on existence of "-device qxl". If
libvirt needs to parse *help*, because there is no QMP support, it
checks for existence of "-vga qxl", but it also parses output of
"-device ?" and sets QEMU_CAPS_DEVICE_QXL too.
Now that libvirt supports only QEMU that has "-device" implemented it's
safe to drop this capability and stop using it.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
This patch simplifies QEMU capabilities for QXL video device. QEMU
exposes this device as *qxl-vga* and *qxl* and they are both the same
device with the same set of parameters, the only difference is that
*qxl-vga* includes VGA compatibility.
Based on QEMU code they are tied together so it's safe to check only for
presence of only one of them.
This patch also removes an invalid test case "video-qxl-sec-nodevice"
where there is only *qxl-vga* device and *qxl* device is not present.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Qemu supports *xen* video device only with XEN and this code was part
of xenner code. We dropped support for xenner in commit de9be0a.
Before this patch if you used 'xen' video type you ended up with
domain without any video device at all. Now we don't allow to start
such domain.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
It was never safe anyway and as such shouldn't have been enabled in the
first place. Future patches will allow hot-(un)pluging of some ivshmem
devices as a workaround.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Due to the switch of parameters in a call to virDomainShmemDefEquals()
no device was found when looking for device with all the information
except address. Also fix the indentation.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
If the last event callback is unregistered while the event loop is
dispatching, it is only marked as deleted, but not removed. The number
of callbacks is more than zero in that case, so the timer is not
removed. Because it can be removed in this function now (but also
accessed afterwards so that we set 'isDispatching = false' and have it
locked), we need to temporarily increase the reference counter of the
state for the duration of this function.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
There is a repeating pattern of code that removes the timer if it's not
needed. So let's move it to a new function. We'll also use it later.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
There should be one more reference because it is being kept in the list
of callbacks as an opaque. We also unref it properly using
virObjectFreeCallback.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Currently Libvirt allows attempts to migrate read only disks. Qemu
cannot handle this as read only disks cannot be written to on the
destination system. The end result is a cryptic error message and a
failed migration.
This patch causes migration to fail earlier and provides a meaningful
error message stating that migrating read only disks is not supported.
Signed-off-by: Corey S. McQuay <csmcquay@linux.vnet.ibm.com>
Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Make sure that the topology results into a sane number of cpus (up to
UINT_MAX) so that it can be sanely compared to the vcpu count of the VM.
Additionally the helper added in this patch allows to fetch the total
number the topology results to so that it does not have to be
reimplemented later.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1378290
In both virLogParseOutput and virLogParseFilter, rather than returning
NULL, goto cleanup since it's possible that for each the first condition
passes, but the || condition doesn't and thus we leak memory.
The dnsmasq man page recommends that dhcp-authoritative "should be
set when dnsmasq is definitely the only DHCP server on a network".
This is the case for libvirt-managed virtual networks.
The effect of this is that VMs that fail to renew their DHCP lease
in time (e.g. if the VM or host is suspended) will be able to
re-acquire the lease even if it's expired, unless the IP address has
been taken by some other host. This avoids various annoyances caused
by changing VM IP addresses.
Sometimes virObjectEventStateFlush can be called without timer (if the
last event was unregistered right when the timer fired). There is a
check for timer == -1, but that triggers warning and other log messages,
which is unnecessary.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Handling of outputs and filters has been changed in a way that splits
parsing and defining. Do the same thing for logging priority as well, this
however, doesn't need much of a preparation.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
This is mainly virLogAddOutputTo* which were replaced by virLogNewOutputTo* and
the previously poorly named ones virLogParseAndDefine* functions. All of these
are unnecessary now, since all the original callers were transparently switched
to the new model of separate parsing and defining logic.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Similar to outputs, parser should do parsing only, thus the 'define' logic
is going to be stripped from virLogParseAndDefineFilters by replacing calls to
this method to virLogSetFilters instead.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Since virLogParseAndDefineOutputs is going to be stripped from 'output defining'
logic, replace all relevant occurrences with virLogSetOutputs call to make the
change transparent to all original callers (daemons mostly).
Signed-off-by: Erik Skultety <eskultet@redhat.com>
This method will eventually replace virLogParseAndDefineFilters which
currently does both parsing and defining.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
This API is the entry point to output modification of the logger. Currently,
everything is done by virLogParseAndDefineOutputs. Parsing and defining will be
split into two operations both handled by this method transparently.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Abstraction added over parsing a single filter. The method parses potentially a
set of logging filters, while adding each filter logging object to a
caller-provided array.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Another abstraction added on the top of parsing a single logging output. This
method takes and parses the whole set of outputs, adding each single output
that has already been parsed into a caller-provided array. If the user-supplied
string contained duplicate outputs, only the last occurrence is taken into
account (all the others are removed from the list), so we silently avoid
duplicate logs.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Same as for outputs, introduce a new method, that is basically the same as
virLogParseAndDefineFilter with the difference that it does not define the
filter. It rather returns a newly created object that needs to be inserted into
a list and then defined separately.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Introduce a method to parse an individual logging output. The difference
compared to the virLogParseAndDefineOutput is that this method does not define
the output, instead it makes use of the virLogNewOutputTo* methods introduced
in the previous patch and just returns the virLogOutput object that has to be
added to a list of object which then can be defined as a whole via
virLogDefineOutputs. The idea remains still the same - split parsing and
defining of the logging primitives (outputs, filters).
Additionally, since virLogNewOutputTo* methods are now finally used,
ATTRIBUTE_UNUSED can be successfully removed from the methods' definitions,
since that was just to avoid compiler complaints about unused static functions.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Now that we're in the critical section, syslog connection can be re-opened
by issuing openlog, which is something that cannot be done beforehand, since
syslog keeps its file descriptor private and changing the tag earlier might
introduce a log inconsistency if something went wrong with preparing a new set
of logging outputs in order to replace the existing one.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Continuing with the effort to split output parsing and defining, these new
functions return a logging object reference instead of defining the output.
Eventually, these functions will replace the existing ones (virLogAddOutputTo*)
which will then be dropped.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Prepare a method that only defines a set of filters. It takes a list of
filters, preferably created by virLogParseFilters. The original set of filters
is reset and replaced by the new user-provided set of filters.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Prepare a method that only defines a set of outputs. It takes a list of
outputs, preferably created by virLogParseOutputs. The original set of outputs
is reset and replaced by the new user-provided set of outputs.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Outputs are a bit trickier than filters, since the user(config)-specified
set of outputs can contain duplicates. That would lead to logging the same
message twice. For compatibility reasons, we cannot just error out and forbid
the daemon to start if we find duplicate outputs which do not make sense.
Instead, we could silently take into account only the last occurrence of the
duplicate output and remove all the previous ones, so that the logger will not
try to use them when it is looping over all of its registered outputs.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
In order to later split output parsing and output defining, introduce a new
function which will create a new virLogOutput object which the parser will
insert into a list with the list being eventually defined.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
There is really no reason why we could not keep journald's fd within the
journald output object the same way as we do for regular file-based outputs.
By doing this we later won't have to special case the journald-based output
(due to the fd being globally shared) when replacing the existing set of outputs
with a new one. Additionally, by making this change, we don't need the
virLogCloseJournald routine anymore, plain virLogCloseFd will suffice.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Right now virLogParse* functions are doing both parsing and defining of filters
and outputs which should be two separate operations. Since the naming is
apparently a bit poor this patch renames these functions to
virLogParseAndDefine* which eventually will be replaced by virLogSet*.
Additionally, virLogParse{Filter,Output} will be later (after the split) reused,
so that these functions do exactly what the their name suggests.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
During first stage of virlog.c refactor, commit 0b231195 forgot to remove the
macro definition along with its usage.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
If the event is already disabled, then don't bother with setting it
disabled again. Causes unnecessary error on systems that don't support
the feature anyway.
The intel-iommu device has existed since QEMU 2.2.0, but
it was only possible to create it with -device since
QEMU 2.7.0, thanks to:
commit 621d983a1f9051f4cfc3f402569b46b77d8449fc
Author: Marcel Apfelbaum <marcel@redhat.com>
Date: Mon Jun 27 18:38:34 2016 +0300
hw/iommu: enable iommu with -device
Use the standard '-device intel-iommu' to create the IOMMU device.
The legacy '-machine,iommu=on' can still be used.
The libvirt capability check & command line formatting code
is thus broken for all QEMU versions 2.2.0 -> 2.6.0 inclusive.
This fixes it to use iommu=on instead.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Since introduction of chardev hotplug the code was wrong for the UDP
case and basically created a TCP socket instead. Use proper objects and
type for UDP.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1377602
Rather than copy-paste - use a macro
Unfortunately due to how the RNG schema was written keeping the 'value'
and 'value'_max next to each other in the XML causes a schema failure,
so the FORMAT has to write out singly rather than optimizing to write
out both values at once
Signed-off-by: John Ferlan <jferlan@redhat.com>
We're about to add more options, let's avoid having multiple if-then-else
which each try to set up the qemuMonitorJSONMakeCommand call with all the
parameters it knows about.
Instead, use the fact that when a NULL is found in the argument list that
processing of the remaining arguments stops and just have call.
Signed-off-by: John Ferlan <jferlan@redhat.com>
We're about to add 6 new options and it appears (from testing) one cannot
utilize both the shorthand (alias) and (much) longer names for the arguments.
So modify the command builder to use the longer name and of course alter the
test output .args to have the similarly innocuous long name.
Also utilize a macro to build that name makes it so much more visually
appealing and saves a few characters or potential cut-n-paste issues.
Signed-off-by: John Ferlan <jferlan@redhat.com>
When I added support for the pcie-expander-bus controller in commit
bc07251f, I incorrectly thought that it only had a single slot
available. Actually it has 32 slots, just like the root complex aka
pcie-root (the part that I *did* get correct is that unlike pcie-root
a pcie-expander-bus doesn't allow any integrated endpoint devices -
only pcie-root-ports and dmi-to-pci-controllers are allowed).
Reduce some cut-n-paste code by creating common helper. Make use of the
recently added virJSONValueObjectStealArray to grab the devices list as
part of the common code (we we can Free the reply) and return devices for
each of the callers to continue to parse.
NB: This also adds error checking to qemuMonitorJSONDiskNameLookup
Signed-off-by: John Ferlan <jferlan@redhat.com>
Provide the Steal API for any code paths that will desire to grab the
object array and then free it afterwards rather than relying to freeing
the whole chain from the reply.
Rather than use stack allocated state context pointers, let's allocate and
free the state context pointer. In doing so, we'll shrink the code a bit
since many routines perform the same initialization sequence.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Since none of the callers check the status, let's just alter it to
a static void.
While we're at it - scrap the local runtime variable and just do the
math in the VIR_DEBUG directly.
Signed-off-by: John Ferlan <jferlan@redhat.com>
If attaching to a qemu process fails after opening the monitor socket
libvirt does not clean up the monitor. As the monitor also holds a
reference to the domain object the qemu attach API basically leaks it.
QEMU also does not interact on a second monitor connection and thus a
further attempt to attach to it would lock up.
Prevent libvirt from leaking the monitor by explicitly closing it.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1378401
Attaching to a existing qemu process allows to get us into a situation
when qemu is new enough to have JSON monitor and new vCPU hotplug but
the json monitor is not used. The vCPU detection code would require it
though. This broke attaching to qemu processes.
Make the condition less strict and just skip the vCPU hotplug detection
if JSON monitor is not available.
Resolves one of the symptoms in:
https://bugzilla.redhat.com/show_bug.cgi?id=1378401
Libvirt, on its own, shouldn't decide whether an expired lease should
stay in the custom leases database or not. It should rather rely on
the 'DEL' event from dnsmasq.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We are about to add 6 new values to fetch. This will put us over the
current limit of 16 (we're at 13 now).
Once there are more than 16 parameters, this will affect existing clients
that attempt to fetch blockiotune config values for the domain from the
remote host since the server side has no mechanism to determine whether
the capability for the emulator exists and thus would attempt to return
all known values from the persistentDef. If attempting to fetch the
blockiotune values from a running domain, the code will check the emulator
capabilities and set maxparams (in qemuDomainGetBlockIoTune) appropriately.
On the client side of the remote connection, it uses this constant in
xdr_remote_domain_get_block_io_tune_ret and virTypedParamsDeserialize
calls, so if a remote server returns more than 16 parameters, then the
client will fail with "Unable to decode message payload".
Signed-off-by: John Ferlan <jferlan@redhat.com>
The REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX was erroneously used in the
remoteDomainBlockStatsFlags and remoteDomainGetBlockIoTune calls. Change
the constant to be the right one.
Fortunately, all 3 are defined as 16.
Signed-off-by: John Ferlan <jferlan@redhat.com>
This breaks vCPU hotplug, because when starting a domain, we
create a copy of domain definition (which becomes live XML) and
during the post parse callbacks we might adjust some tunings so
that vCPU hotplug is possible.
This reverts commit 581b7756af.
This breaks vCPU hotplug, because when starting a domain, we
create a copy of domain definition (which becomes live XML) and
during the post parse callbacks we might adjust some tunings so
that vCPU hotplug is possible.
This reverts commit c0f90799bc.
Certain operations may make the vcpu order information invalid. Since
the order is primarily used to ensure migration compatibility and has
basically no other user benefits, clear the order prior to certain
operations and document that it may be cleared.
All the operations that would clear the order can still be properly
executed by defining a new domain configuration rather than using the
helper APIs.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1370357
virDomainDefSetVcpus was not designed to handle coldplug of vcpus now
that we can set state of vcpus individually.
Introduce qemuDomainSetVcpusConfig that properly handles state changes
of vcpus when coldplugging so that invalid configurations are not
created.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1375939
The current code that validates duplicate vcpu order would not work
properly if the order would exceed def->maxvcpus. Limit the order to the
interval described.
The bitmap indexes for the order duplicate check are shifted to 0 since
vcpu order 0 is not allowed. The error message doesn't need such
treating though.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1370360
https://bugzilla.redhat.com/show_bug.cgi?id=1292984
Hold on to your hats, because this is gonna be wild.
In bd3e16a3 I've tried to expose sanlock io_timeout. What I had
not realized (because there is like no documentation for sanlock
at all) was very unusual way their APIs work. Basically, what we
do currently is:
sanlock_add_lockspace_timeout(&ls, io_timeout);
which adds a lockspace to sanlock daemon. One would expect that
io_timeout sets the io_timeout for it. Nah! That's where you are
completely off the tracks. It sets timeout for next lockspace you
will probably add later. Therefore:
sanlock_add_lockspace_timeout(&ls, io_timeout = 10);
/* adds new lockspace with default io_timeout */
sanlock_add_lockspace_timeout(&ls, io_timeout = 20);
/* adds new lockspace with io_timeout = 10 */
sanlock_add_lockspace_timeout(&ls, io_timeout = 40);
/* adds new lockspace with io_timeout = 20 */
And so on. You get the picture.
Fortunately, we don't allow setting io_timeout per domain or per
domain disk. So we just need to set the default used in the very
first step and hope for the best (as all the io_timeout-s used
later will have the same value).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Currently, we are checking for sanlock_add_lockspace_timeout
which is good for now. But in a subsequent patch we are going to
use sanlock_write_lockspace (which sets an initial value for io
timeout for sanlock). Now, there is no reason to check for both
functions in sanlock library as the sanlock_write_lockspace was
introduced in 2.7 release and the one we are currently checking
for in the 2.5 release. Therefore it is safe to assume presence
of sanlock_add_lockspace_timeout when sanlock_write_lockspace
is detected.
Moreover, the macro for conditional compilation is renamed to
HAVE_SANLOCK_IO_TIMEOUT (as it now encapsulates two functions).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
If this reminds you of a commit message from around a year ago, it's
41c2aa729f and yes, we're dealing with
"the same thing" again. Or f309db1f4d and
it's similar.
There is a logic in place that if there is no real need for
memory-backend-file, qemuBuildMemoryBackendStr() returns 0. However
that wasn't the case with hugepage backing. The reason for that was
that we abused the 'pagesize' variable for storing that information, but
we should rather have a separate one that specifies whether we really
need the new object for hugepage backing. And that variable should be
set only if this particular NUMA cell needs special treatment WRT
hugepages.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1372153
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Due to a copy and paste error, the scheduler 'cap' parameter
was over-writing the 'weight' parameter when preparing the
return parameters in libxlDomainGetSchedulerParametersFlags.
As a result, the scheduler weight was never shown when getting
schedinfo and setting the weight failed as well
virsh schedinfo testvm
Scheduler : credit
cap : 0
virsh schedinfo testvm --cap 50 --weight 500
Scheduler : credit
error: invalid scheduler option: weight
The obvious fix is to assign the 'caps' parameter to the correct
item in the parameter list.
Reported-by: Volo M. <vm@vovs.net>
Add support for formating/parsing libxl channels.
Syntax on xen libxl goes as following:
channel=["connection=pty|socket,path=/path/to/socket,name=XXX",...]
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Acked-by: Jim Fehlig <jfehlig@suse.com>
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
And allow libxl to handle channel element which creates a Xen
console visible to the guest as a low-bandwitdh communication
channel. If type is PTY we also fetch the tty after boot using
libxl_channel_getinfo to fetch the tty path. On socket case,
we autogenerate a path if not specified in the XML. Path autogenerated
is slightly different from qemu driver: qemu stores also on
"channels/target" but it creates then a directory per domain with
each channel target name. libxl doesn't appear to have a clear
definition of private files associated with each domain, so for
simplicity we do it slightly different. On qemu each autogenerated
channel goes like:
channels/target/<domain-name>/<target name>
Whereas for libxl:
channels/target/<domain-name>-<target name>
Should note that if path is not specified it won't persist,
existing only on live XML, unless user had initially specified it.
Since support for libxl channels only came on Xen >= 4.5 we therefore
need to conditionally compile it with LIBXL_HAVE_DEVICE_CHANNEL.
After this patch and having a qemu guest agent:
$ cat domain.xml | grep -a1 channel | head -n 5 | tail -n 4
<channel type='unix'>
<source mode='bind' path='/tmp/channel'/>
<target type='xen' name='org.qemu.guest_agent.0'/>
</channel>
$ virsh create domain.xml
$ echo '{"execute":"guest-network-get-interfaces"}' | socat
stdio,ignoreeof unix-connect:/tmp/channel
{"execute":"guest-network-get-interfaces"}
{"return": [{"name": "lo", "ip-addresses": [{"ip-address-type": "ipv4",
"ip-address": "127.0.0.1", "prefix": 8}, {"ip-address-type": "ipv6",
"ip-address": "::1", "prefix": 128}], "hardware-address":
"00:00:00:00:00:00"}, {"name": "eth0", "ip-addresses":
[{"ip-address-type": "ipv4", "ip-address": "10.100.0.6", "prefix": 24},
{"ip-address-type": "ipv6", "ip-address": "fe80::216:3eff:fe40:88eb",
"prefix": 64}], "hardware-address": "00:16:3e:40:88:eb"}, {"name":
"sit0"}]}
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
So far only guestfwd and virtio were supported. Add an additional
for Xen as libxl channels create a Xen console visible to the guest.
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
The qemucapsprobe helper calls virQEMUCapsNewForBinaryInternal with
caps == NULL, causing the following crash:
Program received signal SIGSEGV, Segmentation fault.
#0 0x00007ffff788775f in virQEMUCapsInitHostCPUModel
(qemuCaps=qemuCaps@entry=0x649680, host=host@entry=0x10) at
src/qemu/qemu_capabilities.c:2969
#1 0x00007ffff7889dbf in virQEMUCapsNewForBinaryInternal
(caps=caps@entry=0x0, binary=<optimized out>,
libDir=libDir@entry=0x4033f6 "/tmp", cacheDir=cacheDir@entry=0x0,
runUid=runUid@entry=4294967295, runGid=runGid@entry=4294967295,
qmpOnly=true) at src/qemu/qemu_capabilities.c:4039
#2 0x0000000000401702 in main (argc=2, argv=0x7fffffffd968) at
tests/qemucapsprobe.c:73
Caused by v2.2.0-182-g68c7011.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1368417
So far, when it comes to 'virsh update-device --config' of disks
we are limiting ourselves for just the disk source update and
just for CDROMs and floppies. This makes no sense. Especially if
you look around and see that we already allow full update to
graphics and net devices. So let's just take whatever XML user
wants to have there and replace our internal definition with it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
libxl events are delivered to libvirt via the libxlDomainEventHandler
callback registered with libxl. Documenation in
$xensrc/tools/libxl/libxl_event.h states that the callback "may occur
on any thread in which the application calls libxl". This can result
in deadlock since many of the libvirt callees of libxl hold a lock on
the virDomainObj they are working on. When the callback is invoked, it
attempts to find a virDomainObj corresponding to the domain ID provided
by libxl. Searching the domain obj list results in locking each obj
before checking if it is active, and its ID equals the requested ID.
Deadlock is possible when attempting to lock an obj that is already
locked further up the call stack. Indeed, Max Ustermann reported an
instance of this deadlock
https://www.redhat.com/archives/libvir-list/2015-November/msg00130.html
Guido Rossmueller also recently stumbled across it
https://www.redhat.com/archives/libvir-list/2016-September/msg00287.html
Fix the deadlock by moving the lookup of virDomainObj to the
libxlDomainShutdownThread. After this patch, libxl events are
enqueued on the libvirt side and processed by dedicated thread,
avoiding the described deadlock.
Reported-by: Max Ustermann <ustermann78@web.de>
Reported-by: Guido Rossmueller <Guido.Rossmueller@gdata.de>
enum types are unsigned and the qemuGetCompressionProgram
function can return -1 on error. It is therefore inappropriate
to return an enum type. This fixes a build error where the
internal 'ret' variable was used in a comparison with -1
../../src/qemu/qemu_driver.c: In function 'qemuGetCompressionProgram':
../../src/qemu/qemu_driver.c:3280:5: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
../../src/qemu/qemu_driver.c:3289:5: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
cc1: all warnings being treated as errors
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
When creating a copy of virDomainDef we save ourselves the
trouble of writing deep-copy functions and just format and parse
back domain/device XML. However, the XML we are parsing was
already fully formatted - there is no reason to run post parse
callbacks (which fill in blanks - there are none!).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This is an internal flag that prevents our two entry points to
XML parsing (virDomainDefParse and virDomainDeviceDefParse) from
running post parse callbacks. This is expected to be used in
cases when we already have full domain/device XML and we are just
parsing it back (i.e. virDomainDefCopy or virDomainDeviceDefCopy)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Just like we did two commits ago, don't try to fetch capabilities
for non-existing binary. Re-use the ones we have for running
domain.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Just like we did two commits ago, don't try to fetch capabilities
for non-existing binary. Re-use the ones we have for running
domain.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
We can't rely on def->emulator path. It may be provided by user
as we give them opportunity to provide their own XML for
migration. Therefore the path may point to just whatever binary
(or even to a non-existent file). Moreover, this path is meant
for destination, but the capabilities lookup is done on source.
What we can do is to assume same capabilities for post parse
callbacks as the running domain has. They will be used just to
add some default models/controllers/devices/... anyway.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Just like virDomainDefPostParseCallback has gained new
parseOpaque argument, we need to follow the logic with
virDomainDeviceDefPostParse.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
We want to pass the proper opaque pointer instead of NULL to
virDomainDefParse and subsequently virDomainDefParseNode too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
We want to pass the proper opaque pointer instead of NULL to
virDomainDefParseXML and subsequently virDomainDefPostParse too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Some callers might want to pass yet another pointer to opaque
data to post parse callbacks. The driver generic one is not
enough because two threads executing post parse callback might
want to see different data (e.g. domain object pointer that
domain def belongs to).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Based upon a patch from Chen Hanxiao <chenhanxiao@gmail.com>, rather than
need to call virFindFileInPath twice, let's just save the path and pass it
along with the compressed type. (NB: the second call would be in virExec as
called from virCommandRunAsync which is called from qemuMigrationToFile
using the argument 'compressor' which up to this point would be the string
from the cfg file that isn't the fully qualified path).
Since we now have the path, we can remove qemuCompressProgramName which
would return NULL or the string representation of the compress type.
There's only one caller and the code is duplicitous just converting the
recently converted cfg image name back into it's string value in order to
get/find the path to the image. A subsequent patch can return this path.
Let's do some more code reuse - there are 3 other callers that care to
check/get the compress program. Each of those though cares whether the
requested cfg image is valid and exists. So, add a parameter to handle
those cases.
NB: We won't need to initialize the returned value in the case where
the cfg image doesn't exist since the called program will handle that.
Add a new parameter 'styleFormat' to be used when printing the
warning message so that it's "clearer" what style of compression
call caused the error. Add that style to both messages as a paremter.
Also a VIR_WARN error message doesn't need to be translated
(e.g. inside _()), so remove the need for the translation.
There's only one caller now anyway... Besides it's just a shell for
getting the compress type. Subsequent patches will return the path
to the compression program.
Split out the guts of getCompressionType to perform the same functionality
in the new helper program with a subsequent patch goal to be reusable for
other callers making similar checks/calls to ensure the compression type
is valid and that the compression program cannot be found.
If passing an empty usbdevice_list to libxl, qemu will always get an
-usb parameter for HVM guests with only non-USB input devices. This
causes qemu to crash when passing pvusb device on HVM guests.
The solution is to allocate the list only when an item to put in it
is found.
Both cpuCompare* APIs are renamed to virCPUCompare*. And they should now
work for any guest CPU definition, i.e., even for host-passthrough
(trivial) and host-model CPUs. The implementation in x86 driver is
enhanced to provide a hint about -noTSX Broadwell and Haswell models
when appropriate.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The function is similar to virCPUDataCheckFeature, but it works directly
on CPU definition rather than requiring it to be transformed into CPU
data first.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The API is supposed to make sure the provided CPU definition does not
use a CPU model which is not supported by the hypervisor (if at all
possible, of course).
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Keeping nfeatures_max set to 0 while nfeatures > 0 and some features are
already stored in features array is just asking for problems once we
want to add a new feature into the array.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The reworked API is now called virCPUUpdate and it should change the
provided CPU definition into a one which can be consumed by the QEMU
command line builder:
- host-passthrough remains unchanged
- host-model is turned into custom CPU with a model and features
copied from host
- custom CPU with minimum match is converted similarly to host-model
- optional features are updated according to host's CPU
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
x86ModelFromCPU is used to provide CPUID data for features matching
@policy. This patch allows callers to set @policy to -1 to get combined
CPUID for all CPU features (including those implicitly provided a CPU
model) specified in CPU def.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The domain capabilities XML is capable of showing whether each guest CPU
mode is supported or not with a possibility to provide additional
details. This patch enhances host-model capability to advertise the
exact CPU model which will be used as a host-model:
<cpu>
...
<mode name='host-model' supported='yes'>
<model fallback='allow'>Broadwell</model>
<vendor>Intel</vendor>
<feature policy='disable' name='aes'/>
<feature policy='require' name='vmx'/>
</mode>
...
</cpu>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The ARM CPU driver wrongly reported host CPU model as "host", which made
host-model to be just an alias for host-passthrough. Let's drop this
insanity.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Host capabilities provide libvirt's view of the host CPU, but for a
useful support for host-model CPUs we really need a hypervisor's view of
the CPU. And since the view can be differ with emulator, qemu
capabilities is the best place to store the host CPU model.
This patch just copies the CPU model from host capabilities, but this
will change in the future.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The function filters all CPU features through a given callback while
copying CPU model related parts of a CPU definition.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The function moves CPU model related parts from one CPU definition to
another. It can be used to avoid unnecessary copies from a temporary CPU
definitions which will be freed anyway.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Useful for copying a CPU definition without model related parts (i.e.,
without model name, feature list, vendor).
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
In case a hypervisor is able to tell us a list of supported CPU models
and whether each CPU models can be used on the current host, we can
propagate this to domain capabilities. This is a better alternative
to calling virConnectCompareCPU for each supported CPU model.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Listing all CPU models supported by QEMU in domain capabilities makes
little sense when libvirt will refuse any model it doesn't know about.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Some CPU drivers (such as arm) do not provide list of CPUs libvirt
supports and just pass any CPU model from domain XML directly to QEMU.
Such driver need to return models == NULL and success from cpuGetModels.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
qemu_command.c should deal with translating our domain definition into a
QEMU command line and nothing else.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The list of supported CPU models in domain capabilities is stored in
virDomainCapsCPUModels. Let's use the same object for storing CPU models
in QEMU capabilities.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The patch adds <cpu> element to domain capabilities XML:
<cpu>
<mode name='host-passthrough' supported='yes'/>
<mode name='host-model' supported='yes'/>
<mode name='custom' supported='yes'>
<model>Broadwell</model>
<model>Broadwell-noTSX</model>
...
</mode>
</cpu>
Applications can use it to inspect what CPU configuration modes are
supported for a specific combination of domain type, emulator binary,
guest architecture and machine type.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Our internal APIs mostly use virArch rather than strings. Switching
cpuGetModels to virArch will save us from unnecessary conversions in the
future.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
By default, virt-manager (and likely other libvirt-based apps) sets
the VIR_MIGRATE_PERSIST_DEST flag when invoking the migrate API, which
fails in a Xen setup since the libxl driver does not support the flag.
Persisting a domain is a trivial task in the grand scheme of migration,
so be nice to libvirt apps and add support for VIR_MIGRATE_PERSIST_DEST
in the libxl driver.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
We have a few of senarios that libvirtd would invoke qemuProcessStop
and leave a "shutting down" in /var/log/libvirt/qemu/$DOMAIN.log.
The shutoff reason showing in debug log is also very important
for us to know why VM shutting down in domain log,
as we seldom enable debug log of libvirtd.
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1322717
During offline migration, no storage is copied. Nor disks, nor
NVRAM file, nor anything. We use qemu for that and because domain
is not running there's nobody to copy that for us.
We should document this to avoid confusing users.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Calling virDomainGetEmulatorPinInfo on a live VM with automatic NUMA
pinning and VIR_DOMAIN_AFFECT_CONFIG would return the automatic pinning
data in some cases which is bogus. Use the autoCpuset property only when
called on a live definition.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1365779
Calling virDomainGetVcpuPinInfo on a live VM with automatic NUMA pinning
and VIR_DOMAIN_AFFECT_CONFIG would return the automatic pinning data
in some cases which is bogus. Use the autoCpuset property only when
called on a live definition.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1365779
Sometimes adding a separate variable to access vm->privateData is not
necessary. Add a macro that will do the typecasting rather than having
to add a temp variable to force the compiler to typecast it.
Old libvirt represents
<graphics type='spice'>
<listen type='none'/>
</graphics>
as
<graphics type='spice' autoport='no'/>
In this mode, QEMU doesn't listen for SPICE connection anywhere and
clients have to use virDomainOpenGraphics* APIs to attach to the domain.
That is, the client has to run on the same host where the domains runs
and it's impossible to tell the client to reconnect to the destination
QEMU during migration (unless there is some kind of proxy on the host).
While current libvirt correctly ignores such graphics devices when
creating graphics migration cookie, old libvirt just sends
<graphics type='spice' port='0' listen='0.0.0.0' tlsPort='-1'/>
in the cookie. After seeing this cookie, we happily would call
client_migrate_info QMP command and wait for SPICE_MIGRATE_COMPLETED
event, which is quite pointless since the doesn't know where to connecti
anyway. We should just ignore such cookies.
https://bugzilla.redhat.com/show_bug.cgi?id=1376083
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Checking if a domain's definition or if it is active before we got a job
is pointless since the domain might have changed in the meantime.
Luckily libvirtd didn't crash when the API tried to talk to an inactive
domain:
debug : qemuDomainObjBeginJobInternal:2914 : Started job: modify
(async=none vm=0x7f8f340140c0 name=ble)
debug : qemuDomainObjEnterMonitorInternal:3137 : Entering monitor
(mon=(nil) vm=0x7f8f340140c0 name=ble)
warning : virObjectLock:319 : Object (nil) ((unknown)) is not a
virObjectLockable instance
debug : qemuMonitorOpenGraphics:3505 : protocol=spice fd=27
fdname=graphicsfd skipauth=1
error : qemuMonitorOpenGraphics:3508 : invalid argument: monitor must
not be NULL
debug : qemuDomainObjExitMonitorInternal:3160 : Exited monitor
(mon=(nil) vm=0x7f8f340140c0 name=ble)
debug : qemuDomainObjEndJob:3068 : Stopping job: modify (async=none
vm=0x7f8f340140c0 name=ble)
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
We can receive NULL as sync reply in two situations. First
is garbage sync reply and this situation is handled by
resending sync message. Second is different cases
of rebooting guest, destroing domain etc and we can
give more meaningful error message. Actually we have
this error message in qemuAgentCommand already which checks
for the same sitatuion. AFAIK case with mon->running
is just to be safe on adding some future(?) cases of
returning NULL reply.
We can easily handle receiving garbage on sync. We don't
have to make client deal with this situation. We just
need to resend sync command but this time garbage is
not be possible.
When we wait for sync reply we can receive delayed
reply to syncs or commands that were sent erlier. We can
safely skip them until we receive sync reply with correct id.
There is no much sense report this situation to client.
Actually with a bit of "luck" if we involve client into
this the play can go on forever: send sync 0, receive
sync reply -1, send sync 1, receive reply 0 ...
After sync is sent we can receive garbare and this is not error.
Consider next regular case:
1. libvirtd sends sync
2. qga sends partial sync reply and die
3. libvirtd sends sync
4. qga sends sync reply
5. libvirtd receives garbage
(half of first reply and second reply together)
We should handle this situation as it is recoverable.
Next sync can succeed. Let's report reply is NULL,
it will be converted to the VIR_ERR_AGENT_UNSYNCED
which signals client to retry.
Errors in qemuAgentIOProcessLine stop agent IO processing just
like any regular IO error, however some of current errors
that this functions spawns are false positives. Consider
next case for example:
1. send sync (unsynced state)
2. receive sync reply (sync established)
3. command send, but timeout occured (unsynced state)
4. receive command reply
Last IO triggers error because current code ignores
only delayed syncs when unsynced
We should not treat any delayed reply as error in unsynced
state. Until client and qga are not in sync delayed reply to any
command is possible. msg == NULL is the exact criterion
that we are not in sync.
Put it into qemuDomainPrepareShmemChardev() so it can be used later.
Also don't fill in the path unless the server option is enabled.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Some checks will need to be performed for newer device types as well, so
let's not duplicate them.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Commit 839a060 tied the lifecycle of virtlogd more
closely to that of libvirtd. Unfortunately, while starting
virtlogd when libvirtd is started is definitely a good idea,
restarting virtlogd or shutting it down at any time outside
of system poweroff is not.
Revert part of that commit by removing the PartOf= lines,
meaning that only startup requests will be propagated from
libvirtd to virtlogd.
Resolves: https://bugzilla.redhat.com/1372576
Now that we have two same implementations for getting path for
huge pages backed guest memory, lets merge them into one function.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
When trying to migrate a huge page enabled guest, I've noticed
the following crash. Apparently, if no specific hugepages are
requested:
<memoryBacking>
<hugepages/>
</memoryBacking>
and there are no hugepages configured on the destination, we try
to dereference a NULL pointer.
Program received signal SIGSEGV, Segmentation fault.
0x00007fcc907fb20e in qemuGetHugepagePath (hugepage=0x0) at qemu/qemu_conf.c:1447
1447 if (virAsprintf(&ret, "%s/libvirt/qemu", hugepage->mnt_dir) < 0)
(gdb) bt
#0 0x00007fcc907fb20e in qemuGetHugepagePath (hugepage=0x0) at qemu/qemu_conf.c:1447
#1 0x00007fcc907fb2f5 in qemuGetDefaultHugepath (hugetlbfs=0x0, nhugetlbfs=0) at qemu/qemu_conf.c:1466
#2 0x00007fcc907b4afa in qemuBuildMemoryBackendStr (size=4194304, pagesize=0, guestNode=0, userNodeset=0x0, autoNodeset=0x0, def=0x7fcc70019070, qemuCaps=0x7fcc70004000, cfg=0x7fcc5c011800, backendType=0x7fcc95087228, backendProps=0x7fcc95087218,
force=false) at qemu/qemu_command.c:3297
#3 0x00007fcc907b4f91 in qemuBuildMemoryCellBackendStr (def=0x7fcc70019070, qemuCaps=0x7fcc70004000, cfg=0x7fcc5c011800, cell=0, auto_nodeset=0x0, backendStr=0x7fcc70020360) at qemu/qemu_command.c:3413
#4 0x00007fcc907c0406 in qemuBuildNumaArgStr (cfg=0x7fcc5c011800, def=0x7fcc70019070, cmd=0x7fcc700040c0, qemuCaps=0x7fcc70004000, auto_nodeset=0x0) at qemu/qemu_command.c:7470
#5 0x00007fcc907c5fdf in qemuBuildCommandLine (driver=0x7fcc5c07b8a0, logManager=0x7fcc70003c00, def=0x7fcc70019070, monitor_chr=0x7fcc70004bb0, monitor_json=true, qemuCaps=0x7fcc70004000, migrateURI=0x7fcc700199c0 "defer", snapshot=0x0,
vmop=VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START, standalone=false, enableFips=false, nodeset=0x0, nnicindexes=0x7fcc95087498, nicindexes=0x7fcc950874a0, domainLibDir=0x7fcc700047c0 "/var/lib/libvirt/qemu/domain-1-fedora") at qemu/qemu_command.c:9547
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Both qemu monitor and agent print the same
log on HUANGUP event, which would be confusing
when reading libvirtd log.
This patch will give a different log message to them.
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Most of QEMU's PCI display device models, such as:
libvirt video/model/@type QEMU -device
------------------------- ------------
cirrus cirrus-vga
vga VGA
qxl qxl-vga
virtio virtio-vga
come with a linear framebuffer (sometimes called "VGA compatibility
framebuffer"). This linear framebuffer lives in one of the PCI device's
MMIO BARs, and allows guest code (primarily: firmware drivers, and
non-accelerated OS drivers) to display graphics with direct memory access.
Due to architectural reasons on aarch64/KVM hosts, this kind of
framebuffer doesn't / can't work in
qemu-system-(arm|aarch64) -M virt
machines. Cache coherency issues guarantee a corrupted / unusable display.
The problem has been researched by several people, including kvm-arm
maintainers, and it's been decided that the best way (practically the only
way) to have boot time graphics for such guests is to consolidate on
QEMU's "virtio-gpu-pci" device.
>From <https://bugzilla.redhat.com/show_bug.cgi?id=1195176>, libvirt
supports
<devices>
<video>
<model type='virtio'/>
</video>
</devices>
but libvirt unconditionally maps @type='virtio' to QEMU's "virtio-vga"
device model. (See the qemuBuildDeviceVideoStr() function and the
"qemuDeviceVideo" enum impl.)
According to the above, this is not right for the "virt" machine type; the
qemu-system-(arm|aarch64) binaries don't even recognize the "virtio-vga"
device model (justifiedly). Whereas "virtio-gpu-pci", which is a pure
virtio device without a compatibility framebuffer, is available, and works
fine.
(The ArmVirtQemu ("AAVMF") platform of edk2 -- that is, the UEFI firmware
for "virt" -- supports "virtio-gpu-pci", as of upstream commit
3ef3209d3028. See
<https://tianocore.acgmultimedia.com/show_bug.cgi?id=66>.)
Override the default mapping of "virtio", from "virtio-vga" to
"virtio-gpu-pci", if qemuDomainMachineIsVirt() evaluates to true.
Cc: Andrea Bolognani <abologna@redhat.com>
Cc: Drew Jones <drjones@redhat.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Martin Kletzander <mkletzan@redhat.com>
Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1372901
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Acked-by: Martin Kletzander <mkletzan@redhat.com>
There is nothing Linux-specific in that function. Also since commit
8c3b5bf481 mingw build is broken due to
the fact that this function is not compiled in the library.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
