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.
The terminator would not be parsed properly since the XPath selector was
looking for an populated element, and also the code did not bother
assigning the terminating virStorageSourcePtr to the backingStore
property of the parent.
Some tests would catch it if there wasn't bigger fallout from the change
to backing store termination in a693fdba01. Fix them properly now.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1509110
https://bugzilla.redhat.com/show_bug.cgi?id=1497410
This reverts commit bc8a99ef06.
The vhostuser is not a TAP. Therefore our QoS code is not able to
set any bandwidth. I don't really understand what I was thinking.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This patch generates a NUMA distance-aware libxl description from the
information extracted from a NUMA distance-aware libvirt XML file.
By default, if no NUMA node distance information is supplied in the
libvirt XML file, this patch uses the distances 10 for local and 20
for remote nodes/sockets.
Signed-off-by: Wim ten Have <wim.ten.have@oracle.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Add support for describing NUMA distances in a domain's <numa> <cell>
XML description.
Below is an example of a 4 node setup:
<cpu>
<numa>
<cell id='0' cpus='0-3' memory='2097152' unit='KiB'>
<distances>
<sibling id='0' value='10'/>
<sibling id='1' value='21'/>
<sibling id='2' value='31'/>
<sibling id='3' value='21'/>
</distances>
</cell>
<cell id='1' cpus='4-7' memory='2097152' unit='KiB'>
<distances>
<sibling id='0' value='21'/>
<sibling id='1' value='10'/>
<sibling id='2' value='21'/>
<sibling id='3' value='31'/>
</distances>
</cell>
<cell id='2' cpus='8-11' memory='2097152' unit='KiB'>
<distances>
<sibling id='0' value='31'/>
<sibling id='1' value='21'/>
<sibling id='2' value='10'/>
<sibling id='3' value='21'/>
</distances>
<cell id='3' cpus='12-15' memory='2097152' unit='KiB'>
<distances>
<sibling id='0' value='21'/>
<sibling id='1' value='31'/>
<sibling id='2' value='21'/>
<sibling id='3' value='10'/>
</distances>
</cell>
</numa>
</cpu>
A <cell> defines a NUMA node. <distances> describes the NUMA distance
from the <cell> to the other NUMA nodes (the <sibling>s). For example,
in above XML description, the distance between NUMA node0 <cell id='0'
...> and NUMA node2 <sibling id='2' ...> is 31.
Valid distance values are '10 <= value <= 255'. A distance value of 10
represents the distance to the node itself. A distance value of 20
represents the default value for remote nodes but other values are
possible depending on the physical topology of the system.
When distances are not fully described, any missing sibling distance
values will default to 10 for local nodes and 20 for remote nodes.
If distance is given for A -> B, then we default B -> A to the same
value instead of 20.
Signed-off-by: Wim ten Have <wim.ten.have@oracle.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
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>
libvirt reports a fake NUMA topology in virConnectGetCapabilities
even if built without numactl support. The fake NUMA topology consists
of a single cell representing the host's cpu and memory resources.
Currently this is the case for ARM and s390[x] RPM builds.
A client iterating over NUMA cells obtained via virConnectGetCapabilities
and invoking virNodeGetMemoryStats on them will see an internal failure
"NUMA isn't available on this host" from virNumaGetMaxNode. An example
for such a client is VDSM.
Since the intention seems to be that libvirt always reports at least
a single cell it is necessary to return "fake" node memory statistics
matching the previously reported fake cell in case NUMA isn't supported
on the system.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
Simply add the 5.2 SDK header to the existing unified framework. No
other special handling is needed as there's no API break between
existing 5.1 and the just added 5.2.
There was a recent report of the xen-xl converter not handling
config files missing an ending newline
https://www.redhat.com/archives/libvir-list/2017-October/msg01353.html
Commit 3cc2a9e0 fixed a similar problem when parsing content of a
file but missed parsing in-memory content. But AFAICT, the better
fix is to properly set the end of the content when initializing the
virConfParserCtxt in virConfParse().
This commit reverts the part of 3cc2a9e0 that appends a newline to
files missing it, and fixes setting the end of content when
initializing virConfParserCtxt. A test is also added to check
parsing in-memory content missing an ending newline.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel P. Berrange <berrange@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>
Split on the last colon and avoid parsing port if the split remainder
contains the closing square bracket, so that IPv6 addresses are
interpreted correctly.
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>
In VirtualBox SAS and SCSI are separate controller types whereas libvirt
does not make such distinction. This patch adds support for attaching
the VBOX SAS controllers by mapping the 'lsisas1068' controller model in
libvirt XML to VBOX SAS controller type. If VBOX VM has disks attached
to both SCSI and SAS controller libvirt domain XML will have two
<controller type='scsci'> elements with index and model attributes set
accordingly. In this case, each respective <disk> element must have
<address> element specified to assign it to respective SCSI controller.
This patch adds <address> element to each <disk> device since device
names alone won't adequately reflect the storage device layout in the
VM. With this patch, the ouput produced by dumpxml will faithfully
reproduce the storage layout of the VM if used with define.
Previously any removable storage device without media attached was
omitted from domain XML dump. They're still (rightfully) omitted in
snapshot XML dump but need to be accounted properly to for the device
names to stay in 'sync' between domain and snapshot XML dumps.
Primer the code for further changes:
* move variable declarations to the top of the function
* group together free/release statements
* error check and report VBOX API calls used
If a VBOX VM has e.g. a SATA and SCSI disk attached, the XML generated
by dumpxml used to produce "sda" for both of those disks. This is an
invalid domain XML as libvirt does not allow duplicate device names. To
address this, keep the running total of disks that will use "sd" prefix
for device name and pass it to the vboxGenerateMediumName which no
longer tries to "compute" the value based only on current and max
port and slot values. After this the vboxGetMaxPortSlotValues is not
needed and was deleted.
Both vboxSnapshotGetReadWriteDisks and vboxSnapshotGetReadWriteDisks do
not need to free the def->disks on cleanup because it's being done by
the caller via virDomainSnaphotDefFree
This patch prepares the vboxSnapshotGetReadOnlyDisks and
vboxSnapshotGetReadWriteDisks functions for further changes so that
the code movement does not obstruct the gist of those future changes.
This is done primarily because we'll need to know the type of vbox
storage controller as early as possible and make decisions based on
that info.
With this patch, the vbox driver will no longer attach all supported
storage controllers by default even if no disk devices are associated
with them. Instead, it will attach only those that are implicitly added
by virDomainDefAddImplicitController based on <disk> element or if
explicitly specified via the <controller> element.
Since the VBOX API requires to register an initial VM before proceeding
to attach any remaining devices to it, any failure to attach such
devices should result in automatic cleanup of the initially registered
VM so that the state of VBOX registry remains clean without any leftover
"aborted" VMs in it. Failure to cleanup of such partial VMs results in a
warning log so that actual define error stays on the top of the error
stack.
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).
To allow merging this with other disk type checks we need to check
qemuCaps only when available, since some of the checks are executed on
disk cold-plug and thus capabilities should not be checked.
Make the checks optional by making them conditional on qemuCaps not
being NULL.
All of the error message are already in a conditional block with known
bus type. Inline the bus type rather than formatting it from a separate
variable.
The disk index validation is used only in very specific cases and does
not need to be performed otherwise. Move it out of the global check into
the usage place.
busid and unitid are ever used only if the device is an SD card due to
the check in qemuDiskBusNeedsDeviceArg. Since the SD card does not have
an bus or unit number, most of the code and command line formatter can
be removed since it will never be used.
In near future we will need more than just a plain VIR_STRDUP().
Better implement that in a separate function and in
qemuBuildMemoryBackendStr() which is complicated enough already.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This function works over domain definition and not domain object.
Its name is thus misleading.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
After the virNetDaemonAddServerPostExec call in virtlogd we should have
netserver refcount set to 2. One goes to netdaemon servers hashtable
and one goes to virt{logd,lock} own reference to netserver. Let's add
the missing increment in virNetDaemonAddServerPostExec itself while
holding the daemon lock.
Since lockd defers management of the @srv object by the presence
in the hash table, virLockDaemonNewPostExecRestart must Unref the
alloc'd Ref on the @srv object done as part of virNetDaemonAddServerPostExec
and virNetServerNewPostExecRestart processing. The virNetDaemonGetServer
in lock_daemon main will also take a reference which is Unref'd during
main cleanup.
Commit id '252610f7d' used a hash table to store the @srv, but
didn't handle the virObjectUnref if virNetDaemonNew failed nor
did it use virObjectUnref once successfully placed into the table
which will now be managing it's lifetime (and would cause the
virObjectRef if successfully inserted into the table).
When coverage build is enabled, gcc complains about it:
In file included from qemu/qemu_agent.h:29:0,
from qemu/qemu_driver.c:47:
qemu/qemu_driver.c: In function 'qemuDomainSetInterfaceParameters':
./conf/domain_conf.h:3397:1: error: inlining failed in call to
'virDomainNetTypeSharesHostView': call is unlikely and code size would
grow [-Werror=inline]
virDomainNetTypeSharesHostView(const virDomainNetDef *net)
^
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
This patch exposes additional methods of the native VBOX API to the
libvirt 'unified' vbox API to deal with IStorageController. The exposed
methods are:
* IStorageController->GetStorageControllerType()
* IStorageController->SetStorageControllerType()
* IMachine->GetStorageControllers()
Original code was checking for non empty disk source before proceeding
to actually attach disk device to VM. This prevented from creating
empty removable devices like DVD or floppy. Therefore, this patch
re-organizes the loop work-flow to allow such configurations as well as
makes the code follow better libvirt practices. Additionally, adjusted
debug logs to be more helpful - removed old ones and added new which
give more valuable info for troubleshooting.
Previously, if one tried to define a VBOX VM and the API failed to
perform the requested actions for some reason, it would just log the
error and move on to process remaining disk definitions. This is not
desired as it could result in incorrectly defined VM without the caller
even knowing about it. So now all the code paths that call
virReportError are now treated as hard failures as they should have
been.
Remove the setting since it's unused as of commit 34364df3 which should
have never copied it in from the old code which ended up getting removed
as part of commit c7c286c6.
This commit primes vboxAttachDrives for further changes so when they
are made, the diff is less noisy:
* move variable declarations to the top of the function
* add disk variable to replace all the def->disks[i] instances
* add cleanup at the end of the loop body, so it's all in one place
rather than scattered through the loop body. It's purposefully
called 'cleanup' rather than 'skip' or 'continue' because future
commit will treat errors as hard-failures.
Previously, the driver was computing VBOX's devicePort/deviceSlot values
based on device name and max port/slot values. While this worked, it
completely ignored <address> values. Additionally, libvirt's built-in
virDomainDiskDefAssignAddress already does a good job setting default
values on virDomainDeviceDriveAddress struct which we can use to set
devicePort and deviceSlot and accomplish the same result while allowing
the customizing those via XML. Also, this allows to remove some code
which will make further patches smaller.
When registering a VM we call OpenMedium on each disk image which adds it
to vbox's global media registry. Therefore, we should make sure to call
Close when unregistering VM so we cleanup the media registry entries
after ourselves - this does not remove disk image files. This follows
the behaviour of the VBoxManage unregistervm command.
Right-aligning backslashes when defining macros or using complex
commands in Makefiles looks cute, but as soon as any changes is
required to the code you end up with either distractingly broken
alignment or unnecessarily big diffs where most of the changes
are just pushing all backslashes a few characters to one side.
Generated using
$ git grep -El '[[:blank:]][[:blank:]]\\$' | \
grep -E '*\.([chx]|am|mk)$$' | \
while read f; do \
sed -Ei 's/[[:blank:]]*[[:blank:]]\\$/ \\/g' "$f"; \
done
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
When a user provides the backing chain, we will not need to re-detect
all the backing stores again, but should move to the end of the user
specified chain. Additionally if a user provides a full terminated chain
we should not attempt any further detection.
Separate it so that it deals only with single virStorageSource, so that
it can later be reused for full backing chain support.
Two aliases are passed since authentication is more relevant to the
'storage backend' whereas encryption is more relevant to the protocol
layer. When using node names, the aliases will be different.
qemuDomainGetImageIds and qemuDomainStorageFileInit are helpful when
trying to access a virStorageSource from the qemu driver since they
figure out the correct uid and gid for the image.
When accessing members of a backing chain the permissions for the top
level would be used. To allow using specific permissions per backing
chain level but still allow inheritance from the parent of the chain we
need to add a new parameter to the image ID APIs.
Until now we ignored user-provided backing chains and while detecting
the code inherited labels of the parent device. With user provided
chains we should keep this functionality, so label of the parent image
in the backing chain will be applied if an image-specific label is not
present.
Until now we ignored user-provided backing chains and while detecting
the code inherited labels of the parent device. With user provided
chains we should keep this functionality, so label of the parent image
in the backing chain will be applied if an image-specific label is not
present.
virSecuritySELinuxSetImageLabelInternal assigns different labels to
backing chain members than to the parent image. This was done via the
'first' flag. Convert it to passing in pointer to the parent
virStorageSource. This will allow us to use the parent virStorageSource
in further changes.
When the user provides backing chain, we don't need the full support for
traversing the backing chain. This patch adds a feature check for the
virStorageSourceAccess API.
The 'file access' module of the storage driver has few feature checks to
determine whether libvirt supports given storage driver method. The code
to retrieve the driver struct needed for the check is the same so it can
be extracted.
We handle incremental storage migration in a different way. The support
for this new (as of QEMU 2.10) parameter is only needed for full
coverage of migration parameters used by QEMU.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
We already support several ways of setting migration bandwidth and this
is not adding another one. With this patch we are able to read and write
this parameter using query-migrate-parameters and migrate-set-parameters
in one call with all other parameters.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The parameters used "migrate" prefix which is pretty redundant and
qemuMonitorMigrationParams structure is our internal representation of
QEMU migration parameters and it is supposed to use names which match
QEMU names.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
We already support setting the maximum downtime with a dedicated
virDomainMigrateSetMaxDowntime API. This patch does not implement
another way of setting the downtime by adding a new public migration
parameter. It just makes sure any parameter we are able to get from a
QEMU monitor by query-migrate-parameters can be passed back to QEMU via
migrate-set-parameters.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The check can be easily replaced with a simple test in the JSON
implementation and we don't need to update it every time a new parameter
is added.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The macro (now called PARSE_SET) is now usable for any type which needs
a *_set bool for indicating a valid value.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Linux kernel shows our "cmt" feature as "cqm". Let's mention the name in
the cpu_map.xml to make it easier to find.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Since vhostuser type is really a tap that is just plugged into
different type of bridge, supporting QoS is trivial.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
For instance, NET_TYPE_MCAST doesn't support setting QoS. Instead
of claiming success and doing nothing, we should be explicit
about that and report an error.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1427049
Use virStorageBackendCreateVolUsingQemuImg to apply the LUKS information
to the logical volume just created. As part of the processing of the
lvcreate command add 2MB to the capacity to account for the LUKS header
when it's determined that the volume desires to use encryption.
Refactor to extract out the LVCREATE command. This also removes the
need for the local @created since the error path can now only be reached
after the creation of the logical volume.
Signed-off-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1490279
Turns out the virStorageBackendVolResizeLocal did not differentiate
whether the target volume was a LUKS volume or not and just blindly
did the ftruncate() on the target volume.
Follow the volume creation logic (in general) and create a qemu-img
resize command to resize the target volume for LUKS ensuring that
the --object secret is provided as well as the '--image-opts' used
by the qemu-img resize logic to describe the path and secret ensuring
that it's using the luks driver on the volume of course.
Since all that was really needed was a couple of fields and building
the object can be more generic, let's alter the args a bit. This will
be useful shortly for adding the secret object for a volume resize
operation on a luks volume that will need a secret object.
Rather than passing just the path, pass the virStorageVolDefPtr as we're
going to need it shortly.
Also fix the order of code and stack variables in the calling function
virStorageBackendVolResizeLocal.
Some globbing chars in the domain name could be used to break out of
apparmor rules, so lets forbid these when in virt-aa-helper.
Also adding a test to ensure all those cases were detected as bad char.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Don't leak @blockNodes in the loop.
==226576== 7,120 bytes in 60 blocks are definitely lost in loss record 122 of 125
==226576== at 0x4835214: calloc (vg_replace_malloc.c:711)
==226576== by 0x4950D7B: virAllocN (viralloc.c:191)
==226576== by 0x49EB5BB: virXPathNodeSet (virxml.c:676)
==226576== by 0x104DB67: virQEMUCapsLoadCPUModels (qemu_capabilities.c:3738)
==226576== by 0x105510D: virQEMUCapsLoadCache (qemu_capabilities.c:3929)
==226576== by 0x104459F: qemuTestParseCapabilities (testutilsqemu.c:498)
==226576== by 0x1040DC9: testQemuCapsCopy (qemucapabilitiestest.c:105)
==226576== by 0x1041F07: virTestRun (testutils.c:180)
==226576== by 0x1040B45: mymain (qemucapabilitiestest.c:181)
==226576== by 0x104320F: virTestMain (testutils.c:1119)
==226576== by 0x1041149: main (qemucapabilitiestest.c:193)
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>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Hot-adding disks does not parse the full XML to generate apparmor rules.
Instead it uses -f <PATH> to append a generic rule for that file path.
580cdaa7: "virt-aa-helper: locking disk files for qemu 2.10" implemented
the qemu 2.10 requirement to allow locking on disks images that are part of
the domain xml.
But on attach-device a user will still trigger an apparmor deny by going
through virt-aa-helper -f, to fix that add the lock "k" permission to the
append file case of virt-aa-helper.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
When adding CPU usability blockers I forgot to properly free them when
in virDomainCapsCPUModelsDispose.
Reported-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The assumption so far was an average of 4 disks per guest.
But some architectures, like s390x, still often use plenty of smaller disks.
To include those in the considerations an assumption of an average of 10
disks is more reasonable.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
The initial assumption was ~2 files per guest, but some common setups
like Openstack drive up to 4 files per guest.
E.g. on Arm where the following XML leads to 4 file handles:
<serial type='file'>
<source path='/var/lib/nova/instances/7c0dcd78-.../console.log'/>
<target port='0'/>
<alias name='serial0'/>
</serial>
<console type='file'>
<source path='/var/lib/nova/instances/7c0dcd78-.../console.log'/>
<target type='serial' port='0'/>
<alias name='serial0'/>
</console>
With that in mind and the target to support 4k guests by default we
should raise the limit to 16k.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
QEMU identified a race condition between the device state serialization
and the end of storage migration. Both QEMU and libvirt needs to be
updated to fix this.
Our migration work flow is modified so that after starting the migration
we to wait for QEMU to enter "pre-switchover", "postcopy-active", or
"completed" state. Once there, we cancel all block jobs as usual. But if
QEMU is in "pre-switchover", we need to resume the migration afterwards
and wait again for the real end (either "postcopy-active" or
"completed" state).
Old QEMU will just enter either "postcopy-active" or "completed"
directly, which is still correctly handled even by new libvirt. The
"pre-switchover" state will only be entered if QEMU supports it and the
pause-before-switchover capability was enabled. Thus all combinations of
libvirt and QEMU will work, but only new QEMU with new libvirt will
avoid the race condition.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
This new capability enables a pause before device state serialization so
that we can finish all block jobs without racing with the end of the
migration. The pause is indicated by "pre-switchover" state. Once we're
done QEMU enters "device" migration state.
This patch just defines the new capability and QEMU migration states and
their mapping to our job states.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
VirutalBox has a IVRDEServerInfo structure available that
gives the effective runtime port that the VM is using when it's
running. This is useful when the "TCP/Ports" VBox property was set to
port range (e.g. via autoport = "yes" or via VBoxManage) in which
case it would be impossible to get the "active" port otherwise.
Originally autoport in vbox driver was setting the port to default value
(3389) which caused multiple VM instances use the same port. Since
libvirt XML does not allow to set port ranges, this patch changes the
"autoport" behavior to set VBox's "TCP/Ports" property to an arbitrary
port range (3389-3689) to avoid that issue.
The VBOX_SESSION_OPEN/CLOSE macros are only called in
_vboxDomainSnapshotRestore and they are unflexible because:
* assume the caller will have variable named "data"
* can only create Write lock type
As per above, it's not that hard to simply use the VBOX API directly.
When starting a domain with managed save image, we try to restore it
first. If the image is corrupted, we silently unlink it and just
normally start the domain. At this point the domain has no managed save
image, yet we did not reset the hasManagedSave flag.
https://bugzilla.redhat.com/show_bug.cgi?id=1460962
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
One of the usecases of iohelper is to read from pipe and write
to file with O_DIRECT. As we read from pipe we can have partial
read and then we fail to write this data because output file
is open with O_DIRECT and buffer size is not aligned.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Instead of enumerating all states which need to be turned into
QEMU_DOMAIN_JOB_STATUS_FAILED (and failing to add all of them), it's
better to mention just the one which needs to be left alone.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Almost every failure in qemuMigrationRun while we are talking to QEMU
monitor results in a jump to exit_monitor label. The only exception is
removed by this patch.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The "ret" variable is used for storing the return value of a function
and should not be used as a temporary variable.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Merge cancel and cancelPostCopy sections with the generic error section,
where we can easily decide whether canceling the ongoing migration is
required.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Let cleanup only do things common to both failure and success paths and
move error handling code inside the new "error" section.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Some code which was supposed to be executed only when migration
succeeded was buried inside the cleanup code.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
When adding a new job state it's useful to let the compiler complain
about places where we need to think about what to do with the new
state.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
We need to format alias even for inactive XMLs since that's the
way how users are going to identify their devices.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Since we will be allowing users to set device aliases and memory
devices are fragile when it comes to aliases we have to make sure
they won't change during migration. Other devices should be fine.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
They have to be unique within the domain. As usual, backwards
compatibility takes its price. In this particular situation we
have a device that is represented twice in a domain and so is its
alias.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
If driver that is calling the parse supports user supplied
aliases, they can be parsed even for inactive XMLs. However, to
avoid any clashes with aliases that libvirt generates, the user
ones have to have "ua-" prefix.
Note, that some drivers don't have notion of device aliases at
all. Also, in order to support user supplied aliases some extra
checks need to be done (e.g. during hotplug). Therefore we can't
just enable this feature for all the drivers. Thus we need a flag
that drivers set to tell parsing code that they can handle user
supplied device aliases.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
When assigning alias to a device we usually iterate over other
devices of its kind trying to find next index. We do this by
stripping down the prefix and then parsing number at the end,
Usually, if the prefix doesn't match the one we are expecting, we
just continue with next iteration. Except for couple of
functions: qemuGetNextChrDevIndex(),
qemuAssignDeviceRedirdevAlias() and qemuAssignDeviceShmemAlias().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Introduced by 6094d6ec7f.
Found by running libvirt-perl tests.
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
The function virEventRegisterImpl() checks the attempt to replace the
registered events. But there is a duplicate variable inside the IF statement.
The variable 'removeHandleImpl' was wrongly repeated. One of them needs to be
replaced by 'removeTimeoutImpl'.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The only remaining user of qemuMonitorGetMigrationCapability is our test
suite. Let's replace qemuMonitorGetMigrationCapability with
qemuMonitorGetMigrationCapabilities there and drop the unused function.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
All calls to qemuMonitorGetMigrationCapability in QEMU driver are
replaced with qemuMigrationCapsGet.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Each time we need to check whether a given migration capability is
supported by QEMU, we call query-migrate-capabilities QMP command and
lookup the capability in the returned list. Asking for the list of
supported capabilities once when we connect to QEMU and storing the
result in a bitmap is much better and we don't need to enter a monitor
just to check whether a migration capability is supported.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The new function is called qemuProcessInitMonitor and it will enter/exit
the monitor so that the caller doesn't have to deal with this.
The goal of this patch is to simplify the code in qemuConnectMonitor
which would otherwise be a bit hairy after the following patch.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Rather than a forward linked list, let's use the virHashTable in
order to manage the objsName data.
Requires numerous changes from List to Object management similar to
many other drivers/vir*obj.c modules
Since the virStorageEncryptionPtr encryption; is a member of
_virStorageSource it really should be allowed to be a subelement
of the disk <source> for various disk formats:
Source{File|Dir|Block|Volume}
SourceProtocol{RBD|ISCSI|NBD|Gluster|Simple|HTTP}
NB: Simple includes sheepdog, ftp, ftps, tftp
That way we can set up to allow the <encryption> element to be
formatted within the disk source, but we still need to be wary
from whence the element was read - see keep track and when it
comes to format the data, ensure it's written in the correct place.
Modify the qemuxml2argvtest to add a parse failure when there is an
<encryption> as a child of <disk> *and* an <encryption> as a child
of <source>.
The virschematest will read the new test files and validate from a
RNG viewpoint things are fine.
Since the virStorageAuthDefPtr auth; is a member of _virStorageSource
it really should be allowed to be a subelement of the disk <source>
for the RBD and iSCSI prototcols. That way we can set up to allow
the <auth> element to be formatted within the disk source.
Since we've allowed the <auth> to be a child of <disk>, we'll need
to keep track of how it was read so that when writing out we'll know
whether to format as child of <disk> or <source>. For the argv2xml
parsing, let's format under <source> as a preference. Do not allow
<auth> to be both a child of <disk> and <source>.
Modify the qemuxml2argvtest to add a parse failure when there is an
<auth> as a child of <disk> *and* an <auth> as a child of <source>.
Add tests to validate that if the <auth> was found in <source>, then
the resulting xml2xml and xml2arg works just fine. The two new .args
file are exact copies of the non "-source" version of the file.
The virschematest will read the new test files and validate from a
RNG viewpoint things are fine
Update the virstoragefile, virstoragetest, and args2xml file to show
the "preference" to place <auth> as a child of <source>.
Since the encryption information can also be disk source specific
move it from qemuDomainDiskPrivate to qemuDomainStorageSourcePrivate
Since the last allocated element from qemuDomainDiskPrivate is
removed, that means we no longer need qemuDomainDiskPrivateDispose.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Since the secret information is really virStorageSource specific
piece of data, let's manage the privateData from there instead of
at the Disk level.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Add the object definition and helpers to store security-related private
data for virStorageSources.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Introduce the bare necessities to add privateData to _virStorageSource.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
When commit id 'da86c6c22' added support for diskPriv->encinfo in
qemuDomainSecretDiskPrepare a change to qemuDomainSecretDiskDestroy
to was missed. Although qemuDomainDiskPrivateDispose probably would
do the trick.
Signed-off-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1495511
When creating new /dev for domain ran in namespace we try to
preserve all sub-mounts of /dev. Well, not quite all. For
instance if /dev/foo/bar and /dev/foo are both mount points, only
/dev/foo needs preserving. /dev/foo/bar is preserved with it too.
Now, to identify such cases like this one STRPREFIX() is used.
That is not good enough. While it works for [/dev/foo/bar;
/dev/foo] case, it fails for [/dev/prefix; /dev/prefix2] where
the strings share the same prefix but are in fact two different
paths. The solution is to use STRSKIP().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
This function is going to make decisions based on the features
set per each driver. For that we need the virDomainXMLOption
object.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Let's move all the virAsprintf()-s into separate functions for
better structure of the code. Later, when somebody wants to
generate a device alias, all they need is to expose the function.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
We have a special function for assigning aliases to RNG devices.
Use that instead of plain virAsprintf().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
It looks like the error message was copied from virsh, because
that's where we have @ctl. Nevertheless, it's @flags which is
invalid, not @ctl.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Back in the times of using 'pci_del', unplugging a device without
a PCI address was not wired up.
After completely removing support for qemu without QEMU_CAPS_DEVICE,
aliases are used to uniquely identify devices in all cases.
Remove the pointless validation of data that was already present
in the domain definition.
There are two more cases where we set an S390/CCW/PCI address
type based on the machine type.
Reuse qemuDomainEnsureVirtioAddress to reduce repetition.
Split out the common code responsible for reserving/assigning
PCI/CCW addresses for virtio disks into a helper function
for reuse by other virtio devices.
We pass the source.file to qemuCheckCCWS390AddressSupport for
the purpose of reporting an error message without actually checking
that the rng device is of type VIR_DOMAIN_RNG_BACKEND_RANDOM.
Change it to a hardcoded "rng" string, which also avoids
referring to the device by a host-side attribute.
There is one limitation for using this API, when the guest is started
with all actions set to "destroy" we put "-no-reboot" on the QEMU
command line. That cannot be changed while QEMU is running and
the QEMU process is always terminated no matter what is configured
for any action.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1460677
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
We need to send allowReboot in the migration cookie to ensure the same
behavior of the virDomainSetLifecycleAction() API on the destination.
Consider this scenario:
1. On the source the domain is started with:
<on_poweroff>destroy</on_poweroff>
<on_reboot>restart</on_reboot>
<on_crash>destroy</on_crash>
2. User calls an API to set "destroy" for <on_reboot>:
<on_poweroff>destroy</on_poweroff>
<on_reboot>destroy</on_reboot>
<on_crash>destroy</on_crash>
3. The guest is migrated to a different host
4a. Without the allowReboot in the migration cookie the QEMU
process on destination would be started with -no-reboot
which would prevent using the virDomainSetLifecycleAction() API
for the rest of the guest lifetime.
4b. With the allowReboot in the migration cookie the QEMU process
on destination is started without -no-reboot like it was started
on the source host and the virDomainSetLifecycleAction() API
continues to work.
The following patch adds a QEMU implementation of the
virDomainSetLifecycleAction() API and that implementation disallows
using the API if all actions are set to "destroy" because we add
"-no-reboot" on the QEMU command line. Changing the lifecycle action
is in this case pointless because the QEMU process is always terminated.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
This will be used later on in implementation of new API
virDomainSetLifecycleAction(). In order to use it, we need to store
the value in status XML to not lose the information if libvirtd is
restarted.
If some guest was started by old libvirt where it was not possible
to change the lifecycle action for running guest, we can safely
detect it based on the current actions from the status XML.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Extract the required data inside a function instead of passing it
all as arguments.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
There is no need to have two different enums where one has the same
values as the other one with some additions.
Currently for on_poweroff and on_reboot we allow only subset of actions
that are allowed for on_crash. This was covered in parse time using
two different enums. Now to make sure that we don't allow setting
actions that are not supported we need to check it while validating
domain config.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Commit v3.8.0-95-gfd885a06a dropped nmodels parameter from several APIs
in src/cpu/cpu.h, but failed to update all callers.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
If we find ourselves in the situation that the 'add' uevent has been
fired earlier than the sysfs tree for a device was created, we should
use the best-effort approach and give kernel some predetermined amount
of time, thus waiting for the attributes to be ready rather than
discarding the device from our device list forever. If those don't appear
in the given time frame, we need to move on, since libvirt can't wait
indefinitely.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1463285
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Since we have a number of places where we workaround timing issues with
devices, attributes (files in general) not being available at the time
of processing them by calling usleep in a loop for a fixed number of
tries, we could as well have a utility function that would do that.
Therefore we won't have to duplicate this ugly workaround even more.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Adjust udevEventHandleThread to be a proper thread routine running in an
infinite loop handling devices. The handler thread pulls all available
data from the udev monitor and only then waits until a wakeup signal for
new incoming data has been emitted by udevEventHandleCallback.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
This patch splits udevEventHandleCallback in two (introduces
udevEventHandleThread) in order to be later able to refactor the latter
to actually become a normal thread which will wait some time for the
kernel to create the whole sysfs tree for a device as we cannot do that
in the event loop directly.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
udevSetupSystemDev only needs the udev data lock to be locked because of
calling udevGetDMIData which accesses some protected structure members,
but it can do that on its own just fine, no need to hold the lock the
whole time.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
The driver locks are unnecessary here, since currently the cleanup is
only called from the main daemon thread, so we can't race here. Moreover
@devs and @privateData are self-lockable objects, so no problem there
either.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Since there's going to be a worker thread which needs to have some data
protected by a lock, the whole code would just simply get unnecessary
complex, since two sets of locks would be necessary, driver lock (for
udev monitor and event handle) and a mutex protecting thread-local data.
Given the future thread will need to access the udev monitor socket as
well, why not protect everything with a single lock, even better, by
converting the driver's private data to a lockable object, we get the
automatic object disposal feature for free.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
We need to perform a sanity check on the udev monitor before every
use so that we know nothing has changed in the meantime. The reason for
moving the code to a separate helper is to enhance readability and shift
the focus on the important stuff within the udevEventHandleCallback
handler.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Even though hal doesn't make use of it, the privileged flag is related
to the daemon/driver rather than the backend actually used.
While at it, get rid of some tab indentation in the driver state struct.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
There were a bunch of commentary blocks that were literally useless in
terms of describing what the code following them does, since most of
them were documenting "the obvious" or it just wouldn't help at all.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
So we have a syntax-check rule to catch all tab indents but it naturally
can't catch tab spacing, i.e. as a delimiter. This patch is a result of
running 'vim -en +retab +wq'
(using tabstop=8 softtabstop=4 shiftwidth=4 expandtab) on each file from
a list generated by the following:
find . -regextype gnu-awk \
-regex ".*\.(rng|syms|html|s?[ch]|py|pl|php(\.code)?)(\.in)?" \
| xargs git grep -lP "\t"
Signed-off-by: Erik Skultety <eskultet@redhat.com>
When formatting an inactive or migratable XML we will need to suppress
backing chain members which were detected from the disk to keep
semantics straight. This means we need to record, whether a
virStorageSource originates from autodetection.
The file object is needed when formatting the command line, but it makes
nesting of the objects less easy for use with blockdev. Separate the
wrapping into the 'file' object into a helper used specifically for disk
sources in the old code path.
Move qemuFreeKeywords into qemu_parse_command.c as
qemuParseKeywordsFree and call it rather than inline code
in multiple places.
Signed-off-by: Kothapally Madhu Pavan <kmp@linux.vnet.ibm.com>
Even though only family and model are used for matching CPUID data with
CPU models from cpu_map.xml, stepping is used by x86DataFilterTSX which
is supposed to disable TSX on CPU models with broken TSX support. Thus
we need to start parsing stepping from QEMU to make sure we don't
disable TSX on CPUs which provide working TSX implementation. See the
following patch for a real world example of such CPU.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
If same boot order is specified twice (or more) in domain xml
we call free for uninitiaziled loadparm on cleanup in virDomainDeviceBootParseXML
and SIGABRT (or similar) as a result.
When libvirt older than 3.9.0 reconnected to a running domain started by
old libvirt it could have messed up the expansion of host-model by
adding features QEMU does not support (such as cmt). Thus whenever we
reconnect to a running domain, revert to an active snapshot, or restore
a saved domain we need to check the guest CPU model and remove the
CPU features unknown to QEMU. We can do this because we know the domain
was successfully started, which means the CPU did not contain the
features when libvirt started the domain.
https://bugzilla.redhat.com/show_bug.cgi?id=1495171
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
When reconnecting to a domain started with a host-model CPU which was
started by old libvirt that did not replace host-model with the real CPU
definition, libvirt replaces the host-model CPU with the CPU from
capabilities (because this is what the old libvirt did when it started
the domain). Without this patch libvirt could use features unknown to
QEMU in the CPU definition which replaced the original host-model CPU.
Such domain would keep running just fine, but any attempt to migrate it
will fail and once the domain is saved or snapshotted, restoring it
would fail too.
In other words whenever we want to use the CPU definition from host
capabilities as a guest CPU definition, we have to filter the unknown
features.
https://bugzilla.redhat.com/show_bug.cgi?id=1495171
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
When migration fails, QEMU may provide a description of the error in
the reply to query-migrate QMP command. We can fetch this error and use
it instead of the generic "unexpectedly failed" message.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Express a properly terminated backing chain by putting a
virStorageSource of type VIR_STORAGE_TYPE_NONE in the chain. The newly
used helpers simplify this greatly.
The change fixes a bug as formatting an incomplete backing chain and
parsing it back would end up in expressing a terminated chain since
src->backingStoreRaw was not populated. By relying on the terminator
object this can be now processed appropriately.
Add helpers that will simplify checking if a backing file is valid or
whether it has backing store. The helper virStorageSourceIsBacking
returns true if the given virStorageSource is a valid backing store
member. virStorageSourceHasBacking returns true if the virStorageSource
has a backing store child.
Adding these functions creates a central points for further refactors.
Storage driver uses virStorageSource only partially to store it's
configuration but fully when parsing backing files of storage volumes.
This patch sets the 'type' field to a value other than
VIR_STORAGE_TYPE_NONE so that further patches can add a terminator
element to backing chains without breaking iteration.
The backing store indexes were not bound to the storage sources in any
way. To allow us to bind a given alias to a given storage source we need
to save the index in virStorageSource. The backing store ids are now
generated when detecting the backing chain.
Since we don't re-detect the backing chain after snapshots, the
numbering needs to be fixed there.
Existing qemuParseCommandLineMem() will parse "-m 4G" format string.
This patch allows it to parse "-m size=8126464k,slots=32,maxmem=33554432k"
format along with existing format. And adds a testcase to validate the changes.
Signed-off-by: Kothapally Madhu Pavan <kmp@linux.vnet.ibm.com>
Hyper-V uses its own specific memory management so no mapping is going to
be perfect. However, it is more correct to map Limit to max_memory (it
really is the upper limit of what the VM may potentially use) and keep
cur_balloon equal to total_memory.
The typical value returned from Hyper-V in Limit is 1 TiB, which is not
really going to work if interpreted as "startup memory" to be ballooned
away later.
Signed-off-by: Ladi Prosek <lprosek@redhat.com>
The code was vulnerable to SQL injection. Likely not a security issue due to
WMI SQL and other constraints but still lame. For example:
virsh # dominfo \"
error: failed to get domain '"'
error: internal error: SOAP fault during enumeration: code 's:Sender', subcode
'n:CannotProcessFilter', reason 'The data source could not process the filter.
The filter might be missing or it might be invalid. Change the filter and try
the request again. ', detail 'The WS-Management service cannot process the
request. The WQL query is invalid. '
This commit fixes the Hyper-V driver by escaping all WMI SQL string parameters.
The same command with the fix:
virsh # dominfo \"
error: failed to get domain '"'
error: Domain not found: No domain with name "
Signed-off-by: Ladi Prosek <lprosek@redhat.com>
"%s is not a Hyper-V server" is not a correct generalization of all possible
error conditions of hypervEnumAndPull. For example:
$ virsh --connect hyperv://localhost/?transport=http
Enter username for localhost [administrator]:
Enter administrator's password for localhost: <enters incorrect password>
error: failed to connect to the hypervisor
error: internal error: localhost is not a Hyper-V server
This commit removes the general virReportError from hypervInitConnection and
also the "Invalid query" virReportError from hypervSerializeEprParam, which
does not correctly describe the error either (virBufferCheckError has
already set a meaningful error message at that point).
The same scenario with the fix:
$ virsh --connect hyperv://localhost/?transport=http
Enter username for localhost [administrator]:
Enter administrator's password for localhost: <enters incorrect password>
error: failed to connect to the hypervisor
error: internal error: Transport error during enumeration: User, password or
similar was not accepted (26)
Signed-off-by: Ladi Prosek <lprosek@redhat.com>
The default_tls_x509_verify (and related) parameters in qemu.conf
control whether the QEMU TLS servers request & verify certificates
from clients. This works as a simple access control system for
servers by requiring the CA to issue certs to permitted clients.
This use of client certificates is disabled by default, since it
requires extra work to issue client certificates.
Unfortunately the code was using this configuration parameter when
setting up both TLS clients and servers in QEMU. The result was that
TLS clients for character devices and disk devices had verification
turned off, meaning they would ignore errors while validating the
server certificate.
This allows for trivial MITM attacks between client and server,
as any certificate returned by the attacker will be accepted by
the client.
This is assigned CVE-2017-1000256 / LSN-2017-0002
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Somewhere around commit 9ff9d9f reserving entire PCI slots was
eliminated, as demonstrated by commit 6cc2014.
Reserve the functions required by the implicit devices:
00:01.0 ISA Bridge
00:01.1 IDE Controller
00:01.2 USB Controller (unless USB is disabled)
00:01.3 Bridge
https://bugzilla.redhat.com/show_bug.cgi?id=1460143
Gather query-cpu-definitions results and use them for testing CPU model
usability blockers in CPUID to virCPUDef translation.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
When decoding CPUID data to virCPUDef we need to be careful about using
a CPU model which cannot be directly used on the current host. Normally,
libvirt would notice the features which prevent the model from being
usable and it would disable them in the computed virCPUDef, but this
won't work in case the definition of the CPU model in QEMU contains more
features than what we have in cpu_map.xml. We need to count with the
usability blockers we got from QEMU and explicitly disable all of them
to make the computed virCPUDef usable.
https://bugzilla.redhat.com/show_bug.cgi?id=1464832
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This internal API can be used to find a specific CPU model in
virDomainCapsCPUModels list.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The "preferred" parameter is not used by any caller of cpuDecode
anymore. It's only used internally in cpu_x86 to implement cpuBaseline.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
All APIs which expect a list of CPU models supported by hypervisors were
switched from char **models and int models to just accept a pointer to
virDomainCapsCPUModels object stored in domain capabilities. This avoids
the need to transform virDomainCapsCPUModelsPtr into a NULL-terminated
list of model names and also allows the various cpu driver APIs to
access additional details (such as its usability) about each CPU model.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
query-cpu-definitions QMP command returns a list of unavailable features
which prevent CPU models from being usable on the current host. So far
we only checked whether the list was empty to mark CPU models as
(un)usable. This patch parses all unavailable features for each CPU
model and stores them in virDomainCapsCPUModel as a list of usability
blockers.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
When a hypervisor marks a CPU model as unusable on the current host, it
may also give us a list of features which prevent the model from being
usable. Storing this list in virDomainCapsCPUModel will help the CPU
driver with creating a host-model CPU configuration.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The API makes a deep copy of a NULL-terminated string list.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Currently, if parsing of device info fails info->alias is freed.
It doesn't make much sense to leave the rest of the struct
behind.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
There's one 'return' in the middle of the function body. It's
very easy to miss and so it makes adding new code harder. Also
the function doesn't follow our style 100%.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1497396
In 0d3d020ba6 I've added capability to accept MAC addresses
for the API too. However, the implementation was faulty. It needs
to lookup the corresponding interface in the domain definition
and pass the ifname instead of MAC address.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Commit id '8708ca01c' added a check to determine whether the NIC had
Switchdev capabilities; however, in doing so inadvertently would cause
network devices without a PCI device to not be added to the node device
database. Thus, network devices having a "computer" as a parent, such
as "net_lo*", "net_virbr*", "net_tun*", "net_vnet*", etc. were not added.
Alter the check to not even check for Switchdev bits if no PCI device found.
https://bugzilla.redhat.com/show_bug.cgi?id=1497396
The other APIs accept both, ifname and MAC address. There's no
reason virDomainInterfaceStats can't do the same.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Every caller reports the error themselves. Might as well move it
into the function and thus unify it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Let's use the RWObjectLockable for the various list lock mgmt.
Only time need Write lock will be for Add/Remove logic.
Signed-off-by: John Ferlan <jferlan@redhat.com>
The command "info migrate" of qemu outputs the dirty-pages-rate during
migration, but page size is different in different architectures. So
page size should be output to calculate dirty pages in bytes.
Page size is already implemented with commit
030ce1f8612215fcbe9d353dfeaeb2937f8e3f94 in qemu.
Now Implement the counter-part in libvirt.
Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
libvirtd throws unhandled signal 11 on ppc while running
virsh cpu-compare with missing model tag in the xml. This
patch errors out in such situation.
Signed-off-by: Nitesh Konkar <nitkon12@linux.vnet.ibm.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Parse the -M (or -machine) command line option before starting
processing in earnest and have a fallback ready in case it's not
present, so that while parsing other options we can rely on
def->os.machine being initialized.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1379218
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
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>
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>
This commit fixes the deadlock introduced by commit
0980764dee. The call getgrouplist() of
the glibc library isn't safe to be called in between fork and
exec (see commit 75c125641a).
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Fixes: 0980764dee ("util: share code between virExec and virCommandExec")
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
These functions are used by an upcoming commit.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Rename virDomainNumaDefCPUFormat to virDomainNumaDefCPUFormatXML,
matching its peer virDomainNumaDefCPUParseXML and the general
vir*{Format,Parse}XML conventions.
Signed-off-by: Wim ten Have <wim.ten.have@oracle.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Generating libvirt packages per make rpm, "with-libxl=1" and "with-xen=1",
adds strict runtime dependencies per libxenlight for xen-libs package from
core libvirt-libs package. This is not necessary and unfortunate since
those dependencies set demand to "xen-libs" package even when there's no
need for libvirt xen or libxl driver components.
This patch is to have two separate xenconfig lib tool libraries: one for
core libvirt (without XL), and a another that contains xl for libxl driver
(libvirt_driver_libxl_impl.la) which when loading the driver, loads the
remaining symbols (xen{Format,Parse}XL. For the user/sysadmin, this means
the xen dependencies are moved into libxl driver, instead of core libvirt.
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Wim ten Have <wim.ten.have@oracle.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
In preparation for privatizing the object, use the accessor to fetch
the obj->def instead of the direct reference.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Modify virStoragePoolObjGetAutostartLink and
virStoragePoolObjGetConfigFile to return "const char *"
since that's how both are used and to ensure no one
tries to VIR_FREE the result.
To avoid any issues later on if paths ever change (unlikely but
possible) and to match the style of other generated rules the paths
of the static rules have to be quoted as well.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
libvirt allows spaces in vm names, there were issues in the past but it
seems not removed so the assumption has to be that spaces are continuing
to be allowed.
Therefore virt-aa-helper should not reject spaces in vm names anymore if
it is going to be refused causing issues then the parser or xml schema
should do so.
Apparmor rules are in quotes, so a space in a path based on the name works.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
If users only specified vendor&product (the common case) then parsing
the xml via virDomainHostdevSubsysUSBDefParseXML would only set these.
Bus and Device would much later be added when the devices are prepared
to be added.
Due to that a hot-add of a usb hostdev works as the device is prepared
and virt-aa-helper processes the new internal xml. But on an initial
guest start at the time virt-aa-helper renders the apparmor rules the
bus/device id's are not set yet:
p ctl->def->hostdevs[0]->source.subsys.u.usb
$12 = {autoAddress = false, bus = 0, device = 0, vendor = 1921, product
= 21888}
That causes rules to be wrong:
"/dev/bus/usb/000/000" rw,
The fix calls virHostdevFindUSBDevice after reading the XML from
virt-aa-helper to only add apparmor rules for devices that could be found
and now are fully known to be able to write the rule correctly.
It uncondtionally sets virHostdevFindUSBDevice mandatory attribute as
adding an apparmor rule for a device not found makes no sense no matter
what startup policy it has set.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Skip purging the backing chain and redetecting it when it was not going
to change during the time we were not present.
The decision is based on the new flag which records whether there were
blockjobs running to the status XML.
https://bugzilla.redhat.com/show_bug.cgi?id=1447169
Since domain can have at most one watchdog it simplifies things a
bit. However, since we must be able to set the watchdog action as
well, new monitor command needs to be used.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Currently we don't do it. Therefore we accept senseless
combinations of models and buses they are attached to.
Moreover, diag288 watchdog is exclusive to s390(x).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
For VMs with persistent config the config may change upon successful
completion of a job. Save it always if a persistent VM finishes a
blockjob. This will simplify further additions.
The status XML would be saved only for the copy job (in case of success)
or on failure even for other jobs. As the status contains the backing
chain data, which change after success we should always save it on
block job completion.
Checking of disk presence accesses storage on the host so it should be
done from the host setup function. Move the code to new function called
qemuProcessPrepareHostStorage and remove qemuDomainCheckDiskPresence.
Introduce a new function to prepare domain disks which will also do the
volume source to actual disk source translation.
The 'pretend' condition is not transferred to the new location since it
does not help in writing tests and also no tests abuse it.
Pass flags to the function rather than just whether we have incoming
migration. This also enforces correct startup policy for USB devices
when reverting from a snapshot.
qemuMigrationPrepareAny called multiple of the functions starting the
qemu process for incoming migration by adding the flags explicitly.
Extract them to a variable so that they can be easily used for other
calls or changed in the future.
Interestingly enough, we don't document the point of view of the
interface statistics. Therefore it's unknown to users if for
instance rx_packets is the number of packets received by domain or
received by host (from domain). Document this explicitly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Similarly to previous patch, for some types of interface domain
and host are on the same side of RX/TX barrier. In that case, we
need to set up the QoS differently. Well, swapped.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1497410
The comment in virNetDevTapInterfaceStats() implementation for
Linux states that packets transmitted by domain are received by
the host and vice versa. Well, this is true but not for all types
of interfaces. For instance, for macvtaps when TAP device is
hooked right onto a physical device any packet that domain sends
looks also like a packet sent to the host. Therefore, we should
allow caller to chose if the stats returned should be straight
copy or swapped.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Small wrapper to lookup interface in domain definition by its
name.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Users might have configured interface so that it's type of
network, but the corresponding network plugs interfaces into an
OVS bridge. Therefore, we have to check for the actual type of
the interface instead of the configured one.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This code compiles only on Linux. Therefore the condition we
check is always true.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
qemu 2.7.0 introduces multiqueue virtio-blk(commit 2f27059).
This patch introduces a new attribute "queues". An example of
the XML:
<disk type='file' device='disk'>
<driver name='qemu' type='qcow2' queues='4'/>
The corresponding QEMU command line:
-device virtio-blk-pci,scsi=off,num-queues=4,id=virtio-disk0
Signed-off-by: Lin Ma <lma@suse.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
When detaching an <interface/> from a domain, the MAC address is
parsed and if not present one is generated. If no corresponding
interface is found in the domain, the following error is
reported:
error: operation failed: no device matching mac address 52:54:00:75:32:5b found
where the MAC address is the auto generated one. This might be
very confusing. Solution to this is to ignore auto generated MAC
address when looking up the device.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
It will come handy to know if the MAC address was generated (e.g.
during XML parse) or if it was parsed since provided by user in
the XML.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Found by Coverity. If virNWFilterHashTablePut, then the 3rd arg @val
must be free'd since it would be leaked.
This also fixes potential problem on the error path where the caller
could assume the virNWFilterHashTablePut was successful when in fact
it failed leading to other issues.
Rather than using loop break;'s in order to force a return
of rc = -1, let's just return -1 immediately on the various
error paths and then return 0 on the success path.
virDomainDiskSourceParse got to the point of being an ugly spaghetti
mess by adding more and more stuff into it. Split out parsing of network
disk information into a separate function so that it stays contained.
On pure success paths, virNWFilterIPAddrMapAddIPAddr was validly
consuming the input @addr; however, on failure paths it was possible
that virNWFilterVarValueCreateSimple succeed, but virNWFilterHashTablePut
failed resulting in virNWFilterVarValueFree being called to clean
up @val which also cleaned up the input @addr. Thus the caller had
no way to determine on failure whether it too should clean up the
passed parameter.
Instead, let's create a copy of the input @addr, then handle that
properly in the API allowing/forcing the caller to free it's own
copy of the input parameter.
Alter qemu command line generation in order to possibly add TLS for
a suitably configured domain.
Sample TLS args generated by libvirt -
-object tls-creds-x509,id=objvirtio-disk0_tls0,dir=/etc/pki/qemu,\
endpoint=client,verify-peer=yes \
-drive file.driver=vxhs,file.tls-creds=objvirtio-disk0_tls0,\
file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,\
file.server.type=tcp,file.server.host=192.168.0.1,\
file.server.port=9999,format=raw,if=none,\
id=drive-virtio-disk0,cache=none \
-device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,\
id=virtio-disk0
Update the qemuxml2argvtest with a couple of examples. One for a
simple case and the other a bit more complex where multiple VxHS disks
are added where at least one uses a VxHS that doesn't require TLS
credentials and thus sets the domain disk source attribute "tls = 'no'".
Update the hotplug to be able to handle processing the tlsAlias whether
it's to add the TLS object when hotplugging a disk or to remove the TLS
object when hot unplugging a disk. The hot plug/unplug code is largely
generic, but the addition code does make the VXHS specific checks only
because it needs to grab the correct config directory and generate the
object as the command line would do.
Signed-off-by: Ashish Mittal <Ashish.Mittal@veritas.com>
Signed-off-by: John Ferlan <jferlan@redhat.com>
Introduce a function to setup any TLS needs for a disk source.
If there's a configuration or other error setting up the disk source
for TLS, then cause the domain startup to fail.
For VxHS, follow the chardevTLS model where if the src->haveTLS hasn't
been configured, then take the system/global cfg->haveTLS setting for
the storage source *and* mark that we've done so via the tlsFromConfig
setting in storage source.
Next, if we are using TLS, then generate an alias into a virStorageSource
'tlsAlias' field that will be used to create the TLS object and added to
the disk object in order to link the two together for QEMU.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Add an optional virTristateBool haveTLS to virStorageSource to
manage whether a storage source will be using TLS.
Sample XML for a VxHS disk:
<disk type='network' device='disk'>
<driver name='qemu' type='raw' cache='none'/>
<source protocol='vxhs' name='eb90327c-8302-4725-9e1b-4e85ed4dc251' tls='yes'>
<host name='192.168.0.1' port='9999'/>
</source>
<target dev='vda' bus='virtio'/>
</disk>
Additionally add a tlsFromConfig boolean to control whether the TLS
setting was due to domain configuration or qemu.conf global setting
in order to decide whether to Format the haveTLS setting for either
a live or saved domain configuration file.
Update the qemuxml2xmltest in order to add a test to show the proper
parsing.
Also update the docs to describe the tls attribute.
Signed-off-by: Ashish Mittal <Ashish.Mittal@veritas.com>
Signed-off-by: John Ferlan <jferlan@redhat.com>
Add a new TLS X.509 certificate type - "vxhs". This will handle the
creation of a TLS certificate capability for properly configured
VxHS network block device clients.
The following describes the behavior of TLS for VxHS block device:
(1) Two new options have been added in /etc/libvirt/qemu.conf
to control TLS behavior with VxHS block devices
"vxhs_tls" and "vxhs_tls_x509_cert_dir".
(2) Setting "vxhs_tls=1" in /etc/libvirt/qemu.conf will enable
TLS for VxHS block devices.
(3) "vxhs_tls_x509_cert_dir" can be set to the full path where the
TLS CA certificate and the client certificate and keys are saved.
If this value is missing, the "default_tls_x509_cert_dir" will be
used instead. If the environment is not configured properly the
authentication to the VxHS server will fail.
Signed-off-by: Ashish Mittal <Ashish.Mittal@veritas.com>
Signed-off-by: John Ferlan <jferlan@redhat.com>
The virNWFilterIPAddrMapAddIPAddr code can consume the @addr parameter
on success when the @ifname is found in the ipAddressMap->hashTable
hash table in the call to virNWFilterVarValueAddValue; however, if
not found in the hash table, then @addr is formatted into a @val
which is stored in the table and on return the caller would be
expected to free @addr.
Thus, the caller has no way to determine on success whether @addr was
consumed, so in order to fix this create a @tmp variable which will
be stored/consumed when virNWFilterVarValueAddValue succeeds. That way
the caller can free @addr whether the function returns success or failure.
The packet with passed FD has the following format:
--------------------------
| len | header | payload |
--------------------------
where "payload" has an additional count of FDs before the actual data:
------------------
| nfds | payload |
------------------
When the packet is received we parse the "header", which as a side
effect updates msg->bufferOffset to point to the beginning of "payload".
If the message call contains FDs, we need to also parse the count of
FDs, which also updates the msg->bufferOffset.
The issue here is that when we attempt to read the FDs data from the
socket and we receive EAGAIN we finish the reading and call poll()
to wait for the data the we need. When the data arrives we already have
the packet in our buffer so we read the "header" again but this time
we don't read the count of FDs because we already have it stored.
That means that the msg->bufferOffset is not updated to point to the
actual beginning of the payload data, but it points to the count of
FDs. After all FDs are processed we dispatch the message to process
it and decode the payload. Since the msg->bufferOffset points to wrong
data, we decode the wrong payload and the API call fails with
error messages:
Domain not found: no domain with matching uuid '67656e65-7269-6300-0c87-5003ca6941f2' ()
Broken by commit 133c511b52 which fixed a FD and memory leak.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
VM private data is cleared when the VM is turned off and also when the
VM object is being freed. Some of the clearing code was duplicated.
Extract it to a separate function.
This also removes the now unnecessary function
qemuDomainClearPrivatePaths.
Calling fallocate on the new (smaller) capacity ensures
that the whole file is allocated, but it does not reduce
the file size.
Also call ftruncate after fallocate.
https://bugzilla.redhat.com/show_bug.cgi?id=1366446
We have been trying to implement the ALLOCATE flag to mean
"the volume should be fully allocated after the resize".
Since commit b0579ed9 we do not allocate from the existing
capacity, but from the existing allocation value.
However this value is a total of all the allocated bytes,
not an offset.
For a sparsely allocated file:
$ perl -e 'print "x"x8192;' > vol1
$ fallocate -p -o 0 -l 4096 vol1
$ virsh vol-info vol1 default
Capacity: 8.00 KiB
Allocation: 4.00 KiB
Treating allocation as an offset would result in an incompletely
allocated file:
$ virsh vol-resize vol1 --pool default 16384 --allocate
Capacity: 16.00 KiB
Allocation: 12.00 KiB
Call fallocate from zero on the whole requested capacity to fully
allocate the file. After that, the volume is fully allocated
after the resize:
$ virsh vol-resize vol1 --pool default 16384 --allocate
$ virsh vol-info vol1 default
Capacity: 16.00 KiB
Allocation: 16.00 KiB
Introduce a new function virFileAllocate that will call the
non-destructive variants of safezero, essentially reverting
my commit 1390c268
safezero: fall back to writing zeroes even when resizing
back to the state as of commit 18f0316
virstoragefile: Have virStorageFileResize use safezero
This means that _ALLOCATE flag will no longer work on platforms
without the allocate syscalls, but it will not overwrite data
either.
Use an empty string to let qemu fill out the default.
This matches what's done in qemuBuildChrChardevStr.
https://bugzilla.redhat.com/show_bug.cgi?id=1454671
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This reverts commit edaf4ebe95.
This uses "reconnect" as attribute for <source> element, but we already
have a <reconnect> element for <source> element for chardev devices.
Since this is the same feature for different device it should be
presented in XML the same way.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Some values we read from the qemu monitor may be changed with the actual
state by the incoming migration. This means that we should refresh
certain things only after the migration has finished.
This is mostly visible in the cdrom tray state, which is by default
closed but may be opened by the guest OS. This would be refreshed before
qemu transferred the actual state and thus libvirt would think that the
tray is closed.
Note that this patch moves only a few obvious query commands. Others may
be moved later after individual assessment.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1463168
When the vcpu is successfully removed libvirt would remove the cgroup.
In cases when removal of the cgroup fails libvirt would report an error.
This does not make much sense, since the vcpu was removed and we can't
really do anything with the cgroup. This patch silences the errors from
cgroup removal.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1462092
It is possible (although possibly not very useful) to leave out
the service attribute when using <source mode='bind'/>
Fix the formatter bug introduced by commit 4a0da34 and format
the host when its present (checked for non-NULL inside
virBufferEscapeString) instead of basing it on the presence
of the service attribute.
https://bugzilla.redhat.com/show_bug.cgi?id=1455825
The alias recorded in disk->info.alias is the alias for the frontend
device but we are interested in the backend drive. This messed up the
disk node name extraction code as qemu reports the drive alias in the
block query commands. This was broken in the node name detector
refactoring done in commit 0175dc6ea0
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1494327
Seeing a log message saying 'flags=93' is ambiguous & confusing unless
you happen to know that libvirt always prints flags as hex. Change our
debug messages so that they always add a '0x' prefix when printing flags,
and '0' prefix when printing mode. A few other misc places gain a '0x'
prefix in error messages too.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Some distros (see diff) chose to backport QMP support rather than rebase
to newer version of qemu. As a hack they added the string 'libvirt' to
the qemu -help output. Remove this as downstream-only hacks should be
carried by downstream and not litter upstream.
This effectively reverts commit ff88cd5905
Because qemuDomainDefCopy needs a string representation of a domain
definition, there's no reason for calling the lower level
qemuDomainDefFormatBuf API.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
virDomainDefFormatInternal (called by qemuDomainDefFormatXMLInternal)
already checks for buffer errors and properly resets the buffer on
failure.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
instead of only unloading it. This makes sure old profiles don't pile up
in /etc/apparmor.d/libvirt and we get updates to modified templates on
VM restart.
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
After commit 8708ca01c0 libvirtd consistently aborts with "stack
smashing detected" when nodedev driver is initialized.
This is caused by nlmsg_parse() being told that its array of nlattr*
has CTRL_CMD_MAX (10) entries, when in fact it is declared to have
CTRL_ATTR_MAX (8) entries. Since all the entries are initialized to
NULL, the result is that nlmsg_parse is overwriting 2*(sizof(nlattr*))
bytes outside the array.
Signed-off-by: Laine Stump <laine@laine.org>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Commit id '5604c056' used the wrong API to generate the
<secret type='%s'..." field. The previous code used the
correct API as was done in commit id '6887af39'. The data
is actually a usage type not an auth type even though the
result is the same.
Passing a NULL value for the argument secAlias to the function
qemuDomainGetTLSObjects would cause a segmentation fault in
libvirtd.
Changed code to check before dereferencing a NULL secAlias.
Signed-off-by: Ashish Mittal <ashmit602@gmail.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1448268
When migrating to a file (e.g. when doing 'virsh save file'),
couple of things are happening in the thread that is executing
the API:
1) the domain obj is locked
2) iohelper is spawned as a separate process to handle all I/O
3) the thread waits for iohelper to finish
4) the domain obj is unlocked
Now, the problem is that while the thread waits in step 3 for
iohelper to finish this may take ages because iohelper calls
fdatasync(). And unfortunately, we are waiting the whole time
with the domain locked. So if another thread wants to jump in and
say copy the domain name ('virsh list' for instance), they are
stuck.
The solution is to unlock the domain whenever waiting for I/O and
lock it back again when it finished.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1471225
Commit id '99a2d6af2' was a bit too aggressive with determining whether
the provided path was a "physical" cd-rom in order to generate a taint
message due to the possibility of some guest and host trying to control
the tray. For cd-rom guest devices backed to some VIR_STORAGE_TYPE_FILE
storage, this wouldn't be a problem and as such it shouldn't be a problem
for guest devices using some sort of block device on the host such as
iSCSI, LVM, or a Disk pool would present.
So before issuing a taint message, let's check if the provided path of
the VIR_STORAGE_TYPE_BLOCK backed device is a "known" physical cdrom name
by comparing the beginning of the path w/ "/dev/cdrom" and "/dev/sr".
Also since it's possible the provided path could resolve to some /dev/srN
device, let's get that path as well and perform the same check.
Signed-off-by: John Ferlan <jferlan@redhat.com>
The virSocketAddrFormat() allocates the string and it's caller
responsibility to free it afterwards.
==28857== 11 bytes in 1 blocks are definitely lost in loss record 37 of 168
==28857== at 0x4C2BEDF: malloc (vg_replace_malloc.c:299)
==28857== by 0x9A81D79: strdup (in /lib64/libc-2.23.so)
==28857== by 0x5DA3BF0: virStrdup (virstring.c:902)
==28857== by 0x5D96182: virSocketAddrFormatFull (virsocketaddr.c:427)
==28857== by 0x5D95E13: virSocketAddrFormat (virsocketaddr.c:352)
==28857== by 0x5706890: qemuBuildHostNetStr (qemu_command.c:3891)
==28857== by 0x57138D3: qemuBuildInterfaceCommandLine (qemu_command.c:8597)
==28857== by 0x5713D6A: qemuBuildNetCommandLine (qemu_command.c:8699)
==28857== by 0x57176F6: qemuBuildCommandLine (qemu_command.c:10027)
==28857== by 0x5769D61: qemuProcessCreatePretendCmd (qemu_process.c:6004)
==28857== by 0x4056EC: testCompareXMLToArgv (qemuxml2argvtest.c:502)
==28857== by 0x41DF40: virTestRun (testutils.c:180)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Since commit v2.2.0-199-g7ce711a30e libvirt stores an updated guest CPU
in domain's live definition and there's no need to update it every time
we want to format the definition. The commit itself tried to address
this in qemuDomainFormatXML, but forgot to fix qemuDomainDefFormatLive.
Not to mention that masking a previously set flag is only acceptable if
the flag was set by a public API user. Internally, libvirt should have
never set the flag in the first place.
https://bugzilla.redhat.com/show_bug.cgi?id=1485022
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
When a user requested a domain XML description with
VIR_DOMAIN_XML_UPDATE_CPU flag, libvirt would use the host CPU
definition from host capabilities rather than the one which will
actually be used once the domain is started.
https://bugzilla.redhat.com/show_bug.cgi?id=1481309
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
