Commit Graph

41 Commits

Author SHA1 Message Date
Takashi Natsume dfef52f3a6 Fix wrong assertion methods
Change-Id: I0e1ce867c76fcf4fb5784550c1f1f051498176a4
Closes-Bug: 1989280
Signed-off-by: Takashi Natsume <takanattie@gmail.com>
2023-04-07 00:20:10 +09:00
Eric Harney 1dc057296b Tests: Fix invalid assert_called_once calls in driver tests
These drivers all typo'd the same assertion, resulting in
the tests not testing code correctly.

Change-Id: I4e0bc023a5d8f45a8c488d56495a4feb18b6521a
2022-03-02 14:15:15 -05:00
Gorka Eguileor 25eb0a7d76 NFS: Update connection info on online snap create
The NFS snapshot creation for an attached volume requires interaction
between Nova and Cinder, and a new qcow2 file is used after the
attachment completes.

This means that the connection properties stored in the Attachment is no
longer valid, as it is pointing to the old qcow2 file, so if Nova tries
to use that attachment it will start writing on the old qcow2 file.

A flow showing this issue is:

- Attach NFS volume
- Create snapshot
- Hard reboot

After that the VM will start using the base image, breaking the qcow2
chain.

If we delete the snapshot in the meantime, then the VM will fail to
reboot.

This patch fixes this inconsistency by updating the connection info
field inside the remotefs driver.

We usually prefer that drivers don't to touch the DB, directly or
indirectly (using OVOs), but in this case we are using OVOs methods
instead of the usual model update on the volume manager because there
are cases in the driver where a snapshot is created (cloning via
snapshot) and we have to update the attachment without the manager, as
it is unaware that a temporary snapshot is being created.

Besides that main reason there are other less critical reasons to do the
attachment update in the driver:

- Smaller code change
- Easier to backport
- Limit change impact on other areas (better for backport)
- The snapshot_create model update code in the manager does not support
  updating attachments.
- There are cases in the cinder volume manager where the model update
  values returned by snapshot_create are not being applied.

Snapshot deletion belonging to in-use volumes are not affected by this
issue because we only do block commit when the snapshot file we are
deleting is not the active file.  In _delete_snapshot_online:

        if utils.paths_normcase_equal(info['active_file'],
                                      info['snapshot_file']):

Closes-Bug: #1860913
Change-Id: I62fcef3169dcb9f4363a5344af4b2711edfef632
2021-07-07 16:21:26 +02:00
Rajat Dhasmana 81e5a92903 NFS: Fix for groups and cloning
Recently we merged change I43036837274a7c8dba612db53b34a6ce2cfb2f07
which adds format info support for *fs type volumes.
This seems to cause issues when volume has groups or we are cloning
from a bootable volume.
This patch disables the format info support for volumes with groups
and fixes the issue with cloning.

Change-Id: I41ffedfa1abd0708c84494a0aaaa3815810102b8
2021-03-16 06:13:05 -04:00
Rajat Dhasmana ace1748218 Support format info in fs type drivers
This feature adds format info in filesystem type
drivers with the following changes:
1) Store format info in admin_metadata while creating/cloning
volumes
2) Use format info while extending volumes
3) Modify volume format when performing snapshot delete
(blockRebase) operation on attached volume.
4) Return format in connection_info

blueprint add-support-store-volume-format-info

Change-Id: I43036837274a7c8dba612db53b34a6ce2cfb2f07
2021-03-12 12:41:12 -05:00
Alex Deiter 11b5c9d97b Creating snapshot on NFS backend fails
Fixed an issue with creating a snapshot on an NFS backend
if the snapshot name is not specified.

Closes-Bug: 1886222
Change-Id: I33da9c65ef56b3e4967170e3a0fb25f12e067876
2020-07-21 13:34:27 +00:00
Eric Harney 44c7da9a44 NFS encrypted volume support
Volume encryption helps provide basic data protection in
case the volume back-end is either compromised or outright
stolen. The contents of an encrypted volume can only be
read with the use of a specific key;

Volume encryption: Check volume encryption key in
'_do_create_volume' method (remotefs) and use
'_create_encrypted_volume_file' when encryption is
required.

