Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always remove the RO/RW tag from VDIs in case of failure #673

Merged

Conversation

Wescoeur
Copy link
Contributor

@Wescoeur Wescoeur commented Feb 5, 2024

During VDI activation in the blktap module and in case of failure in "sm.VDI.from_uuid" call, the RO/RW tag is never removed. As a result a VDI can no longer be used correctly: an assert is triggered each time we try to re-activate this volume because the tag is still present.

@@ -1615,7 +1617,6 @@ def _activate_locked(self, sr_uuid, vdi_uuid, options):
driver_info = target.sr.srcmd.driver_info
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These few lines of code used to be conditional on self.tap_wanted() but now they're run unconditionally. Is that deliberate? Or an accidental side-effect of the way you've moved the beginning of the try block?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well spotted, that's a bug

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed this change a little too quickly... So fixed I guess this time. :)

@@ -1615,7 +1617,6 @@ def _activate_locked(self, sr_uuid, vdi_uuid, options):
driver_info = target.sr.srcmd.driver_info
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well spotted, that's a bug

@Wescoeur Wescoeur force-pushed the fix-pause-tag-never-removed branch from ef7a9b9 to b1533b5 Compare February 6, 2024 15:39
@MarkSymsCtx
Copy link
Contributor

Being picky it's not actually the pause tag you're ensuring is removed by rather the active on host RW/RO key. The change itself looks fine and I'm not overly bothered if the commit message is a bit misleading.

During VDI activation in the blktap module and in case of failure
in "sm.VDI.from_uuid" call, the RW/RO tag is never removed.
As a result a VDI can no longer be used correctly: an assert is
triggered each time we try to re-activate this volume because
the tag is still present.

Signed-off-by: Ronan Abhamon <ronan.abhamon@vates.fr>
@Wescoeur Wescoeur force-pushed the fix-pause-tag-never-removed branch from b1533b5 to 05b947b Compare February 6, 2024 17:33
@Wescoeur Wescoeur changed the title Always remove the pause tag from VDIs in case of failure Always remove the RO/RW tag from VDIs in case of failure Feb 6, 2024
@Wescoeur
Copy link
Contributor Author

Wescoeur commented Feb 6, 2024

Right. I modified the description.

@MarkSymsCtx MarkSymsCtx merged commit a13a270 into xapi-project:master Feb 7, 2024
2 checks passed
@Wescoeur Wescoeur deleted the fix-pause-tag-never-removed branch February 12, 2024 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants