Skip to content

Storage fixes - #7369

Open
last-genius wants to merge 3 commits into
xapi-project:masterfrom
last-genius:asv/storage-fixes
Open

last-genius wants to merge 3 commits into
xapi-project:masterfrom
last-genius:asv/storage-fixes

Conversation

@last-genius

@last-genius last-genius commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

First commit fixes multiple VDI.deactivate running in parallel.

Second commit cleans up on VBD.plug failure for dom0 VBDs.

Third commit cleans up stale datapaths before re-using them.

We've seen a case where multiple VDI.deactivate calls running in parallel
get into a race between get_other_config and add_other_config, failing with
the Duplicate_key exception.

If one of the calls succeeded in adding other_config=content_id:, then simply
continue.

Signed-off-by: Andrii Sultanov <andriy.sultanov@vates.tech>

@gthvn1 gthvn1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just a remark, otherwise LGTM

Comment thread ocaml/xapi/storage_access.ml Resolved
XCP-ng uses VBDs in dom0 for some use-cases, and failures can result in
leftover datapaths breaking unrelated VBDs.

To help avoid this, if attach succeeded but activate failed, detach
the dom0 VBD. This requires reordering the functions so that
attach_and_activate can call deactivate_and_detach.

Details of this issue:

This script can be used to check for leftover datapaths:

    $ cat check_datapath.py
    import json
    d = json.load(open('/var/run/nonpersistent/xapi/storage.db'))
    claims = {}
    for sr, srv in d['host']['srs'].items():
        for vdi, vv in (srv.get('vdis') or {}).items():
            for dp, state in (vv.get('dps') or {}).items():
                claims.setdefault(dp, []).append((sr, vdi, state))
    for dp, owners in sorted(claims.items()):
        print(dp, '  <-- DUPLICATE' if len(owners) > 1 else '')
        for sr, vdi, state in owners:
            print('    ', sr, vdi, state)

The following sequence simulates a failure to activate a VBD and shows how two
VBDs using the same datapath results in further errors down the line:

    $ touch /tmp/fist_blktap_activate_inject_failure
    $ xe vbd-plug uuid=$FIRST_VBD_UUID
    Error code: SR_BACKEND_FAILURE_1200
    Error parameters: , injected failure,

    $ rm -f /tmp/fist_blktap_activate_inject_failure
    $ python3 check_datapath.py
    vbd/0/xvda
         1d4a39c0-5029-a134-77c1-b189d2e60dfc 6016c6ad-1101-4d0a-bc32-bc9604d930ab ['Attached', 'RW']

    $ xe vbd-destroy uuid=$FIRST_VBD_UUID
    $ xe vbd-plug uuid=$SECOND_VBD_UUID
    $ python3 check_datapath.py
    vbd/0/xvda   <-- DUPLICATE
         1d4a39c0-5029-a134-77c1-b189d2e60dfc 7ceaa7c6-4c51-4536-9c84-7d25d11e3698 ['Activated', 'RW']
         1d4a39c0-5029-a134-77c1-b189d2e60dfc 6016c6ad-1101-4d0a-bc32-bc9604d930ab ['Attached', 'RW']

    $ xe vbd-unplug uuid=$SECOND_VBD_UUID
    message: Expected 0 or 1 VDI with datapath, had 2

Only reboots will fix this, otherwise the storage.db needs to be manually
modified to drop the stale entry.

Signed-off-by: Andrii Sultanov <andriy.sultanov@vates.tech>
Previous commit fixed one of the causes of "stale" VDIs taking up datapaths:
activation failure. There are, however, probably more ways these can happen.
Additionally, already existing stale datapaths need to be cleaned up.

This commit changes the clean-up of leaked DPs that happens before VDI.attach.
The function now also cleans up a stale datapath used by a different VDI (a
stale datapath is defined as that which has been attached but not activated)
before re-using this datapath for a new VDI.

This is done at attach time to cover all possible reasons for occurrence of
such a stale datapath. The clean-up needs to be done before the new datapath is
attached, since the rest of the module assumes only 0 or 1 VDIs use a datapath
at the same time.

This should be safe to do, since xapi already let the VBD be created (so it
thinks its device is not in use), and the datapath is not activated.

This requires refactoring the function to handle removing datapaths for
different VDIs, not just the one we are creating a datapath for.

Also specify the exact reason the datapath is being removed to enable easier
debugging in case of an issue.

Signed-off-by: Andrii Sultanov <andriy.sultanov@vates.tech>
@last-genius

Copy link
Copy Markdown
Contributor Author

Added another commit - this one to clean up a stale datapath before re-using it for a different VDI. This should hopefully cover all bases, so that the issue of stale datapaths does not result in two VDIs using the same datapath.

@last-genius
last-genius requested a review from gthvn1 November 4, 2026 16:15
@last-genius

last-genius commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor Author

I tested the last commit manually following the sequence in the second commit (with the second commit dropped so that it doesn't interfere). There are no 'duplicate' datapaths anymore and no more of the "expected 0 or 1, got 2" error. The log shows that the stale datapath was cleaned up:

Nov  4 16:59:14 hpm3c14 xapi: [ info||Storage_smapiv1_wrapper] Attempting to destroy stale datapath dp:vbd/0/xvda sr:X vdi:FIRST_VDI

@last-genius last-genius changed the title Minor storage fixes Storage fixes Nov 4, 2026

This branch has not been deployed

No deployments
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.

2 participants