Snapshot encryption: To create an encrypted volume from a
snapshot, we need to pass the Barbican key of the
snapshot.volume to 'convert_image' method. Because of this
I've added 'src_passphrase_file' parameter.

This patch doesn't handle encrypted volume -> unencrypted
Current error prompted: 'Invalid input received: Invalid
volume_type provided: aeac5517-6bc8-4b59-9eb2-76e84369bd0
(requested type is not compatible; recommend omitting the
type argument). (HTTP 400)'

Implements: blueprint nfs-volume-encryption
Co-Authored-By: Sofia Enriquez <lsofia.enriquez@gmail.com>

Change-Id: I896f70d204ad103e968ab242ba9045ca984827c4
2020-06-23 22:00:05 +00:00
Sofia Enriquez e52ab1a3ad Creating image-volume cache on NFS backend fails
snapshot.id doesn't have `tmp-snap` in it as id
is not passed as a option while creating snapshot
object. This should be change to
`snapshot.display_name` at same location

Closes-Bug: #1875570
Change-Id: I66da0212a61d778fe9795d382b3af5b225a8aa54
2020-05-29 13:53:41 +00:00
Eric Harney d4eb4a9ba1 Move unit test code under tests/unit/
test.py was previously not in the tests
directory.  This means that downstream packagers
of Cinder have to specifically exclude it from
the main Cinder package (which does not typically
include unit tests).

Move it under the cinder/tests/unit/ dir where it
should be, to clean this up.

Change-Id: I65c50722f5990f540d84fa361b997302bbc935c5
2020-04-30 18:13:54 -04:00
Sean McGinnis 3eb9b422f4
Introduce flake8-import-order extension
This adds usage of the flake8-import-order extension to our flake8
checks to enforce consistency on our import ordering to follow the
overall OpenStack code guidelines.

Since we have now dropped Python 2, this also cleans up a few cases for
things that were third party libs but became part of the standard
library such as mock, which is now a standard part of unittest.

Some questions, in order of importance:

Q: Are you insane?
A: Potentially.

Q: Why should we touch all of these files?
A: This adds consistency to our imports. The extension makes sure that
   all imports follow our published guidelines of having imports ordered
   by standard lib, third party, and local. This will be a one time
   churn, then we can ensure consistency over time.

Q: Why bother. this doesn't really matter?
A: I agree - but...

We have the issue that we have less people actively involved and less
time to perform thorough code reviews. This will make it objective and
automated to catch these kinds of issues.

But part of this, even though it maybe seems a little annoying, is for
making it easier for contributors. Right now, we may or may not notice
if something is following the guidelines or not. And we may or may not
comment in a review to ask for a contributor to make adjustments to
follow the guidelines.

But then further along into the review process, someone decides to be
thorough, and after the contributor feels like they've had to deal with
other change requests and things are in really good shape, they get a -1
on something mostly meaningless as far as the functionality of their
code. It can be a frustrating and disheartening thing.

I believe this actually helps avoid that by making it an objective thing
that they find out right away up front - either the code is following
the guidelines and everything is happy, or it's not and running local
jobs or the pep8 CI job will let them know right away and they can fix
it. No guessing on whether or not someone is going to take a stand on
following the guidelines or not.

This will also make it easier on the code reviewers. The more we can
automate, the more time we can spend in code reviews making sure the
logic of the change is correct and less time looking at trivial coding
and style things.

Q: Should we use our hacking extensions for this?
A: Hacking has had to keep back linter requirements for a long time now.
   Current versions of the linters actually don't work with the way
   we've been hooking into them for our hacking checks. We will likely
   need to do away with those at some point so we can move on to the
   current linter releases. This will help ensure we have something in
   place when that time comes to make sure some checks are automated.

Q: Didn't you spend more time on this than the benefit we'll get from
   it?
A: Yeah, probably.

Change-Id: Ic13ba238a4a45c6219f4de131cfe0366219d722f
Signed-off-by: Sean McGinnis <sean.mcginnis@gmail.com>
2020-01-06 09:59:35 -06:00
Zuul b20561141b Merge "RemoteFS: Use dest vol id instead of source id in snapshot temp name" 2019-11-15 23:27:37 +00:00
David Hill f3ed9d436b RemoteFS: Use dest vol id instead of source id in snapshot temp name
Use the destination volume id instead of the source volume id
in the temporary snapshot file name.  This is likely not strictly
needed after Change I64e041fe, which ensures that multiple clone
volume operations won't run simultaneously from the same source
volume, but is still a good idea to ensure that there is less
that can go wrong in a failure scenario.

