Commit Graph

16 Commits

Author SHA1 Message Date
whoami-rajat 1a8ea0eac4 Skip sparse copy during volume reimage
When rebuilding a volume backed instance, while copying the new
image to the existing volume, we preserve sparseness.
This could be problematic since we don't write the zero blocks of
the new image and the data in the old image can still persist
leading to a data leak scenario.

To prevent this, we are using `-S 0`[1][2] option with the `qemu-img convert`
command to write all the zero bytes into the volume.

In the testing done, this doesn't seem to be a problem with known 'raw'
images but good to handle the case anyway.

Following is the testing performed with 3 images:

1. CIRROS QCOW2 to RAW
======================

Volume size: 1 GiB
Image size (raw): 112 MiB

CREATE VOLUME FROM IMAGE (without -S 0)

LVS (10.94% allocated)
  volume-91ea43ef-684c-402f-896e-63e45e5f4fff stack-volumes-lvmdriver-1 Vwi-a-tz-- 1.00g stack-volumes-lvmdriver-1-pool 10.94

REBUILD (with -S 0)

LVS (10.94% allocated)
  volume-91ea43ef-684c-402f-896e-63e45e5f4fff stack-volumes-lvmdriver-1 Vwi-aotz-- 1.00g stack-volumes-lvmdriver-1-pool 10.94

Conclusion:
Same space is consumed on the disk with and without preserving sparseness.

2. DEBIAN QCOW2 to RAW
======================

Volume size: 3 GiB
Image size (raw): 2 GiB

CREATE VOLUME FROM IMAGE (without -S 0)

LVS (66.67% allocated)
  volume-edc42b6a-df5d-420e-85d3-b3e52bcb735e stack-volumes-lvmdriver-1 Vwi-a-tz-- 3.00g stack-volumes-lvmdriver-1-pool 66.67

REBUILD (with -S 0)

LVS (66.67% allocated)
  volume-edc42b6a-df5d-420e-85d3-b3e52bcb735e stack-volumes-lvmdriver-1 Vwi-aotz-- 3.00g stack-volumes-lvmdriver-1-pool 66.67

Conclusion:
Same space is consumed on the disk with and without preserving sparseness.

3. FEDORA QCOW2 TO RAW
======================

CREATE VOLUME FROM IMAGE (without -S 0)

Volume size: 6 GiB
Image size (raw): 5 GiB

LVS (83.33% allocated)
  volume-efa1a227-a30d-4385-867a-db22a3e80ad7 stack-volumes-lvmdriver-1 Vwi-a-tz-- 6.00g stack-volumes-lvmdriver-1-pool 83.33

REBUILD (with -S 0)

LVS (83.33% allocated)
  volume-efa1a227-a30d-4385-867a-db22a3e80ad7 stack-volumes-lvmdriver-1 Vwi-aotz-- 6.00g stack-volumes-lvmdriver-1-pool 83.33

Conclusion:
Same space is consumed on the disk with and without preserving sparseness.

Another testing was done to check if the `-S 0` option actually
works in OpenStack setup.
Note that we are converting qcow2 to qcow2 image which won't
happen in a real world deployment and only for test purposes.

DEBIAN QCOW2 TO QCOW2
=====================

CREATE VOLUME FROM IMAGE (without -S 0)

LVS (52.61% allocated)
  volume-de581f84-e722-4f4a-94fb-10f767069f50 stack-volumes-lvmdriver-1 Vwi-a-tz-- 3.00g stack-volumes-lvmdriver-1-pool 52.61

REBUILD (with -S 0)

LVS (66.68% allocated)
  volume-de581f84-e722-4f4a-94fb-10f767069f50 stack-volumes-lvmdriver-1 Vwi-aotz-- 3.00g stack-volumes-lvmdriver-1-pool 66.68

Conclusion:
We can see that the space allocation increased hence we are not preserving sparseness when using the -S 0 option.

[1] https://qemu-project.gitlab.io/qemu/tools/qemu-img.html#cmdoption-qemu-img-common-opts-S
[2] abf635ddfe/qemu-img.c (L182-L186)

Closes-Bug: #2045431

Change-Id: I5be7eaba68a5b8e1c43f0d95486b5c79c14e1b95
2023-12-26 13:08:58 +05:30
Gorka Eguileor e071ee2632 Docs: Document clone_image driver method
This patch improves and fixes the documentation of the clone_image
driver method and updates the parameters in various places where the
method is declared where they presented the wrong parameters.

Since there is a valid default implementation for the method, a small
text has been added to indicate that some of the core methods, like this
one, are optional.

To avoid getting docstrings from the driver and the interface (which is
the one used for the documentation) out of sync we replace the
description on cinder/volume/driver.py with a text referencing the
interface docstring.

Change-Id: I09a8357e15b9041790285ab1f9a3cdd7a18243b4
2022-05-12 20:02:56 +02:00
Gorka Eguileor 39e518456c Fix reported storage_protocol
Cinder has multiple drivers reporting the same storage_protocol in their
stats but with different strings:

For example we have the following variants:
- nfs and NFS
- nveof, NVMe-oF, NVMeOF
- iSCSI, iscsi
- FC, fc, fibre_channel

This patch documents the right values (to use existing constants) and
makes the scheduler treat all variants as if they were equal.

This is done by changing the storage_protocol into a list in the
scheduler upon reception of the stats from the volume.  Most of the code
in the scheduler is already able to handle this, the only changes
necessary are on the filter and goodness function code, which will now
run the respective functions for all the different protocol alternatives
and select the right one, but only when the function actually uses the
storage_protocol.

The API will now report the preferred protocol name in operations like
"cinder get-pools --detail".

Closes-Bug: #1966103
Change-Id: I07d74078dbb102490dd722029e32c74cec3aa44c
2022-04-20 18:48:24 +02:00
Gorka Eguileor d5a8c72032 Remove attach and detach volume driver methods
As part of the effort agreed in the last PTG to simplify and properly
define the driver interface this patch removes "attach_volume" and
"detach_volume" methods from the driver interface and from the SolidFire
driver, the only driver that is using it.

Change-Id: Ieb08a6870da92e438b3e7d6f48c1bdeb4d560e22
2022-01-31 15:46:22 +01:00
Gorka Eguileor bb8771436e Add explanations on safe delete
We keep seeing people concerned about data leakage and thorough deletion
of data from volumes on volume deletion [1][2], and Cinder currently
only worries about the data leakage, not the stealing of the physical
disks.

In order to clarify this for administrators and to reduce the number of
specs proposed to address this issue we are adding additional
documentation on security aspects as well as data leakage.

This patch was decided as an action from the discussion during the
Wallaby PTG [3].

[1]: https://review.opendev.org/#/c/758375/
[2]: https://review.opendev.org/#/c/759553/
[3]: https://wiki.openstack.org/wiki/CinderWallabyPTGSummary#Two_proposed_specs_on_the_same_topic_.28mutually_assured_destruction.29

Change-Id: I9f8d413cf8337e75241ad24c85692135b34a17fc
Implements: blueprint specify-data-secure-deletion
2021-03-17 14:04:20 +01:00
Ivan Kolodyazhny fdd0a3bf5e Move get_volume_stats impl to the base volume driver
Change-Id: I7f26b270d5a85cd40ffbb3b33006a41b2e4852a1
2020-07-01 12:41:20 +03:00
Sean McGinnis effe719e8f
Elaborate on terminate_connection documentation
We have had issues with drivers not handling terminate_connection
correctly when the connector argument is None. In these cases, the
driver is expected to terminate all connections to the volume.

This adds additional information to the volume driver interface
docstring to try to better point out this expectation.

Change-Id: Ibbf481da981428cd921fb9cb6bec6af8f6f330b3
Signed-off-by: Sean McGinnis <sean.mcginnis@gmail.com>
2019-12-04 09:21:15 -06:00
Erlon R. Cruz f33b234aa0 Make scheduler check online_extend_support capability
Since Pike release, Cinder supports volume online extending, and by
default it assumes that every backend supports this feature. This
assumption causes a bug on those backends that don't support it. On
such backends, an online extending attempt leaves the volume in
error_extending state.

This patch allows a backend to report to the scheduler if it does not
support online extending. This way, an online extending attempt will
fail, without leaving the volume in error_extending state.

