Can't restore volume to a different type than the snapshot

[Cory Stone]

There is now a volume_type_id on the snapshot model and you cannot create a volume from that snapshot as any other type. It changes the type of the volume to the one on the snapshot.

This is complicated by the fact that the migration that added this column didn't insert any data into this column for any existing snapshots, so they have a NULL volume_type_id value.

[Duncan Thomas (duncan-thomas)]

Re the migration being broken, yuck.

I don't think you should be able to create a volume from a snapshot of a different type - snapshots are lightweight, let's keep them that way? We can't provide both an abstraction of fundamentally different storage types and a 100% feature matrix if we allow every conceivable operation. This should be done with a create and a retype IMO

[Avishay Traeger (avishay-il)]

Definitely need to fix the migration.

To clarify: Snapshots are handled efficiently by the storage (efficiently in time and space), and so when you create a snapshot, the snapshot will reside on the same backend as the volume. Similarly, when you create a new volume from the snapshot, this new volume will also be that same backend because this can generally also be done efficiently. Allowing a user to assign ANY new type to the new volume may require having the volume reside on a different backend than the original volume and the snapshot (in the case where the current backend does not have the capabilities required by the type).

So our options are:
1. Use the original type, and let the user retype if necessary
2. Allow setting a new type, but fail the create operation if the backend where the snapshot resides cannot accept the type - this would be confusing to users.
3. Package retype into this and make it transparent. I'd say this would be good to do eventually, but definitely not in Icehouse, and probably not in Juno - would want to wait until migration and retype are thoroughly tested and debugged before putting them into the create_volume path.

So in summary, the migration issue should be fixed ASAP, but the bigger "issue" is wishlist IMO.

[John Griffith (john-griffith)]

We can certainly turn the enforcement back off here as it seems to be creating a problem for most people (except HP Cloud). I'm not opposed to going back to allowing it and if it involves migration and migration isn't enabled failing.

[Duncan Thomas (duncan-thomas)]

HPCloud only has one type in production, so it isn't directly affecting us.

The basic problem is for most drivers, I plain won't work, the missing enforcement was a bug. The fact it happens to work for Rax is a coincidence based on their design.

My objection to the feature at all is that it breaks the 'snaps are an optimised, backend specific thing' model...

[John Strunk (johndstrunk)]

Thats the point isn't it? Such enforcement doesn't belong in the Cinder level, any incompatibility should be up to the driver level. The mere fact that it didnt need to be there and still was being held to until now shows its not necessary at that level.

It shouldn't matter if the backend storage is usb sticks, floppy drives, sparse images, sans, etc Cinder should remain simply iscsi to iscsi and again any such enforcement to incompatibility should be at the driver level who will have more insight into the actual storage medium and process. By enforcing this, you are in fact forcing a compatibility matrix mentioned above.

At the very least it should be an option to disable / enable.

[John Griffith (john-griffith)]

Ok, I like John's idea of giving an option here. How about we default to "not allow" but provide a conf setting to enable dissimilar types for clone as well as create-from-snap?

[Duncan Thomas (duncan-thomas)]

@johndstrunk drivers being able to decide what the API behaviour should be, and this differing depending on what your driver/backened is a truely terrible idea, and against the whole point of cinder - there's no point having a 'standard' api if it isn't standard. If cinder breaks workload portability between backends for the core feature set then we might as well give up on even trying.

[John Strunk (johndstrunk)]

Let me clarify again. iSCSI to iSCSI is completely compatible. As stated above, an iSCSI protocol compatible export is completely compatible with another block to block. We are dealing with blocks of data not file systems remember. It doesn't matter the hardware backend for the export. It can be a usb stick, floppy drive, lvm, sparse image, hard drive, anything and as long as that storage hardware is compatible with iSCSI standards and drivers and able to export there is no need for type restriction. To the connecting device, they look the same and react the same. Which is why this restore to a different type has been working this way without, supposedly, the type restriction working. If you're going to have Cinder handle type restrictions, you are in fact limiting this product to a select set of hardware compatibly matrix and you're asking the Cinder community to systematically evaluate every hardware backend for compatibility.

Cinder has no place in that area, it should simply be an iSCSI handling API and kept simple. If there is any hardware backend which does not support compatible iSCSI commands and protocol, and for which I can't think off the top of my head as its likely impossible, it should be the driver implementation which either ignores that request or returns an error.