Change-Id: I5bd185d04dbda673a5882d61aaab7acdd99b74a6
Related-Bug: #1851512
Closes-Bug: #1840712
2019-11-13 12:19:22 -05:00
Eric Harney de789648e5 Rename volume/utils.py to volume/volume_utils.py
Much of our code renames this at import already --
just name it "volume_utils" for consistency, and
to make code that imports other modules named "utils"
less confusing.

Change-Id: I3cdf445ac9ab89b3b4c221ed2723835e09d48a53
2019-09-09 15:00:07 -04:00
Silvan Kaiser 78f2101fc5 Add context to cloning snapshots in remotefs driver
Adds context to _create_cloned_volume method in order
to allow taking a snapshot during cloning. Also saves
the temp_snapshot in order to enable Nova to update the
snapshot state. Destroys the temp_snapshot after usage.
This affects the NFS, WindowsSMBFS, VZStorage and
Quobyte drivers.

Closes-Bug: #1744692

Change-Id: I328a02d0f26e8c3d41ec18a0487da6fd20b39b04
2019-07-10 10:41:15 +02:00
Eric Harney ade7d89c2e Revert "Remove truncate from rootwrap filters"
This reverts commit a62c9dfdd4.

This did not account for cases where truncate is
called w/o elevated privileges.

Related-Bug: #1818504
Change-Id: I3cb85be854e68fda525cfebe254ce7c85d8e3d37
2019-03-06 10:08:10 -05:00
Charles Short a62c9dfdd4 Remove truncate from rootwrap filters
Use oslo.privsep for the truncate command.

Change-Id: Ic287c64a4e0f663738e23d22e819b6ffee9c84c1
Signed-off-by: Charles Short <chucks@redhat.com>
2019-02-21 16:34:01 -05:00
Lucian Petrut 133617828c Avoid using 'truncate' on Windows
The Windows SMB driver has been broken by a recent change [1],
which is attempting to handle info file permissions, using the
'truncate' command along the way, which is not available on Windows.

We're not handling file permissions on Windows, so we can just skip
this step, avoiding this issue.

Change-Id: I1f92cef19cd2eed7ba845210f7ff89d88ba78f49
Closes-Bug: #1811369
2019-01-11 13:38:35 +02:00
Alan Bishop 1fb342cba8 Fix permissions with NFS-backed snapshots and backups
Fix snapshot issues that occur if cinder does not have regular (non-root)
write permission on an NFS share. This is done by following the volume
driver's model of creating files as root, followed by calling
_set_rw_permissions().

Fix backup issues that occur if cinder's group ID changes, which can
happen when migrating from bare metal to a containerized service. If the
share's group ownership and permission needs to be modified, then do it
recursively so that previously made backups remain accessible.

Closes-Bug: #1807760
Change-Id: I6c20c4825af0a365b6a20fb633c810c2f2fe48b0
2018-12-11 08:13:48 -05:00
Zuul 6f88e910af Merge "Adds Overlay Volumes Created from Snapshots to Quobyte" 2018-08-21 23:29:17 +00:00
Zuul 00956b26ab Merge "Fix remotefs driver report wrong value" 2018-08-02 19:03:33 +00:00
Kien Ha b629e4eba1 Fix remotefs driver report wrong value
allocated_capacity_gb should not be reported by drivers and let the
manager handle this.
provisioned_capacity_gb is reporting allocated physical space. Return
provisioned.

Change-Id: I7e1b8d5b399a8dd15bb0dee477090a45c2faa6fa
Closes-Bug: #1746233
Signed-off-by: Kien Ha <kienha9922@gmail.com>
2018-07-30 19:00:13 -04:00
Silvan Kaiser 0bf81e69d9 Adds Overlay Volumes Created from Snapshots to Quobyte
In order to further prevent delays during creation of volumes from
snapshots this adds the creation of volumes as overlay files backed by
volumes in the volume_from_snapshot cache (introduced in
Change I2142b1c0a0cc2c4f85794416e702a326d3406b9d). This speeds up
the creation of new volumes from a snapshot as the new volume is created
as an overlay file for the cached volume instead of beeing a full copy
of the cached file.
Such overlay based volumes are tracked via softlinks in the
volume_from_snapshot cache directory in order to ensure deletion of the
cached volume only if all related volumes & snapshots have already been
removed.

