Skip to content

Conversation

@sirtimid
Copy link
Contributor

@sirtimid sirtimid commented Jun 9, 2025

Closes #532

Remove support for launching vats outside a subcluster.
Update all that rely on launching standalone vats.

Screenshot 2025-06-09 at 16 16 20

@sirtimid sirtimid requested a review from a team as a code owner June 9, 2025 14:14
@sirtimid sirtimid changed the title Make kernel launchVat a private method and prevent loading rogue vats feat: Remove support for launching vats outside a subcluster Jun 9, 2025
@sirtimid sirtimid requested a review from rekmarks June 9, 2025 17:16
@sirtimid sirtimid force-pushed the sirtimid/subclusters branch from bca937f to 8b5d454 Compare June 11, 2025 11:12
@sirtimid sirtimid force-pushed the sirtimid/remove-public-launch-vat branch from f225996 to 7c9465e Compare June 11, 2025 11:16
Base automatically changed from sirtimid/subclusters to main June 11, 2025 14:10
@sirtimid sirtimid force-pushed the sirtimid/remove-public-launch-vat branch from 7c9465e to 53a0e20 Compare June 11, 2025 15:59
@sirtimid sirtimid requested a review from grypez June 11, 2025 17:43
grypez
grypez previously requested changes Jun 11, 2025
Copy link
Contributor

@grypez grypez left a comment

Choose a reason for hiding this comment

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

This change leaves dead code in support of rogue vats.

Since no rogue vats are started internally, privatizing #launchVat ought to make the getVatSubcluster method in the kernel store well-defined (if it returns undefined, either the given vat does not exist or the kernel has failed).

function getVatSubcluster(vatId: VatId): SubclusterId {
  const currentMap = getVatToSubclusterMap();
  return currentMap[vatId] ?? Fail`Vat ${vatId} has no subcluster`;
}

Then the rogue vat code can be removed entirely.

}
for (const vat of rogueVats) {
await this.launchVat(vat.config);
await this.#launchVat(vat.config);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
await this.#launchVat(vat.config);

I think we can remove this loop and the const rogueVats = ... entirely.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shoot.. I think my rebase messed up things a bit and I also forgot to clean up. Done 97bbd58

@sirtimid sirtimid requested a review from grypez June 11, 2025 21:27
@sirtimid sirtimid enabled auto-merge (squash) June 11, 2025 21:28
@rekmarks rekmarks linked an issue Jun 11, 2025 that may be closed by this pull request
@grypez grypez dismissed their stale review June 12, 2025 14:20

addressed

Copy link
Contributor

@grypez grypez left a comment

Choose a reason for hiding this comment

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

LGTM

@sirtimid sirtimid merged commit ae3878c into main Jun 12, 2025
22 checks passed
@sirtimid sirtimid deleted the sirtimid/remove-public-launch-vat branch June 12, 2025 14:49
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.

Migrate kernel to support only subcluster-based vat launch

4 participants