The latter is impossible to mock on platforms that use the
gnulib implementation, such as FreeBSD, while the former
doesn't suffer from this limitation.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The storage file drivers are currently loaded as a side effect of
loading the storage driver. This is a bogus dependancy because the
storage file code has no interaction with the storage drivers, and
even ultimately be running in a completely separate daemon.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The storage file code needs to be run in the hypervisor drivers, while
the storage backend code needs to be run in the storage driver. Split
the source code as a preparatory step for creating separate loadable
modules.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The storage file code needs to be run in the hypervisor drivers, while
the storage backend code needs to be run in the storage driver. Split
the source code as a preparatory step for creating separate loadable
modules.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The driver.{c,h} files are primarily targetted at loading hypervisor
drivers and some helper functions in that area. It also, however,
contains a generically useful function for loading extension modules
that is called by the storage driver. Split that functionality off
into a new virmodule.{c,h} file to isolate it.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Currently the driver module loading code does not report an error if the
driver module is physically missing on disk. This is useful for distro
packaging optional pieces. When the daemons are split up into one daemon
per driver, we will expect module loading to always succeed. If a driver
is not desired, the entire daemon should not be installed.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The virFileFindResource method merely builds up the expected fully
qualified path to the resource. It does not actually check if it exists
on disk. The loadable module callers were mistakenly thinking a NULL
indicates the file doesn't exist on disk, whereas it in fact indicates
an out of memory error.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Now that we've activated two hacks to prevent unloading of modules,
there is no point passing back a pointer to the loaded library handle.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Ensuring that we don't call the virDrvConnectOpen method with a NULL URI
means that the drivers can drop various checks for NULL URIs. These were
not needed anymore since the probe functionality was split
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Declare what URI schemes a driver supports in its virConnectDriver
struct. This allows us to skip trying to open the driver entirely
if the URI scheme doesn't match.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Add a localOnly flag to the virConnectDriver struct which allows a
driver to indicate whether it is local-only, or permits remote
connections. Stateful drivers running inside libvirtd are generally
local only. This allows us to remote the check for uri->server != NULL
from most drivers.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This patch adds support to qcow2 formatted filesystem object storage by
instructing qemu-img to build them with preallocation=falloc whenever the
XML described storage <allocation> matches its <capacity>. For all other
cases the filesystem stored objects are built with preallocation=metadata.
Signed-off-by: Wim ten Have <wim.ten.have@oracle.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The storagePoolLookupByTargetPath() method in the storage driver is used
by the QEMU driver during block migration. If there's a valid use case
for this in the QEMU driver, then external apps likely have similar
needs. Exposing it in the public API removes the direct dependancy from
the QEMU driver to the storage driver.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The virStorageTranslateDiskSourcePool method modifies a virDomainDiskDef
to resolve any storage pool reference. For some reason this was added
into the storage driver code, despite working entirely in terms of the
public APIs. Move it into the domain conf file and rename it to match the
object it modifies.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The QEMU driver loadable module needs to be able to resolve all ELF
symbols it references against libvirt.so. Some of its symbols can only
be resolved against the storage_driver.so loadable module which creates
a hard dependancy between them. By moving the storage file backend
framework into the util directory, this gets included directly in the
libvirt.so library. The actual backend implementations are still done as
loadable modules, so this doesn't re-add deps on gluster libraries.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The storage driver backends are serving the public storage pools API,
while the storage file backends are serving the internal QEMU driver and
/ or libvirt utility code.
To prep for moving this storage file backend framework into the utility
code, split out the backend definitions.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Now that we can open connections to the secondary drivers on demand,
there is no need to pass a virConnectPtr into all the backend
functions.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Instead of passing around a virConnectPtr object, just open a connection
to the nodedev driver at time of use. Opening connections on demand will
be beneficial when the nodedev driver is in a separate daemon. It also
solves the problem that a number of callers just pass in a NULL
connection today which prevents nodedev lookup working at all.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Instead of passing around a virConnectPtr object, just open a connection
to the secret driver at time of use. Opening connections on demand will
be beneficial when the secret driver is in a separate daemon. It also
solves the problem that a number of callers just pass in a NULL
connection today which prevents secret lookup working at all.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Allow the possibility of opening a connection to only the storage
driver, by defining storage:///system and storage:///session URIs
and registering a fake hypervisor driver that supports them.
The hypervisor drivers can now directly open a storage driver
connection at time of need, instead of having to pass around a
virConnectPtr through many functions. This will facilitate the later
change to support separate daemons for each driver.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
By convention the last thing in the driver.c files should be the driver
callback table and function to register it.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Commit 000e950455 tried to fix improper bracketing when refreshing disk
volume stats for a backing volume. Unfortunately the condition is still
wrong as in cases as the backing store being inaccessible
storageBackendUpdateVolTargetInfo returns -2 if instructed to ignore
errors. The condition does not take this into account.
Dumping XML of a volume which has inacessible backing store would then
result into:
# virsh vol-dumpxml http.img --pool default
error: An error occurred, but the cause is unknown
Properly ignore -2 for backing volumes.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1540022
Alter the logic such that we only add the volume to the pool once
we've filled in all the information and cause failure to go to a
common error: label. Patches to place the @vol into a few hash tables
will soon "require" that at least the keys (name, target.path, and key)
be populated with valid data.
For a disk backend, the deleteVol code will clear all the
volumes in the pool and perform a pool refresh, thus the
storageVolDeleteInternal should not use access @voldef
after deleteVol succeeds.
After commit a693fdb 'vol-dumpxml' missed the ability to show backingStore
information. This commit adds a volume type for files that fixes this
problem.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1529663
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Now that we have a private storage pool list, we can take the next
step and convert to using objects. In this case, we're going to use
RWLockable objects (just like every other driver) with two hash
tables for lookup by UUID or Name.
Along the way the ForEach and Search API's will be adjusted to use
the related Hash API's and the various FindBy functions altered and
augmented to allow for HashLookup w/ and w/o the pool lock already
taken.
After virStoragePoolObjRemove we will need to virObjectUnref(obj)
after to indicate the caller is "done" with it's reference. The
Unlock occurs during the Remove.
The NumOf, GetNames, and Export functions all have their own callback
functions to return the required data and the FindDuplicate code
can use the HashSearch function callbacks.
Commit id '5ab746b8' introduced the function as perhaps a copy
of storageVolLookupByPath; however, it did not use the @cleanpath
variable even though it used the virFileSanitizePath. So in essance
the only "check" being done for failure is whether it was possible
to strdup the path.
Looking at the virStoragePoolDefParseXML one will note that the
target.path is stored using the result of virFileSanitizePath.
Therefore, this function should sanitize and use the input @path
for the argument to storagePoolLookupByTargetPathCallback which
is comparing against stored target.path values.
Additionally, if there was an error we should use the proper error
of VIR_ERR_NO_STORAGE_POOL (instead of VIR_ERR_NO_STORAGE_VOL).
virStorageFileReportBrokenChain uses data from the driver private data
pointer to print the user and group. This would lead to a crash in call
paths where we did not initialize the storage backend as recently added
in commit 24e47ee2b9 to qemuDomainDetermineDiskChain.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1522682
Commit id '5d5c732d7' had an incorrect assignment and was found
by travis build:
storage/storage_driver.c:1668:14: error: equality comparison with extraneous
parentheses [-Werror,-Wparentheses-equality]
if ((obj == virStoragePoolObjListSearch(&driver->pools,
~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Create an API to search through the storage pool objects looking for
a specific truism from a callback API in order to return the specific
storage pool object that is desired.
Create an API to walk the pools->objs[] list in order to perform a
callback function for each element of the objs array that doesn't care
about whether the action succeeds or fails as the desire is to run the
code over every element in the array rather than fail as soon as or if
one fails.
For now it'll just call the virStoragePoolObjUnlock, but a future
adjustment will do something different. Since the new API will check
for a NULL object before the Unlock call, callers no longer need to
check for NULL before calling.
The virStoragePoolObjUnlock is now private/static to virstorageobj.c
with a short term forward reference.
Resolve a storage driver crash as a result of a long running
storageVolCreateXML when the virStorageVolPoolRefreshThread is
run as a result of when a storageVolUpload completed and ran the
virStoragePoolObjClearVols without checking if the creation
code was currently processing a buildVol after incrementing
the driver->asyncjob count.
The refreshThread will now check the pool asyncjob count before
attempting to pursue the pool refresh. Adjust the documentation
to describe the condition.
Crash from valgrind is as follows (with a bit of editing):
==21309== Invalid read of size 8
==21309== at 0x153E47AF: storageBackendUpdateVolTargetInfo
==21309== by 0x153E4C30: virStorageBackendUpdateVolInfo
==21309== by 0x153E52DE: virStorageBackendVolRefreshLocal
==21309== by 0x153DE29E: storageVolCreateXML
==21309== by 0x562035B: virStorageVolCreateXML
==21309== by 0x147366: remoteDispatchStorageVolCreateXML
...
==21309== Address 0x2590a720 is 64 bytes inside a block of size 336 free'd
==21309== at 0x4C2F2BB: free
==21309== by 0x54CB9FA: virFree
==21309== by 0x55BC800: virStorageVolDefFree
==21309== by 0x55BF1D8: virStoragePoolObjClearVols
==21309== by 0x153D967E: virStorageVolPoolRefreshThread
...
==21309== Block was alloc'd at
==21309== at 0x4C300A5: calloc
==21309== by 0x54CB483: virAlloc
==21309== by 0x55BDC1F: virStorageVolDefParseXML
==21309== by 0x55BDC1F: virStorageVolDefParseNode
==21309== by 0x55BE5A4: virStorageVolDefParse
==21309== by 0x153DDFF1: storageVolCreateXML
==21309== by 0x562035B: virStorageVolCreateXML
==21309== by 0x147366: remoteDispatchStorageVolCreateXML
...
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 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.
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.
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.
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>
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
This commit adds new events for two methods and operations: *PoolBuild() and
*PoolDelete(). Using the event-test and the commands set below we have the
following outputs:
$ sudo ./event-test
Registering event callbacks
myStoragePoolEventCallback EVENT: Storage pool test Defined 0
myStoragePoolEventCallback EVENT: Storage pool test Created 0
myStoragePoolEventCallback EVENT: Storage pool test Started 0
myStoragePoolEventCallback EVENT: Storage pool test Stopped 0
myStoragePoolEventCallback EVENT: Storage pool test Deleted 0
myStoragePoolEventCallback EVENT: Storage pool test Undefined 0
Another terminal:
$ sudo virsh pool-define test.xml
Pool test defined from test.xml
$ sudo virsh pool-build test
Pool test built
$ sudo virsh pool-start test
Pool test started
$ sudo virsh pool-destroy test
Pool test destroyed
$ sudo virsh pool-delete test
Pool test deleted
$ sudo virsh pool-undefine test
Pool test has been undefined
This commits can be a solution for RHBZ #1475227.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1475227
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Introduce virStoragePoolObjForEachVolume to scan each volume
calling the passed callback function until all volumes have been
processed in the storage pool volume list, unless the callback
function returns an error.
Introduce virStoragePoolObjSearchVolume to search each volume
calling the passed callback function until it returns true
indicating that the desired volume was found.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Create/use virStoragePoolObjAddVol in order to add volumes onto list.
Create/use virStoragePoolObjRemoveVol in order to remove volumes from list.
Create/use virStoragePoolObjGetVolumesCount to get count of volumes on list.
For the storage driver, the logic alters when the volumes.obj list grows
to after we've fetched the volobj. This is an optimization of sorts, but
also doesn't "needlessly" grow the volumes.objs list and then just decr
the count if the virGetStorageVol fails.
Signed-off-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1464313
If a Disk pool was defined/created using XML that either didn't
specify a specific format or specified format type='unknown', then
restarting a pool after an initial disk backend build with overwrite
would fail after a libvirtd restart for a non-autostarted pool.
This is because the persistent pool data is not updated during pool
build w/ overwrite processing to have the VIR_STORAGE_POOL_DISK_DOS
default format.
So in addition to the alteration done during disk build processing,
alter the default expectation for disk startup to be DOS if nothing
has been defined yet. That will either succeed if the pool had been
successfully built previously using the default DOS format or fail
with a message indicating the format is something else that does not
match the expect format 'dos'.
https://bugzilla.redhat.com/show_bug.cgi?id=1437797
Rather than using refreshVol which essentially only updates the
allocation, capacity, and permissions for the volume, but not
the format which does get updated in a pool refresh - let's use
the same helper that pool refresh uses in order to update the
volume target.
Our backing probing code handles directory file types properly in
virStorageFileGetMetadataRecurse(), by that I mean it leaves them
alone. However its caller, the virStorageFileGetMetadata() resets the
type to raw before probing, without even checking the type. We need
to special-case TYPE_DIR in order to achieve desired results.
Also, in order to properly test this, we need to stop resetting format
of volumes in tests for TYPE_DIR (probably the reason why we didn't
catch that and why the test data didn't need to be modified).
Partially-resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1443434
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Driver modules proved to be reliable for a long time. Since support for
not building modules complicates the code and makefiles drop it.
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Disallow providing the wwnn/wwpn of the HBA in the adapter XML:
<adapter type='fc_host' [parent='scsi_hostN'] wwnn='HBA_wwnn'
wwpn='HBA_wwpn'/>
This should be considered a configuration error since a vHBA
would not be created. In order to use the HBA as the backing the
following XML should be used:
<adapter type='scsi_host' name='scsi_hostN'/>
So add a check prior to the checkParent call to validate that
the provided wwnn/wwpn resolves to a vHBA and not an HBA.
Signed-off-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1458708
If the parent provided for the storage pool adapter is not vHBA
capable, then issue a configuration error even though the provided
wwnn/wwpn were found.
It is a configuration error to provide a mismatched parent to
the wwnn/wwpn. The @parent is optional and is used as a means to
perform duplicate pool source checks.
https://bugzilla.redhat.com/show_bug.cgi?id=1472277
Commit id '106930aaa' altered the order of checking for an existing
vHBA (e.g something created via nodedev-create functionality outside
of the storage pool logic) which inadvertantly broke the code to
decide whether to alter/force the fchost->managed field to be 'yes'
because the storage pool will be managing the created vHBA in order
to ensure when the storage pool is destroyed that the vHBA is also
destroyed.
This patch moves the check (and checkParent helper) for an existing
vHBA back into the createVport in storage_backend_scsi. It also
adjusts the checkParent logic to more closely follow the intentions
prior to commit id '79ab0935'. The changes made by commit id '08c0ea16f'
are only necessary to run the virStoragePoolFCRefreshThread when
a vHBA was really created because there's a timing lag such that
the refreshPool call made after a startPool from storagePoolCreate*
wouldn't necessarily find LUNs, but the thread would. For an already
existing vHBA, using the thread is unnecessary since the vHBA already
exists and the lag to configure the LUNs wouldn't exist.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Currently, @port is type of string. Well, that's overkill and
waste of memory. Port is always an integer. Use it as such.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Alter the virStoragePoolObjNumOfVolumes, virStoragePoolObjVolumeGetNames,
and virStoragePoolObjVolumeListExport APIs to take a virStoragePoolObjPtr
instead of the &obj->volumes and obj->def.
Signed-off-by: John Ferlan <jferlan@redhat.com>
A virStoragePoolObjPtr will be an 'obj'.
A virStoragePoolPtr will be a 'pool'.
A virStorageVolPtr will be a 'vol'.
A virStorageVolDefPtr will be a 'voldef'.
Signed-off-by: John Ferlan <jferlan@redhat.com>