Besides changing the Quobyte driver this also adds using specialized
backing file templates when running qemu-img info commands in RemoteFS
based drivers.
Default behaviour is unchanged but drivers may now provide an optional
modified matching template, if required.

Partial-Bug: #1715078

Change-Id: I2a213b456514c15ce54d7082e272adff6a59cbd7
2018-07-03 10:53:14 +02:00
Sean McGinnis 09654af518 Fix invalid escape sequence warnings
There are a lot of DeprecationWarning message emited when running
under python 3.6 due mostly to non-raw strings being used for
regex expressions.

DeprecationWarning: invalid escape sequence \

These had previously been ignored, but starting with python 3.6,
and the commit for issue 27364, these are now considered deprecated
and will no longer be supported going forward. Per the discussion
on that issue, these strings should really be raw strings any way:

https://bugs.python.org/issue27364#msg272696

Change-Id: If6ff206e4bbcf10ab52d2895f606dafad2936ddb
2018-06-20 16:22:26 -05:00
Lucian Petrut 8aa2f5a5b6 SMBFS: fix creating volume from snapshot
In some cases, attempting to create a volume from an in-use snapshot
fails because of Hyper-V locks.

The reason is that in order to locate the backing image that needs
to be *cloned*, we may need to query the currently attached image,
which is not allowed.

In order to avoid this, we're going to store the backing file
information in the DB using the snapshot object metadata field,
at the same time preserving backwards compatibility.

Note that we cannot rely on model changes passed to the manager as
snapshots are used internally in a few cases in which the manager
would not expect snapshot object changes. Also, when deleting
snapshots, we're updating the metadata for more recent snapshots
as the backing file changes.

Fake volume/snapshot object usage throughout the smbfs driver unit
tests had to be updated due to the object changes performed by it.

Change-Id: I88e9dad5cf36621c4b86acc7b7e972850e061119
Closes-Bug: #1694635
2018-01-31 18:38:35 +02:00
sunyandi 1cfd567449 modify volume spelling errors
Change-Id: I14497ed20c6938c0c9662fc94d641ecaf65e984a
2018-01-18 18:46:57 +08:00
Zuul 5eef4b5cdb Merge "SMBFS: fix detecting if a volume is in-use" 2017-12-20 00:07:14 +00:00
Lucian Petrut fb48a1fe64 SMBFS: fix detecting if a volume is in-use
The SMBFS driver along with the remotefs module rely on the volume
status field when checking if it's attached.

While in most cases this is fine, it will cause issues when backing
up volumes. In this situation, the volume status will be 'backing-up',
in which case the driver will wrongfully consider that the volume is
detached (by not having the 'in-use' state).

In particular, this affects the volume snapshot workflow.

This change fixes this check, adding a helper method which detects
the volume attach status by looking at the volume 'attach_status'
field.

Change-Id: I154d85cde693c6e77e22a677cae7150be48fce07
Partial-Implements: windows-smb-backup
2017-12-19 15:17:59 +02:00
Eric Harney 524a74c6ba qemu-img info --force-share for NFS driver
NFS initialize_connection fails due to new locking
code in qemu-img info.  Bug 1718133 tracked a related issue.

Fix the NFS driver to work with the new version of
qemu-img info by disabling locking for qemu-img info
queries during initialize_connection and create_snapshot.

Closes-Bug: #1731992
Change-Id: Ia3623af2048d6cbda65deebf4404e6b5fefe1bfc
2017-12-14 15:42:36 +00:00
Lucian Petrut ed945da6bf SMBFS: manageable volumes
This change allows the SMBFS driver to claim (manage) vhd/x images
present on pre-configured shares.

Note that images having backing (parent) files will be rejected.
Managing snapshots will not be supported either.

The images may reside in subdirectories (and get moved afterwards),
which may be useful when having a folder dedicated to images that
are to be imported in Cinder.

