https://bugzilla.redhat.com/show_bug.cgi?id=1349696
As it turns out using only the 'parent' to achieve the goal of a
consistent vHBA parent has issues with reboots where the scsi_hostX
parent could change to scsi_hostY causing either failure to create
the vHBA or usage of the wrong HBA for our vHBA.
Thus add the ability to search for the "parent" by the parent wwnn/
wwpn values or just a fabric_name if someone only cares to ensure
usage of the same SAN for the vHBA.
Add new fields to the fchost structure to allow creation of a vHBA via
the storage pool when a parent_wwnn/parent_wwpn or parent_fabric_wwn is
supplied in the storage pool XML.
https://bugzilla.redhat.com/show_bug.cgi?id=1349696
When creating a vHBA, the process is to feed XML to nodeDeviceCreateXML
that lists the <parent> scsi_hostX to use to create the vHBA. However,
between reboots, it's possible that the <parent> changes its scsi_hostX
to scsi_hostY and saved XML to perform the creation will either fail or
create a vHBA using the wrong parent.
So add the ability to provide "wwnn" and "wwpn" or "fabric_wwn" to
the <parent> instead of a name of the scsi_hostN that is the parent.
The allowed XML will thus be:
<parent>scsi_host3</parent> (current)
or
<parent wwnn='$WWNN' wwpn='$WWPN'/>
or
<parent fabric_wwn='$WWNN'/>
Using the wwnn/wwpn or fabric_wwn ensures the same 'scsi_hostN' is
selected between hardware reconfigs or host reboots. The fabric_wwn
Using the wwnn/wwpn pair will provide the most specific search option,
while fabric_wwn will at least ensure usage of the same SAN, but maybe
not the same scsi_hostN.
This patch will add the new fields to the nodedev.rng for input purposes
only since the input XML is essentially thrown away, no need to Format
the values since they'd already be printed as part of the scsi_host
data block.
New API virNodeDeviceGetParentHostByWWNs will take the parent "wwnn" and
"wwpn" in order to search the list of devices for matching capability
data fields wwnn and wwpn.
New API virNodeDeviceGetParentHostByFabricWWN will take the parent "fabric_wwn"
in order to search the list of devices for matching capability data field
fabric_wwn.
Qemu has abandoned the +/-feature syntax in favor of key=value. Some
architectures (s390) do not support +/-feature. So we update libvirt to handle
both formats.
If we detect a sufficiently new Qemu (indicated by support for qmp
query-cpu-model-expansion) we use key=value else we fall back to +/-feature.
Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
When qmp query-cpu-model-expansion is available probe Qemu for its view of the
host model. In kvm environments this can provide a more complete view of the
host model because features supported by Qemu and Kvm can be considered.
Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
query-cpu-model-expansion is used to get a list of features for a given cpu
model name or to get the model and features of the host hardware/environment
as seen by Qemu/kvm.
Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
On s390, the host's features are heavily influenced by not only the host
hardware but also by hardware microcode level, host OS version, qemu
version and kvm version. In this environment it does not make sense to
attempt to report exact host details.
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Acked-by: Jiri Denemark <jdenemar@redhat.com>
Implement compare for s390. Required to test the guest against the host for
guest cpu model runnability checking. We always return IDENTICAL to bypass
Libvirt's checking. s390 will rely on Qemu to perform the runnability checking.
Implement update for s390. required to support use of cpu "host-model" mode.
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Acked-by: Jiri Denemark <jdenemar@redhat.com>
Just so it doesn't bite us in the future, even though it's unlikely.
And fix the comment above it as well. Commit e08ee7cd3405 took the
info from the function it's calling, but that was lie itself in the
first place.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
The resulting function virFileGetMountSubtreeImpl() just uses
virStringSortRevCompare or virStringSortCompare which uses strcmp().
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
With my namespace patches, we are spawning qemu in its own
namespace so that we can manage /dev entries ourselves. However,
some filesystems mounted under /dev needs to be preserved in
order to be shared with the parent namespace (e.g. /dev/pts).
Currently, the list of mount points to preserve is hardcoded
which ain't right - on some systems there might be less or more
items under real /dev that on our list. The solution is to parse
/proc/mounts and fetch the list from there.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Our STREQ_NULLABLE and STRNEQ_NULLABLE macros are too
complicated. This was a result of some broken version of gcc.
However, that is long gone and therefore we can simplify the
macros.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
If we restart libvirtd while VM was doing external memory snapshot, VM's
state be updated to paused as a result of running a migration-to-file
operation, and then VM will be left as paused state. In this case we must
restart the VM's CPUs to resume it.
Signed-off-by: Wang King <king.wang@huawei.com>
Rather than extraneous VIR_FREE's depending on where we are in the code,
move them to the top of the loop and in the cleanup path.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Move the check for an already existing vHBA to the top of the function.
No sense in first decoding a provided parent if the next thing we're going
to do is fail if a provided wwnn/wwpn already exists.
Signed-off-by: John Ferlan <jferlan@redhat.com>
If a <parent> is not supplied in the XML used to create a non-persistent
vHBA, then instead of failing, let's try to find a "vports" capable node
device and use that.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Extract out code from virNodeDeviceGetParentHost into helpers - it's
going to be reused in upcoming patches to search on more fields
Create virNodeDeviceFindVPORTCapDef in order to return a virNodeDevCapsDefPtr
of the VPORT_OPS and virNodeDeviceFindFCParentHost to use the function and
generate an error message if the device doesn't have the capability.
Also clean up the processing in virNodeDeviceGetParentHost to remove
need for goto's.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Commit 4b951d1e38259ff5d03e9eedb65095eead8099e1 missed the fact that the
VM needs to be resumed after a live external checkpoint (memory
snapshot) where the cpus would be paused by the migration rather than
libvirt.
Again, not something that I'd hit, but there is a chance in
theory that this might bite us. Currently the way we decide
whether or not to create /dev entry for a device is by marching
first four characters of path with "/dev". This might be not
enough. Just imagine somebody has a disk image stored under
"/devil/path/to/disk". We ought to be matching against "/dev/".
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Not that I'd encounter any bug here, but the code doesn't look
100% correct. Imagine, somebody is trying to attach a device to a
domain, and the device's /dev entry already exists in the qemu
namespace. This is handled gracefully and the control continues
with setting up ACLs and calling security manager to set up
labels. Now, if any of these steps fail, control jump on the
'cleanup' label and unlink() the file straight away. Even when it
was not us who created the file in the first place. This can be
possibly dangerous.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1406837
Imagine you have a domain configured in such way that you are
assigning two PCI devices that fall into the same IOMMU group.
With mount namespace enabled what happens is that for the first
PCI device corresponding /dev/vfio/X entry is created and when
the code tries to do the same for the second mknod() fails as
/dev/vfio/X already exists:
2016-12-21 14:40:45.648+0000: 24681: error :
qemuProcessReportLogError:1792 : internal error: Process exited
prior to exec: libvirt: QEMU Driver error : Failed to make device
/var/run/libvirt/qemu/windoze.dev//vfio/22: File exists
Worse, by default there are some devices that are created in the
namespace regardless of domain configuration (e.g. /dev/null,
/dev/urandom, etc.). If one of them is set as backend for some
guest device (e.g. rng, chardev, etc.) it's the same story as
described above.
Weirdly, in attach code this is already handled.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Clang 3.9 refuses to compile the existing code with the
following error:
util/virfirewall.c:425:20: error: passing an object that undergoes
default argument promotion to 'va_start'
has undefined behavior [-Werror,-Wvarargs]
va_start(args, layer);
^
util/virfirewall.c:420:37: note: parameter of type 'virFirewallLayer'
is declared here
virFirewallLayer layer,
^
This happens because 'layer' is of type virFirewallLayer, which
is an enum type and not a standard type such as eg. void* or int.
To solve the issue, turn virFirewallAddRule() from a very thin
wrapper around virFirewallAddRuleFullV() to a macro that expands
to a call to virFirewallAddRuleFull() - itself a very thin wrapper
around the aforementioned virFirewallAddRuleFullV() - with no loss
of functionality or type safety.
https://bugzilla.redhat.com/show_bug.cgi?id=1405269
If a secret was not provided for what was determined to be a LUKS
encrypted disk (during virStorageFileGetMetadata processing when
called from qemuDomainDetermineDiskChain as a result of hotplug
attach qemuDomainAttachDeviceDiskLive), then do not attempt to
look it up (avoiding a libvirtd crash) and do not alter the format
to "luks" when adding the disk; otherwise, the device_add would
fail with a message such as:
"unable to execute QEMU command 'device_add': Property 'scsi-hd.drive'
can't find value 'drive-scsi0-0-0-0'"
because of assumptions that when the format=luks that libvirt would have
provided the secret to decrypt the volume.
Access to unlock the volume will thus be left to the application.
After 478ddedc12 a bug is fixed where we wrongly presumed loopack
device name on non-Linux systems. It's lo0. However, the fix is
not reflected in the tests which are failing now.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Due to nature of operations we do over the string list (more
precisely due to how virStringListRemove() works), it is not the
best idea to use dataFree callback. Problem is, on MAC address
remove, the string list remove function modifies the original
list in place. Then, virHashUpdateEntry() is called which frees
all the data stored in the list rendering @newMacsList point to
freed data.
==16002== Invalid read of size 8
==16002== at 0x50BC083: virFree (viralloc.c:582)
==16002== by 0x513DC39: virStringListFree (virstring.c:251)
==16002== by 0x51089B4: virMacMapHashFree (virmacmap.c:67)
==16002== by 0x50EF30B: virHashAddOrUpdateEntry (virhash.c:352)
==16002== by 0x50EF4FD: virHashUpdateEntry (virhash.c:415)
==16002== by 0x5108BED: virMacMapRemoveLocked (virmacmap.c:129)
==16002== by 0x51092D5: virMacMapRemove (virmacmap.c:346)
==16002== by 0x402F02: testMACRemove (virmacmaptest.c:107)
==16002== by 0x403F15: virTestRun (testutils.c:180)
==16002== by 0x4032C4: mymain (virmacmaptest.c:205)
==16002== by 0x405A3B: virTestMain (testutils.c:992)
==16002== by 0x403D87: main (virmacmaptest.c:237)
==16002== Address 0xdd5a4d0 is 0 bytes inside a block of size 24 free'd
==16002== at 0x4C2AD6F: realloc (vg_replace_malloc.c:693)
==16002== by 0x50BB99B: virReallocN (viralloc.c:245)
==16002== by 0x513DC0B: virStringListRemove (virstring.c:235)
==16002== by 0x5108BA6: virMacMapRemoveLocked (virmacmap.c:124)
==16002== by 0x51092D5: virMacMapRemove (virmacmap.c:346)
==16002== by 0x402F02: testMACRemove (virmacmaptest.c:107)
==16002== by 0x403F15: virTestRun (testutils.c:180)
==16002== by 0x4032C4: mymain (virmacmaptest.c:205)
==16002== by 0x405A3B: virTestMain (testutils.c:992)
==16002== by 0x403D87: main (virmacmaptest.c:237)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
In virMacMapRemoveLocked() we have two variables: @macsList and
@newMacsList. Obviously, @newMacsList is supposed to hold pointer
to modified list but in fact it holds pointer to the old list.
It's confusing.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Before, boot devices information for CTs was always empty and we
didn't indicate that containers can boot from disk.
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>
Virtuozzo SDK interface doesn't differ filesystems from disks and sees them as disks.
Before, we always mistakenly presented disks based on files as filesystems, which is
not completely correct. Now we are going to show either disks or filesystems depending
on a hint, which uses boot device section of VZ config. Though this information
doesn't change booting order of a CT, it is used by vz libvirt interface as a hint
for libvirt representation of disks. Since now, if we have filesystems in input xml,
then we add them to VZ booting devices list and rely on this information to show
corresponding libvirt xml.
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>
Implicit devices like controllers are confusing for CTs and
function virDomainDefAddImplicitDevices never intended to be called
for CTs.
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>
virQEMUCapsSupportsChardev existing checks returns true
for spapr-vty alone. Instead verify spapr-vty validity
and let the logic to return true for other device types
so that virtio-console passes.
The non-pseries machines dont have spapr-vio-bus. So, the
function always returned false for them before.
Fixes - https://bugzilla.redhat.com/show_bug.cgi?id=1257813
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1332019
This function will essentially be a wrapper to virStorageVolInfo in order
to provide a mechanism to have the "physical" size of the volume returned
instead of the "allocation" size. This will provide similar capabilities to
the virDomainBlockInfo which can return both allocation and physical of a
domain storage volume.
NB: Since we're reusing the _virStorageVolInfo and not creating a new
_virStorageVolInfoFlags structure, we'll need to generate the rpc APIs
remoteStorageVolGetInfoFlags and remoteDispatchStorageVolGetInfoFlags
(although both were originally created from gendispatch.pl and then
just copied into daemon/remote.c and src/remote/remote_driver.c).
The new API will allow the usage of a VIR_STORAGE_VOL_GET_PHYSICAL flag
and will make the decision to return the physical or allocation value
into the allocation field.
In order to get that physical value, virStorageBackendUpdateVolTargetInfoFD
adds logic to fill in physical value matching logic in qemuStorageLimitsRefresh
used by virDomainBlockInfo when the domain is inactive.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Although the virStorageBackendUpdateVolTargetInfo will update the
target.physical value, there is no way to provide that information
via the virStorageGetVolInfo API since it only returns the capacity
and allocation of a volume. So as described in commit id '0282ca45',
it should be possible to generate an output only <physical> value
for that purpose.
This patch generates the <physical> value in the volume XML output
for the sole purpose of being able to view/see the value to allow
someone to parse the XML in order to obtain the value.
Update the documentation to describe the output only nature.
Signed-off-by: John Ferlan <jferlan@redhat.com>
According to commit id '0282ca45a' the 'physical' value should
essentially be the last offset of the image or the host physical
size in bytes of the image container. However, commit id '15fa84ac'
refactored the GetBlockInfo to use the same returned data as the
GetStatsBlock API for an active domain. For the 'entry->physical'
that would end up being the "actual-size" as set through the
qemuMonitorJSONBlockStatsUpdateCapacityOne (commit '7b11f5e5').
Digging deeper into QEMU code one finds that actual_size is
filled in using the same algorithm as GetBlockInfo has used for
setting the 'allocation' field when the domain is inactive.
The difference in values is seen primarily in sparse raw files
and other container type files (such as qcow2), which will return
a smaller value via the stat API for 'st_blocks'. Additionally
for container files, the 'capacity' field (populated via the
QEMU "virtual-size" value) may be slightly different (smaller)
in order to accomodate the overhead for the container. For
sparse files, the state 'st_size' field is returned.
This patch thus alters the allocation and physical values for
sparse backed storage files to be more appropriate to the API
contract. The result for GetBlockInfo is the following:
capacity: logical size in bytes of the image (how much storage
the guest will see)
allocation: host storage in bytes occupied by the image (such
as highest allocated extent if there are no holes,
similar to 'du')
physical: host physical size in bytes of the image container
(last offset, similar to 'ls')
NB: The GetStatsBlock API allows a different contract for the
values:
"block.<num>.allocation" - offset of the highest written sector
as unsigned long long.
"block.<num>.capacity" - logical size in bytes of the block device
backing image as unsigned long long.
"block.<num>.physical" - physical size in bytes of the container
of the backing image as unsigned long long.
This patch detects a misconfiguration between the disk bus type and disk
address type for controller based disk buses (SATA, SCSI, FDC and
IDE). The addresses of these bus types are all managed in common code so
it's possible to decide in common code whether the disk address and bus
type are compatible or not.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Disk->info is not live updatable so add a check for this. Otherwise
libvirt reports success even though no data was updated.
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 function will be needed by the QEMU driver in an upcoming
patch. Additionally, removed a useless empty line.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
