snapshot_volume_backed races, could result in data corruption

[Matthew Booth]

snapshot_volume_backed() in compute.API does not set a task_state during execution. However, in essence it does:

if vm_state == ACTIVE:
  quiesce()
snapshot()
if vm_state == ACTIVE:
  unquiesce()

There is no exclusion here, though, which means a user could do:

quiesce()
                   quiesce()
snapshot()
                   snapshot()

unquiesce() --snapshot() now running after unquiesce -> corruption
                   unquiesce()

or:

suspend()
snapshot()
  NO QUIESCE (we're suspended)
  snapshot()
                   resume()
  --snapshot() now running after resume -> corruption

Same goes for stop/start.

Note that snapshot_volume_backed() is a separate top-level entry point from snapshot(). snapshot() does not suffer from this problem, because it atomically sets the task state to IMAGE_SNAPSHOT_PENDING when running, which prevents the user from performing a concurrent operation on the instance. I suggest that snapshot_volume_backed() should do the same.

[Chris Martin (cm876n)]

I believe this is an actual bug. I tested this on my debstack on in an ubuntu environment running on vmware 14.04 and when I ran the command to snapshot the instance booted from volume from two different command prompts at almost the same time. When I did this I had multiple 'cirros-rootfs' now booted on my sidebar yet when I delete the instance and it's associated images only one of the 'cirros-rootfs' volumes would be deleted.