implement the capability to desactivate CBT on all vdi snapshot child #108
Conversation
Lankou66
force-pushed
the
allow_CBT_disabling_on_snapshot
branch
from
6642746 to
fc458c8
Compare
|
Here's a reminder regarding the commit message conventions. The commit title is largely over 70 chars, here :) https://docs.xcp-ng.org/project/development-process/commit-message-conventions/ |
Lankou66
force-pushed
the
allow_CBT_disabling_on_snapshot
branch
from
fc458c8 to
315794d
Compare
Lankou66
changed the title
Lankou66
force-pushed
the
allow_CBT_disabling_on_snapshot
branch
from
315794d to
2648b19
Compare
drivers/VDI.py
Outdated
| return [ | ||
| self.session.xenapi.VDI.get_uuid(ref) | ||
| for ref in record.get("snapshots", []) | ||
| if self.session.xenapi.VDI.get_uuid(ref) |
There was a problem hiding this comment.
Two call for the XAPI is not optimal but also, this will raise if ref is not a correct OpaqueRef
There was a problem hiding this comment.
You can also return the ref directly since you use it later
There was a problem hiding this comment.
i've removed the comprehension list
now we get ant return the list of rest directly
drivers/VDI.py
Outdated
| blktap2.VDI.tap_unpause(self.session, sr_uuid, vdi_uuid) | ||
|
|
||
| def _disable_cbt(self, vdi_uuid, visited=None): | ||
| # Prevent infite loop |
There was a problem hiding this comment.
| # Prevent infite loop | |
| # Prevent infinite loop |
I don't think you need to do this, you should call for the VDI and then the same function doing the disable on the snapshots.
The snapshot won't have snapshots.
There was a problem hiding this comment.
i removed it
indeed this protection was useless
Lankou66
force-pushed
the
allow_CBT_disabling_on_snapshot
branch
from
2648b19 to
832aef0
Compare
drivers/VDI.py
Outdated
| finally: | ||
| blktap2.VDI.tap_unpause(self.session, sr_uuid, vdi_uuid) | ||
|
|
||
| def _disable_cbt(self, vdi_uuid ): |
There was a problem hiding this comment.
| def _disable_cbt(self, vdi_uuid ): | |
| def _disable_cbt(self, vdi_uuid): |
drivers/VDI.py
Outdated
| lock = Lock("cbtlog", str(vdi_uuid)) | ||
| lock.acquire() | ||
| try: | ||
| self.uuid = vdi_uuid |
There was a problem hiding this comment.
That's not in the original code. You are in a VDI object refererring to a specific, modifying this parameter could have adverse effect later.
drivers/VDI.py
Outdated
| self.session.xenapi.VDI.set_cbt_enabled(vdi_ref, False) | ||
| for snapshot_ref in self._list_vdi_snapshots(self.uuid): | ||
| snapshot_uuid = self.session.xenapi.VDI.get_uuid(snapshot_ref) | ||
| self._disable_cbt(snapshot_uuid) |
There was a problem hiding this comment.
I think you can do a function doing the above with just the ref and not the snapshot loop.
And call this once for the normal VDI then for the snapshot refs.
It might need to go deeper in self._cbt_op to know if they use info stored in self that are from the main VDI.
- Recursive CBT disabling on VDI all child snapshots - Delete CBT log files when present This resolves issues where CBT deactivation was incomplete when processing multi-level snapshot chains. Fixes: incomplete CBT deactivation on snapshot chains Signed-off-by: Goulven Riou <[email protected]>
Lankou66
force-pushed
the
allow_CBT_disabling_on_snapshot
branch
from
832aef0 to
c9dcd5f
Compare
Implement recursive CBT deactivation for VDI snapshot chains
Add recursive CBT deactivation for VDI and all child snapshots Delete CBT log files when present
Update parent/child references in CBT metadata
Fixes: Incomplete CBT deactivation on snapshot chains