The compromise, which I agree with John Griffith above, is allowing for an option, on by default for those using only one type of backend, but able to be disabled for those that are and are aware of the compatibility between types for which they will be responsible. To just cut off this ability just seems to punish only those who do have multiple back ends, for the vast majority of the rest of the community, it will never become an issue. For those that do though, its a huge problem being restricted in this way.

[John Griffith (john-griffith)]

Ok, this is digressing a bit; there are two sides to this and I can point out facts to bolster either side.

Bottom line, John I think we're on the same page and agree that a compromise is appropriate. I'll get a patch in hopefully later tonight to enable this. Regardless of opinion I don't think it's unreasonable to offer this, especially as a compatibility offering. Even if people classify the original behavior as a bug, sometimes bugs truly are features and people deploy installations that utilize those bugs/features and that's perfectly valid IMO.

I'll get a patch for Juno as soon as tonight or tomorrow.

Thanks!

[Duncan Thomas (duncan-thomas)]

> Let me clarify again. iSCSI to iSCSI is completely compatible. As stated above, an iSCSI protocol compatible export is completely
> compatible with another block to block. We are dealing with blocks of data not file systems remember. It doesn't matter the
> hardware backend for the export. It can be a usb stick, floppy drive, lvm, sparse image, hard drive, anything and as long as that
> storage hardware is compatible with iSCSI standards and drivers and able to export there is no need for type restriction. To the
> connecting device, they look the same and react the same. Which is why this restore to a different type has been working this
> way without, supposedly, the type restriction working. If you're going to have Cinder handle type restrictions, you are in fact
> limiting this product to a select set of hardware compatibly matrix and you're asking the Cinder community to systematically
> evaluate every hardware backend for compatibility.

Cinder is a very long way from being an iSCSI-only system - we have SMB, NFS, iSCSI, ceph, sheepdog, REST and other transfer technologies. If your view of cinde ris that it is a shim over iSCSI, I'm afraid you way wrong. There are already API rules in that don't need to apply to some drivers, but we maintain a consistent API across them all so that workloads are portable and can work on any backend, at least if you stick to the core API (which includes snapshots). The only way to implement generic support for create-from-snap-of-a-different type is to fall back to dd, which if people /really/ want to do then fine, I don't like it, the performance will suck but at least we stick to the core value of 'no vendor lock-in'. Otherwise I'm going to continue to push back hard against a supported option that breaks workload compatibility.

[Duncan Thomas (duncan-thomas)]

> Cinder has no place in that area, it should simply be an iSCSI handling API and kept simple.
> If there is any hardware backend which does not support compatible iSCSI commands and
> protocol, and for which I can't think off the top of my head as its likely impossible, it
> should be the driver implementation which either ignores that request or returns an error.

We decided some time ago that for cinder to go down that road lies madness. If a driver can't support all of the core features, we don't merge it.

[John Griffith (john-griffith)]

Removing targeting, may get something submitted but not holding RC for it.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/181268

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/182327

[OpenStack Infra (hudson-openstack)]

Change abandoned by John Griffith (<email address hidden>) on branch: master
Review: https://review.openstack.org/181268
Reason: This is now decoupled from the abandoned dependency, new review is here: https://review.openstack.org/#/c/182327/

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/182327
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=706cbf40f5f8c6f756277057c92bf41f1ef1cd80
Submitter: Jenkins
Branch: master

commit 706cbf40f5f8c6f756277057c92bf41f1ef1cd80
Author: John Griffith <email address hidden>
Date: Tue May 12 08:03:27 2015 -0600

    Check type match on create from source/snap

    We used to allow creating from source/snap and specifying a
    different type than the originating resource when doing so.
    Once we started getting more drivers and more multi-backend
    configurations, we implemented a check in volume.api that
    took this away (broke it). There have been a number of
    arguments about whether this should be allowed or not, and
    that it could fail after the rpc call leaving the user with
    nothing more than a "failed" volume and no explanation as to
    why.

    This patch allows the capability, but checks validity at the
    API layer before issuing the create call. There are two
    requirements for the new type specification to be valid:
    1. There is only one backend (cinder-volume) topic configured
    2. Both types in question specify the same volume_backend_name
    If neither of these requirements are met, the user will receive
    an "invalid type" error explaining that the type combination is
    not compatible and that they should omit the type argument altogether.

    Change-Id: I08bc5e9a8800ce3b27c7db90b7bff86d7d14359a
    Closes-Bug: #1289931