CP-308455 VM.sysprep if CD insert fails, remove ISO #6632
Conversation
| Client.VBD.insert ~rpc ~session_id ~vdi ~vbd ; | ||
| ( try Client.VBD.insert ~rpc ~session_id ~vdi ~vbd | ||
| with e -> | ||
| debug "%s: failed to insert CD, removing ISO %s: %s" __FUNCTION__ iso |
There was a problem hiding this comment.
Should it be warn or error?
| with e -> | ||
| debug "%s: failed to insert CD, removing ISO %s: %s" __FUNCTION__ iso | ||
| (Printexc.to_string e) ; | ||
| Sys.remove iso ; |
There was a problem hiding this comment.
Can Sys.remove fail? What happen if it fails?
There was a problem hiding this comment.
It can fail if the file does not exist, which should not happen. I could wrap this.
| Sys.remove iso | ||
| with exn -> | ||
| Sys.remove iso ; | ||
| (* still remove ISO to protect it *) |
There was a problem hiding this comment.
This behaviour is a little uncommon. Can you describe in detail?
What's the relation between vbd and iso? If vbd eject fail, will it still exist? Then if we remove iso, will it be inconsistent?
There was a problem hiding this comment.
The ISO may contain secrets so we would like to not keeping it around. This is an ISO SR where we can add and remove ISOs and the SM layer will update xapi. I am not sure what happens from the VMs perspective, though.
|
The PR needs to be rebased on top of the latest master to run all the required checks |
If inserting the CD fails, remove the ISO to protect its contents. Likewise, remove it when eject fails. Signed-off-by: Christian Lindig <[email protected]>
lindig
force-pushed
the
private/christianlin/vm-sysprep
branch
from
1ab0037 to
c97250d
Compare
6 participants
If inserting the CD fails, remove the ISO to protect its contents. Likewise, remove it when eject fails.