All the logic is included in the new "RemoteFSManageableVolumesMixin"
class so that any related driver may inherit it. For the record,
this works out of the box for the NFS driver (special care has been
taken in this sense).

Implements: blueprint remotefs-manage-unmanage-volume
Change-Id: I54695655e563d84e4fb1b76c42f0127c5fb909f7
2017-11-23 11:14:59 +02:00
Zuul 3e6e2bc101 Merge "SMBFS: add fixed image support" 2017-11-22 19:35:47 +00:00
AlexMuresan 54c2787132 SMBFS: add fixed image support
The SMBFS driver currently supports only dynamic vhd/x images.

This change will allow configuring the driver to use fixed images
using the 'nas_volume_prov_type' config option.

Note that fixed images will always be zeroed out. We recommend
placing the images on ReFS/SSD backed storage.

Implements: blueprint smbfs-fixed-images

Change-Id: Ic9e724fcda83fea384406f45f792f7a2d4b979bb
2017-11-03 11:47:23 +02:00
Alexandru Muresan 413664d11c RemoteFS: revert snapshot support
This change implements the 'revert_to_snapshot' method at the remotefs
base driver level in a generic way. This should work right away with
most (if not all) remotefs based drivers.

Note that we only consider the case in which the latest snapshot is
being reverted (the Cinder API won't allow reverting volumes to other
snapshots anyway, at least for the time being).

The RemoteFS based drivers (e.g. NFS driver, SMB driver) use COW images
(e.g. qcow2, differencing vhdx) in order to provide volume snapshot
functionality. Reverting a volume to its latest snapshot is a trivial
operation in this case and can be achived by simply cleaning up (e.g.
recreating) the latest image from chain.

In order to avoid breaking drivers, the 'revert_to_snapshot' method
is implemented in a mixin that may be inherited by any RemoteFS based
driver.

Implements: blueprint remotefs-revert-snapshot

Change-Id: Id66d3f0fe36cf7c32ef3875bf5fcb1c7a4678273
2017-10-18 12:17:40 +03:00
Lucian Petrut 1dd3dea3cd SMBFS: enhance volume cloning
This change enhances the volume clone operation by simply copying
the image file if the cloned volume doesn't have any snapshots.

This way, we don't have to convert a chain of images. Also, this
means that we may benefit from copy offloading.

This improvement especially helps image caching, which relies on
efficient volume cloning.

This is basically implemented in the base remotefs module, any
similar driver leveraging it may enable it by switching a flag,
assuming that it implements the required methods.

Note that the copied image needs to be extended according to the
new volume size. We cannot simply call the 'extend_volume' method
as most of the RemoteFS based drivers will apply a lock to it, in
which case we'd get into a deadlock. For this reason, we follow
the common practice among those drivers, involving a helper that
won't get a lock decorator.

Change-Id: I96ac2f019d522ebbeb6c28fc4c11b41d9ba8fdc0
2017-06-27 21:26:18 +00:00
Lucian Petrut 1076e1bd67 Windows: case insensitive path comparisons
While Windows paths are case insensitive, some checks performed in the
SMBFS driver as well as its parent class are not, breaking the driver's
logic.

We're most affected by the fact that some of the Windows API functions
disregard path case sensitivity, always returning uppercase strings in
some situations.

This change adds a helper function that compares paths having the os
type in mind and uses it throughout the remotefs and smbfs modules.

Change-Id: Icfef1c8b9ae011d2b463aa5c1fb7f512388c8f88
Closes-Bug: #1694648
2017-06-26 12:37:09 +03:00
Lucian Petrut 63b9f0fb8e RemoteFS: enable image volume cache
At the moment, the RemoteFS based drivers cannot leverage the generic
image volume cache feature.

The reason is that this involves cloning a newly created volume while
still being in 'downloading' state, which is an unacceptable state
from the driver's point of view.

This change adds 'downloading' as an acceptable state for the volume
clone operation as well as for creating/deleting snapshots used
internally by the volume clone operation.

Note that 'regular' volume clones will be rejected before reaching
the driver when having a source volume in 'downloading' state.

