'tftp' storage protocol was supported by qemu until 2.7.0. Add an
interlock when blockdev is used and drop the test case for it as it's
IMO not worth adding another test file just for that.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
==156803== 58 (40 direct, 18 indirect) bytes in 1 blocks are definitely lost in loss record 306 of 463
==156803== at 0x4839EC6: calloc (vg_replace_malloc.c:762)
==156803== by 0x5791AC0: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.6400.1)
==156803== by 0x48F60DC: virAlloc (viralloc.c:48)
==156803== by 0x18DD74: qemuStorageSourcePrivateDataAssignSecinfo (qemu_domain.c:2384)
==156803== by 0x18DFD5: qemuStorageSourcePrivateDataParse (qemu_domain.c:2433)
==156803== by 0x49EC884: virDomainStorageSourceParse (domain_conf.c:9857)
==156803== by 0x49ECBA3: virDomainDiskBackingStoreParse (domain_conf.c:9909)
==156803== by 0x49F129D: virDomainDiskDefParseXML (domain_conf.c:10785)
==156803== by 0x4A1804E: virDomainDefParseXML (domain_conf.c:21543)
==156803== by 0x4A1B60C: virDomainObjParseXML (domain_conf.c:22254)
==156803== by 0x4A1BFE9: virDomainObjParseNode (domain_conf.c:22429)
==156803== by 0x4A1C0B4: virDomainObjParseFile (domain_conf.c:22443
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Modern way to store <auth> and <encryption> of a <disk> is under
<source>. This was added to mirror how <backingStore> handles these and
in fact they are relevant to the source rather than to any other part of
the disk. Historically we allowed them to be directly under <disk> and
we need to keep compatibility.
This wasn't a problem until introduction of -blockdev in qemu using of
<auth> or <encryption> plainly wouldn't work with backing chains.
Now that it works in backing chains and can be moved back and forth
using snapshots/block-commit we need to ensure that the original
placement is properly kept even if the source changes.
To achieve the above semantics we need to store the preferred placement
with the disk definition rather than the storage source definitions and
also ensure that the modern way is chosen when the VM started with
<source/encryption> only in the backing store.
https://bugzilla.redhat.com/show_bug.cgi?id=1822878
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Any non-raw block layer feature will not work with raw SCSI command
passthrough via 'scsi-block'. Explicitly refuse use of luks encryption,
storage slices and copy on read.
https://bugzilla.redhat.com/show_bug.cgi?id=1820040
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Historically the virtio-blk frontend by default enabled SCSI emulation
and tried to do SCSI command passthrough. As this was enabled by default
there's a fallback mechanism in place in cases when the backend doesn't
support SCSI for any reason.
This is not the case when disk type=lun is used with 'scsi-block' via
'virtio-scsi'.
We did not restrict configurations when the user picks 'qcow2' or any
other format as format of the disk, in which case the emulation is
disabled as such configuration doesn't make sense.
This patch unifies the approach so that 'raw' is required both when used
via 'virtio-blk' and 'virtio-scsi' so that the user is presented with
the expected configuration. Note that use of <disk type='lun'> is
already very restrictive as it requires a block device or iSCSI storage.
Additionally the scsi emulation is now deprecated by qemu with
virtio-blk as it conflicts with virtio-1 and the alternative is to use
'virtio-scsi' which performs better and is along for a very long time.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
SD cards need to be instantiated via -drive if=sd. This means that all
cases where we use the blockdev path need to be special-cased for SD
cards.
Note that at this point QEMU_CAPS_BLOCKDEV is still cleared if the VM
config has a SD card.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We still have to use -drive to instantiate sd disks. Combining that with
the new logic for blockjobs would be very complicated and not worth it
given that 'sd' cards work only on few rarely used machine types of
non-common architectures and libvirt didn't implement support for 'sd'
bus controllers. This will allow us to use -blockdev for other kinds on
such machines while sacrificing block jobs.
Note: this is currently no-op as we mask-out the QEMU_CAPS_BLOCKDEV
capability if any of the disks has bus='sd'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In case of 'sd' cards we'll use pre-blockdev code also if qemu supports
blockdev. In that specific case we'll need to mask out blockdev support
for 'sd' disks. Plumb in a boolean to allow it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We have a framework to register cleanup callbacks that are run
when a domain is shut down. The idea is to run callbacks in
reverse order than they were registered. However, looking at the
code this is not the case. Fortunately, this framework is used to
register a single callback and a single callback only -
qemuMigrationDstPrepareCleanup() - therefore there was no problem
just yet.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Instead of the following pattern:
type ret;
...
ret = func();
return ret;
we can use:
return func()
directly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
If a backup job fails midway it's hard to figure out what happened as
it's running asynchronous. Use the VIR_DOMAIN_JOB_ERRMSG job statistics
field to pass through the error from the first failed backup-blockjob
so that both the consumer of the virDomainGetJobStats and the
corresponding event can see the error.
event 'job-completed' for domain backup-test:
operation: 9
time_elapsed: 46
disk_total: 104857600
disk_processed: 10158080
disk_remaining: 94699520
success: 0
errmsg: No space left on device
virsh domjobinfo backup-test --completed --anystats
Job type: Failed
Operation: Backup
Time elapsed: 46 ms
File processed: 9.688 MiB
File remaining: 90.312 MiB
File total: 100.000 MiB
Error message: No space left on device
https://bugzilla.redhat.com/show_bug.cgi?id=1812827
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The field can be used by jobs to add an optional error message to a
completed (failed) job.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
In order to add a string to qemuDomainJobInfo we must ensure that it's
freed and copied properly. Add helpers to copy and free the structure
and adjust the code to use them properly for the new semantics.
Additionally also allocation is changed to g_new0 as it includes the
type and thus it's very easy to grep for all the allocations of a given
type.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Catch the individual usage not removed in previous commits.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Previously, we used virCapabilitiesDomainDataLookup() to fill
machine type in post parse callback if none was provided in the
domain XML. If machine type couldn't be filled in an error was
reported. After 4a4132b462 we've changed it to
virQEMUCapsGetPreferredMachine() which returns NULL, but we no
longer report an error and proceed with the post parse callbacks
processing. This may lead to a crash because the code later on
assumes def->os.machine is not NULL.
Fixes: 4a4132b462
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Mores <pmores@redhat.com>
The cfg->root is going away, therefore get the info right from
the driver structure.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The virDomainGenerateMachineName() function doesn't belong in
src/conf/ really, because it has nothing to do with domain XML
parsing. It landed there because of lack of better place in the
past. But now that we have src/hypervisor/ the function should
live there. At the same time, the function name is changed to
match new location.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Introduced in v1.2.17-rc1~121, the assumption was that the
driver->privileged is immutable at the time but it might change
in the future. Well, it did not ever since. It is still immutable
variable. Drop the needless accessor then.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Now that all its helper functions are in qemu_validate.c, we can
move the function itself. The helpers can become static again since
they're all in the same file.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
This will allow to move qemuDomainDeviceDefValidate() itself in
the next patch in a cleaner way.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Move the function and all its static helper functions.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This function will remain public due to its usage in qemublocktest.c
even after moving qemuDomainDeviceDefValidate(). The position of its
header in qemu_validate.h is no accident.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
This function alone requires other 3 static functions to be
moved as well, thus let's move it in its own patch.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
qemuDomainChrDefValidate() has a lot of static helpers functions
that needed to be moved as well.
Other functions from qemuDomainDeviceDefValidate() that were
also moved:
- qemuValidateDomainSmartcardDef
- qemuValidateDomainRNGDef
- qemuValidateDomainRedirdevDef
- qemuValidateDomainWatchdogDef
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
The next big task is to move qemuDomainDeviceDefValidate() to
qemu_validation.c, which is a function that calls a lot of
other static helper functions. This patch starts it by moving
qemuDomainDeviceDefValidateAddress().
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Move the static functions qemuDomainValidateDef() uses, as well as
qemuDomainValidateDef() itself to qemu_validate.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This patch introduces a new file to host domain validations from
the QEMU driver. And to get things started, let's move
qemuDomainDefValidateFeatures() to this new file.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
If the storage source has the query part set, format it in the output.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Commit 5540acb9a2 added a minimum size verification for the target
size of ppc64 NVDIMMs but forgot to remove a MAX() size check that
was being used in earlier reviews of that commit. The size
verification makes this check unneeded since we're making sure
that guestArea will always be at least equal to ppc64AlignSize.
Fixes: 5540acb9a2
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The PMU feature is enabled by default in ppc64 guests and can't
be disabled via Libvirt or QEMU [1]. The current PMU feature
implementation does not allow PMU to enabled or disabled in the
ppc64 guest. Declaring the PMU feature will make the 'pmu'
property to be passed on to QEMU, but this property isn't
available for ppc64:
qemu-kvm: can't apply global host-powerpc64-cpu.pmu=on: Property '.pmu' not found
A similar error is thrown when trying to disable the PMU.
This patch standardizes the PMU handling for ppc64 guests by:
- throwing an error if the user attempts to set the feature to
'off', given that this feature can't be turned off at all;
- allowing the feature to be declared as 'on' in the domain XML.
This is done by skipping ppc64 guests when creating the command
line for this feature.
[1] https://www.redhat.com/archives/libvir-list/2020-March/msg00874.html
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Hyperv features are supported by both x86 and aarch64. The <hyperv/>
declaration in the XML by itself is benign to other architectures,
but any of its 14 current features will break QEMU with an error
like this (from ppc64):
qemu-kvm: Expected key=value format, found hv_relaxed
This is a more extreme case than the one for apic eoi because we
would need an extra 'switch' statement, with all current Hyperv
features in the body of qemuDomainDefValidateFeatures(), to
check if the user attempted to activate any of them. It's easier to
simply fail to launch with any 'hyperv' declaration in the XML for
every arch which is not x86 and aarch64.
A fair disclaimer about Windows and PowerPC: the last Windows version
that ran in the architecture is the hall of famer Windows NT 4.0,
launched in 1996 and with end of extended support for the Server
version in 2004 [1]. I am acknowledging that there might be Windows
NT 4.0 users running in PowerPC, but not enough people running it
under KVM/QEMU to justify Libvirt allowing 'hyperv' to exist in the
domain XML of ppc64 domains.
[1] https://en.wikipedia.org/wiki/Windows_NT_4.0
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The 'pvspinlock' feature is x86 only. The "<pvspinlock/>" declaration
will always have a value 'on' or 'off', and both will break QEMU when
launching non-x86 guests. This is the error message for
"<pvspinlock state='on'/>" when running a ppc64 guest:
qemu-kvm: Expected key=value format, found +kvm_pv_unhalt
A similar error message is thrown for "<pvspinlock state='off'/>".
This patch prevents non-x86 guests from launching with any
pvspinlock setting with a more informative error message:
error: unsupported configuration: The 'pvspinlock' feature is not
supported for architecture 'ppc64' or machine type 'pseries'
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The "<apic/>" feature, although it's available only for x86 guests,
can be declared in the domain XML of other archs without errors.
But setting its 'eoi' attribute will break QEMU. For "<apic eoi='on'/>",
in a ppc64 guest:
qemu-kvm: Expected key=value format, found +kvm_pv_eoi
A similar error happens with eoi='off'.
One can argue that it's better to simply forbid launching non-x86
guests with "<apic/>" declared in the XML - it is a feature that
the architecture doesn't support and this would make it clearer
about it. This is sensible, but there are non-x86 guests that are
running with "<apic/>" declared in the domain (and A LOT of guests
running with "<acpi/>" for that matter, probably reminiscent of x86
templates that were reused for other archs) that will stop working if we
go this route.
A more subtle approach is to detect if the 'eoi' element is being set
for non-x86 guests and warn the user about it with a better error
message than the one QEMU provides. This is the new error message
when any value is set for the 'eoi' element in a ppc64 XML:
error: unsupported configuration: The 'eoi' attribute of the 'apic'
feature is not supported for architecture 'ppc64' or machine type
'pseries'.
https://bugzilla.redhat.com/show_bug.cgi?id=1236440
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Helper processes may have their state migrated with QEMU data stream
thanks to the QEMU "dbus-vmstate".
libvirt maintains the list of helpers to be migrated. The
"dbus-vmstate" is added when required, and given the list of helper
Ids that must be migrated, on save & load sides.
See also:
https://git.qemu.org/?p=qemu.git;a=blob;f=docs/interop/dbus-vmstate.rst
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This avoids trying to start a dbus-daemon when its already running.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This code was based on a per-helper instance and peer-to-peer
connections. The code that landed in qemu master for v5.0 is relying
on a single instance and DBus bus.
Instead of trying to adapt the existing dbus-vmstate code, let's
remove it and resubmit. That should make reviewing easier.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Introduce qemuBlockStorageSourceGetCookieString which does the
concatenation so that we can reuse it later.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Using the 'uuid' element for ppc64 NVDIMM memory added in the
previous patch, use it in qemuBuildMemoryDeviceStr() to pass
it over to QEMU.
Another ppc64 restriction is the necessity of a mem->labelsize,
given than ppc64 only support label-area backed NVDIMMs.
Finally, we don't want ppc64 NVDIMMs to align up due to the
high risk of going beyond the end of file with a 256MiB
increment that the user didn't predict. Align it down
instead. If target size is less than the minimum of
256MiB + labelsize, error out since QEMU will error out
if we attempt to round it up to the minimum.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The @devPath variable is not modifiable. It merely just points to
string containing path where private devtmpfs is being
constructed. Make it const so it doesn't look weird that it's not
freed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Mores <pmores@redhat.com>
If building namespace fails somewhere in the middle (that is some
files exists under devMountsSavePath[i]), then plain rmdir() is
not enough to remove dir. Umount the temp location and use
virFileDeleteTree() to remove the directory.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Mores <pmores@redhat.com>
The virFileMakePathWithMode() which is our recursive version of
mkdir() fails, it simply just returns a negative value with errno
set. No error is reported (as compared to virFileTouch() for
instance).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Mores <pmores@redhat.com>
Introduce qemuBlockStorageSourceNeedsStorageSliceLayer which will hold
the decision logic and fix all places that open-code it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In one of my previous commits I've introduced code that creates
all devices for given (possible) multipath target. But I've made
a mistake there - the code accesses 'next->path' without checking
if the disk source is local. Note that the 'next->path' is
NULL/doesn't make sense for VIR_STORAGE_TYPE_NVME.
Fixes: a30078cb83
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1814947
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
So far, when using the qemu:///embed driver, management
applications can't chose whether they want to register their
domains in machined or not. While having that option is certainly
desired, it will require more work. What we can do meanwhile is
to generate names that include part of hash of the root
directory. This is to ensure that if two applications using
different roots but the same domain name (and ID) start the
domain no clashing name for machined is generated.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
