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>
If a remote call fails during event registration (more than likely from
a network failure or remote libvirtd restart timed just right), then when
calling the virObjectEventStateDeregisterID we don't want to call the
registered @freecb function because that breaks our contract that we
would only call it after succesfully returning. If the @freecb routine
were called, it could result in a double free from properly coded
applications that free their opaque data on failure to register, as seen
in the following details:
Program terminated with signal 6, Aborted.
#0 0x00007fc45cba15d7 in raise
#1 0x00007fc45cba2cc8 in abort
#2 0x00007fc45cbe12f7 in __libc_message
#3 0x00007fc45cbe86d3 in _int_free
#4 0x00007fc45d8d292c in PyDict_Fini
#5 0x00007fc45d94f46a in Py_Finalize
#6 0x00007fc45d960735 in Py_Main
#7 0x00007fc45cb8daf5 in __libc_start_main
#8 0x0000000000400721 in _start
The double dereference of 'pyobj_cbData' is triggered in the following way:
(1) libvirt_virConnectDomainEventRegisterAny is invoked.
(2) the event is successfully added to the event callback list
(virDomainEventStateRegisterClient in
remoteConnectDomainEventRegisterAny returns 1 which means ok).
(3) when function remoteConnectDomainEventRegisterAny is hit,
network connection disconnected coincidently (or libvirtd is
restarted) in the context of function 'call' then the connection
is lost and the function 'call' failed, the branch
virObjectEventStateDeregisterID is therefore taken.
(4) 'pyobj_conn' is dereferenced the 1st time in
libvirt_virConnectDomainEventFreeFunc.
(5) 'pyobj_cbData' (refered to pyobj_conn) is dereferenced the
2nd time in libvirt_virConnectDomainEventRegisterAny.
(6) the double free error is triggered.
Resolve this by adding a @doFreeCb boolean in order to avoid calling the
freeCb in virObjectEventStateDeregisterID for any remote call failure in
a remoteConnect*EventRegister* API. For remoteConnect*EventDeregister* calls,
the passed value would be true indicating they should run the freecb if it
exists; whereas, it's false for the remote call failure path.
Patch based on the investigation and initial patch posted by
fangying <fangying1@huawei.com>.
It is necessary for some parts of the code to refresh just data
based on the based on the backing store string. Add a convenience
function that will retrieve this data.
Use ATTRIBUTE_FALLTHROUGH, introduced by commit
5d84f5961b, instead of comments to
indicate that the fall through is an intentional behavior.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
These flags to APIs will tell if caller wants to use sparse
stream for storage transfer. At the same time, it's safe to
enable them in storage driver frontend and rely on our backends
checking the flags. This way we can enable specific flags only on
some specific backends, e.g. enable
VIR_STORAGE_VOL_DOWNLOAD_SPARSE_STREAM for filesystem backend but
not iSCSI backend.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Basically, what is needed here is to introduce new message type
for the messages passed between the event loop callbacks and the
worker thread that does all the I/O. The idea is that instead of
a queue of read buffers we will have a queue where "hole of size
X" messages appear. That way the event loop callbacks can just
check the head of the queue and see if the worker thread is in
data or a hole section and how long the section is.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
There should be no need to make dir based pools world/group readable.
So use 0711, not 0755, as the default perms for storage dirs.
Updates in v2:
- adapt commit wording to mention dropping group readable as well
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1439132
During 'matrix' testing of all possible combinations I found that if
device is formated with "gpt" first, then an attempt is made to format
using "mac", a startup will fail.
Deeper analysis by Peter Krempa indicates that the "mac" table fits
into the first block on the disk. Since the GPT disklabel is stored
at LBA address 1 it is not overwritten at all. Thus it's apparent that
the (blkid) detection tool then prefers GPT over a older disklabel.
The GPT disklabel has also a secondary copy at the last LBA of the disk.
So, follow the same logic as the logical pool in clearing a 1MB swath
at the beginning and end of the device to avoid potential issues with
larger sector sizes for the device.
Also fixed a minor formatting nit in virStorageBackendDeviceIsEmpty call.
Create a wrapper/helper that can be used to call the storage backend
wipe helper - storageBackendVolWipeLocalFile for future use by logical
and disk backends to clear out the partition table rather than having
each open code the same algorithm.
Add bool 'zero_end' and logic that would allow a caller to wipe specific
portions of a target device either from the beginning (the default) or
from the end when zero_end is true.
This will allow for this code to wipe out partition table information
from a device.
Mostly code motion to move storageConnectList[Defined]StoragePools
and similar test driver code into virstorageobj.c and rename to
virStoragePoolObjGetNames.
Also includes a couple of variable name adjustments to keep code consistent
with other drivers.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Unify the NumOf[Defined]StoragePools API into virstorageobj.c from
storage_driver and test_driver. The only real difference between the
two is the test driver doesn't call using the aclfilter API.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Mostly code motion to move storagePoolListVolumes code into virstorageobj.c
and rename to virStoragePoolObjVolumeGetNames.
Also includes a couple of variable name adjustments to keep code consistent
with other drivers.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Unify the NumOfVolumes API into virstorageobj.c from storage_driver and
test_driver. The only real difference between the two is the test driver
doesn't call using the aclfilter API.
Signed-off-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1439132
Add "bsd" to the list of format types to not checked during blkid
processing even though it supposedly knows the format - for some
(now unknown) reason it's returning partition table not found. So
let's just let PARTED handle "bsd" too.
Signed-off-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1439132
Commit id 'a48c674fb' added a check for format types "dvh" and "pc98"
to use the parted print processing instead of using blkid processing
in order to validate the label on the disk was what is expected for
disk pool startup. However, commit id 'a4cb4a74f' really messed things
up by missing an else condition causing PARTEDFindLabel to always
return DIFFERENT.
Signed-off-by: John Ferlan <jferlan@redhat.com>
After restart of libvirtd the 'checkPool' method is supposed to validate
that the pool is online. Since libvirt then refreshes the pool contents
anyways just return whether the pool was supposed to be online so that
the code can be reached. This is necessary since if a pool does not
implement the method it's automatically considered as inactive.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1436065
Use the relative lookup specifier rather than the global one. Otherwise
only the first name would be looked up. Add a test case to cover the
scenario.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1436574
The native gluster pool source list data differs from the data used for
attaching gluster volumes as netfs pools. Currently the only difference
was the format. Since native pools don't use it and later there will be
more differences add a more deterministic way to switch between the
types instead.
https://bugzilla.redhat.com/show_bug.cgi?id=1371892
The 'capacity' value (e.g. guest logical size) for a LUKS volume is
smaller than the 'physical' value of the file in the file system, so
we need to account for that.
When peeking at the encryption information about the volume add a fetch
of the payload_offset which is described as the offset to the start of
the volume data (in 512 byte sectors) in QEMU's QCryptoBlockLUKSHeader.
Then adjust the ->capacity appropriately when we determine that the
volume target encryption has a payload_offset value.
If a transient storage pool is deemed inactive after libvirtd restart it
would not be deleted from the list. Reuse virStoragePoolUpdateInactive
along with a refactor necessary to properly update the state.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1242801
After a pool is made inactive the definition objects need to be updated
(if a new definition is prepared) and transient pools need to be
completely removed. Split out the code doing these steps into a separate
function for later reuse.
When registering a storage poll backend, the code would use
virStorageTypeToString instead of virStoragePoolTypeToString. The
following message would be logged:
virDriverLoadModuleFunc:71 : Lookup function 'virStorageBackendSCSIRegister'
virStorageBackendRegister:174 : Registering storage backend '(null)'
off_t is signed and it's size is the same as long only on 64b archs.
Thus it cannot be formatted as %lu.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1430679
As it turns out some file headers (e.g. ext4) may be larger/longer than
the 512 bytes of zeros being written prior to a pvcreate, so let's write
out 2048 bytes similar to how the pvcreate sources would peek at the first
4 sectors of the device.
Make sure there is at enough bytes on the device to clear before doing
doing the clear - just to be sure.
There is no reason for it not to be in the utils, all global symbols
under that file already have prefix vir* and there is no reason for it
to be part of DRIVER_SOURCES because that is just a leftover from
older days (pre-driver modules era, I believe).
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Use "virStoragePoolObj" as a prefix for any external API in virstorageobj.
Also a couple of functions were local to virstorageobj.c, so remove their
external defs iin virstorageobj.h.
NB: The virStorageVolDef* API's won't change.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Move all the StoragePoolObj related API's into their own module
virstorageobj from the storage_conf
Purely code motion at this point, plus adjustments to cleanly build
Signed-off-by: John Ferlan <jferlan@redhat.com>
Rather than returning true/false and having the caller check if the
vHBA was actually created, let's do that check within the CreateVport
function. That way the caller can faithfully assume success based
on a name start the thread looking for the LUNs. Prior to this change
it's possible that the vHBA wasn't really created (e.g if the call to
virVHBAGetHostByWWN returned NULL), we'd claim success, but in reality
there'd be no vHBA for the pool. This also fixes a second yet seen
issue that if the nodedev was present, but the parent by name wasn't
provided (perhaps parent by wwnn/wwpn or by fabric_name), then a failure
would be returned. For this path it shouldn't be an error - we should
just be happy that something else is managing the device and we don't
have to create/delete it.
The end result is that the createVport code can now just start the
refresh thread once it gets a non NULL name back.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Move the bulk of createVport and rename to virNodeDeviceCreateVport.
Remove the deleteVport entirely and replace with virNodeDeviceDeleteVport
Signed-off-by: John Ferlan <jferlan@redhat.com>
The function is actually in virutil.c, but prototyped in virfile.h.
This patch fixes that by renaming the function to virWaitForDevices,
adding the prototype in virutil.h and libvirt_private.syms, and then
changing the callers to use the new name.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Move the virStoragePoolSourceAdapter from storage_conf.h and rename
to virStorageAdapter.
Continue with code realignment for brevity and flow.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Rework the code to use the new FCHost specific adapter structures.
Also rework the parameters to only pass what's need and leave logic in
the caller for the adapter type and the need to call the helpers.
Signed-off-by: John Ferlan <jferlan@redhat.com>
$ virsh vol-clone /tmp/test.iso new.iso
error: Failed to clone vol from test.iso
error: internal error: Child process (/bin/qemu-img convert -f iso -O iso /tmp/test.iso /tmp/new.iso) unexpected exit status 1: qemu-img: Could not open '/tmp/test.iso': Unknown driver 'iso'
Map iso->raw before sending the format value to qemu-img
https://bugzilla.redhat.com/show_bug.cgi?id=972784https://bugzilla.redhat.com/show_bug.cgi?id=1419395
The build system for libvirt correctly detects the location of blkid
using PKG_CONFIG_PATH environment variable. The file blkid.pc states
that the include flags should be: 'Cflags: -I${includedir}/blkid' but
libvirt searches for blkid.h inside ${includedir}/blkid/blkid, which is
wrong. Until now, the compilation for libvirt succeeded because of pure
luck, as it had -I/usr/include as a CFLAG. This issue was faced while
compiling libvirt on Ubuntu 16.04.2 with bare minimum dev packages and a
custom compiled blkid kept in a non-standard $prefix.
Signed-off-by: Nehal J Wani <nehaljw.kkd1@gmail.com>
Add a new storage driver registration function that will force the
backend code to fail if any of the storage backend modules can't be
loaded. This will make sure that they work and are present.
If driver modules are enabled turn storage driver backends into
dynamically loadable objects. This will allow greater modularity for
binary distributions, where heavyweight dependencies as rbd and gluster
can be avoided by selecting only a subset of drivers if the rest is not
necessary.
The storage modules are installed into 'LIBDIR/libvirt/storage-backend/'
and users can override the location by using
'LIBVIRT_STORAGE_BACKEND_DIR' environment variable.
rpm based distros will at this point install all the backends when
libvirt-daemon-driver-storage package is installed.
Add APIs that allow to dynamically register driver backends so that the
list of available drivers does not need to be known during compile time.
This will allow us to modularize the storage driver on runtime.
Create a virscsihost.c and place the functions there. That removes the
last #ifdef __linux__ from virutil.c.
Take the opporunity to also change the function names and in one case
the parameters slightly
Use the new virNodeDeviceGetParentName instead. Modify the callers to
build the node device scsi_host# name string in order to call the new
function so that proper lookup occurs.
Rather than have them mixed in with the virutil apis, create a separate
virvhba.c module and move the vHBA related calls into there. Soon there
will be more added.
Also modify the names of the functions and some arguments to be more
indicative of what is really happening. Adjust the callers respectively.
While I was changing fchosttest, rather than the non-descriptive names
test1...test6, rename them to match what the test is doing.
Right now, we use simple string comparison both on the source paths
(mount's output vs pool's source) and the target (mount's mnt_dir vs
pool's target). The problem are symlinks and mount indeed returns
symlinks in its output, e.g. /dev/mappper/lvm_symlink. The same goes for
the pool's source/target, so in order to successfully compare these two
replace plain string comparison with virFileComparePaths which will
resolve all symlinks and canonicalize the paths prior to comparison.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1417203
Signed-off-by: Erik Skultety <eskultet@redhat.com>
When FS pool's source is already mounted on the target location instead
of just simply marking the pool as active, thus starting it we fail with
an error stating that the source is indeed already mounted on the target.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Commit id '5f07c3c07' broke the freebsd build in the libvirt CI test
environment because the UMOUNT was not defined unless WITH_STORAGE_FS
is defined.
So remove the virStorageBackendUmountLocal from storage_util.c,h and
restore the code back in the storage_backend_fs.c and _vstorage.c
modules.
Added create/define/etc pool operations for vstorage backend.
Used the common/local pool API's from storage_util for operations
that are not specific to vstorage. In particular Refresh and Delete
Pool operations as well as all the Volume operations.
Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com>
Added general definitions for vstorage pool backend including
the build options to add --with-storage-vstorage checking.
In order to use vstorage as a backend for a storage pool
vstorage tools (vstorage and vstorage-mount) need to be installed.
Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com>
Move all the volume functions to storage_util to create local/common helpers
using the same naming syntax as the existing upload, download, and wipe
virStorageBackend*Local API's.
In the process of doing so, found more API's that can now become local
to storage_util. In order to distinguish between local/external - I
changed the names of the now local only ones from "virStorageBackend..."
to just "storageBackend..."
Signed-off-by: John Ferlan <jferlan@redhat.com>
Move some pool functions to storage_util to create local/common helpers
using the same naming syntax as the existing upload, download, and wipe
virStorageBackend*Local API's.
In the process of doing so, found a few API's that can now become local
to storage_util. In order to distinguish between local/external - I
changed the names of the now local only ones from "virStorageBackend..."
to just "storageBackend..."
Signed-off-by: John Ferlan <jferlan@redhat.com>
Just moving code around with minor adjustment to have the Stop
code combine with the Unmount code since all the Stop code did
was call the Unmount code.
Previous commit tried to change configure logic such that the
GLUSTER_CLI parameter would always be set:
commit 9e97c8c0f0
Author: Peter Krempa <pkrempa@redhat.com>
Date: Mon Jan 9 15:56:12 2017 +0100
storage: gluster: Remove build-time dependency on the 'gluster' cli tool
This missed the fact that the AC_PATH_PROG call was itself inside an 'if'
conditional that would not be called in with_storage_gluster was false. As
a result, GLUSTER_CLI was still conditionally defined.
Just kill the GLUSTER_CLI parameter and AC_PATH_PROG call entirely and pass a
bare "gluster" string to virFindFileInPath instead.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The iSCSI backend driver was using stuff from the SCSI driver without
making sure that it's compiled in. Move the common code into the
storage_util.c since it does not contain any specific code.
The file backend code was mistakenly put into #if WITH_STORAGE_FS. This
is not necessary since all the backends just access files on disk, and
thus the code for WITH_STORAGE_DIR is sufficient to compile everything.
The file became a garbage dump for all kinds of utility functions over
time. Move them to a separate file so that the files can become a clean
interface for the storage backends.
https://bugzilla.redhat.com/show_bug.cgi?id=1346566
If libvirt_parthelper is erroneously told to append the partition
separator 'p' onto the generated output for a disk pool using device
mapper that has 'user_friendly_names' set to true, then the error
recovery path will fail to find volume resulting in the pool being
in an unusable state.
So, augment the documentation to provide the better hint that the
part_separator='yes' should be set when user_friendly_names are not
being used. Additionally, once we're in the error path where the
returned name doesn't match the expected partition name try to see
if the reason is because the 'p' was erroneosly added. If so alter
the about to be removed vol->target.path so that the DiskDeleteVol
code can find the partition that was created and remove it.
If the voldef type is VIR_STORAGE_VOL_BLOCK, then as long as the
format is known, let's allow the probe to happen - gets a truer value
and the same probe/update would be allowed for the same volume defined
in a domain.
For volume processing in virStorageBackendUpdateVolTargetInfo to get
the capacity commit id 'a760ba3a7' added the ability to probe a volume
that didn't list a target format. Unfortunately, the code used the
virStorageSource (e.g. target->type - virStorageType) rather than
virStorageVolDef (e.g. vol->type - virStorageVolType) in order to
make the comparison. As it turns out target->type for a volume is
not filled in at all for a voldef as the code relies on vol->type.
Ironically the result is that only VIR_STORAGE_VOL_BLOCK's would get
their capacity updated.
This patch will adjust the code to check the "vol->type" field instead
as an argument. This way for a voldef, the correct comparison is made.
Additionally for a backingStore, the 'type' field is never filled in;
however, since we know that the provided path is a location at which
the backing store can be accessed on the local filesystem thus just
pass VIR_STORAGE_VOL_FILE in order to satisfy the adjusted voltype
check. Whether it's a FILE or a BLOCK only matters if we're trying to
get more data based on the target->format.
The tool is used for pool discovery. Since we call an external binary we
don't really need to compile out the code that uses it. We can check
whether it exists at runtime.
In commit 4090e15399 we went back from reporting no errors if no storage
pools were found on a given host to reporting a bad error. And only in
cases when gluster was not installed.
Report a less bad error in case there are no volumes. Also report the
error when gluster is installed but no volumes were found, since
virStorageBackendFindGlusterPoolSources would return success in that
case.
For case VIR_STORAGE_BLKID_PROBE_DIFFERENT, clean up the message to
avoid using the virsh like --overwrite syntax. Additionally provide
a different error message when not writing the label to avoid confusion.
Rather than special casing the VIR_STORAGE_BLKID_PROBE_UNKNOWN after
calling virStorageBackendBLKIDFindPart, just allow the switch statement
handle setting ret = -2.
If neither BLKID or PARTED is available and we're not writing, then
just return 0 which allows the underlying storage pool to generate
a failure. If both are unavailable and we're writing, then generate
a more generic error message.
The check is pointless since LVM is capable to detect it's own members
and the check is flawed as it would fail if neither libblkid nor parted
is installed.
We don't really need to babysit LVM in this way.
This reverts commit cb38b6cbc7.
The check does not work properly (crashes) with netfs filesystems and
also checking that a device is not empty when attempting to mount a
filesystem is not very usefull since the mount will fail anyways.
As the code would improve only a very minor corner case I don't really
see a reason to have this code at all.
This code would also fail if libvirt is compiled without support for
blkid and without parted.
This reverts commit a11fd69735.
Commit id 'a48c674f' caused problems for systems without PARTED installed.
So move the PARTED probing code back to storage_backend_disk.c and create
a shim within storage_backend.c to call it if WITH_STORAGE_DISK is true;
otherwise, just return -1 with the error.
At startup time, rather than blindly trusting the target devices are
still properly formatted, let's check to make sure the pool's target
devices are all properly formatted before attempting to start the pool.
Signed-off-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1373711
Add support and documentation for the [NO_]OVERWRITE flags for the
logical backend.
Update virsh.pod with a description of the process for usage of
the flags and building of the pool's volume group.
Signed-off-by: John Ferlan <jferlan@redhat.com>
If the build fails, then we need to ensure that we've run pvremove
on any devices which we've run pvcreate on; otherwise, a subsequent
build could fail since running pvcreate twice on a device requires
special force arguments.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Currently as long as the disk is formatted using a known parted format
type, the algorithm is happy to continue. However, that leaves a scenario
whereby a disk formatted using "pc98" could be used by a pool that's defined
using "dvh" (or vice versa). Alter the check to be match and different
and adjust the caller.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Rather than have the Disk code having to use PARTED to determine if
there's something on the device, let's use the virStorageBackendDeviceProbe.
and only fallback to the PARTED probing if the BLKID code isn't built in.
This will also provide a mechanism for the other current caller (File
System Backend) to utilize a PARTED parsing algorithm in the event that
BLKID isn't built in to at least see if *something* exists on the disk
before blindly trying to use. The PARTED error checking will not find
file system types, but if there is a partition table set on the device,
it will at least cause a failure.
Move virStorageBackendDiskValidLabel and virStorageBackendDiskFindLabel
to storage_backend and rename/rework the code to fit the new model.
Update the virsh.pod description to provide a more generic description
of the process since we could now use either blkid or parted to find
data on the target device.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Prior to starting up, let's be sure the target volume device is
formatted as we expect; otherwise, inhibit the start.
Signed-off-by: John Ferlan <jferlan@redhat.com>
It's possible that the API could be called from a startup path in
order to check whether the label on the device matches what our
format is. In order to handle that condition, add a 'writelabel'
boolean to the API in order to indicate whether a write or just
read is about to happen.
This alters two "error" conditions that would care about knowing.
Signed-off-by: John Ferlan <jferlan@redhat.com>
A device may be formatted using some sort of disk partition format type.
We can check that using the blkid_ API's as well - so alter the logic to
allow checking the device for both a filesystem and a disk partition.
Signed-off-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1363586
Commit id '27758859' introduced the "NO_OVERWRITE" flag check for
file system backends; however, the implementation, documentation,
and algorithm was inconsistent. For the "flag" description for the
API the flag was described as "Do not overwrite existing pool";
however, within the storage backend code the flag is described
as "it probes to determine if filesystem already exists on the
target device, renurning an error if exists".
The code itself was implemented using the paradigm to set up the
superblock probe by creating a filter that would cause the code
to only search for the provided format type. If that type wasn't
found, then the algorithm would return success allowing the caller
to format the device. If the format type already existed on the
device, then the code would fail indicating that the a filesystem
of the same type existed on the device.
The result is that if someone had a file system of one type on the
device, it was possible to overwrite it if a different format type
was specified in updated XML effectively trashing whatever was on
the device already.
This patch alters what NO_OVERWRITE does for a file system backend
to be more realistic and consistent with what should be expected when
the caller requests to not overwrite the data on the disk.
Rather than filter results based on the expected format type, the
code will allow success/failure be determined solely on whether the
blkid_do_probe calls finds some known format on the device. This
adjustment also allows removal of the virStoragePoolProbeResult
enum that was under utilized.
If it does find a formatted file system different errors will be
generated indicating a file system of a specific type already exists
or a file system of some other type already exists.
In the original virsh support commit id 'ddcd5674', the description
for '--no-overwrite' within the 'pool-build' command help output
has an ambiguous "of this type" included in the short description.
Compared to the longer description within the "Build a given pool."
section of the virsh.pod file it's more apparent that the meaning
of this flag would cause failure if a probe of the target already
has a filesystem.
So this patch also modifies the short description to just be the
antecedent of the 'overwrite' flag, which matches the API description.
This patch also modifies the grammar in virsh.pod for no-overwrite
as well as reworking the paragraph formats to make it easier to read.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Rename virStorageBackendFileSystemProbe and to virStorageBackendBLKIDFindFS
and move to the more common storage_backend module.
Create a shim virStorageBackendDeviceIsEmpty which will make the call
to the virStorageBackendBLKIDFindFS and check the return value.
Signed-off-by: John Ferlan <jferlan@redhat.com>
The code at the very bottom of the DAC secdriver that calls
chown() should be fine with read-only data. If something needs to
be prepared it should have been done beforehand.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The public virSecret object has a single "usage_id" field
but the virSecretDef object has a different 'char *' field
for each usage type, but the code all assumes every usage
type has a corresponding single string. Get rid of the
pointless union in virSecretDef and just use "usage_id"
everywhere. This doesn't impact public XML format, only
the internal handling.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
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.
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>
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>
Instead of having duplicated code in qemuStorageLimitsRefresh and
virStorageBackendUpdateVolTargetInfo to get capacity specific data
about the storage backing source or volume -- create a common API
to handle the details for both.
As a side effect, virStorageFileProbeFormatFromBuf returns to being
a local/static helper to virstoragefile.c
For the QEMU code - if the probe is done, then the format is saved so
as to avoid future such probes.
For the storage backend code, there is no need to deal with the probe
since we cannot call the new API if target->format == NONE.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Instead of having duplicated code in qemuStorageLimitsRefresh and
virStorageBackendUpdateVolTargetInfoFD to fill in the storage backing
source or volume allocation, capacity, and physical values - create a
common API that will handle the details for both.
The common API will fill in "default" capacity values as well - although
those more than likely will be overridden by subsequent code. Having just
one place to make the determination of what the values should be will
make things be more consistent.
For the QEMU code - the data filled in will be for inactive domains
for the GetBlockInfo and DomainGetStatsOneBlock API's. For the storage
backend code - the data will be filled in during the volume updates.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Commit id '03e750f3' added support for checking the PLOOP type; however,
it used 'target.type' which no storage code ever fills in, so it will
never be set. Change to just vol->type (could use vol->target.format
as well).
We had a lot of rados_conf_set and check works.
Use helper virStorageBackendRBDRADOSConfSet for them.
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
We have couple of functions that operate over NULL terminated
lits of strings. However, our naming sucks:
virStringJoin
virStringFreeList
virStringFreeListCount
virStringArrayHasString
virStringGetFirstWithPrefix
We can do better:
virStringListJoin
virStringListFree
virStringListFreeCount
virStringListHasString
virStringListGetFirstWithPrefix
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
New line character in name of storagepool is now forbidden because it
mess virsh output and can be confusing for users.
Validation of name is done in driver, after parsing XML to avoid
problems with dissappeared pools which was already created with
new-line char in name.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1366460
When using the --overwrite switch on a pool-build or pool-create, the
The mkfs.ext{2|3|4} commands use mke2fs which requires using the '-F' switch
in order to force overwriting the current filesystem on the whole disk.
Likewise, the mkfs.vfat command uses mkfs.fat which requires using the '-I'
switch in order to force overwriting the current filesystem on the whole disk.
Rather than use stack allocated state context pointers, let's allocate and
free the state context pointer. In doing so, we'll shrink the code a bit
since many routines perform the same initialization sequence.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Since none of the callers check the status, let's just alter it to
a static void.
While we're at it - scrap the local runtime variable and just do the
math in the VIR_DEBUG directly.
Signed-off-by: John Ferlan <jferlan@redhat.com>
A LUKS volume uses the volume secret type just like the QCOW2 secret, so
adjust the loading of the default secrets to handle any volume that the
virStorageFileGetMetadataFromBuf code has deemed to be an encrypted volume
to search for the volume's secret. This lookup is done by volume usage
where the usage is expected to be the path to volume.
https://bugzilla.redhat.com/show_bug.cgi?id=1362349
When adding the ability to build the pool during the start pool processing
using the similar flags as buildPool processing would use, the code was
essentially cut-n-pasted from storagePoolCreateXML. However, that included
a call to virStoragePoolObjRemove which shouldn't happen within the
storagePoolCreate path since that'll remove the pool from the list of
pools only to be rediscovered if libvirtd restarts.
So on failure, just fail and return as we should expect
There was a missing check for vol->target.encryption being NULL
at one particular place (modified by commit a48c71411) which caused a crash
when user attempted to create a raw volume using a non-raw file volume as
source.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1363636
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Let's cleanly differentiate what wiping a volume does for ploop and
other volumes so it's more readable what is done for each one instead of
branching out multiple times in different parts of the same function.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Some functions use volume specification merely to use the target path
from it. Let's change it to pass the path only so that it can be used
for other files than just volumes.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
This is done in order to call them in next patches from each other and
definitions would be missing otherwise.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1356436
Commit id '56057900' altered the discovery of iSCSI node targets by
using the "--op nonpersistent". This caused issues for clean environments
or if by chance a "-m node -o delete" was executed.
Since each iSCSI Storage Pool has the required iSCSI target path, use
that and the virISCSINodeNew API in order to generate the iSCSI node record.
The current LUKS support has a "luks" volume type which has
a "luks" encryption format.
This partially makes sense if you consider the QEMU shorthand
syntax only requires you to specify a format=luks, and it'll
automagically uses "raw" as the next level driver. QEMU will
however let you override the "raw" with any other driver it
supports (vmdk, qcow, rbd, iscsi, etc, etc)
IOW the intention though is that the "luks" encryption format
is applied to all disk formats (whether raw, qcow2, rbd, gluster
or whatever). As such it doesn't make much sense for libvirt
to say the volume type is "luks" - we should be saying that it
is a "raw" file, but with "luks" encryption applied.
IOW, when creating a storage volume we should use this XML
<volume>
<name>demo.raw</name>
<capacity>5368709120</capacity>
<target>
<format type='raw'/>
<encryption format='luks'>
<secret type='passphrase' uuid='0a81f5b2-8403-7b23-c8d6-21ccd2f80d6f'/>
</encryption>
</target>
</volume>
and when configuring a guest disk we should use
<disk type='file' device='disk'>
<driver name='qemu' type='raw'/>
<source file='/home/berrange/VirtualMachines/demo.raw'/>
<target dev='sda' bus='scsi'/>
<encryption format='luks'>
<secret type='passphrase' uuid='0a81f5b2-8403-7b23-c8d6-21ccd2f80d6f'/>
</encryption>
</disk>
This commit thus removes the "luks" storage volume type added
in
commit 318ebb36f1
Author: John Ferlan <jferlan@redhat.com>
Date: Tue Jun 21 12:59:54 2016 -0400
util: Add 'luks' to the FileTypeInfo
The storage file probing code is modified so that it can probe
the actual encryption formats explicitly, rather than merely
probing existance of encryption and letting the storage driver
guess the format.
The rest of the code is then adapted to deal with
VIR_STORAGE_FILE_RAW w/ VIR_STORAGE_ENCRYPTION_FORMAT_LUKS
instead of just VIR_STORAGE_FILE_LUKS.
The commit mentioned above was included in libvirt v2.0.0.
So when querying volume XML this will be a change in behaviour
vs the 2.0.0 release - it'll report 'raw' instead of 'luks'
for the volume format, but still report 'luks' for encryption
format. I think this change is OK because the storage driver
did not include any support for creating volumes, nor starting
guets with luks volumes in v2.0.0 - that only since then.
Clearly if we change this we must do it before v2.1.0 though.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
As gluster natively supports multiple hosts for failover reasons we can
easily add the support to the storage driver code in libvirt.
Extract the code setting an individual host into a separate function and
call them in a loop. The new code also tries to keep the debug log
entries sane.
Commit id '5e46d7d6' did not take into account that usage of a luks
volume will require usage of the master key encrypted passphrase for
a QEMU environment. So rather than allow creation of something that
won't be usable, just fail the creation.
Partially resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=1301021
If the volume xml was looking to create a luks volume take the necessary
steps in order to make that happen.
The processing will be:
1. create a temporary file (virStorageBackendCreateQemuImgSecretPath)
1a. use the storage driver state dir path that uses the pool and
volume name as a base.
2. create a secret object (virStorageBackendCreateQemuImgSecretObject)
2a. use an alias combinding the volume name and "_luks0"
2b. add the file to the object
3. create/add luks options to the commandline (virQEMUBuildLuksOpts)
3a. at the very least a "key-secret=%s" using the secret object alias
3b. if found in the XML the various "cipher" and "ivgen" options
Signed-off-by: John Ferlan <jferlan@redhat.com>
Vz containers are able to use ploop volumes from storage pools
to work upon.
To use filesystem type volume, pool name and volume name should be
specifaed in <source> :
<filesystem type='volume' accessmode='passthrough'>
<driver type='ploop' format='ploop'/>
<source pool='guest_images' volume='TEST_POOL_CT'/>
<target dir='/'/>
</filesystem>
The information about pool and volume is stored in ct dom configuration:
<StorageURL>libvirt://localhost/pool_name/vol_name</StorageURL>
and can be easily obtained via PrlVmDevHd_GetStorageURL sdk call.
The only shorcoming: if storage pool is moved somewhere the ct
should be redefined in order to refresh the information aboot path
to root.hdd
Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com>
The modification of .volWipe callback wipes ploop volume using one of
given wiping algorithm: dod, nnsa, etc.
However, in case of ploop volume we need to reinitialize root.hds and DiskDescriptor.xml.
v2:
- added check on ploop tools presens
- virCommandAddArgFormat changed to virCommandAddArg
Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com>
In order to use more common code and set up for a future type, modify the
encryption secret to allow the "usage" attribute or the "uuid" attribute
to define the secret. The "usage" in the case of a volume secret would be
the path to the volume as dictated by the backwards compatibility brought
on by virStorageGenerateQcowEncryption where it set up the usage field as
the vol->target.path and didn't allow someone to provide it. This carries
into virSecretObjListFindByUsageLocked which takes the secret usage attribute
value from from the domain disk definition and compares it against the
usage type from the secret definition. Since none of the code dealing
with qcow/qcow2 encryption secrets uses usage for lookup, it's a mostly
cosmetic change. The real usage comes in a future path where the encryption
is expanded to be a luks volume and the secret will allow definition of
the usage field.
This code will make use of the virSecretLookup{Parse|Format}Secret common code.
Signed-off-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1316370
Consider the following disk for a domain:
<disk type='volume' device='cdrom'>
<driver name='qemu' type='raw'/>
<auth username='libvirt'>
<secret type='iscsi' usage='libvirtiscsi'/>
</auth>
<source pool='iscsi-secret-pool' volume='unit:0:0:0' mode='direct' startupPolicy='optional'/>
<target dev='sda' bus='scsi'/>
<readonly/>
<address type='drive' controller='0' bus='0' target='0' unit='0'/>
</disk>
Now, startupPolicy is currently not allowed for iscsi disks, so
one would expect an error message to be thrown. But what a
surprise is waiting for users if they try to start up such
domain:
==15724== Invalid free() / delete / delete[] / realloc()
==15724== at 0x4C2B1F0: free (vg_replace_malloc.c:473)
==15724== by 0x54B7A69: virFree (viralloc.c:582)
==15724== by 0x552DC90: virStorageAuthDefFree (virstoragefile.c:1549)
==15724== by 0x552F023: virStorageSourceClear (virstoragefile.c:2055)
==15724== by 0x552F054: virStorageSourceFree (virstoragefile.c:2067)
==15724== by 0x55556AA: virDomainDiskDefFree (domain_conf.c:1562)
==15724== by 0x5557ABE: virDomainDefFree (domain_conf.c:2547)
==15724== by 0x1B43CC42: qemuProcessStop (qemu_process.c:5918)
==15724== by 0x1B43BA2E: qemuProcessStart (qemu_process.c:5511)
==15724== by 0x1B48993E: qemuDomainObjStart (qemu_driver.c:7050)
==15724== by 0x1B489B9A: qemuDomainCreateWithFlags (qemu_driver.c:7104)
==15724== by 0x1B489C01: qemuDomainCreate (qemu_driver.c:7122)
==15724== Address 0x21cfbb90 is 0 bytes inside a block of size 48 free'd
==15724== at 0x4C2B1F0: free (vg_replace_malloc.c:473)
==15724== by 0x54B7A69: virFree (viralloc.c:582)
==15724== by 0x552DC90: virStorageAuthDefFree (virstoragefile.c:1549)
==15724== by 0x12D1C8D4: virStorageTranslateDiskSourcePool (storage_driver.c:3475)
==15724== by 0x1B4396E4: qemuProcessPrepareDomain (qemu_process.c:4896)
==15724== by 0x1B43B880: qemuProcessStart (qemu_process.c:5466)
==15724== by 0x1B48993E: qemuDomainObjStart (qemu_driver.c:7050)
==15724== by 0x1B489B9A: qemuDomainCreateWithFlags (qemu_driver.c:7104)
==15724== by 0x1B489C01: qemuDomainCreate (qemu_driver.c:7122)
==15724== by 0x561CA97: virDomainCreate (libvirt-domain.c:6787)
==15724== by 0x12B6FD: remoteDispatchDomainCreate (remote_dispatch.h:4116)
==15724== by 0x12B61A: remoteDispatchDomainCreateHelper (remote_dispatch.h:4092)
The problem is, in virStorageTranslateDiskSourcePool disk
def->src->auth is freed, but the pointer is not set to NULL. So
later, when qemuProcessStop starts to free the domain definition,
virStorageAuthDefFree() tries to free the memory again, instead
of jumping out immediately.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Create a function to return a temporary file path to be used in a mkostemp
type call using the path to the stateDir + pool->def->name + vol->name
Signed-off-by: John Ferlan <jferlan@redhat.com>
The VIR_STORAGE_POOL_EVENT_REFRESHED constant does not
reflect any change in the lifecycle of the storage pool.
It should thus not be part of the storage pool lifecycle
event set, but rather be a top level event in its own
right. Thus we introduce VIR_STORAGE_POOL_EVENT_ID_REFRESH
to replace it.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
In the unlikely case the iSCSI session path exists, but does not
contain an entry starting with "target", we would silently use
an initialized value.
Rewrite the function to correctly report errors.
The directories we iterate over are unlikely to contain any entries
starting with a dot, other than '.' and '..' which is already skipped
by virDirRead.
Move the enum into a new src/util/virsecret.h, rename it to be
virSecretLookupType. Add a src/util/virsecret.h in order to perform
a couple of simple operations on the secret XML and virSecretLookupTypeDef
for clearing and copying.
This includes quite a bit of collateral damage, but the goal is to remove
the "virStorage*" and replace with the virSecretLookupType so that it's
easier to to add new lookups that aren't necessarily storage pool related.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Every driver provides a refreshPool impl, and many other critical
places in the code unconditionally call it without checking if
it exists, so this check is pointless
