Skip to content

Conversation

@smklein
Copy link
Collaborator

@smklein smklein commented Apr 28, 2022

Fixes #990

Easy repro:

TMPDIR=/home/smklein/dangerzone cargo test -p internal-dns 

Before: On success, the temporary directory grew. After: It's empty.

@smklein smklein requested a review from davepacheco April 28, 2022 20:35
Copy link
Collaborator

@davepacheco davepacheco left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for doing this.

@smklein smklein enabled auto-merge (squash) April 28, 2022 20:38
@smklein smklein merged commit 1aaf2a2 into main Apr 28, 2022
@smklein smklein deleted the dns-test-cleanup branch April 28, 2022 21:20
iximeow added a commit that referenced this pull request Jan 8, 2026
Propolis changes are pretty important, it's unfortunate I'd overlooked
driving the changes through to Omicron. The full list of changes:

* Do not lose wakeups for block device workers (#973)
* fix arg for dtrace script (#978)
  - no changes to Propolis
* Add support for programmable SMBIOS Type 1 table (#977)
  - unused in Nexus
* NVMe reset can discard request Permits early (#983)
* Wire up viona notify fast path (#754)
* distinguish probes across NVMe devices (#993)
* update dropshot to 0.16.6, dropshot-api-manager to 0.3.0 (#992)
* distinguish file backend thread names across backend instances (#997)
* bump softnpu (#1000)
* block/file: do pread/pwrite from Propolis heap instead of VM memory (#985)
* bins: intial CPU binding support (#991)

At least some of these are fixes for bugs we've seen in the last month
or two (#983 made for a very coreful dogfood update as Propolises were
stopped), many are either unused by Nexus (#977, #987, #993, #997) or
just not reachable from a Nexus-configured VM *yet* (#973, #985).

Both #991 and #754 are effectful today.

The initial CPU binding in Propolis is such that if a VM would use more
than half of a sled's CPUs, we explicitly bind vCPU threads to uppermost
CPUs 1:1. This helps keep VM exit/reentry quick, mostly interesting
while under high I/O load. In the limit this binding should be chosen by
Nexus, and probably should be applied for all VMs.

Additionally, #754 affects all VM NICs. It's just to avoid going out to
Propolis and back for what really can be handled in-kernel; there should
be no behavioral change here.

Finally, while there *is* a new `propolis-server` API version, and I
have sled-agent using it, I'm intentionally not plumbing that through to
Nexus. There isn't anything valuable for Nexus to do with it, so it
would be a new sled-agent API rev to include the new Propolis API type
.. for no reason. Instead, I've opted to have sled-agent do the lossless
conversion from older Propolis spec as provided by Nexus, to newer
Propolis spec.

We'll definitely have more changes to the Propolis API that will go
through to Nexus, such as aforementioned CPU binding assignments, so I
figure this will get more normalized sooner than later.
askfongjojo pushed a commit that referenced this pull request Jan 9, 2026
Propolis changes are pretty important, it's unfortunate I'd overlooked
driving the changes through to Omicron. The full list of changes:

* Do not lose wakeups for block device workers (#973)
* fix arg for dtrace script (#978)
  - no changes to Propolis
* Add support for programmable SMBIOS Type 1 table (#977)
  - unused in Nexus
* NVMe reset can discard request Permits early (#983)
* Wire up viona notify fast path (#754)
* distinguish probes across NVMe devices (#993)
* update dropshot to 0.16.6, dropshot-api-manager to 0.3.0 (#992)
* distinguish file backend thread names across backend instances (#997)
* bump softnpu (#1000)
* block/file: do pread/pwrite from Propolis heap instead of VM memory (#985)
* bins: intial CPU binding support (#991)

At least some of these are fixes for bugs we've seen in the last month
or two (#983 made for a very coreful dogfood update as Propolises were
stopped), many are either unused by Nexus (#977, #987, #993, #997) or
just not reachable from a Nexus-configured VM *yet* (#973, #985).

Both #991 and #754 are effectful today.

The initial CPU binding in Propolis is such that if a VM would use more
than half of a sled's CPUs, we explicitly bind vCPU threads to uppermost
CPUs 1:1. This helps keep VM exit/reentry quick, mostly interesting
while under high I/O load. In the limit this binding should be chosen by
Nexus, and probably should be applied for all VMs.

Additionally, #754 affects all VM NICs. It's just to avoid going out to
Propolis and back for what really can be handled in-kernel; there should
be no behavioral change here.

Finally, while there *is* a new `propolis-server` API version, and I
have sled-agent using it, I'm intentionally not plumbing that through to
Nexus. There isn't anything valuable for Nexus to do with it, so it
would be a new sled-agent API rev to include the new Propolis API type
.. for no reason. Instead, I've opted to have sled-agent do the lossless
conversion from older Propolis spec as provided by Nexus, to newer
Propolis spec.

We'll definitely have more changes to the Propolis API that will go
through to Nexus, such as aforementioned CPU binding assignments, so I
figure this will get more normalized sooner than later.
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.

internal_dns tests don't clean up after themselves

3 participants