QEMU code does not work well with too big numbers on the JSON monitor so
our monitor code supports sending only numbers up to LLONG_MAX. Avoid a
weird error message by limiting the size of the 'bandwidth' parameter
for block copy.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1532542
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
These APIs are not required anywhere outside the migration code so need
not be exported to the rest of the QEMU driver.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The qemuMigrationPrecreateStorage method needs a connection
to access the storage driver. Instead of passing it around,
open it at time of use.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
There's a few places in startup code paths which pass around a
virConnectPtr which is no longer required. Specifically, the
qemuProcessStart() method now only requires a non-NULL connection if
autodestroy is requested.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When setting up graphics, we sometimes need to resolve networks,
requiring the caller to pass in a virConnectPtr, except sometimes they
pass in NULL. Use virGetConnectNetwork() to acquire the connection to
the network driver when it is needed.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
During domain startup there are many places where we need to acquire
secrets. Currently code passes around a virConnectPtr, except in the
places where we pass in NULL. So there are a few codepaths where ability
to start guests using secrets will fail. Change to acquire a handle to
the secret driver when needed.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Rather than expecting callers to pass a virConnectPtr into the
virDomainDiskTranslateSourcePool() method, just acquire a connection
to the storage driver when needed.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
There is a long standing hack to pass a virConnectPtr into the
qemuMonitorStartCPUs method, so that when the text monitor prompts
for a disk password, we can lookup virSecretPtr objects. This causes
us to have to pass a virConnectPtr around through countless methods
up the call chain....except some places don't have any virConnectPtr
available so have always just passed NULL. We can finally fix this
disastrous design by using virGetConnectSecret() to open a connection
to the secret driver at time of use.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This wires up the previously added Chassis strings XML schema to be able to
generate comamnd line args for QEMU. This requires QEMU >= 2.1 release
containing this patch:
SMBIOS: Build aggregate smbios tables and entry point
https://git.qemu.org/?p=qemu.git;a=commit;h=c97294ec1b9e36887e119589d456557d72ab37b5
Signed-off-by: Zhuang Yanying <ann.zhuangyanying@huawei.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The documentation for the JSON/qapi type 'UnixSocketAddress' states that
the unix socket path field is named 'path'. Unfortunately qemu uses
'socket' in case of the gluster driver (despite documented otherwise).
Add logic which will format the correct fields while keeping support of
the old spelling.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1544325
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1515533
We're already checking if IPv4 prefix isn't too long. But we are
not checking if it isn't too short. QEMU supports prefixes longer
than 4 (including). I haven't find anything similar related to
IPv6 in qemu sources.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The array indexes are formatted if the JSON->commandline translator is
translating an array type. It does not at all depend on this function.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Instead of storing separately whether the feature is enabled
or not and what resizing policy should be used, store both of
them in a single place.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Instead of storing separately whether the feature is enabled
or not and what driver should be used, store both of them in
a single place.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
When no GIC version is specified, we currently default to GIC v2;
however, that's not a great default, since guests will fail to
start if the hardware only supports GIC v3.
Change the behavior so that a sensible default is chosen instead.
That basically means using the same algorithm whether the user
didn't explicitly enable the GIC feature or they explicitly
enabled it but didn't specify any GIC version.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Keep them along with other arch/machine type checks for
features instead of waiting until command line generation
time.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The compiler can make sure we are handling all features.
While reworking the logic, also change error messages to a more
consistent style.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
We want to perform all feature verification in a single spot, but
some of it (eg. GIC) is currently being performed at command line
generation time, and moving it to PostParse() would cause guests
to disappear. Moving verification to Validate() allows us to
side-step the issue.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This reverts commit 038eb472a0.
On reflection adding defaults for arbitrary guest XML device config
settings to the qemu.conf is not a sustainable path. Removing the
support for rx/tx queue size so that it doesn't set a bad precedent.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Commit 'f0f2a5ec2' neglected to adjust the if condition to split
out the possibility that the @watchdog is NULL when altering the
message to add detail about the model.
Just split out the condition and use previous/original message, but
with the new message code.
Found by Coverity
Signed-off-by: John Ferlan <jferlan@redhat.com>
The floppy command formatting is special-cased since it does not
directly translate to a single '-device' argument.
Move the code from qemuBuildDiskDriveCommandLine to a new helper
function so that all the related code is together.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
We forgot to free alloced mem when failed to
dup ifname or macaddr.
Also use VIR_STEAL_PTR to simplify codes.
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Several PCI controllers have the same options, and thus
can be handled together.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This is a hard error, and should be handled as such.
Introduced in 2461476022.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Loadable drivers must never depend on each other. Over time some usage
mistakenly crept in for the storage and network drivers, but now this is
eliminated the syntax-check rules can enforce this separation once more.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The storagePoolLookupByTargetPath() method in the storage driver is used
by the QEMU driver during block migration. If there's a valid use case
for this in the QEMU driver, then external apps likely have similar
needs. Exposing it in the public API removes the direct dependancy from
the QEMU driver to the storage driver.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The virStorageTranslateDiskSourcePool method modifies a virDomainDiskDef
to resolve any storage pool reference. For some reason this was added
into the storage driver code, despite working entirely in terms of the
public APIs. Move it into the domain conf file and rename it to match the
object it modifies.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Currently the QEMU driver will call directly into the network driver
impl to modify resolve the atual type of NICs with type=network. It
has todo this before it has allocated the actual NIC. This introduces
a callback system to allow us to decouple the QEMU driver from the
network driver.
This is a short term step, as it ought to be possible to achieve the
same end goal by simply querying XML via the public network API. The
QEMU code in question though, has no virConnectPtr conveniently
available at this time.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The QEMU driver calls into the network driver to get the first IP
address of the network. This information is readily available via the
formal public API by fetching the XML doc and then parsing it.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Currently the QEMU driver will call directly into the network driver
impl to modify network device bandwidth for interfaces with
type=network. This introduces a callback system to allow us to decouple
the QEMU driver from the network driver.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Currently virt drivers will call directly into the network driver impl
to allocate domain interface devices where type=network. This introduces
a callback system to allow us to decouple the virt drivers from the
network driver.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The QEMU driver loadable module needs to be able to resolve all ELF
symbols it references against libvirt.so. Some of its symbols can only
be resolved against the storage_driver.so loadable module which creates
a hard dependancy between them. By moving the storage file backend
framework into the util directory, this gets included directly in the
libvirt.so library. The actual backend implementations are still done as
loadable modules, so this doesn't re-add deps on gluster libraries.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Refreshing the halted state can cause VM performance issues. Since
s390 is currently the only architecture with a known interest in
the halted state, we're avoiding to call QEMU on other platforms.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
Since it may be possible that the state is unknown in some cases we
should store it as a tristate so that other code using it can determine
whether the state was updated.
Don't extract the halted state into a separate array, but rater access
the vcpu structures directly. We still need to call the vcpu helper to
retrieve the performance statistics though.
https://bugzilla.redhat.com/show_bug.cgi?id=916061
If the QEMU version running is new enough (based on the DUMP_COMPLETED
event), then we can add a 'detach' boolean to the dump-guest-memory
command in order to tell QEMU to run in a thread. This ensures that we
don't lock out other commands while the potentially long running dump
memory is completed.
This allows the usage of a qemuDumpWaitForCompletion which will wait
for the event while the qemuDomainGetJobInfoDumpStats can be used via
qemuDomainGetJobInfo in order to query QEMU to determine how far along
the job is.
Now that we have a true async job, we'll only set the dump_memory_only
flag only when @detach=false; otherwise, we note that the job is a
for stats dump this allows the opposite end for job info to determine
what to copy.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Add an API to allow fetching the memory only dump statistics
for a job via the qemuDomainGetJobInfo API.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Add the query-dump API's in order to allow the dump-guest-memory
to be used to monitor progress. This will use the dump stats
extraction helper to fill a return buffer.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Handle a DUMP_COMPLETED event processing the status, stats, and
error string. Use the @status in order to copy the error that
was generated whilst processing the @stats data. If an error was
provided by QEMU, then use that instead.
If there's no async job, we can just ignore the data.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
The event will be fired when the domain memory only dump completes.
Fill in a return buffer to store/pass along the dump statistics that
will be eventually shared by a query-dump command. Also pass along
the status of the filling and any possible error received.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Define the qemuMonitorDumpStats as a new job JobStatsType to handle
being able to get memory dump statistics. For now do nothing with
the new TYPE_MEMDUMP.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Add a TYPE_SAVEDUMP so that when coalescing stats for a save or
dump we don't needlessly try to get the mirror stats for a migration.
Other conditions can still use MIGRATION and SAVEDUMP interchangably
including usage of the @migStats field to fetch/store the data.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Convert the stats field in _qemuDomainJobInfo to be a union. This
will allow for the collection of various different types of stats
in the same field.
When starting the async job that will end up being used for stats,
set the @statsType value appropriately. The @mirrorStats are
special and are used with stats.mig in order to generate the
returned job stats for a migration.
Using the NONE should avoid the possibility that some random
async job would try to return stats for migration even though
a migration is not in progress.
For now a migration and a save job will use the same statsType
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Since one of the things in capabilities (info from resctrl updated with data
about caches) can be change on the system by remounting the /sys/fs/resctrl with
different options, the capabilities need to be refreshed. There is a better fix
in the works, but it's going to be way bigger than this (hence the XXX note
there), so for the time being let's workaround this. And in order not to slow
down the domain starting, only get the capabilities if there are any cachetunes.
Relates-to: https://bugzilla.redhat.com/show_bug.cgi?id=1540780
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Add and use qemuProcessEventFree for freeing qemuProcessEvents. This
is less error-prone as the compiler can help us make sure that for
every new enumeration value of qemuProcessEventType the
qemuProcessEventFree function has to be adapted.
All process*Event functions are *only* called by
qemuProcessHandleEvent and this function does the freeing by itself
with qemuProcessEventFree. This means that an explicit freeing of
processEvent->data is no longer required in each process*Event
handler.
The effectiveness of this change is also demonstrated by the fact that
it fixes a memory leak of the panic info data in
qemuProcessHandleGuestPanic.
Reported-by: Wang Dong <dongdwdw@linux.vnet.ibm.com>
Signed-off-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Use the return value of virObjectRef directly. This way, it's easier
for another reader to identify the reason why the additional reference
is required.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Add the DUMP_COMPLETED check to the capabilities. This is the
mechanism used to determine whether the dump-guest-memory command
can support the "-detach" option and thus be able to wait on the
event and allow for a query of the progress of the dump.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Extract out the parts of qemuDomainGetJobStatsInternal that get
the migration stats. We're about to add the ability to get just
dump information.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Use a switch statement instead of if-else-if statements. Move the
command line building of the iothread attribute into the common path
as the SCSI controller attributes are already validated.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Move the SATA controller check from command line building to
controller def validation. This includes copying the SATA
skip check found in qemuBuildSkipController.
Move the qemuCaps checks over to qemuDomainControllerDefValidatePCI.
This requires two test updates in order to set the correct capability
bit for an xml2xml test as well as setting up the similar capability
for the pseries memlocktest.
Excluding the qemuCaps checks, move the remainder of the checks
that validate whether the PCI definition is valid or not into
qemuDomainControllerDefValidatePCI.
Similar to the checking the modelName vs. NAME_NONE, let's make the
ModelNameTypeToString check more generic too within the checking done
in controller validation (with the same ignore certain models.
NB: We need to keep the ModelNameTypeToString fetch in command line
validation since we use it, but at least we can assume it returns
something valid now.
Move the various modelName == NAME_NONE from the command line
generation into domain controller validation. Also rather than
have multiple cases with the same check, let's make the code
more generic, but also note that it was the modelName option
that caused the failure. We also have to be sure not to check
the PCI models that we don't care about.
For the remaining checks in command line building, we can use
the field name in the error message to be more specific about
what causes the failure.
We format the 'chassis' and 'port' properties on the QEMU command
line later on, so we should make sure they've been set.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Move PCI validation checks out of qemu_command into the proper
qemu_domain validation helper.
Since there's a lot to move, we'll start slow by replicating the
pcie-root and pci-root avoidance from qemuBuildSkipController and
the first switch found in qemuBuildControllerDevStr.
Move SCSI validation from qemu_command into qemu_domain.
Rename/reorder the args in qemuCheckSCSIControllerIOThreads
to match the caller as well as fixing up the comments to
remove the previously removed qemuCaps arg.
Modify the SCSI controller switch during command line building
to account for all virDomainControllerModelSCSI types rather
than using the default label.
Move the checks that various attributes are not set on any controller
other than SCSI controller using virtio-scsi model into the common
controller validate checks.
https://bugzilla.redhat.com/show_bug.cgi?id=1461214
Since fec8f9c49a we try to use predictable file names for
'memory-backend-file' objects. But that made us provide full path
to qemu when hot plugging the object while previously we provided
merely a directory. But this makes qemu behave differently. If
qemu sees a path terminated with a directory it calls mkstemp()
and unlinks the file immediately. But if it sees full path it
just calls open(path, O_CREAT ..); and never unlinks the file.
Therefore it's up to libvirt to unlink the file and not leave it
behind.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
In my first approach in 4b480d1076 I overlooked the comment in
qemuMigrationRunIncoming stating that during actual migration the
qemuMigrationRunIncoming does not wait until the migration is complete
but rather offloads that to the Finish phase of migration.
This means that during actual migration qemuProcessRefreshState was
called prior to qemu actually transferring the full state and thus the
queries did not get the correct information. The approach worked only
for restore, where we wait for the migration to finish during qemu
startup.
Fix the issue by calling qemuProcessRefreshState both from
qemuProcessStart if there's no incomming migration and from
qemuMigrationFinish so that the code actually works as expected.
In 2074ef6cd4 and c56cdf259 (and friends) we've added two
attributes to virtio NICs: rx_queue_size and tx_queue_size.
However, sysadmins might want to set these on per-host basis but
don't necessarily have an access to domain XML (e.g. because they
are generated by some other app). So let's expose them under
qemu.conf (the settings from domain XML still take precedence as
they are more specific ones).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Commit id 'bc444666f' added a check if the returned data
buffer had an error, but failed to adjust the event from
VIR_DOMAIN_BLOCK_JOB_COMPLETED to VIR_DOMAIN_BLOCK_JOB_FAILED
in order to propagate an error such as "File descriptor in bad
state" that may be returned from QEMU when both @offset and
@len are set to 0 such as is the case when performing an async
block job read on a read only filesystem.
Signed-off-by: Jie Wang <wangjie88@huawei.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Use the DEVICE_MISSING error code when helpers fail to find
the requested device. This makes it easier for consumers to
key off the error code rather than the error message.
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Modify OPERATION_FAILED and INTERNAL_ERROR error codes to
use DEVICE_MISSING instead for failures associated with the
inability to find the device. This makes it easier for consumers
to key off the error code rather than the error message.
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Now that the controller model is updated during post parse callback,
this code no longer needs to fetch the model based on the capabilities
and can just return the model directly if the controller is found.
Removal of @qemuCaps cascades through various callers which are now
updated to not pass the capabilities.
Now that post parse processing handles setting the SCSI controller
model, there's no need to call qemuDomainGetSCSIControllerModel to
get the "default controller" when building the command line controller
string or when assigning the spaprvio address since the controller
model value will already be filled in.
During post parse processing, let's force setting the controller
model to default value if not already set for defined controllers
(e.g. the non implicit ones).
If we're going to add a controller to the domain, let's set the
default SCSI model value if we cannot find another SCSI controller
already present.
NB: Requires updating the live output test data since the model
will now be formatted.
Rename and rework qemuDomainSetSCSIControllerModel since we're
really not setting the SCSI controller model. Instead the code
is either returning the existing SCSI controller model value, the
default value based on the capabilities, or -1 with the error set.
Rather than repeat multiple steps in order to find the SCSI
controller model, let's combine them into one helper that will
return either the model from the definition or the default
model based on the capabilities.
This patch adds an extra check/error that the controller
that's being found actually exists. This just clarifies that
the error was because the controller doesn't exist rather
than the more generic error that we were unable to determine
the model from qemuDomainSetSCSIControllerModel when a -1
was passed in and the capabilities were unable to find one.
As it turns out virDomainDeviceFindControllerModel was only ever
called for SCSI controllers using VIR_DOMAIN_CONTROLLER_TYPE_SCSI
as a parameter.
So rename to virDomainDeviceFindSCSIController and rather than
return a model, let's return a virDomainControllerDefPtr to let
the caller reference whatever it wants.
Rather than one function serving two purposes, let's split out the
else condition which is checking whether the model can be used
during command line building based on the capabilities.
Signed-off-by: John Ferlan <jferlan@redhat.com>
During reconnect we need to reconstruct the paths of all cachetunes so that they
get cleaned up when the domain is stopped.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
The virresctrl will use this as well and we need to have that info after restart
to properly clean up /sys/fs/resctrl.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Pointed out during review on one or two places, but it actually appears in lot
more places. So let's be consistent.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
After processing the processEvent->data for a qemuProcessEventHandler
callout, it's expected that the called processEvent->eventType helper
will perform the proper free on the data field. In this case it's
a qemuMonitorEventPanicInfoPtr.
We've been building up to this. This adds support for cputune/cachetune
settings for domains in the QEMU driver. The addition into
qemuProcessSetupVcpu() automatically adds support for hotplug. For hot-unplug
we need to remove the allocation only if all the vCPUs were unplugged. But
since the threads are left running, we can't really do much about it now.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1289368
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
This wires up the previously added OEM strings XML schema to be able to
generate comamnd line args for QEMU. This requires QEMU >= 2.12 release
containing this patch:
commit 2d6dcbf93fb01b4a7f45a93d276d4d74b16392dd
Author: Daniel P. Berrange <berrange@redhat.com>
Date: Sat Oct 28 21:51:36 2017 +0100
smbios: support setting OEM strings table
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
We recently added a generic XHCI USB3 controller to QEMU, and libvirt
supports adding that controller rather than the NEC XHCI USB3
controller, but when auto-adding a USB controller to Q35 domains we
were still adding the vendor-specific NEC controller. This patch
changes to add the generic controller instead, if it's available in
the QEMU binary that will be used.
Signed-off-by: Laine Stump <laine@laine.org>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Whenever a different kernel is booted, some capabilities related to KVM
(such as CPUID bits) may change. We need to refresh the cache to see the
changes.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
qemuDomainDefValidateVideo() (called from qemuDomainDefValidate()) is
just a loop performing various checks on each video device. Rather
than maintaining this separate function, just fold the validations
into qemuDomainDeviceDefValidateVideo(), which is called once for each
video device.
Commit 10c73bf1 fixed a bug that I had introduced back in commit
70249927 - if a vhost-scsi device had no manually assigned PCI
address, one wouldn't be assigned automatically. There was a slight
problem with the logic of the fix though - in the case of domains with
pcie-root (e.g. those with a q35 machinetype),
qemuDomainDeviceCalculatePCIConnectFlags() will attempt to determine
if the host-side PCI device is Express or legacy by examining sysfs
based on the host-side PCI address stored in
hostdev->source.subsys.u.pci.addr, but that part of the union is only
valid for PCI hostdevs, *not* for SCSI hostdevs. So we end up trying
to read sysfs for some probably-non-existent device, which fails, and
the function virPCIDeviceIsPCIExpress() returns failure (-1).
By coincidence, the return value is being examined as a boolean, and
since -1 is true, we still end up assigning the vhost-scsi device to
an Express slot, but that is just by chance (and could fail in the
case that the gibberish in the "hostside PCI address" was the address
of a real device that happened to be legacy PCI).
Since (according to Paolo Bonzini) vhost-scsi devices appear just like
virtio-scsi devices in the guest, they should follow the same rules as
virtio devices when deciding whether they should be placed in an
Express or a legacy slot. That's accomplished in this patch by
returning early with virtioFlags, rather than erroneously using
hostdev->source.subsys.u.pci.addr. It also adds a test case for PCIe
to assure it doesn't get broken in the future.
https://bugzilla.redhat.com/show_bug.cgi?id=1536461
This reverts commit aeda1b8c56.
Problem is that we need mon->lastError to be set because it's
used all over the place. Also, there's nothing wrong with
reporting error if one occurred. I mean, if there's a thread
executing an API and which currently is talking on monitor it
definitely wants the error reported.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
When migrating a shutoff domain (i.e., offline migration), we have no
statistics to report and thus jobInfo will be NULL in
qemuMigrationFinish.
Broken by me in v3.10.0-183-ge8784e7868.
https://bugzilla.redhat.com/show_bug.cgi?id=1536351
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
We read from QEMU until seeing a \r\n pair to indicate a completed reply
or event. To avoid memory denial-of-service though, we must have a size
limit on amount of data we buffer. 10 MB is large enough that it ought
to cope with normal QEMU replies, and small enough that we're not
consuming unreasonable mem.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Commit 7a931a4204 refactored the code and probably forgot to add
this line.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Add a check if it's a iSCSI hostdev and if it's not then don't use the
union member 'iscsi'. The segmentation fault occured when accessing
secinfo->type, but this can vary from case to case.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Libvirt 3.7.0 and earlier libvirt reported a migration job as completed
immediately after QEMU finished sending migration data at which point
migration was not really complete yet. Commit v3.7.0-29-g3f2d6d829e
fixed this, but caused a regression in reporting statistics for
completed jobs which started reporting the job as still running. This
happened because the completed job statistics including the job status
are copied from the running job before we finally mark it as completed.
Let's make sure QEMU_DOMAIN_JOB_STATUS_COMPLETED is always set in the
completed job info even when the job has not finished yet.
https://bugzilla.redhat.com/show_bug.cgi?id=1523036
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
When reconnecting to a running domain with host-model CPU started by old
libvirt which did not store the actual CPU in the status XML, we need to
ignore the fallback attribute to make sure we can translate the detected
host CPU model to a model which is supported by the running QEMU.
https://bugzilla.redhat.com/show_bug.cgi?id=1532980
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
virSecurityManagerDomainSetPathLabel is used to make a path known
to the security modules, but today is used interchangably for
- paths to files/dirs to be accessed directly
- paths to a dir, but the access will actually be to files therein
Depending on the security module it is important to know which of
these types it will be.
The argument allowSubtree augments the call to the implementations of
DomainSetPathLabel that can - per security module - decide if extra
actions shall be taken.
For now dac/selinux handle this as before, but apparmor will make
use of it to add a wildcard to the path that was passed.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1527740
Users might use a block device as UEFI VAR store. Or even have
OVMF stored there. Therefore, when starting a domain and separate
mount namespace is used, we have to create all the /dev entries
that are configured for the domain.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Commit id '162efa1a' added support hotplug a redirdev, but
did not add the hot unplug. This patch will add that support
to allow usage of the detach-device --live on the device.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1528502
So imagine you have /dev/blah symlink which points to /dev/sda.
You attach /dev/blah as disk to your domain. Libvirt correctly
creates the /dev/blah -> /dev/sda symlink in the qemu namespace.
However, then you detach the disk, change the symlink so that it
points to /dev/sdb and tries to attach the disk again. This time,
however, the attach fails (well, qemu attaches wrong disk)
because the code assumes that symlinks don't change. Well they
do.
This is inspired by test fix written by Eduardo Habkost.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
When the -machine pseries,max-cpu-compat=X is supported use
machine parameter instead of -cpu host,compat=X parameter as
that is deprecated now with qemu >= v2.10.
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1519146
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Since we have user aliases it may happen that users want to
change it using 'update-device'. Instead of ignoring it silently,
error out loudly. Note that we don't limit the check just for
"ua-" prefixes because users might try to change libvirt
generated aliases too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The qemuMonitorJSONMakeCommand can properly handle a NULL string
by using the "S:" parameter instead of "s:", so let's use that
of having in if/else condition that only adds the "s:".
A microcode update can cause the CPUID bits to change; an example
from the past was the update that disabled TSX on several Haswell
and Broadwell machines.
Therefore, place microcode version in the virQEMUCaps struct and
XML, and rebuild the cache if the versions do not match.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
When qemuDomainFindOrCreateSCSIDiskController adds a controller,
let's use the same model as a currently found controller under the
assumption that the reason to add the controller in hotplug is
because virDomainHostdevAssignAddress determined that there were
too many devices on the existing controller, but only assigned a
new controller index and did not add a new controller and we
desire to use the same controller model as any existing controller
and not take a chance that qemuDomainSetSCSIControllerModel would
use a default that may be incompatible.
All calls to virDomainAuditCgroupPath() were passing 'rc == 0' as
argument, when it was supposed to pass the 'rc' value directly.
As a consequence, the audit events that were supposed to be
logged (actual cgroup changes) were never being logged, and bogus
audit events were logged when using regular files as disk image.
Fix all calls to use the return value of
virCgroup{Allow,Deny}Device*() directly as the 'rc' argument.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1448149
If a domain has no numa nodes, that means we don't put any
memory-backend-file onto the qemu command line. That in turn
means we can't set access='shared'. Therefore, we should produce
an error instead of ignoring the setting silently.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The PROBE macro used in qemuMonitorIOProcess and the VIR_DEBUG message
in qemuMonitorJSONIOProcess create a lot of logging churn when debug
logging is enabled during monitor communication.
The messages logged from the PROBE macro are rather useless since they
are reporting the partial state of receiving the reply from qemu. The
actual full reply is still logged in qemuMonitorJSONIOProcessLine once
the full message is received.
QEMU 2.7 and newer don't allow guests to start unless the initial
vCPUs count is a multiple of the vCPU hotplug granularity, so
validate it and report an error if needed.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1283700
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
While at the moment we're only performing a single check that is
connected to vCPU hotplugging, we're going to introduce a second
one soon. Move the topology check underneath the capability check
to make that easier; since, after this change, the 'topologycpus'
variable doesn't need to have function scope, we move its
declaration to the inner scope as well.
The comments around the check are modified in order to explain
the different QEMU versions involved.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Similar to qemuDomainAddChardevTLSObjects let's move the chardev
source must be TCP and it has the @haveTLS flag set checks before
trying to delete the TLS objects.
For the Chr device this represents no change; however, for RNG device
this is an additionaly check that was missed in commit id '68808516'.
Before adding the objects, TCP and haveTLS are checked.
Let's make a comment deletion helper similar to the Add helper
that can be called after the ExitMonitor.
The modify qemuDomainRemoveChrDevice and qemuDomainRemoveRNGDevice
to call the helper instead of inlining the copy and pasted code.
ebtables/iptables processing is skipped for any interface connected to
Open vSwitch (they have their own packet filtering), likewise for
midonet (according to
http://blog.midokura.com/2016/04/midonet-rule-chains), but libvirt
would allow adding a <filterref> to interfaces connected in these
ways, so the user might mistakenly believe they were being protected.
This patch checks for a non-NULL <virtualport> element for an
interface (or its network) and logs an error if <virtualport> and
<filterref> are both present. This could cause some previously working
domains to no longer start, but that's really the whole point of this
patch - to warn people that their filterref isn't protecting them as
they might have thought.
I don't bother checking this during post-parse validation, because
such a check would be incomplete - it's possible that a network would
have a <virtualport> that would be applied to an interface, and you
can't know that until the domain is started.
Resolves: https://bugzilla.redhat.com/1502754
When the <bandwidth> of an interface is changed with update-device,
the old settings are cleared with tc, then new settings added with
tc. But if the <bandwidth has been removed, the old settings weren't
being removed, so the bandwidth restrictions would still be active on
the interface although the interface status in libvirt showed that
they had been removed.
This patch fixes it by calling virNetDevBandwidthClear() if the
"modification" to the interface bandwidth was to completely clear
it.
An alternative could have been to modify virNetDevBandwidthSet() to
always clear existing bandwith settings at the beginning of the
function (currently it short circuits in that case, doing nothing),
but that would have led to cases where virNetDevBandwidthClear() was
now being called in cases where it previously wasn't, and while many
of those cases would be NOPs, there could be cases where it would
cause an error. The way this patch works, the ...Clear() function is
only called in cases where the ...Set() function had previously been
called successfully, so the risk of regression is minimized.
Resolves: https://bugzilla.redhat.com/1454709
Also call qemuDomainRemoveInputDevice if we receive the
event after the Detach API ends.
Commit 67486bb failed to include this.
https://bugzilla.redhat.com/show_bug.cgi?id=1524837
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
VM drivers may need to store additional private data to the status XML
so that it can be restored after libvirtd restart. Since not everything
is needed add a callback infrastructure, where VM drivers can add only
stuff they need.
Note that the private data is formatted as a <privateData> sub-element
of the <disk> or <backingStore> <source> sub-element. This is done since
storing it out of band (in the VM private data) would require a complex
matching process to allow to put the data into correct place.
Commit id '70249927b' neglected to cover this case because the test
had taken the "shortcut" to already add the <address>; however, when
the PCI address assignment code was adjusted by commit id '70249927'
the vhost-scsi (VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST) wasn't
covered thus returning a 0 for pciFlags. So I altered the tests too
to make sure it doesn't happen again.
Previously the qemuxml2xmloutdata was a softlink to the source
qemuxml2argvdata, so I unlinked and recreated the output file to
force generation of the adddress. Without the test changes, an
address generation returns:
libvirt: Domain Config error : internal error: Cannot automatically
add a new PCI bus for a device with connect flags 00
if an address was supplied in the test, a restart of libvirtd or
edit of a guest would display the following opaque message:
warning : qemuDomainCollectPCIAddress:1237 :
qemuDomainDeviceCalculatePCIConnectFlags() thinks that the device
with PCI address 0000:00:09.0 should not have a PCI address
where the address is related to the guest PCI address provided.
Commit id 'c5c96545' neglected to validate that the srcPriv was
non-NULL before dereferencing. Similar problem to what was fixed
by commit id '8056721c' but missed during multiple rebases and
code reworks.
ncpus would be -1 on error and the cleanup for loop would not be skipped
in this case.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1522706
If domain is active, but the undefine API was called without the
VIR_DOMAIN_UNDEFINE_KEEP_NVRAM flag set, the following incorrect
error message is produced:
error: Requested operation is not valid: cannot delete inactive domain with nvram
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Move the IDE controller check from command line building to
controller def validation. Also explicitly include the avoidance
check for the implicit IDE controller from qemuBuildSkipController.
Cause the IDE case for command line building to generate a
failure if called to add an IDE since that shouldn't happen
if the Validate code did the right thing.
Move the call to qemuDomainCheckCCWS390AddressSupport from
qemuBuildControllerDevStr to qemuDomainDeviceDefValidateController.
This means we will get the qemuCaps from the driver opaque
variable passed to qemuDomainDeviceDefValidate.
When reconnecting to a running domain started by old libvirt, which did
not change host-model into a custom CPU definition, we replace the CPU
definition with a specific CPU model from host capabilities. However,
that CPU model may not be supported by the running qemu process. We need
to translate the CPU model to one of the models which libvirt could have
used when starting the domain.
https://bugzilla.redhat.com/show_bug.cgi?id=1521202
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
virQEMUCapsProbeQMPCPUDefinitions is now a small wrapper which fills in
qemuCaps with CPU models fetched by virQEMUCapsFetchCPUDefinitions.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Since we are re-detecting the backing chain after pivoting to the active
block commit target (or block copy target) the disk index needs to be
reset to 0. This is necessary since we move a member of the backing
chain to disk->src but clear indexes only starting from
disk->src->backingStore. The freshly detected images have indexes
starting from 1, but since we've pivoted into an image which was
previously a backing store it would have a non-0 index.
The lookup function would then return the top of the chain for queries
like 'vda[1]' instead of the first backing store.
This problem will not be present once we keep the disk indexes stable.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1519745
Separate the logic of creating devices from their gathering.
Use this new function in qemuDomainNamespaceSetupHostdev and
qemuDomainNamespaceSetupDisk.
This patch pass event error up to the place where we can
use it. Error is passed only for sync blockjob event mode
as we can't use the error in async mode. In async mode we
just pass the event details to the client thru event API
but current blockjob event API can not carry extra parameter.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
==899== 39 bytes in 1 blocks are definitely lost in loss record 732 of 1,003
==899== at 0x4C2AEDF: malloc (vg_replace_malloc.c:299)
==899== by 0x8B68CE7: vasprintf (in /lib64/libc-2.25.so)
==899== by 0x55498D2: virVasprintfInternal (virstring.c:708)
==899== by 0x55499E7: virAsprintfInternal (virstring.c:729)
==899== by 0x2BECFFF0: qemuGetMemoryBackingBasePath (qemu_conf.c:1757)
==899== by 0x2BF23225: qemuStateInitialize (qemu_driver.c:893)
==899== by 0x563073D: virStateInitialize (libvirt.c:770)
==899== by 0x124CC4: daemonRunStateInit (libvirtd.c:834)
==899== by 0x55521CD: virThreadHelper (virthread.c:206)
==899== by 0x88D9686: start_thread (in /lib64/libpthread-2.25.so)
==899== by 0x8BEAEFE: clone (in /lib64/libc-2.25.so)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Raw local files do not pass through the backing store detector and thus
the code did not allocate the required backing store terminator for
them. Previously the terminating element would be formatted into the XML
since the default values used for the metadata allowed that. This is a
regression since a693fdba01 which was not detected in the review.
This patch also reverts all the changes in the test files.
Until now we would skip loading of the backing chain for files which
don't support backing chains only when starting up the VM. Move the
check from qemuProcessPrepareHostStorage with some adaptations so that's
always applied.
In status XML, we do not store the QEMU version information, we only
format all the capabilities. We dropped QEMU_CAPS_PCI_MULTIBUS
in commit 5b783379 which was released in libvirt 3.2.0.
Therefore the only way of telling if the already running domain
at the time of daemon restart has been started with a QEMU that does
use 'pci.0' or not on PPC is to look at the pci-root controller's
alias. This is not an option if the domain has a user-specified alias
for the pci-root.
Instead of reintroducing the capability, assume 'pci.0' when we have
no version information. That way the only left broken use case would
be the combination of user aliases and very old QEMU.
Partially reverts commit 3a37af1e4.
https://bugzilla.redhat.com/show_bug.cgi?id=1518148
We do not fill out qemuCaps->arch when parsing status XML.
Use def->os.arch like we do for PPC.
This fixes hotplug after daemon restart for domains that use
a user alias for the implicit pci-root on x86.
https://bugzilla.redhat.com/show_bug.cgi?id=1518148
For some corner cases, virQEMUCapsHasPCIMultiBus depends on the QEMU
version, which is by design not stored in the status XML and therefore
it cannot be fixed for all existing running domains.
Prefer the controller alias read from the status XML when formatting
PCI addresses and only fall back to using virQEMUCapsHasPCIMultiBus
if the alias is a user alias.
This fixes hotplug after daemon restart for domains not using user
aliases.
Partially reverts commit 937f3195.
https://bugzilla.redhat.com/show_bug.cgi?id=1518148
Adjust function descriptions of virQEMUCapsInitCPUModelS390 and
virQEMUCapsInitCPUModel to the changes introduced with
commitID 74fc32a955.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Even though we never format the device on the QEMU command line,
as it's a platform serial device that's not user-instantiable,
we should still make sure it's available before using it.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
All serial devices shoule have an associated capability.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
We should make sure the isa-serial device is available before
formatting it on the QEMU command line.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
All serial devices shoule have an associated capability.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Now that <serial> and <console> on s390/s390x behave a bit more like the
other architectures, remove this extra differentation, and use sclp
console by default for new guests. New virtio consoles can still be
added, and it is actually needed because of the limited number of
instances for sclp and sclplm.
This reverts commit b1c88c1476, whose
reasons are not totally clear.
Signed-off-by: Pino Toscano <ptoscano@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Introduce specific a target types with two models for the console
devices (sclp and sclplm) used in s390 and s390x guests, so isa-serial
is no more used for them.
This makes <serial> usable on s390 and s390x guests, with at most only
a single sclpconsole and one sclplmconsole devices usable in a single
guest (due to limitations in QEMU, which will enforce already at
runtime).
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1449265
Signed-off-by: Pino Toscano <ptoscano@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
We can finally introduce a specific target model for the pl011 device
used by mach-virt guests, which means isa-serial will no longer show
up to confuse users.
We make sure migration works in both directions by interpreting the
isa-serial target type, or the lack of target type, appropriately
when parsing the guest XML, and skipping the newly-introduced type
when formatting if for migration. We also verify that pl011 is not
used for non-mach-virt guests and add a bunch of test cases.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=151292
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The existing implementation set the address type for all serial
devices to spapr-vio, which made it impossible to use other devices
such as usb-serial and pci-serial; moreover, some decisions were
made based on the address type rather than the device type.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1512934
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
We can finally introduce a specific target model for the spapr-vty
device used by pSeries guests, which means isa-serial will no longer
show up to confuse users.
We make sure migration works in both directions by interpreting the
isa-serial target type, or the lack of target type, appropriately
when parsing the guest XML, and skipping the newly-introduced type
when formatting if for migration. We also verify that spapr-vty is
not used for non-pSeries guests and add a bunch of test cases.
This commit is best viewed with 'git show -w'.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1511421
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Instead duplicating the capability check for each possible target
model, introduce a small helper that matches the target model with
the corresponding capability and collapse all existing checks into
a single one.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Now that we've created a distinction between target type and target
model, with the latter being the concrete device name, it's time to
switch to formatting the model instead of the type.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Target model and target type must agree for the configuration
to make sense, so check that's actually the case and error out
otherwise.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Instead of validating each target type / address type combination
separately, create a small helper to perform the matching and
collapse all existing checks into a single one.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Instead of waiting until we get to command line generation, we can
validate the target for a char device much earlier.
Move all the checks out of qemuBuildSerialChrDeviceStr() and into
the new fuction. This will later allow us to validate the target
for platform devices.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
This is the first step in getting rid of the assumption that
isa-serial is the default target type for serial devices.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Having a separate function for char device handling is better than
adding even more code to qemuDomainDeviceDefPostParse().
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1425757
The blockdev-add code provides a mechanism to sanely provide user
and password-secret arguments for iscsi without placing them on the
command line to be viewable by a 'ps -ef' type command or needing
to create separate -iscsi devices for each disk/volume found.
So modify the iSCSI command line building to check for the presence
of the capability in order properly setup and use the domain master
secret object to encrypt the password in a secret object and alter
the parameters for the command line to utilize.
Modify the xml2argvtest to exhibit the syntax for both disk and
hostdev configurations.
Detect the capability via the query-qmp-schema for blockdev-add
to find the 'password-secret' parameter that will allow the iSCSI
code to use the master secret object to encrypt the secret for an
and only need to provide the object id of the secret on the command
line thus obsfuscating the passphrase.
Rather than picking apart the two pieces we need/want (path, hosts,
and auth)- let's allocate/use a virStorageSourcePtr for iSCSI storage.
The end result is that qemuBuildSCSIiSCSIHostdevDrvStr doesn't need
to "fake" one for the qemuBuildNetworkDriveStr call.
Libvirt prints an error on startup when it is missing host cpu model
information for any queried qemu binary. On s390 we only have host cpu model
information for kvm enabled qemu instances. So when virt type is not kvm, this
is actually not an error on s390.
This patch adds virt type as a parameter to virQEMUCapsInitCPUModelS390, and a
new return code 2 for virQEMUCapsInitCPUModel and virQEMUCapsInitCPUModelS390.
If the virt type is not kvm then we skip printing the scary error message
and return 2 because this case is actually expected behavior. The new return
code is meant to differentiate between the failure case and the case where we
simply expect the cpu model information to be unattainable.
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Move the setup of the disk attribute to the disk source prepare function
which will allow proper usage with JSON props and move the fallback
(legacy) generating code into the block which is executed with legacy
options.
As a side-effect of this change we can clean up propagation of 'cfg'
into the command generator.
Also it's nice to see that the test output is the same even when the
value is generated in a different place.
The 'file.password-secret' injection should be used only if we are using
the old formatter. When formatting the source string from the JSON
properties, the property should be added there.
Also drop the comment which refers to stuff that will not be used in
libvirt since -blockdev is the way to go.
Qemu has now an internal mechanism for locking images to fix specific
cases of disk corruption. This requires libvirt to mark the image as
shared so that qemu lifts certain restrictions.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1378242
'share-rw' for the disk device configures qemu to allow concurrent
access to the backing storage.
The capability is checked in various supported disk frontend buses since
it does not make sense to partially backport it.
Creating a snapshot would introduce a possibly unsupported member for
sharing into the backing chain. Add a check to prevent that from
happening.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1511480
Disk sharing between two VMs may corrupt the images if the format driver
does not support it. Check that the user declared use of a supported
storage format when they want to share the disk.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1511480
Storage source format backing a shared device (e.g. running a cluster
filesystem) needs to support the sharing so that metadata are not
corrupted. Add a central function for checking this.
Since we already have such support for libxl all we need is qemu
driver adjustment. And a test case.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This capability says if qemu is capable of specifying distances
between NUMA nodes on the command line. Unfortunately, there's no
real way to check this and thus we have to go with version check.
QEMU introduced this in 0f203430dd8 (and friend) which was
released in 2.10.0.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
When QEMU dies, we read its output stored in a log file and use it for
reporting a hopefully useful error. However, virReportError will trim
the message to (VIR_ERROR_MAX_LENGTH - 1) characters, which means the
end of the log (which likely contains the error message we want to
report) may get lost. We should trim the beginning of the log instead.
https://bugzilla.redhat.com/show_bug.cgi?id=1335534
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
When reading QEMU log for reporting it as an error message, we want to
skip "char device redirected to" line. However, this string is not
printed at the beginning of a line, which means STRPREFIX will never
find it.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Historically we've formatted a lot of the attributes of a disk (disk
geometry, etc) with -drive. Since we use -device now, they should be
formatted there. Extract them to a separate function for keeping
compatibility with SDcards which still use only -drive.
Start this by moving the geometry into a separate function.
When doing block commit we need to allow write for members of the
backing chain so that we can commit the data into them.
qemuDomainDiskChainElementPrepare was used for this which since commit
786d8d91b4 calls qemuDomainNamespaceSetupDisk which has very adverse
side-effects, namely it relabels the nodes to the same label it has in
the main namespace. This was messing up permissions for the commit
operation since its touching various parts of a single backing chain.
Since we are are actually not introducing new images at that point add a
flag for qemuDomainDiskChainElementPrepare which will refrain from
calling to the namespace setup function.
Calls from qemuDomainSnapshotCreateSingleDiskActive and
qemuDomainBlockCopyCommon do introduce new members all calls from
qemuDomainBlockCommit do not, so the calls are anotated accordingly.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1506072
https://bugzilla.redhat.com/show_bug.cgi?id=1434451
Just like in 9324f67a57 we need to put default sata alias
(which is hardcoded to "ide", obvious, right?) onto the command
line instead of the one provided by user.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This function only queries domain @def. It doesn't change it.
Therefore it should take const pointer.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The compiler can warn us if we add a value to the
virDomainChrSerialTargetType enumeration but forget to handle
it properly in the code. Let's take advantage of that.
This commit is best viewed with 'git diff -w'.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Add a separate capability for the sclplmconsole device, and check it
specifically instead of using QEMU_CAPS_DEVICE_SCLPCONSOLE for that too.
Signed-off-by: Pino Toscano <ptoscano@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Give a better name to the capability for the sclpconsole device.
Signed-off-by: Pino Toscano <ptoscano@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Up until now we assumed the spapr-vty device would always be
present, which is not very nice. Check for its availability before
using it instead.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Starting from qemu 2.11, the `-device vmcoreinfo` will create a fw_cfg
entry for a guest to store dump details, necessary to process kernel
dump with KASLR enabled and providing additional kernel details.
In essence, it is similar to -fw_cfg name=etc/vmcoreinfo,file=X but in
this case it is not backed by a file, but collected by QEMU itself.
Since the device is a singleton and shouldn't use additional hardware
resources, it is presented as a <feature> element in the libvirt
domain XML.
The device is arm/x86 only for now (targets that support fw_cfg+dma).
Related to:
https://bugzilla.redhat.com/show_bug.cgi?id=1395248
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Truncate the output so that it is only as big as is needed to fit all
the bits, not all the units from the map. This will be needed in the
future in order to properly format bitmaps for kernel's sysfs files.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This follows the virBitmapToData() function and, similarly to
virBitmapNewData(), we'll be able to have virBitmapNewString() later
on without name confusion.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This is similar to the virDomainQemuMonitorCommand API, it can change
the domain state in a way that libvirt may not understand.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Most of the time it's okay to leave this up to negotiation between
the guest and the host, but in some situations it can be useful to
manually decide the behavior, especially to enforce its availability.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1308743
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Setup everything related to disks in one place rather than calling in
from various places.
The change to ordering of the setup steps is necessary since secrets
need the master key to be present.
In some cases it does not make sense to pursue that the private data
will be allocated (especially when we don't need to put anything in it).
Ensure that the code works without it.
This also fixes few crashes pointed out in
https://bugzilla.redhat.com/show_bug.cgi?id=1510323
If creation of the main JSON object containing the storage portion of a
virStorageSource would fail but we'd allocate the server structure we'd
leak it. Found by coverity.
Return NULL right away in qemuBlockStorageSourceGetBackendProps when an
invalid storage source is presented so that virJSONValueObjectAdd isn't
called with a NULL argument.
Found by coverity.
https://bugzilla.redhat.com/show_bug.cgi?id=1434451
When testing user aliases it was discovered that for 440fx
machine type which has default IDE bus builtin, domain cannot
start if IDE controller has the user provided alias. This is
because for 440fx we don't put the IDE controller onto the
command line (since it is builtin) and therefore any device that
is plugged onto the bus must use the default alias.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
In 4f15707202 I've tried to make duplicates detection for
nested /dev mount better. However, I've missed the obvious case
when there are two same mount points. For instance if:
# mount --bind /dev/blah /dev/blah
# mount --bind /dev/blah /dev/blah
Yeah, very unlikely (in qemu driver world) but possible.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
In some cases management application needs to allocate memory for
qemu upfront and then just let qemu use that. Since we don't want
to expose path for memory-backend-file anywhere in the domain
XML, we can generate predictable paths. In this case:
$memoryBackingDir/libvirt/qemu/$shortName/$alias
where $shortName is result of virDomainDefGetShortName().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
When removing path where huge pages are call virFileDeleteTree
instead of plain rmdir(). The reason is that in the near future
there's going to be more in the path than just files - some
subdirs. Therefore plain rmdir() is not going to be enough.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
At the same time, move its internals into a separate function so
that they can be reused.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Very soon qemuBuildMemoryBackendStr() is going to use memory cell
aliases. Therefore set one. At the same time, move it a bit
further - if virAsprintf() fails, there's no point in setting
rest of the members.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Libvirt historically stores storage source path including the volume as
one string in the XML, but that is not really flexible enough when
dealing with the fields in the code. Previously we'd store the slash
separating the two as part of the image name. This was fine for gluster
but it's not necessary and does not scale well when converting other
protocols.
Don't store the slash as part of the path. The resulting change from
absolute to relative path within the gluster driver should be okay,
as the root directory is the default when accessing gluster.
Extract the part formatting the basic URI part so that it can be reused
to format JSON backing definitions. Parts specific to the command line
format will remain in qemuBuildNetworkDriveURI. The new function is
called qemuBlockStorageSourceGetURI.
Original implementation used 'SocketAddress' equivalent from qemu for
the disk server field, while qemu documentation specifies
'InetSocketAddress'. The backing store parser uses the correct parsing
function but the formatter used the incorrect one (and also with the
legacy mode enabled which was wrong).
