Now we have everything prepared so that @model doesn't have to be
rewritten. The correct model can be chosen right from the
beginning.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
We want to call qemuBuildVirtioDevStr() from
qemuBuildDeviceVideoStr() but only for some models (currently
"virtio-gpu" and "vhost-user-gpu"), not all of them. Move this
logic into qemuDeviceVideoGetModel() because this logic will be
refined.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This may look like a step backwards, but it isn't. The point is
that in near future the chosen model will depend on more than
just video type.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This may look like a step backwards, but it isn't. The point is
that in near future the chosen model will depend on more than
just video type.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
There is the same check written twice (whether given video card
is primary one and whether it supports VGA mode). Write it just
once and store it in a boolean variable.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The code that decides video card model is going to be reworked
and expanded. Separate it out into a function.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This function doesn't modify passed video definition. Make the
argument const.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
QEMU 6.1 will replace the virgl property of virtio-vga device to
virtio-vga-gl device. Adapt to that update.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/167
Signed-off-by: Han Han <hhan@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
QEMU 6.1 will add virtio-gpu-gl-pci device to replace the virgl property
of virtio-gpu-pci device. Adapt to that change.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1967356
Signed-off-by: Han Han <hhan@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The devices virtio-gpu-gl-pci and virtio-vga-gl, aimed to replace the
virgl property, are valid for 3d accerlation as well.
Signed-off-by: Han Han <hhan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This flag will be used for the device virtio-gpu-gl-pci which is introduced
since QEMU 6.1.
Signed-off-by: Han Han <hhan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
"avx-vvni" was introduced to qemu in commit
c1826ea6a052084f2e6a0bae9dd5932a727df039, adding it Cooperlake.
This feature is currently not used by any libvirt CPU models, but its
addition silences a warning from sync_qemu_i386.py:
```
warning: Unknown feature 'CPUID_7_1_EAX_AVX_VNNI'
warning: Feature unknown to libvirt: CPUID_7_1_EAX_AVX_VNNI
```
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Connecting a tap device to an Open vSwitch is done by adding a "port"
to the switch with the ovs-vsctl "add-port" command. The port will
have the same name as the tap device, but it is a separate entity, and
can survive beyond the destruction of the tap device (although under
normal circumstances the port will be deleted around the same time the
tap device is deleted).
This makes it possible for a port of a particular name to already
exist at the time libvirt calls ovs-vsctl to add that port. The
original commit of Open vSwitch support (commit df81004632, libvirt
0.9.10, Feb. 2012) used the "--may-exist" option to the add-port
command to indicate that a port of the desired name might already
exist, and that it was okay to simply re-use this port (rather than
failing with an error message).
Then in commit 33445ce844 (libvirt 1.2.7, April 2014) the command
was changed to use "--if-exists del-port blah" instead of
"--may-exist". The reason given was that there was a bug in OVS where
a stale port would be unusable even though it still existed; the
workaround was to forcibly delete any existing port prior to adding
the new port (of the same name). This is the ovs-vsctl command still
in use by libvirt today.
It recently came up in the discussion of a bug concerning guest packet
loss during OpenStack upgrades (https://bugzilla.redhat.com/1963164)
that the bug in OVS that necessitated the del-port workaround was
fixed quite a long time ago (August 2015):
e21c6643a0
thus rendering the workaround in libvirt unnecessary. The assertion in
that discussion is that this workaround is now the cause of the packet
loss being experienced during OpenStack upgrades. I'm not convinced
this is the case, but it does appear that there is no reason to carry
this workaround in libvirt any longer, so this patch reverts the code
back to the original behavior (using "--may-exist" instead of
"--if-exists del-port").
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
I've identified some places (mostly by looking for
virBufferUse()) that can use virXMLFormatElement() instead of
open coded version of it. I'm sure there are many more places
that could use the same treatment. Let's cure them some other
time.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The semicolon in question makes the pipeline fail over a style checker
complaint.
Introduced-in: 360b8eb2d2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This was introduced in qemu commit
e11fd68996fb27c040552320f01a7d30a15a7cc1.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This member is unused (apart from only being set in
virCHDriverConfigNew()), and never freed really (leading to a
memleak).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
If the chStateInitialize method fails, we call chStateCleanup
which free's all global state. It fails to set the global
'ch_driver' to NULL, however, so a later attempt to open the
cloud hypervisor driver will succeed and then crash attempting
to access freed memory.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The iscsi-direct storage pool backend works merely like this: a
connection is established to the target (usually done via
virStorageBackendISCSIDirectSetConnection()), intended action is
executed (e.g. reporting LUNs, volume wiping), and at the end the
connection is closed via virISCSIDirectDisconnect().
The problem is that virISCSIDirectDisconnect() reports its own
errors which may overwrite error that occurred during LUN
reporting, or volume wiping or whatever.
To fix this, use virErrorPreserveLast() + virErrorRestore()
combo, which either preserves previously reported error message,
or is NOP if there's no error reported.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1797879
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Xen only supports one firmware, making autoselection easy to implement.
In fact, <os firmware='efi'> is probably preferable in the Xen driver,
where libxl supports a firmware setting with accepted values such as
bios, ovmf, uefi (currently same semantics as ovmf), seabios, etc.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Xen+ovmf does not support secure boot. Fail domain def validation
if secure boot is enabled.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Introduce libxlDomainDefValidate and move the existing validation
check from libxlDomainDefPostParse. Additional validation will be
introduced in subsequent patches.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The audit log contains the following denials from libvirtd
apparmor="DENIED" operation="capable" profile="libvirtd" pid=6012 comm="daemon-init" capability=17 capname="sys_rawio"
apparmor="DENIED" operation="capable" profile="libvirtd" pid=6012 comm="rpc-worker" capability=39 capname="bpf"
apparmor="DENIED" operation="capable" profile="libvirtd" pid=6012 comm="rpc-worker" capability=38 capname="perfmon"
Squelch the denials and allow the capabilities in the libvirtd
apparmor profile.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In fcdcf8f70c the remoteGetUNIXSocket() function was changed and
one new variable was introduced (among other things): @env_name.
However, for WIN32 case the variable changed name to @env_path
which builds mingw builds.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The virNetSocketNewConnectUNIX() function was changed in
48f66cfe3e. And its WIN32 version (which just reports an error)
was updated too, but this new argument @spawnDaemonPath was not
marked as unused.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
We want to use those shared drivers provided by libvirt to avoid
implementing our own.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Wei Liu <liuwe@microsoft.com>
After previous patches, the @ret variable and the 'cleanup'
label are redundant. Remove them.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
There are two variables that can be freed automatically: @cmd
(which allows us to drop explicit virCommandFree() call at the
end of the function) and @help which was never freed (and thus
leaked).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The CH driver needs "cloud-hypervisor" binary. And if none was
found then the initialization of the driver fails as
chStateInitialize() returns VIR_DRV_STATE_INIT_ERROR. This in
turn means that whole daemon fails to initialize. Let's return
VIR_DRV_STATE_INIT_SKIPPED in this particular case, which
disables the CH drvier but lets the daemon run.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
After previous patches, there's not much value in
chExtractVersion(). Rename chExtractVersionInfo() to
chExtractVersion() and have it use virCHDriver directly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The only caller, chExtractVersion() passes not NULL. Therefore,
it's redundant to check for NULL.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
If chExtractVersionInfo() fails, in some cases it reports error
and in some it doesn't. Fix those places and drop reporting error
from chExtractVersion() which would just overwrite more specific
error.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
When the default driver mode requests the modular daemons, we still
defaulted to spawning libvirtd if the URI was NULL, because we don't
know which driver specific daemon to spawn. virtproxyd has logic
that can handle this as it is used for compatibility when accepting
incoming TCP connections with a NULL URI.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The "spawnDaemon" and "binary" parameters are co-dependant, with the
latter non-NULL, if-and-only-if the former is true. Getting rid of the
"spawnDaemon" parameter simplifies life for the callers and eliminates
an error checking scenario.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When deciding what socket to connect to, we build the daemon path
that we need to autostart. This path only needs to be populated
if we actually intend to use autostart.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The remoteGetUNIXSocket method currently just returns the daemon name
and the caller then converts this to a path. Except the SSH helper
didn't do this, so it was relying on later code expanding $PATH, and
this doesn't allow for build root overrides.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We have helper methods that return boolans for ro/user/autostart
properties. We then pack them into a flags parameter, and later
unpack them again. This makes the code consistently use flags
throughout.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This enum will shortly be used by the remote driver sockets helper
methods too.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The g_build_filename() would decide which separator
to use instead of hardcoding in g_strdup_printf().
Related issue: https://gitlab.com/libvirt/libvirt/-/issues/12
Signed-off-by: Luke Yue <lukedyue@gmail.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Cloud-Hypervisor is a KVM virtualization using hypervisor. It
functions similarly to qemu and the libvirt Cloud-Hypervisor driver
uses a very similar structure to the libvirt driver.
The biggest difference from the libvirt perspective is that the
"monitor" socket is seperated into two sockets one that commands are
issued to and one that events are notified from. The current
implementation only uses the command socket (running over a REST API
with json encoded data) with future changes to add support for the
event socket (to better handle shutdowns from inside the VM).
This patch adds support for the following initial VM actions using the
Cloud-Hypervsior API:
* vm.create
* vm.delete
* vm.boot
* vm.shutdown
* vm.reboot
* vm.pause
* vm.resume
To use the Cloud-Hypervisor driver, the v15.0 release of
Cloud-Hypervisor is required to be installed.
Some additional notes:
* The curl handle is persistent but not useful to detect ch process
shutdown/crash (a future patch will address this shortcoming)
* On a 64-bit host Cloud-Hypervisor needs to support PVH and so can
emulate 32-bit mode but it isn't fully tested (a 64-bit kernel and
32-bit userspace is fine, a 32-bit kernel isn't validated)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: William Douglas <william.douglas@intel.com>
When processing node devices, the udevProcessStorage() will be
called if the device is some form of storage. In here, ID_TYPE
attribute is queried and depending on its value one of more
specialized helper functions is called. For instance, for
ID_TYPE=="cd" the udevProcessCDROM() is called, for
ID_TYPE=="disk" the udevProcessDisk() is called, and so on.
But there's a problem with ID_TYPE and its values. Coming from
udev, we are not guaranteed that ID_TYPE will contain "cd" for
CDROM devices. In fact, there's a rule installed by sg3_utils
that will overwrite ID_TYPE to "cd/dvd" leaving us with an
unhandled type. Fortunately, this was fixed in their upstream,
but there are still versions out there, on OS platforms that we
aim to support that contain the problematic rule. Therefore, we
should accept both strings.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1848875
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Let's use a different variable for storing retvals of helper
functions. This way the usual function pattern can be restored.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This function can't fail really as it's returning 0 no matter
what. This is probably a residue from old days when we cared
about propagating OOM errors. Now we just abort. Make its return
type void then.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This function can't fail really as it's returning 0 no matter
what. This is probably a residue from old days when we cared
about propagating OOM errors. Now we just abort. Make its return
type void then.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
libxl objects are supposed to be initialized and disposed. Adjust
libxlMakeNic to use an already initialized object owned by the caller.
Adjust libxlMakeNicList to initialize the list of objects, before they
are filled by libxlMakeNic. The libxl_domain_config object passed to
libxlMakeNicList is owned by the caller and will be disposed with
libxl_domain_config_dispose, which also disposes embedded objects such
as libxl_device_nic.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Acked-by: Olaf Hering <olaf@aepfle.de>
Before the mentioned commit we always parsed the whole disk definition
for qemuDomainBlockCopy API but we only used the @src part. Based on
that assumption the code was changed to parse only the disk <source>
element.
Unfortunately that is not correct as we need to parse some parts of
<driver> element as well.
Fixes: 0202467c4b
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Attribute `type` and sub-element `metadata_cache` are internally stored
in the `virStorageSource` structure. Sometimes we only care about the
disk source bits so we need a dedicated helper for that.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
libacl is Linux-only, so we don't need to explicitly check for
either the target platform or header availability, and we can
simply rely on cc.find_library() instead. The corresponding
preprocessor define is renamed to more accurately reflect the
nature of the check.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
QEMU_DOMAIN_DISK_PRIVATE(disk)->transientOverlayCreated flag
gets true unexpectedly on qemuProcessSetupDisksTransientSnapshot() when
the disk has <transient shareBacking='yes'> option.
The flag should be enabled on qemuDomainAttachDiskGeneric() after the
overlay setup is completed.
Skip enabling transientOverlayCreated for the disk here.
Fixes: 75871da0ec
Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
We supported autostart of node devices via an xml element, but this
is not consistent with other libvirt objects which use an explicit API
for setting autostart status. So revert this and implement it as an
official API in a future commit.
The initial support was refactored after merging, so this commit reverts
both of those previous commits.
Revert "virNodeDevCapMdevParseXML: Use virXMLPropEnum() for ./start/@type"
This reverts commit 9d4cd1d1cd.
Revert "nodedev: support auto-start property for mdevs"
This reverts commit 42a5585499.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
While we couldn't historically connect to the remote session daemon
automatically, we do allow the user to set an explicit socket path
to enable the connections to work. This ability was accidentally
lost in
commit f8ec7c842d
Author: Daniel P. Berrangé <berrange@redhat.com>
Date: Wed Jul 8 17:03:38 2020 +0100
rpc: use new virt-ssh-helper binary for remote tunnelling
We need to force use of 'netcat' when a 'socket' path is given in
the URI parameters.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Currently the defaults for the proxy/mode settings are set before
parsing URI parameters. A following commit will introduce a dependancy
on the URI parsing for the defaults, so they need to move.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
In one of my recent commits I've done some renaming. But whilst
doing so I also mistakenly replaced 'goto cleanup' with 'return
-1' in virCapabilitiesHostNUMAInitReal() which was incorrect.
Fixes: fe25224fda
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The remoteGetUNIXSocketHelper method always returns a non-NULL string.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
virFileFindResource needs to be given the absolute build path otherwise
its results will vary according to the CWD, leading to spurious failures
in dev testing.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When <transient shareBacking='yes'> is set to a disk and the overlay
disk already exists because of something abnormal, libvirt is terminated
by Segmentation fault.
# virsh start Test0
error: Disconnected from qemu:///system due to end of file
error: Failed to start domain 'Test0'
error: End of file while reading data: Input/output error
Add NULL check for snapdiskdef so that the rollback can work correctly.
Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Fixes: 2e94002d2a
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
and re-adjust if the hotplug fails.
This fixes a bug found during testing of
https://bugzilla.redhat.com/1939776, which was supposed to be resolved
by commit 98e22ff749, but failed to account for the case of device
hotplug.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
An upcoming patch will be checking if the addition of a new net device
requires adjusting the domain locked memory limit, which must be done
prior to sending the command to qemu to add the new device. But
qemuDomainAdjustMaxMemLock() checks all (and only) the devices that
are currently in the domain definition, and currently we are adding
new net devices to the domain definition only at the very end of the
hotplug operation, after qemu has already executed the device_add
command.
In order for the upcoming patch to work, this patch changes
qemuDomainAttachNetDevice() to add the device to the domain nets list
at an earlier time. It can't be added until after PCI address and
alias name have been determined (because both of those examine
existing devices in the domain to figure out a unique value for the
new device), but must be done before making the qemu monitor call.
Since the device has been added to the list earlier, we need to
potentially remove it on failure. This is done by replacing the
existing call to virDomainNetRemoveHostdev() (which checks if this is
a hostdev net device, and if so removes it from the hostdevs list,
since it could have already been added to that list) with a call to
the new virDomainNetRemoveByObj(), which looks for the device on both
nets and hostdevs lists, and removes it where it finds it.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virDomainNetRemove() requires the index of the net device you want to
remove from the list, but in some cases you may not have the index
handy, only the object itself (or the object may not have been added
to the domain's list). virDomainNetRemoveByObj() first tries to find
the given object in the nets list, and deletes that if it is found.
As with virDomainNetRemove() it always unconditionally tries to remove
the device from the hostdevs list (in case it is the ridiculous
combined net+hostdev device created for <interface type='hostdev'>).
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We have many places where the earliest error returns from a function
skip any cleanup label at the bottom (the assumption being that it is
so early in the function that there isn't yet anything that needs to
be explicitly undone on failure). But in general it is a bad sign if
there are any direct "return" statements in a function at any time
after there has been a "goto cleanup" - that indicates someone thought
that an earlier point in the code had done something needing cleanup,
so we shouldn't be skipping it.
There were two occurences of a "return -1" after "goto cleanup" in
qemuDomainAttachDeviceNet(). The first of these has been around for a
very long time (since 2013) and my assumption is that the earlier
"goto cleanup" didn't exist at that time (so it was proper), and when
the code further up in the function was added, the this return -1 was
missed. The second was added during a mass change to check the return
from qemuInterfacePrepareSlirp() in several places (commit
99a1cfc438); in this case it was erroneous from the start.
Change both of these "return -1"s to "goto cleanup". Since we already
have code paths earlier in the function that goto cleanup, this should
not cause any new problem.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There was a recent change in libxml2 that caused a trouble for
us. To us, <metadata/> in domain or network XMLs are just opaque
value where management application can store whatever data it
finds fit. At XML parser/formatter level, we just make a copy of
the element during parsing and then format it back. For
formatting we use xmlNodeDump() which allows caller to specify
level of indentation. Previously, the indentation was not
applied onto the very first line, but as of v2.9.12-2-g85b1792e
libxml2 is applying indentation also on the first line.
This does not work well with out virBuffer because as soon as we
call virBufferAsprintf() to append <metadata/> element,
virBufferAsprintf() will apply another level of indentation.
Instead of version checking, let's skip any indentation added by
libxml2 before virBufferAsprintf() is called.
Note, the problem is only when telling xmlNodeDump() to use
indentation, i.e. level argument is not zero. Therefore,
virXMLNodeToString() which also calls xmlNodeDump() is safe as it
passes zero.
Tested-by: Bjoern Walk <bwalk@linux.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
I guess this is more of an academic problem, because if
<metadata/> content was problematic we would have caught the
error during parsing. Anyway, as is this function returns -1
without any error reported. Fix it by reporting one.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
So far, we have to places where we format <metadata/> into XMLs:
domain and network. Bot places share the same code. Move it into
a helper function and just call it from those places.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
There's an if-else statement in libxlCapsInitNuma() that can
really be just two standalone if()-s. Writing it as such helps
with code readability.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Implement this behaviour by skipping the disks on traditional
commandline and hotplug them before resuming CPUs. That allows to use
the support for hotplugging of transient disks which inherently allows
sharing of the backing image as we open it read-only.
This commit implements the validation code to allow it only with buses
supporting hotplug and the hotplug code while starting up the VM.
When we have such disk we need to issue a system-reset so that firmware
tables are regenerated to allow booting from such device.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
In case the user wants to share the disk image between multiple VMs the
qemu driver needs to hotplug such disks to instantiate the backends.
Since that doesn't work for all disk configs add a switch to force this
behaviour.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The qemuDomainAttachDiskGeneric will also be used on startup for
transient disks which share the overlay. The VM startup code passes the
asyncJob around so we need to pass it into qemuDomainAttachDiskGeneric.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Add code which creates the transient overlay after hotplugging the disk
backend before attaching the disk frontend.
The state of the topmost image is modified to be already read-only to
prevent the need to open the image in read-write mode.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
In preparation for hotplug of <transient> disks we'll need to track
whether the overlay file was created individually per-disk.
Add 'transientOverlayCreated' to 'struct _qemuDomainDiskPrivate' and
remove 'inhibitDiskTransientDelete' from 'qemuDomainObjPrivate' and
adjust the code for the change.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Split up the monitor contexts to attach the backend of the disk and the
frontend device in preparation for hotplugging transient disks where
we'll need to add the code for adding the transient overlay between
these two steps.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Modify the rollback section to use its own monitor context so that we
can later split up the hotplug into multiple steps and move the
detachment of the extension device into the rollback section rather than
doing it inline.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Similarly to previous refactors we want to move all hotplug related
setup which isn't strictly relevant to attaching the disk into
qemuDomainAttachDeviceDiskLiveInternal.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Remove the 'ret' variable and 'cleanup' label in favor of directly
returning the value since we don't have anything under the 'cleanup:'
label.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Remove two empty lines.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Move the auditing entry and insertion into the disk definition from the
function which deals with qemu to 'qemuDomainAttachDeviceDiskLiveInternal'
which deals with the hotplug related specifics.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
qemuDomainAttachDeviceDiskLiveInternal already sets up certain pieces of
the disk definition so it's better suited to move the setup of the
virStorageSource structs, granting access to the storage and allocation
of the alias from qemuDomainAttachDiskGeneric which will be just
handling the qemu interaction.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
We can call it in one place as all per-device-type subcases use the same
code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Move the validation of the SCSI device address and the attachment of the
controller into qemuDomainAttachDeviceDiskLiveInternal as there's no
specific need for a special helper.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Move the specific device setup and address reservation code into the
main hotplug helper as it's just one extra function call.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Move the specific device setup and address reservation code into the
main hotplug helper as it's just one extra function call.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Unify the handling of the copy-on-read filter by changing the handling
to use qemuBlockStorageSourceChainData.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Fill in the required fields in qemuBlockStorageSourceChainData to handle
the hotplug so that we can simplify the cleanup code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
qemuBlockStorageSourceChainData encapsulates the backend of the disk for
startup and hotplug operations. Add the handling for the copy-on-read
filter so that the hotplug code doesn't need to have separate cleanup.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Replace the last use of the function by virDomainDiskInsert and remove
the unused helper.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Pre-extending the disk array size is pointless nowadays since we've
switched to memory APIs which don't return failure.
Switch all uses of reallocation of the array followed by
'virDomainDiskInsertPreAlloced' with direct virDomainDiskInsert.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The "machine-loadparm-multiple-disks-nets-s390" case now requires the
QEMU_CAPS_CCW feature to pass validation.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
We can skip the formatting of the bootindex for floppies directly at the
place where it's being formatted.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The logic assigning the bootindices from the legacy boot order
configuration was spread through the command line formatters for the
disk device and for the floppy controller.
This patch adds 'effectiveBootindex' property to the disk private data
which holds the calculated boot index and moves the logic of determining
the boot index into 'qemuProcessPrepareDomainDiskBootorder' called from
'qemuProcessPrepareDomainStorage'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Later patches will implement sharing of the backing file, so we'll need
to be able to discriminate the overlays per VM.
Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The code deals with the startup of the VM and just uses the snapshot
code to achieve the desired outcome.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
We store the virQEMUDriverConfig object in the context.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Remove all the arguments which are present in qemuSnapshotDiskContext.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The config is used both with the preparation and execution functions, so
we can store it in the context to simplify other helpers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Rather than filling various parts of the context from arguments pass in
the whole context.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Creating the overlay for the disk is needed when starting a new VM only.
Additionally for now migration with transient disks is forbidden
anyways.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The code will be later reused when adding support for sharing the
backing image of the snapshot.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
None of them are currently needed to pass our upstream CI, most were
either for ancient clang versions or coverity for silencing false
positives.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
They were added mostly randomly and we don't really want to keep working
around of false positives.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
After previous patches we have two structures:
virCapsHostNUMACellDistance and virNumaDistance which express the
same thing. And have the exact same members (modulo their names).
Drop the former in favor of the latter.
This change means that distances with value of 0 are no longer
printed out into capabilities XML, because domain XML code allows
partial distance specification and thus threats value of 0 as
unspecified by user (see virDomainNumaGetNodeDistance() which
returns the default LOCAL/REMOTE distance for value of 0).
Also, from ACPI 6.1 specification, section 5.2.17 System Locality
Distance Information Table (SLIT):
Distance values of 0-9 are reserved and have no meaning.
Thus we shouldn't be ever reporting 0 in neither domain nor
capabilities XML.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Expose virNumaDistance XML formatter so that it can be re-used by
other parts of the code.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
There's nothing domain specific about NUMA distances. Rename the
virDomainNumaDistance structure to just virNumaDistance.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The virCapsHostNUMACellSiblingInfo structure really represents
distance to other NUMA node. Rename the structure and variables
of that type to make it more obvious.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This reverts commit <1b22dd6dd44202094e0f78f887cbe790c00e9ebc>.
First of all, the reverted commit is incomplete. It only sets
cpuset.mems in the VM root cgroup when the API is used but there is no
code that would do the same when the VM is started.
Libvirt never places any process into the VM root cgroup directly. All
the supporting processes like slirp-helper or dbus-daemon are placed
into the emulator sub-cgroup and all the QEMU threads are distributed
between emulator, vcpu* and iothread* sub-cgroups. The scenario
described in the reverted commit can happen only if someone manually
adds any process there which we should not care about.
If we would like to set the limit in the VM root cgroup we need to
introduce better logic:
- set both (old and new) numa group in the VM root cgroup
- change the numa group in all sub-cgroups to new value
- finally set only the new value in the VM root cgroup
The simplest fix now is to revert the commit.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The used libxl_domain_build_info, which is contained in
libxl_domain_config, is owned and already initialized by the caller.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
The passed libxl_domain_create_info is owned, and already initialized,
by the caller.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
libxl objects are supposed to be initialized and disposed.
Correct the usage of libxl_device_disk objects which are allocated on
the stack. Initialize each one prior usage, and dispose them once done.
Adjust libxlMakeDisk to use an already initialized object, it is owned
by the caller.
Adjust libxlMakeDiskList to initialize the list of objects, before they
are filled by libxlMakeDisk. In case of error, the objects are disposed
by libxl_domain_config_dispose.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
The description of the function says that the return value is a
file descriptor on success and negative errno on failure which is
not true. If the 'if' case with check on security labels fails,
the return value is -1 not -errno. The solution is to return
'-EINVAL' instead.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Previously, nvram file was created with user/group owner as
'root', rather than specifications defined in libvirtd.conf. The
solution is to call qemuDomainOpenFile(), which creates file with
defined permissions and qemuSecurityDomainSetPathLabel() to set
security label for created nvram file.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1783255
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
From QEMU docs/interop/qcow2.txt :
Byte 20 - 23: cluster_bits
Number of bits that are used for addressing an offset
within a cluster (1 << cluster_bits is the cluster size).
With this patch libvirt will be able to report the current cluster_size
for all existing storage volumes managed by storage driver.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The default value hard-coded in QEMU (64KiB) is not always the ideal.
Having a possibility to set the cluster_size by user may in specific
use-cases improve performance for QCOW2 images.
QEMU internally has some limits, the value has to be between 512B and
2048KiB and must by power of two, except when the image has Extended L2
Entries the minimal value has to be 16KiB.
Since qemu-img ensures the value is correct and the limit is not always
the same libvirt will not duplicate any of these checks as the error
message from qemu-img is good enough:
Cluster size must be a power of two between 512 and 2048k
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/154
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When building a commandline for a DIMM memory device with
non-default access mode, the qemuBuildMemoryBackendProps() will
tell QEMU to allocate memory from per-domain memory backing dir.
But later, when preparing the host, the
qemuProcessNeedMemoryBackingPath() does not check for memory
devices at all resulting in per-domain memory backing dir not
being created which upsets QEMU.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1961114
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We do not need to look for a suitable binary in the vhost-user
description files, if we aren't the ones starting it.
Otherwise startup will fail with:
error: Failed to start domain 'vm1'
error: operation failed: Unable to find a satisfying virtiofsd
https://bugzilla.redhat.com/show_bug.cgi?id=1855789
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
QEMU gained support for 'win-dmp' format in it's release of 3.0,
but libvirt doesn't implement it yet. Fortunately, there not much
needed: new value to virDomainCoreDumpFormat public enum, which
unfortunately means that QEMU driver has to be updated in the
same commit, because of VIR_ENUM_IMPL().
Luckily, we don't need any extra QEMU capability - the code
already checks supported formats via
'query-dump-guest-memory-capability' just before issuing
'dump-guest-memory'.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Clang complains:
../libvirt/src/conf/node_device_conf.c:1945:74: error: result of comparison of unsigned enum expression < 0 is always false [-Werror,-Wtautological-unsigned-enum-zero-compare]
if ((mdev->start = virNodeDevMdevStartTypeFromString(starttype)) < 0) {
Fixes: 42a5585499
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
This strictens the parser to disallow negative values (interpreted as
`ULLONG_MAX + value + 1`) for attribute `reg`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute, as it
refers to a 32 bit address space.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `aw_bits`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `id`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `latency`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virNetDevOpenvswitchInterfaceGetMaster is declared twice in
src/util/virnetdevopenvswitch.h. Remove the last one.
Signed-off-by: Peng Liang <liangpeng10@huawei.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This adds a new element to the mdev capabilities xml schema that
represents the start policy for a defined mediated device. The actual
auto-start functionality is handled behind the scenes by mdevctl, but it
wasn't yet hooked up in libvirt.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The passed libxl_domain_config is owned, and already initialized, by the
caller.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
The initial variant of libxlDomainChangeEjectableMedia could just leave
the function earlier. With refcounting this does not work anymore.
Fixes commit a5bf06ba34
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `number`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `bufferCount`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `bufferCount`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `port`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `timeout`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attributes `cyls`, `heads` and `secs`.
Allowing negative numbers to be interpreted this way makes no sense for
these attributes.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `startport`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
Since Xen 4.5 libxl allows to set affinities during domain creation.
This enables Xen to allocate the domain memory on NUMA systems close to
the specified pcpus.
Libvirt can now handle <domain/cputune/vcpupin> in domU.xml correctly.
Without this change, Xen will create the domU and assign NUMA memory and
vcpu affinities on its own. Later libvirt will adjust the affinity,
which may move the vcpus away from the assigned NUMA node.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
The aim of this function is to return whether domain definition
and/or memory device that user intents to hotplug needs a private
path inside cfg->memoryBackingDir. The rule for the memory device
that's being hotplug includes checking whether corresponding
guest NUMA node needs memoryBackingDir. Well, while the rationale
behind makes sense it is not necessary to check for that really -
just a few lines above every guest NUMA node was checked exactly
for that.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The aim of qemuProcessNeedHugepagesPath() is to return whether
guest needs private path inside HugeTLBFS mounts (deducted from
domain definition @def) or whether the memory device that user is
hotplugging in needs the private path (deducted from the @mem
argument). The actual creation of the path is done in the only
caller qemuProcessBuildDestroyMemoryPaths().
The rule for the first case (@def) and the second case (@mem) is
the same (domain has a DIMM device that has HP requested) and is
written twice. Move the logic into a function to deduplicate the
code.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
As of b4cbdbe90b (and friends) the
minimal QEMU version required is 2.11.0. Let's update our
QEMU_MIN_* macros to reflect that.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The basic use case of VIR_IDENTITY_AUTORESTORE() is in
conjunction with virIdentityElevateCurrent(). What happens is
that virIdentityElevateCurrent() gets current identity (which
increases the refcounter of thread local virIdentity object) and
returns a pointer to it. Later, when the variable goes out of
scope the virIdentityRestoreHelper() is called which calls
virIdentitySetCurrent() over the old identity. But this means
that the refcounter is increased again.
Therefore, we have to explicitly decrease the refcounter by
calling g_object_unref().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Recently, a new code was added to virGetConnectGeneric() that
saves the original error into a variable so that it's not lost in
virConnectClose() called under the 'error' label.
However, the error saving code uses virSaveLastError() +
virSetError() combo which leaks the memory allocated for the
error copy. Using virErrorPreserveLast() + virErrorRestore() does
the same job without the memleak.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
A secret can be marked with the "private" attribute. The intent was that
it is not possible for any libvirt client to be able to read the secret
value, it would only be accesible from within libvirtd. eg the QEMU
driver can read the value to launch a guest.
With the modular daemons, the QEMU, storage and secret drivers are all
running in separate daemons. The QEMU and storage drivers thus appear to
be normal libvirt client's from the POV of the secret driver, and thus
they are not able to read a private secret. This is unhelpful.
With the previous patches that introduced a "system token" to the
identity object, we can now distinguish APIs invoked by libvirt daemons
from those invoked by client applications.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When fetching the value of a private secret, we need to use an elevated
identity otherwise the secret driver will deny access.
When using the modular daemons, the elevated identity needs to be active
before the secret driver connection is opened, and it will apply to all
APIs calls made on that conncetion.
When using the monolithic daemon, the identity at time of opening the
connection is ignored, and the elevated identity needs to be active
precisely at the time the virSecretGetValue API call is made.
After acquiring the secret value, the elevated identity should be
cleared.
This sounds complex, but is fairly straightfoward with the automatic
cleanup callbacks.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The drivers can all call virGetConnectXXX to open a connection to a
secondary driver. For example, when creating a encrypted storage volume,
the storage driver has to open a secret driver connection, or when
starting a guest, the QEMU driver has to open the network driver to
lookup a virtual network.
When using monolithic libvirtd, the connection has the same effective
identity as the client, since everything is still in the same process.
When using the modular daemons, however, the remote daemon sees the
identity of the calling daemon. This is a mistake as it results in
the modular daemons seeing the client with elevated privileges.
We need to pass on the current identity explicitly when opening the
secondary drivers. This is the same thing that is done by daemon RPC
dispatcher code when it is directly forwarding top level API calls
from virtproxyd and other daemons.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This is essentially a way to determine if the current identity
is that of another libvirt daemon.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When talking to the secret driver, the callers inside libvirt daemons
need to be able to run with an elevated privileges that prove the API
calls are made by a libvirt daemon, not an end user application.
The virIdentityElevateCurrent method will take the current identity
and, if not already present, add the system token. The old current
identity is returned to the caller. With the VIR_IDENTITY_AUTORESTORE
annotation, the old current identity will be restored upon leaving
the codeblock scope.
... early work with regular privileges ...
if (something needing elevated privs) {
VIR_IDENTITY_AUTORESTORE virIdentity *oldident =
virIdentityElevateCurrent();
if (!oldident)
return -1;
... do something with elevated privileges ...
}
... later work with regular privileges ...
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When creating the system identity set the system token. The system
token is currently stored in a local path
/var/run/libvirt/common/system.token
Obviously with only traditional UNIX DAC in effect, this is largely
security through obscurity, if the client is running at the same
privilege level as the daemon. It does, however, reliably distinguish
an unprivileged client from the system daemons.
With a MAC system like SELinux though, or possible use of containers,
access can be further restricted.
A possible future improvement for Linux would be to populate the
kernel keyring with a secret for libvirt daemons to share.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We want a way to distinguish between calls from a libvirt daemon, and a
regular client application when both are running as the same user
account. This is not possible with the current set of attributes
recorded against an identity, as there is nothing that is common to all
of the modular libvirt daemons, while distinct to all other processes.
We thus introduce the idea of a system token, which is simply a random
hex string that is only known by the libvirt daemons, to be recorded
against the system identity.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
A random token is simply a string of random bytes formatted in
hexidecimal.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The width of `unsigned long` differs on 32 bit and 64 bit architectures.
There is no compelling reason why the maximum DHCP lease time should
depend on the architecture.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When QEMU introduces new firmware features libvirt will fail until we
list that feature in our code as well which doesn't sound right.
We should simply ignore the new feature until we add a proper support
for it.
Reported-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Some variables are needed only inside for() loop. They were
declared at the beginning of the function because of VIR_FREE()
calls, but since they are auto-freed they can be declared inside
the loop.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
What this function really does it takes ownership of all pointers
passed (well, except for the first one - caps - to which it
registers new NUMA node). But since all info is passed as a
single pointer it's hard to tell (and use g_auto*). Let's use
double pointers to make the ownership transfer obvious.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The @cpus variable is an array of structs in which each item
contains a virBitmap member. As such it is not enough to just
VIR_FREE() the array - each bitmap has to be freed too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The rest of virCapabilities format functions take virBuffer as
the first argument and struct to format as the second. Also, they
accept NULL (as the second argument). Fix
virCapabilitiesHostNUMAFormat() so that it follows this logic.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Since its introduction in v0.9.1~65 the virOnce() was expected to
follow the usual retval logic (0 for success, a negative number
for failure). However, that was never the case.
On the other hand, looking into glibc and musl the pthread_once()
never returns anything other than zero (uclibc-ng seems to not
implement pthread_once()), therefore we never really hit any
problem. But for code cleanliness (and to match POSIX
documentation), let's change to code so that our retval logic is
honoured.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Compilers aren't able to see whether @result is set or not and thus
don't warn of a potential use of uninitialized value. Always set @result
to prevent uninitialized use.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There are few cases where we set a default value when using
virXMLPropEnum which can be converted to virXMLPropEnumDefault.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The helper is almost identical to virXMLPropEnum but it allows to pass a
default value to initialize the result to.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Commit 8391cfbc2d converted the code to use virXMLPropEnum unfaithfully
ommitting the check where 'backend' must be non-zero when parsed from the
user.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Commit 38180f87f5 converted the code to use virXMLPropEnum unfaithfully
ommitting the check where 'format' must be non-zero when parsed from the
user.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Compilers aren't able to see whether @result is set or not and thus
don't warn of a potential use of uninitialized value. Always set @result
to prevent uninitialized use.
In two cases the code needed to be adjusted to preserve functionality.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virXMLPropTristateBool already initializes the value to
VIR_TRISTATE_BOOL_ABSENT so we no longer need to do that for certain
local variables.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Compilers aren't able to see whether @result is set or not and thus
don't warn of a potential use of uninitialized value. Always set @result
to prevent uninitialized use.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Set the backup mode to VIR_TRISTATE_BOOL_YES after virXMLPropTristateBool
left it set to VIR_TRISTATE_BOOL_ABSENT. This will allow fixing
virXMLPropTristateBool to always initialize @result.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Compilers aren't able to see whether @result is set or not and thus
don't warn of a potential use of uninitialized value. Always set @result
to prevent uninitialized use.
This is done by adding a @defaultResult argument to virXMLPropInt since
many places have a non-0 default.
In certain cases such as in virDomainControllerDefParseXML we pass the
value from the original value, which will still trigger compiler checks
if unused while preserving the existing functionality of keeping the
previous value.
This commit fixes 3 uses of uninitialized value parsed by this function:
in virDomainDiskSourceNetworkParse introduced by 38dc25989c
in virDomainChrSourceDefParseTCP introduced by fa48004af5
in virDomainGraphicsListenDefParseXML introduced by 0b20fd3754
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Automatically free 'iothrid' and remove all the cleanup cruft.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Register virDomainIOThreadIDDefFree to do the cleanup.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Compilers aren't able to see whether @result is set or not and thus
don't warn of a potential use of uninitialized value. Always set @result
to prevent uninitialized use.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
VIR_XML_PROP_NONE has value of 0 so it's pointless to include it in an
binary-or expression.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Assign the vcpu count when virXMLPropUInt returns '0' meaning that the
cpu count was not present in the XML. This will allow to always
initialize the value of @result in virXMLPropUInt to prevent use of
uninitialized values.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Compilers aren't able to see whether @result is set or not and thus
don't warn of a potential use of uninitialized value. Always set @result
to prevent uninitialized use.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virXMLPropTristateBool/virXMLPropTristateSwitch/virXMLPropEnum can be
implemented using the same internal code. Extract it into a new function
called virXMLPropEnumInternal, which will also simplify adding versions
of these functions with a custom default value.
This way we'll be able to always initialize @result so that unused value
bugs can be prevented.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attributes `voices` (typically 1),
`bufferLength` (measured in milliseconds), `frequency` (in Hz, typically
44100), and `channels` (typically 2 for stereo).
None of these properties benefit from or have a sensible use-case for
wrap-around behavior.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Attributes are mandatory and were incorrectly made optional recently.
Fixes: 2a5e16398e
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
It was always allowed, but in a very unusual and weird way. Just
look at the original commit that introduced it (78fc843c7b).
Also, we document that "io" value is accepted (which translates
to VIR_DOMAIN_VIDEO_VGACONF_IO with value of zero).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
While reworking the patch I've mistakenly mangled the attribute
names for VIR_DOMAIN_CHR_TYPE_NMDM.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Commit fc7e1b2f03 which refactored the
video driver parse helper introduced a use of uninitialized variable,
which caused test failure at least when compiled with clang.
Pass 'def->vgaconf' directly to virXMLPropEnum. 'def' needs to be
converted to use g_autofree to handle error scenarios.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
xc_get_max_cpus from Xen version 4.3 may return 0 in case xc_physinfo
fails. This has been fixed in Xen 4.4. Remove the obsolete result check
from libvirt. Just convert libxl error codes to plain -1.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
In Xen 4.2 struct libxl_event_hooks had a member which was erroneously
declared const. Since libvirt requires at least Xen 4.6, remove the dead
code.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
The feature is present in all supported qemu versions (>2.11) and there
isn't a reasonable way to detect it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The feature is present in all supported qemu versions (>2.11) and there
isn't a reasonable way to detect it.
In addition the capability wasn't even used to gate any functionality
except for reporting the presence in the domain capabilities XML.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The feature is present in all supported qemu versions (>2.11) and there
isn't a reasonable way to detect it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The feature is present in all supported qemu versions (>2.11) and there
isn't a reasonable way to detect it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The feature is present in all supported QEMU versions and there isn't a
more elegant way to detect it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
All supported qemus have it, there isn't an elegant way to detect it and
it's unlikely to be ever removed on purpose.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
'query-commandline-options' never returned 'vmport' but we can detect it
in the list of supported object types. This removes it from all non-x86
originating test data as it's platform specific.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The feature is no longer asserted. Remove the checks related to it and
make the code work properly with QEMU_CAPS_DEVICE_INTEL_IOMMU.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
All supported QEMU versions now support query-qmp-schema. In the future
it will be possible to use the output of query-qmp-schema to also detect
commands reliably.
Since we are at the point where we have the least amount of .replies
files needing changing for a long time, move the 'query-qmp-schema' bits
before 'query-commands' to prepare for the future.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Modern code uses QMP schema to query for active commit support.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Modern code uses QMP schema to query for supported event types.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
All supported qemu versions have 'query-qmp-schema' so we can remove the
check whether it exists and all logic conntected to it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Remove the slot reservation for the vga card which doesn't make sense
with supported qemus any more for the q35 machine type.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Remove the slot reservation for the vga card which doesn't make sense
with supported qemus any more for the i440fx machine type.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
All supported qemus now support using '-device' for adding a graphics
device.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
These conveniently don't have any test fallout.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Until we clean up and remove all capabilities which no longer make sense
to have separately, we should use virQEMUCapsInitQMPBasicArch to set the
defaults as it's used by qemuxml2argvtest when testing with fake
capabilities.
This allows us to prevent testing dead code paths with the fake
capability tests.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Move it under AARCH 64, since it's a platform specific feature, thus it
will be removed from all other platforms.
Since virQEMUCapsInitQMPBasicArch is used in qemuxml2argv test to
initiate qemuCaps for tests with fake capabilities, all the tests gain
GIC support now.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT and
QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT are now always asserted on PPC
machine types, move them to virQEMUCapsInitQMPBasicArch.
It's now always set for AARCH64, move it into the function setting basic
caps for the emulator.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
It's now always set for AARCH64, move it into the function setting basic
caps for the emulator.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Now that minimum supported qemu version is 2.11, we can remove the
conditions.
Note that the check enabling QEMU_CAPS_TCG was for < 2.10.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
We no longer have to mask out IOMMU and NVDIMM support as we no longer
support the broken qemu versions.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The qemuCaps is left for the device commandline formatters for now as it
might come in handy.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
All machine types which have PCI support multibus since qemu 2.0
according to the logic we had, thus we can remove all the machine type
and version checks which are now dead code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
These strings are not constant really. They are allocated in
qemuDomainObjBeginJobInternal() and freed in
qemuDomainReset*Job(). Freeing a pointer to const looks weird.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `bufferCount`.
`bufferCount` does not benefit from being referable as e.g. "-7" for
requesting 4294967289 buffers, as this value is distinctly out of range
for normal use.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `retries`. UINT_MAX holds no
special significance for this attribute and is distinctly out of range
for normal use.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `id`.
`id` must be greater than 0 and does not benefit from being referable as
e.g. "-7" for host audio backend 4294967289, as this value is distinctly
out of range for normal use.
Additionally, this patch fixes a use of NULL string with printf's %s
modifier if the `model` attribute is absent.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
When updating entries in a bridge forwarding database (i.e., when
macTableManager='libvirt' is configured for the bridge), we may end up
in a situation when the entry we want to add is already present. Let's
just ignore the error in such a case.
This fixes an error to resume a domain when fdb entries were not
properly removed when the domain was paused:
virsh # resume test
error: Failed to resume domain test
error: error adding fdb entry for vnet2: File exists
For some reason, fdb entries are only removed when libvirt explicitly
stops CPUs, but nothing happens when we just get STOP event from QEMU.
An alternative approach would be to make sure we always remove the
entries regardless on why a domain was paused (e.g., during migration),
but that would be a significantly more disruptive change with possible
side effects.
https://bugzilla.redhat.com/show_bug.cgi?id=1603155
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
Commit 28a8699316 ( v6.9.0-179-g28a8699316 ) incorrectly replaced
VIR_EXPAND_N by g_renew.
VIR_EXPAND_N has these two extra effects apart from reallocating memory:
1) The newly allocated memory is zeroed out
2) The number of elements in the array which is passed to VIR_EXPAND_N
is increased.
This comes into play when used with virDomainLeaseInsertPreAlloced,
which expects that the array element count already includes the space
for the added 'lease', by plainly just assigning to
'leases[nleases - 1]'
Since g_renew does not increase the number of elements in the array
any existing code which calls virDomainLeaseInsertPreAlloced thus either
overwrites a lease definition or corrupts the heap if there are no
leases to start with.
To preserve existing functionality we revert the code back to using
VIR_EXPAND_N which at this point doesn't return any value, so other
commits don't need to be reverted.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1953577
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
virCommandRun() already handles the case where the cmd argument is NULL,
so there's no need for the caller to check. Make all callers consistent
and remove unnecessary NULL checks.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Coverity complained that the 'default' case of the switch in
nodeDeviceGetMdevctlCommand() was falling through without initializing
'cmd'. Return NULL in this case even though it should never happen.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When returning early due to errors, cmd will be leaked. Use an autoptr
to handle these early returns without leaking memory.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Jumping back in the code is an anti-pattern that should be avoided if
possible.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We don't require that the data is consistent on the destination if
aborting the migration.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Rename the parameter so that it's more clear what state we are in and
fix all callees.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We want to unify on one block job cancellation API. Use
qemuMonitorBlockJobCancel which has more features.
In case of job refresh, we are killing off any unknown jobs so we don't
care about their fate.
Another difference is that an possible error from the block job
cancellation might be reported, but we don't really care here ince
it's a very unlikely scenario and we also report a warning.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We want to unify on one block job cancellation API. Use
qemuMonitorBlockJobCancel which has more features.
In case of backup jobs we can cancel the jobs forcefully since the code
is on a cleanup path when the job fails.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
'block-job-cancel' has one very important semantic difference to
'job-cancel', docummented in qemu as:
Note that if you issue 'block-job-cancel' after 'drive-mirror' has indicated
(via the event BLOCK_JOB_READY) that the source and destination are
synchronized, then the event triggered by this command changes to
BLOCK_JOB_COMPLETED, to indicate that the mirroring has ended and the
destination now has a point-in-time copy tied to the time of the cancellation.
Since libvirt advertises the block copy job as having the synchronous
abort feature we must not use 'job-cancel' here.
Fixes: 4817b5ca1d
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In certain cases such as when aborting migration we don't really care
for completion of the blockjob. Add 'force' as parameter of
'block-job-cancel'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use automatic memory freeing and remove the cleanup section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Don't try to setup disk migration and the NBD stuff if we end up
migrating nothing.
The destination side has luckily no setup for the non-NBD cases so
omitting the element fully is okay.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Don't even try to setup storage migration if there are no eligible
disks.
This also fixes migration from older libvirts which didn't format an
empty <nbd/> element in the migration cookie if there weren't any disks
to migrate.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Base the decision on the main API flags (VIR_MIGRATE_NON_SHARED_DISK,
QEMU_MONITOR_MIGRATE_NON_SHARED_INC) via a boolean 'storageMigration'
rather than juggling everything trhough 'migration_flags'.
After this patch 'migration_flags' is updated to contain the legacy
storage migration flags only when we'll be about to use it rather than
setting it and then resetting it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
'migrate_flags' can be updated in the only caller and since
qemuMigrationSrcNBDStorageCopy already takes @flags which contains
VIR_MIGRATE_NON_SHARED_INC (used to set
QEMU_MONITOR_MIGRATE_NON_SHARED_INC) we can completely remove the
parameter.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Tested-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In case the 'nbdURI' schema is not known the code would report an error
but wouldn't return failure.
Fixes: 49186372db
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Tested-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Commit 518be41aaa refactored qemuMigrationCookieNBDXMLFormat to use
virXMLFormatElement which in comparison to the previous code doesn't
format the element if it's empty.
Unfortunately some crusty bits of our migration code use questionable
logic to assert use of the old-style storage migration parameters which
breaks if no disks are being migrated and the <nbd/> element is not
present.
While later patches will fix the code, re-instate formatting of empty
<nbd/> for increased compatibility.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Tested-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Add a helper which will format an XML element with attributes and
children, but compared to virXMLFormatElement it also formats an empty
element if both buffers are empty.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Tested-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Function incorrectly returns 0 when property was successfully read.
Fixes: ab5d2776c9
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
When placing vCPUs into CGroups the qemuProcessSetupPid() is
called which then enters a for() loop (around its middle) where
it calls virDomainNumaGetNodeCpumask() for each guest NUMA node.
But the latter returns only a pointer not new reference/copy and
thus the caller must not free it. But the variable is decorated
with g_autoptr() which leads to a double free.
Fixes: 2d37d8dbc9
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
libssh2 has ECDSA and ED25519 support beginning with v1.9.0. libvirt cannot
make use of those because it will handle them as unknown key types.
Add support for those host key types.
Signed-off-by: Bastian Germann <bastiangermann@fishpost.de>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Calling VIR_FREE on a virDomainDef* does not free its various contained
pointers.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Preparatory step to remove virDomainChrSourceDefParseMode.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This is the bug I'm facing. I deliberately configured a container
so that the source of a <filesystem/> to passthrough doesn't
exist. The start fails with:
lxcContainerPivotRoot:669 : Failed to create /non-existent/path/.oldroot: Permission denied
which is expected. But what is NOT expected is that CGroup
hierarchy is left behind. This is because the controller sets up
the CGroup hierarchy, user namespace, moves interfaces, etc. and
finally checks whether container setup (done in a separate
process) succeeded. Only after all this the error is propagated
to the LXC driver. The driver aborts the startup and tries to
perform the cleanup, but this is missing CGroups because those
weren't detected yet.
Ideally, whenever a function fails, it tries to unroll back so
that is has no artifacts left behind (look at all those frees/FD
closes/etc. at end of functions). But with CGroups it is
different - the controller process can't clean up after itself,
because it is still running inside that CGroup.
Therefore, what we have to do is to let the driver detect CGroups
as soon as they are created, and proceed with controller
execution only after that.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Currently, there is only a single pipe passed to lxc_controller
and it is used by lxc_controller to signal to the LXC driver that
the container is set up and ready to run. However, in the next
commit we will need to signal that the LXC driver has done its
part of startup process and thus the controller can proceed.
Unfortunately, virCommand handshake can't be used for this,
because it's already used to read controller's PID.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Future commits will want to reuse the handshakeFd and thus it
mustn't be closed in virLXCControllerDaemonHandshake(). Do the
closing explicitly afterwards.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The lxc_controller has a structure that's keeping its internal
state, including so called handshakeFd which is the write end of
a pipe that's used to signal to the LXC driver that the container
is set up and ready to run. However, the struct member is not
initialized to -1, so if anything fails before it is set then the
virLXCControllerFree() function tries to close FD 0 (stdin).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Convenience function to return the value of an unsigned long long XML
attribute.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
With the last usage of `aes` and `dea` as int gone, these two can
become virTristateSwitch.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This adds a new XML element
<filesystem>
<binary>
<sandbox mode='chroot|namespace'/>
</binary>
</filesystem>
This will be used by qemu virtiofs
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
The code in storage_backend_fs is used for storage_dir and storage_fs
drivers so some parts need to be guarded by checking for
WITH_STORAGE_FS.
Fixes: 16c69e7aae
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Copy the socket path in qemuExtDevicesStart, because
for libvirt-managed virtiofsd daemons the path is filled there
in qemuVirtioFSStart.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Allow passing a socket of an externally launched virtiofsd
to the vhost-user-fs device.
<filesystem type='mount'>
<driver type='virtiofs' queue='1024'/>
<source socket='/tmp/sock/'/>
</filesystem>
https://bugzilla.redhat.com/show_bug.cgi?id=1855789
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
So far VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH is always set
in virDomainFSDefPostParse, but future commits aim to change
that.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Move the default setting of accessmode to the post-parse phase.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `speed`, which does not make sense for
a value measured in Mbits per second.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
In certain rare occasions qemu can transition a block job which was
already 'ready' into 'standby' and then back. If this happens in the
following order libvirt will get confused about the actual job state:
1) the block copy job is 'ready' (job->state == QEMU_BLOCKJOB_STATE_READY)
2) user calls qemuDomainBlockJobAbort with VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT
flag but without VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC
3) the block job is switched to synchronous event handling
4) the block job blips to 'standby' and back to 'ready', the event is
not processed since the blockjob is in sync mode for now
5) qemuDomainBlockJobPivot is called:
5.1) 'job-complete' QMP command is issued
5.2) job->state is set to QEMU_BLOCKJOB_STATE_PIVOTING
6) code for synchronous-wait for the job completion in qemuDomainBlockJobAbort
is invoked
7) the waiting loop calls qemuBlockJobUpdate:
7.1) job->newstate is QEMU_BLOCKJOB_STATE_READY due to 4)
7.2) qemuBlockJobEventProcess is called
7.3) the handler for QEMU_BLOCKJOB_STATE_READY overwrites
job->state from QEMU_BLOCKJOB_STATE_PIVOTING to QEMU_BLOCKJOB_STATE_READY
8) qemuDomainBlockJobAbort is looking for a finished job, so waits again
9) qemu finishes the blockjob and transitions it into 'concluded' state
10) qemuBlockJobUpdate is triggered again, this time finalizing the job.
10.1) job->newstate is = QEMU_BLOCKJOB_STATE_CONCLUDED
job->state is = QEMU_BLOCKJOB_STATE_READY
10.2) qemuBlockJobEventProcessConcluded is called, the function
checks whether there was an error with the blockjob. Since
there was no error job->newstate becomes
QEMU_BLOCKJOB_STATE_COMPLETED.
10.3) qemuBlockJobEventProcessConcludedTransition selects the action
for the appropriate block job type where we have:
case QEMU_BLOCKJOB_TYPE_COPY:
if (job->state == QEMU_BLOCKJOB_STATE_PIVOTING && success)
qemuBlockJobProcessEventConcludedCopyPivot(driver, vm, job, asyncJob);
else
qemuBlockJobProcessEventConcludedCopyAbort(driver, vm, job, asyncJob);
break;
Since job->state is QEMU_BLOCKJOB_STATE_READY,
qemuBlockJobProcessEventConcludedCopyAbort is called.
This patch forbids transitions to QEMU_BLOCKJOB_STATE_READY if the
previous job state isn't QEMU_BLOCKJOB_STATE_RUNNING or
QEMU_BLOCKJOB_STATE_NEW.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1951507
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Future patch will remove MKFS define as we will no longer check it
during compilation.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The function in question uses "tc" binary so virnetdevbandwidth feels
like better place for it.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This will allow us to run tests using firewall on hosts where the mocked
binaries are not available/installed instead of skipping these tests.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Following patches will make this change necessary as we will stop
detecting the full path during compile time.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
We always pass DNSMASQ so there is no need for the argument at all.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
We always pass DNSMASQ so there is no need for the argument at all.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Instead of removing binaryPath let's drop the function completely as
it is not used anywhere.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Instead of removing binaryPath let's drop the function completely as
it is not used anywhere.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
We will never call dnsmasqCapsRefresh() so reflect what actually
happens.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The new enum helpers use a set of flags to modify their behaviour, but
the declared set of flags is semantically confusing:
typedef enum {
VIR_XML_PROP_OPTIONAL = 0, /* Attribute may be absent */
VIR_XML_PROP_REQUIRED = 1 << 0, /* Attribute may not be absent */
Since VIR_XML_PROP_OPTIONAL is declared as 0 any other flag shadows it
and makes it impossible to detect. The functions are not able to detect
a semantic nonsense of VIR_XML_PROP_OPTIONAL | VIR_XML_PROP_REQUIRED and
it's a perfectly valid statement for the compilers.
In general having two flags to do the same boolean don't make sense and
the implementation doesn't fix any shortcomings either.
To prevent mistakes, rename VIR_XML_PROP_OPTIONAL to VIR_XML_PROP_NONE,
so that there's always an enum value used with the calls but it doesn't
imply that the flag makes the property optional when the actual value is
0.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
As I've pointed out in my review, the negative number wrapping for
unsigned variables is an anti-feature which should not be promoted in
any way.
Remove VIR_XML_PROP_WRAPNEGATIVE which would make it more accessible.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
xmlDocSetRootElement removes the node from its previous document tree,
effectively removing the "<cpu>" node from "<domain>" in virCPUDefParseXML.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The g_path_is_absolute() considers more situations
than just a simply "path[0] == '/'".
Related issue: https://gitlab.com/libvirt/libvirt/-/issues/12
Signed-off-by: Luke Yue <lukedyue@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
These per-command generator functions were only exposed in the header to
allow the commandline generation to be tested. Now that we have a
generic mdevctl command generator, we can get rid of the per-command
wrappers and reduce the noise in the header.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
Currently there are dedicated wrappers to construct mdevctl command.
These are mostly fine except for the one that translates both "start"
and "define" commands, only because mdevctl takes the same set of
arguments. Instead, keep the wrappers, but let them call a single
global translator that handles all the mdevctl command differences and
commonalities.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
This is not a 1:1 mapping to mdevctl commands because mdevctl doesn't
support a separate 'create' command. mdevctl uses 'start' for both
starting a pre-defined device as well as for creating and starting a new
transient device. The libvirt code will be more readable if we treat
these as separate commands. When we need to actually execute mdevctl,
the 'create' command will be translated into the appropriate 'mdevctl
start' command.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
rather than using short opentions (e.g. "-p 0000:00:02.0"), use long
options everywhere (e.g. "--parent=0000:00:02.0")
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
"start" in libvirt means - "take this object and create an
instance out of it"
"create" in libvirt most of the time means - "take and XML description,
make an object out of it and use it to create an instance"
This gets confusing with mdevctl which uses "start" for both. So, this
patch proposes to use virMdevctlStart in cases where from libvirt's POV
we're starting a defined device (unlike mdevctl). Similarly, use
virMdevctlCreate in scenarios where XML description is passed to
libvirt and a transient device is supposed to be created.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
These errors are demoted to debug statements[1] since they're only
intended to be used as return values for public APIs. This makes it
difficult to debug the problem when something goes wrong since no error
message is logged. Switch instead to VIR_ERR_INTERNAL_ERROR so that the
error is logged as expected.
[1] See the implementation of daemonErrorLogFilter() for details:
e2f82a3704/src/remote/remote_daemon.c (L89)
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
The calling function will log the error. Just return NULL if a device
cannot be found.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Use the new virXMLProp helpers and XPath queries to get rid of the old
style of iteration through element children.
Note that in case of def->blockio.logical_block_size,
def->blockio.physical_block_size and def->rotation_rate the wraparound
behaviour of 'virStrToLong_ui' was _not_ forward ported to the new code
as it makes no sense with the attributes.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use the appropriate type for the variable and refactor the XML parser to
parse it correctly using virXMLPropEnum.
Changes to other places using switch statements were required.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use the appropriate type for the variable and refactor the XML parser to
parse it correctly using virXMLPropEnum.
Changes to other places using switch statements were required.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Unfortunately virDomainSnapshotLocation is declared in snapshot_conf.h
which includes domain_conf.h. To avoid a circular dependency use
'unsigned int' for now.
Use XML parser can use virXMLPropEnum.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use the appropriate type for the variable and refactor the XML parser to
parse it correctly using virXMLPropEnum.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use the appropriate type for the variable and refactor the XML parser to
parse it correctly using virXMLPropEnum.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use the appropriate type for the variable and refactor the XML parser to
parse it correctly using virXMLPropEnum.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use the appropriate type for the variable and refactor the XML parser to
parse it correctly using virXMLPropEnum.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Move the rest of the validations to the vaidation code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Move the setting of read-only state, the default disk bus and setting of
'snapshot' state for read-only disks to the post parse callback to clean
up the disk parser.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Mark it explicitly as read only in accordance with the comment outlining
configuration.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Add a disk bus value represending no selected bus. This will help split
up the XML parser.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Modifications of the data such as this one don't belong into the parser.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The moved code contains only checks and does not modify the parsed
document so it doesn't belong into the PostParse code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Unify the two distinct disk definition validators.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Consolidate the checks for '<reservations/>' and viritio queues under
already existing blocks which have the same condition.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There's no code which would assert it at this point. Remove the flag.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Extract all code related to parsing data which ends up in the 'src'
member of a virDomainDiskDef.
This allows to use the new function directly in
virDomainDiskDefParseSource and removes the use of the
VIR_DOMAIN_DEF_PARSE_DISK_SOURCE parser flag.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Separate the validation of the source so that it can be reused once we
split up the XML parser too.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The <disk> XML element parser is going to be modified so that the
virStorageSource bits are pre-parsed. Add virDomainDiskDefNewSource,
which uses an existing 'src' pointer.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
qemuDomainBlockCopy needs just the source portion of the disk but uses
the disk parser for it. Since we have a specific function now, refactor
the code to avoid having to deal with the unused virDomainDiskDef.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Add a helper function which will parse the source portion of a <disk>.
The idea is to replace *virDomainDiskDefParse with
VIR_DOMAIN_DEF_PARSE_DISK_SOURCE with the new helper in the future.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use the new macro instead of virXMLParseStringCtxt in places where the
root node is being validated.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Some callers want to validate the root XML node name. Add the capability
to the parser helper to prevent open-coding.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This allows users to restrict memory nodes without setting any specific
memory policy, then 'restrictive' mode is useful.
Signed-off-by: Luyao Zhong <luyao.zhong@intel.com>
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The command line argument is called --hanshakefd (check out
lxc_controller.c:main()). But the command line builder puts only
--handshake. This works, because there is no other argument
sharing the prefix.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
I've encountered the following bug, but only on Gentoo with
systemd and CGroupsV2. I've started an LXC container successfully
but destroying it reported the following error:
error: Failed to destroy domain 'amd64'
error: internal error: failed to get cgroup backend for 'pathOfController'
Debugging showed, that CGroup hierarchy is full of surprises:
/sys/fs/cgroup/machine.slice/machine-lxc\x2d861\x2damd64.scope/
└── libvirt
├── dev-hugepages.mount
├── dev-mqueue.mount
├── init.scope
├── sys-fs-fuse-connections.mount
├── sys-kernel-config.mount
├── sys-kernel-debug.mount
├── sys-kernel-tracing.mount
├── system.slice
│ ├── console-getty.service
│ ├── dbus.service
│ ├── system-getty.slice
│ ├── system-modprobe.slice
│ ├── systemd-journald.service
│ ├── systemd-logind.service
│ └── tmp.mount
└── user.slice
For comparison, here's the same container on recent Rawhide:
/sys/fs/cgroup/machine.slice/machine-lxc\x2d13550\x2damd64.scope/
└── libvirt
Anyway, those nested directories should not be a problem, because
virCgroupKillRecursiveInternal() removes them recursively, right?
Sort of. The function really does remove nested directories, but
it assumes that every directory has the same controller as the
rest. Just take a look at virCgroupV2KillRecursive() - it gets
'Any' controller (the first one it found in ".scope") and then
passes it to virCgroupKillRecursiveInternal().
This assumption is not true though. The controllers found in
".scope" are the following:
cpuset cpu io memory pids
while "libvirt" has fewer:
cpuset cpu io memory
Up until now it's not problem, because of how we order
controllers internally - "cpu" is the first and thus picking
"Any" controller returns just that. But the rest of directories
has no controllers, their "cgroup.controllers" is just empty.
What fixes the bug is dropping @controller argument from
virCgroupKillRecursiveInternal() and letting each iteration work
pick its own controller.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The VIR_CGROUP_BACKEND_CALL() macro gets a backend for controller
and calls corresponding callback in it. If either is NULL then an
error message is printed out. However, the error message contains
only the intended callback func and not controller or backend
found.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Currently, only a subset of virCgroupKillRecursiveInternal()
arguments is printed into debug logs. Print all of them.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Use the appropriate type for the variable and refactor the XML parser to
parse it correctly using virXMLPropEnum.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
Use the appropriate type for the variable and refactor the XML parser to
parse it correctly using virXMLPropEnum.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
