It appears that there are certain cases where a detach
and a delete call can overlap causing a race in the
remove_iscsi_target method. The result is that when
the slower method gets around to the final removal of
the persistence file it's no longer there, so we
hit an unhandled exception.
This patch does two things:
1. It adds the volume lock to the detach_volume
method in the manager (already in place for delete).
This should mitigate the race and allow us to avoid it.
2. In the iscsi_remove_target method check if the
persist file has already been removed when we get
to the last check. The fact is that if it was somehow
removed before we got here we don't really care, we're
just going to delete it anyway.
Avishay Traeger [Sat, 8 Feb 2014 20:59:58 +0000 (22:59 +0200)]
Storwize/SVC: Fix races in host-related functions
In the Storwize/SVC driver, initialize_connection checks if a host
object exists on the storage, and if not, creates one. Similarly,
terminate_connection checks if there are any more mappings for this
host, and deletes the host object if there are none. Consequently, there
exist races between two initialize_connection calls, two
terminate_connection calls, or one and one.
The easy solution here is to use locks. Because locks are files on the
local machine, this implies that all cinder-volume processes managing a
given Storwize/SVC contoller run on the same machine.
Jeff Applewhite [Thu, 30 Jan 2014 19:11:08 +0000 (14:11 -0500)]
Fixes cinder failed to create/restore a backup with NFS driver
The action cinder backup-create fails when trying to backup a newly
created volume when using cinder.volume.drivers.nfs.NfsDriver. This
patch removes not implemented stubs for backup_volume and
restore_volume. The inherited methods in cinder/volume/driver.py
succeed in creating a backup and in restoring it. After manual
testing of this change backups/restores succeed without the not
implemented stubs. This change also adds the not implemented
stubs to the glusterfs driver until a fix is submitted.
Eric Harney [Wed, 29 Jan 2014 15:45:03 +0000 (10:45 -0500)]
Rename self.tgtadm to self.target_helper
Using "tgtadm" for this member variable is confusing because it
actually represents tgt, IET, and LIO. Rename to a more generic
term to make things more readable.
Also remove "tgtadm" from method names and references in tests
where it is not tgtadm-specific.
I'm choosing the name "target_helper" because "tgtadm" = tgtd,
and "iscsi_helper" doesn't fit when using RDMA in ISERDriver.
There was a race condition between the two Event() objects being
created within loopingcall and Retry in api module. Did away with
the Event object in Retry and used loopingcall the right way.
This commit brings the implementation of the test_rbd and
test_backup_ceph unit tests more inline and tries to ensure
that no mock is set without being unset so as not to tread
on the toes of any other tests.
John Griffith [Fri, 31 Jan 2014 03:53:37 +0000 (03:53 +0000)]
Remove create_export from volume create
Currently the LVM driver creates iscsi targets on
volume creation, even though it's only used at
attach time. This isn't necessary and in fact
causes issues if a volume is extended because the
target information isn't currently updated after the
extend. In addition a number of other drivers inclduing
the LIO iscsi driver require the target be created with
initiator info, so the tgt that's created initially
again isn't valid.
This change removes the create_export call from the
volume create process and makes it part of the
managers intialize_connection routine which is
more appropriate.
This change also removes the target on detach since
it's not used any longer and again as the volume may
be modified or extended.
Time unit is missing in the description of vmdk driver option
"vmware_task_poll_interval". Due to this, the vmdk driver config
reference document doesn't have the timeunit in its description
column. This fix adds the missing time unit (seconds) in the
driver option.
Avishay Traeger [Sun, 26 Jan 2014 10:39:27 +0000 (12:39 +0200)]
Allow spaces in host names in the storwize driver
Storwize/SVC naming rules allow host objects to have spaces in their
names. The SSH injection filter will currently reject such names, and so
we surround the names with quotes. Please note that this doesn't allow
to have an arbitrary character in there except a space (anything else
will be cleaned up by the driver or rejected by the SSH injection filter
up the call chain).
Bertrand Lallau [Sat, 1 Feb 2014 13:33:33 +0000 (14:33 +0100)]
Remove a catching exception during delete_volume
GlanceMetadataNotFound Exception was catching during
a call to volume_glance_metadata_delete_by_volume().
This exception is never thrown by the implementation.
This catching has been removed.
This patch migrates all of the communication
to the 3PAR array into the client library.
Some of the calls to the array happen over ssh
and others happen over REST. Now the drivers
don't care.
This allows us to change the external client
library to replace SSH calls to REST calls,
without the need of driver changes.
Ryan McNair [Mon, 27 Jan 2014 21:48:51 +0000 (21:48 +0000)]
Add support for special char in volume metadata
Using special characters such as "&" in volume metadata caused SAX
parser to split text into multiple child nodes, as stated in the
specs for SAX parsers. In this case extract_text would return
'None' because text was return if there was exactly
one child node. Concatenating the values of child nodes accounts
for SAX parsers splitting contigous characters into
multiple chunks.
Brianna Poulos [Wed, 29 Jan 2014 20:49:20 +0000 (15:49 -0500)]
Stop volume_type_encryption creation when in use
This bug fix addresses bug #1274252. It requires a volume
type to have no volumes with the type before allowing it
to be made encrypted. This prevents a situation where
non-encrypted volumes could have an encrypted volume type.
This fix also adds a unit test to confirm functionality.
john-griffith [Wed, 29 Jan 2014 04:30:59 +0000 (21:30 -0700)]
Revert initialize_connection changes
A change was merged that reusulted in interittent
gate failures. It seems that in certain cases the
intitialize is doing an update targets and is
deleting the existing targets and not recreating them.
The idea of this patch was to fix up the iscis targets
for cases where volumes are extended.
A better solution is in progress that will remove the
target creation from the create_volume process altogether.
Currently we seem to have create_export, ensure_export and
now initialize that all are designed to do very similar
things.
A bug has been filed to address this and will attempt
to collapse these functions to be done at attach time.
Florian Haas [Mon, 20 Jan 2014 21:42:11 +0000 (22:42 +0100)]
VolumeManager: initialize even if a volume can't be found
If a previously volume cannot be exported (for example, because
an iSCSI target's backing LV is no longer present), VolumeManager
would previously bail out, leaving the volume service uninitialized.
This left volumes dangling in limbo: their state would be reported
as available (clearly not a reflection of reality), and they could
not be deleted (because the volume service that would be responsible
for deletion was unavailable).
Instead, catch an exception raised by ensure_export() separately,
log it, and set the volume to the error state.
For the tgt backend, this will also remove the volume_path file.
Previously this would cause an ISCSITargetRemoveFailed exception
when the volume would be removed. Instead, simply log a warning
and return, the way other backends (example: RBD) already do.
Also, add an info message that reflects the actual path and
contents of the volume_path file.
Finally, fix up the tgtadm unit test so that it tests for
identical IQNs,
Eric Harney [Mon, 27 Jan 2014 23:46:26 +0000 (18:46 -0500)]
Add create_iscsi_target stub to TargetAdmin
This should be defined in TargetAdmin, as it is assumed to be
present in all subclasses by callers.
IetAdm and LioAdm may need additional work to behave as desired
in all circumstances (such as volume extend), but this should
at least stabilize things in the meantime.
This patch validates the keys of the extra specs
before setting them.
This will make possible to remove those keys.
It allows alphanumeric characters, underscores,
periods, colons and hyphens.
Zhiteng Huang [Thu, 9 Jan 2014 06:54:22 +0000 (14:54 +0800)]
Make sure report_interval is less than service_down_time
Services that inherit service.py/Service class would register
themselves to DB and then update stats periodically (every
report_interval second). The consumer of this kind of information,
like scheduler or 'os-service' API extension, will consider a service
is 'up' (active) if last update from that service is not longer than
'service_down_time' ago.
The problem is if 'report_interval' was configured/provided greater
than 'service_down_time' by mistake, services would then be always
considered in 'down' state, which can result in unsuccesful placement
of volume create request for example. This is what Bug #1255685 is
about.
In previous fix: https://review.openstack.org/#/c/60760/, a
configuration check helper function basic_config_check() was added
*wrongly* to WSGIService class instead of Service class. This patch
moves the configuration check helper function and the check to the
right place to make sure 'report_interval' is less then
'service_down_time'.
Code Review / openstack-build / cinder-build.git / log