Change-Id: I84bb3f062637031f7f46b414d6cbbff0bb0292ea
Closes-Bug: #1685277
2017-06-07 19:07:40 +03:00
Lucian Petrut daeab949a7 Add RemoteFSPoolMixin
This change adds a mixin class to the remotefs module so that
any related volume driver can easily start reporting each share
as a storage pool.

Partially Implements: blueprint remotefs-pools-support

Change-Id: I52da65b1987f4ebdcb5c513f2a316bba9de8e892
2017-05-25 11:46:18 +03:00
Evgeny Antyshev 3acf94de13 Fix double call to "qemu-img create"
Introduced by https://review.openstack.org/#/c/147186,
and unit tests didn't notice (integrated gate testing too)
We noticed this in Virtuozzo Storage CI, as it requires
backing_fmt to be defined when snapshot is created,
because otherwise it accesses backing file, which could
be held by Qemu and not available (our storage doesn't
allow simultaneous open from different mountpoints)

Change-Id: I9adcc57ffc79548872e7a5c5b0935a2cb4a2dea7
2017-01-26 15:11:22 +00:00
Eric Harney 2d77a7a87d NFS snapshots
This patch adds support for snapshots to the NFS driver.

This functionality is only enabled if
"nfs_snapshot_support" is set to True in cinder.conf
and nas_secure_file_operations is disabled.

This is because compute nodes running libvirt <1.2.7 will
encounter problems with snapshot deletion, which includes
Ubuntu 14.04 and requires write access to the volumes
created by a different user. Therefore, deployers must
opt-in to this functionality if their environment is known
to support it.

Due libvirt limitations[1], apparmor must be disabled in the
compute nodes or the volume export location be added to
libvirt's default profile template[3].

Clonning volumes is only supported if the source volume
is not attached.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1361592
[2] http://paste.openstack.org/show/570296/
[3] http://paste.openstack.org/show/570297/

Co-Authored-By: Ankit Agrawal <ankit11.agrawal@nttdata.com>
Co-Authored-By: Jay S. Bryant <jsbryant@us.ibm.com>
Co-Authored-By: Erlon R. Cruz <erlon.cruz@fit-tecnologia.org.br>
Implements: blueprint nfs-snapshots

DocImpact: A new parameter is added and the clonning
limitation must be documented.

Closes-bug: #1614249
Change-Id: Iae35c722eb4b6b7d02a95690abbc07a63da77ce7
2017-01-25 14:12:11 -05:00
Ji-Wei ad36b6948c Make divisible py3 compatible in remotefs driver
the code:
import platform
print('Python', platform.python_version())
print('3 / 2 =', 3 / 2)
print('3 // 2 =', 3 // 2)
print('3 / 2.0 =', 3 / 2.0)
print('3 // 2.0 =', 3 // 2.0)

in py2
('Python', '2.7.10')
('3 / 2 =', 1)
('3 // 2 =', 1)
('3 / 2.0 =', 1.5)
('3 // 2.0 =', 1.0)

in py3
Python 3.4.3+
3 / 2 = 1.5
3 // 2 = 1
3 / 2.0 = 1.5
3 // 2.0 = 1.0

So it need to modify the code to adapt to this difference.

Change-Id: I6811f849ffcd2aaa4a6fefc8ce864097b447cf5f
Closes-Bug: #1596139
2016-10-08 13:03:28 +00:00
Dmitry Guryanov 79b3a92e63 vzstorage: fix create/delete snapshots
vzstorage doesn't support simultaneous open from different
clients. It means you can't open qcow2 file from cinder-volume
side, if it's used by an instance on nova-compute side.
The solution is to cache QemuImgInfo instance
under <path>.qemu_img_cache for image file <path>

Closes-Bug: #1615667
Change-Id: I8949ddc244e0eef699b2af224303f86507f6f30e
Co-Authored-By: Evgeny Antyshev <eantyshev@virtuozzo.com>
2016-08-23 08:53:08 +00:00
Ivan Kolodyazhny 39e0c888ba Move drivers unit tests to unit.volume.drivers directory
This change makes our tests directory cleaner a bit and
helps to find tests faster.

Some of these tests freeze for a long time, so they are skipped.

Related-Bug: #1578986

Change-Id: Ic292b9608cd3d174a2b9b30e7d941e728f32547c
2016-07-25 17:39:50 -04:00