Cinder LVM-over-iSCSI should use O_DIRECT by default

Bug #1441935 reported by Pavel Boldin
14
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Cinder
Fix Released
Wishlist
Pavel Boldin

Bug Description

There is the `LVM-over-iSCSI' Cinder driver that allows OpenStack VM to use Logical Volumes over the iSCSI bus. In one of the implementation this uses `tgtd' SCSI provider with `iscsiadm' target configuration utility. The `tgtd' then uses a logical Logical Volume as a backing device that indeed stores the data.

By default, Cinder instructs `tgtd' to open backing device with the page caching enabled.

While this theoretically speeds up I/O it also introduces the following issues:
1. Over-reliance from the VM on the data storage. VM can assume that the data is written while it is still in the cache on the `tgtd' side. Sudden power-off or other failure can break the data.
2. Double-caching of the data: VM can have filesystem and other caches itself. This virtually moves data cache control outside of the VM. This is the derivative of the above.
3. A VM under QEMU hangs up during the `fdatasync' syscall made on the Cinder-provided devices as the consequence of the above. This is due to the flushing of the enormous page cache. See [1] for details.

Cons are the following:
1. `O_DIRECT' mode is not supported by all the devices and there is no good API for it. However, since we use LVM as backing devices we can be sure that this requirement is met.
2. Performance is still under the question. We have to investigate this fully but our intermediate results favour `O_DIRECT' over `O_SYNC' and write-through caches.

[1] https://bugs.launchpad.net/mos/6.1.x/+bug/1375245

Pavel Boldin (pboldin)
Changed in cinder:
assignee: nobody → Pavel Boldin (pboldin)
Revision history for this message
John Griffith (john-griffith) wrote :

This has come up on and off over the years and both could be argued as "correct". I think the best option here may be to provide this as an option via config. Leaving the default as is, but providing the ability to change it. Honestly, I thought we already did this but looking back at config file not seeing it. Maybe an enhancement we could add?

Changed in cinder:
importance: Undecided → Wishlist
Revision history for this message
Pavel Boldin (pboldin) wrote :

John, I'm able to provide patches and going to do this later this week.

Revision history for this message
Mike Perez (thingee) wrote :

I agree with John. Config option sounds good.

Revision history for this message
Mitsuhiro Tanino (mitsuhiro-tanino) wrote :

Hi Pavel,

Just curious. How about LIO scsi target case?
Are there same issue reported?

Revision history for this message
Pavel Boldin (pboldin) wrote :

Mitsuhiro, I'm not sure of LIO. I will check if required.

But let's do it during the patch review process if there are no objections.

Revision history for this message
Mitsuhiro Tanino (mitsuhiro-tanino) wrote :

Hi Pavel,

That is not required, but in the future I think Cinder might change default SCSI target from tgtd to LIO.
So, this is good timing to think about same issue for LIO, IMHO.

>> But let's do it during the patch review process if there are no objections.
I agree. If there is an other problem, we can a post separate patch.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (master)

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

Changed in cinder:
status: New → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on cinder (master)

Change abandoned by Pavel Boldin (<email address hidden>) on branch: master
Review: https://review.openstack.org/182871
Reason: Please mark this patch and the according bug as "unwish list".

Revision history for this message
Pavel Boldin (pboldin) wrote :

Well, guys. I have faced so much of the troubles committing this patch into the Cinder Upstream that I decided not to break one's imagined rules and just abandon the patch.

Please move it to the "Unwishlist".

Changed in cinder:
assignee: Pavel Boldin (pboldin) → John Griffith (john-griffith)
Pavel Boldin (pboldin)
Changed in cinder:
status: In Progress → Invalid
status: Invalid → Opinion
Pavel Boldin (pboldin)
Changed in cinder:
assignee: John Griffith (john-griffith) → Pavel Boldin (pboldin)
status: Opinion → In Progress
Revision history for this message
Pavel Boldin (pboldin) wrote :

@John, I made a re-roll workarounding the defaults bug in oslo.config and even got yours +2.

There are comments from Gorka that are decent enough to be addressed.

What is the ritual here? Should I address them or having that much +2s is a blocker for any patch updates?

What is the current status of the patch?

Changed in cinder:
status: In Progress → Fix Committed
Thierry Carrez (ttx)
Changed in cinder:
milestone: none → liberty-1
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in cinder:
milestone: liberty-1 → 7.0.0
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.