Closes-bug: #1765182
Change-Id: I2c31b5c171574074a8fc7ba86f94f983fc9658f7
Co-Authored-By: Lucio Seki <luciomitsuru.seki@fit-tecnologia.org.br>
2018-06-26 14:42:34 -03:00
Jay S. Bryant ad5b05bbb7 Fix documentation error
There was a mistake in an earlier patch where
'snapshot' was used instead of 'volume'.  This
follow-up patch corrects the wording.

Change-Id: Ib1a03521c277c12d1b7f1c91eb1f2df1972e883f
2018-06-04 12:03:06 -05:00
Sean McGinnis 147dac80b9 Add snapshots to minimum driver interface
Snapshot functionality was excluded from the base volume
driver functionality while issues were being worked on
with NFS based drivers. Those have since been resolved
and we should now include the snapshot calls in the base
interface for volume backend drivers.

Change-Id: Ic7d6ca1de84d36f54a00e3afe3647865d6edd211
2018-05-10 13:52:32 +00:00
Jay S. Bryant abab90d331 [DOC BLD FIX] Remove todo:: directive from volume_driver
There was a '.. todo::' directive in cinder.interface.volume_driver
which is not a legitimate directive.  This was causing sphinx to
throw a warning.

This patch changes the format of the todo to avoid the warning.

Change-Id: I540b21053642409ef072ca51a03f95773ac5202c
2017-07-17 18:23:25 -05:00
Patrick East e9cd1bc89e Allow Pure drivers to handle detach with no host
Apparently force-detach is ok with no connector
object passed in. This changes the driver to assume
that no connector means just detach the volume from
anything its attached to.

I updated the base driver to document this quirk
too.

This should only ever happen with force detach and
"old" style connections, which means multi-attach
shouldn't be in the picture so this is kind of
safe... probably..

Change-Id: Ia0710d66bdbc85afffdb6e4f22f3ce6ca642ec75
Closes-Bug: #1696215
2017-06-07 10:59:42 -07:00
XieYingYun 1293e25115 Fix some format error in docstrings
Probably the most common format for documenting arguments is reST field
lists [1]. This change updates some docstrings to comply with the field
lists syntax.

[1] http://sphinx-doc.org/domains.html#info-field-lists

Change-Id: I0fe2d2faa7a1abd6d5e84f05e7fdbb13661a94dc
2017-03-30 10:01:18 +08:00
Vipin Balachandran 92c704b6e1 Fix devref create_volume doc formatting
Documentation generated for create_volume is not formatted correctly
due to invalid todo directive. Using the correct todo directive to
fix this.

Change-Id: Ifc6f40fd0444d4ec77f4b62dd2f84924c4472062
2017-01-09 21:11:08 +05:30
Vipin Balachandran 80c25785e6 Driver documentation cleanup
Adding more details to volume driver interface documenation.

Change-Id: I79117625aa3b2d7adf57ddab7e5889bebb450176
2016-08-25 15:51:37 +05:30
Sean McGinnis e7b40242f8 Add driver interface checks
This is the start of an effort to both validate that drivers fully
implement the expected minimum requirements as well as to create a clear
place for driver developers to learn what needs to be implemented and get
documentation explaining what is expected for each method.

This also enables us to create tooling for documenting the available
drivers and their capabilities, to some degree. A follow up patch will
show some of what I'm thinking there, but it will make it possible to write
scripts for different needs.

This is somewhat a cleanup attempt to the ABC work that was started a
while back. This does not aim to replace that effort, but give a
mechanism for some of the things expected out of that effort that ended
up not being possible with how it evolved.

In most cases we do not really care if a driver is inherited from a
certain base class, just that it conforms to the given interface.

The interface/inheritance work really centers around two separate
things:

 * Ensuring drivers conform to an expected interface
 * Allowing code reuse and common implementation

This is really for the first item. Additional work is needed to complete
the ABC work we've done, but that really focuses on the second item, and
is out of scope for the intent of this patch.

Change-Id: I4168225126fe88c31712d94f0a130e9e7ede3446
2016-06-13 15:21:47 +00:00