When re-defining an active storage pool, due to a bug the config
file on disk is not changed. This is because we are passing old
definition instead of new one to virStoragePoolObjSaveDef.
This issue was introduced by bfcd8fc9,
Signed-off-by: Changkuo Shi <shi.changkuo@h3c.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This commit renames and adds other macros to support aother filesystems
when a reflink is performed. After that, XFS filesystems (and others)
with reflink support will be able to clone.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1565004
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Starting with QEMU 2.9, encryption convert processing requires
a multi-step process in order to generate an encrypted image from
some non encrypted raw image.
Processing requires to first create an encrypted image using the
sizing parameters from the input source and second to use the
--image-opts, -n, and --target-image-opts options along with inline
driver options to describe the input and output files, generating
two commands such as:
$ qemu-img create -f luks \
--object secret,id=demo.img_encrypt0,file=/path/to/secretFile \
-o key-secret=demo.img_encrypt0 \
demo.img 500K
Formatting 'demo.img', fmt=luks size=512000 key-secret=demo.img_encrypt0
$ qemu-img convert --image-opts -n --target-image-opts \
--object secret,id=demo.img_encrypt0,file=/path/to/secretFile \
driver=raw,file.filename=sparse.img \
driver=luks,file.filename=demo.img,key-secret=demo.img_encrypt0
$
This patch handles the convert processing by running the processing
in a do..while loop essentially reusing the existing create logic and
arguments to create the target vol from the inputvol and then converting
the inputvol using new arguments.
This then allows the following virsh command to work properly:
virsh vol-create-from default encrypt1-luks.xml data.img --inputpool default
where encrypt1-luks.xml would provided the path and secret for
the new image, while data.img would be the source image.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Since we no longer support creating qcow2 encryption format
volumes, we no longer have to possibly create some secret and
have no real need for the function, so move the remaining
functionality to build the secret path back into the caller
storageBackendCreateQemuImg.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Since we only generate the @encinfo when there's a secret object
and thus we need to reference it in the options,
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Remove the checks for qcow encryption since both callers (create
and resize) would have already disallowed usage.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1526382
Since commit c4eedd793 disallowed qcow2 encrypted images to be
used for domains, it no longer makes sense to allow a qcow2
encrypted volume to be created or resized.
Add a test that will exhibit the failure of creation as well
as the xml2xml validation of the format still being correct.
Update the documentation to note the removal of the capability
to create and use qcow/default encrypted volumes.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Change from @enc to @encinfo leaving @enc for the vol->target.encryption
in the storageBackendCreateQemuImgSetOptions code path.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Allowing a NULL @secretPath for virStorageBackendCreateQemuImgCmdFromVol
would result in a generated command line with a dangling "file=" output.
So let's make sure the @secretPath exists before processing.
This means we should pass a dummy path from the storage test.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Fix the case when creating a luks encrypted volume
via an xml file without 'secret' element.
libvirtd was receiving SIGSEGV, now proper error is reported for
the missing element.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1468422
Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com>
Modify virStorageBackendLogicalLVCreate to ensure if encryption
is requested that only type LUKS is supported; otherwise, error.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1560946
Similar to the the Logical backend, use qemu-img on the created
disk partition device to set up for LUKS encryption. Secret mgmt
for the device can be complicated by a reboot possibly changing
the path to the device if the infrastructure changes.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Peter Krempa <pkrempa@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1400475
In order to avoid a possible error as a result of kernel interactions
with the partition helper, let's use virWaitForDevices to force things
to settle down before attempting to open and read the partition. This
is related to https://bugzilla.redhat.com/show_bug.cgi?id=1264719.
Although perhaps overkill to have too many places to settle, since
we know that the act of reading the partitions via libvirt_parthelper
will cause udev activity/events - we just need to ensure udev has
been settled before proceding with usage of the device.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Extract out command line setup and run from storageBackendCreateQemuImg
as we'll need to run it twice soon.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Split up virStorageBackendCreateQemuImgCmdFromVol into two parts.
It's too long anyway and virStorageBackendCreateQemuImgCmdFromVol
should just handle the command line processing.
NB: Requires changing info.* into info->* references.
Signed-off-by: John Ferlan <jferlan@redhat.com>
The only way preallocate could be set is if the info->format was
not RAW (see storageBackendCreateQemuImgSetBacking), so let's just
extract it from the if/else surrounding the application of the
encryption options.
Signed-off-by: John Ferlan <jferlan@redhat.com>
The only way backing_fmts could be set is if the info->format was
not RAW (see storageBackendCreateQemuImgSetBacking), so let's just
extract it from the if/else surrounding the application of the
encryption options.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Remove the "luks" distinction as the code is about to become more
generic and be able to support qcow encryption as well.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Move generation of secretPath to storageBackendGenerateSecretData
and simplify a bit since we know vol->target.encryption is set plus
we have a local @enc.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Rather than having storageBackendCreateQemuImgCheckEncryption
perform the virStorageGenerateQcowEncryption, let's just do that
earlier during storageBackendCreateQemuImg so that the check
helper is just a check helper rather doing something different
based on whether the format is qcow[2] or raw based encryption.
This fixes an issue in the storageBackendResizeQemuImg processing
for qcow encryption where if a secret was not available for a
volume, a new secret will not be generated and instead an error
message will be generated.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Commit id 'a48c71411' altered the logic a bit and didn't
remove an unnecessary check as info.encryption is true when
vol->target.encryption != NULL, so if we enter the if segment
with info.format == VIR_STORAGE_FILE_RAW && vol->target.encryption
!= NULL, then there's no way info.encryption could be false.
Signed-off-by: John Ferlan <jferlan@redhat.com>
We have been checking whether qemu-img supports the -o compat
option by scraping the -help output.
Since we require QEMU 1.5.0 now and this option was introduced in 1.1,
assume we support it and ditch the help parsing code along with the
extra qemu-img invocation.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
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
