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

Correctly set silo field when opening job object #1437

Merged
merged 2 commits into from
Jun 29, 2022

Conversation

dcantah
Copy link
Contributor

@dcantah dcantah commented Jun 22, 2022

We don't set the silo field on Open of an existing job object today.
This is useful if once opening a job we want to bind a file that only
that silo can see as it relies on atomically checking the silo u32
field to determine if we can carry out the operation.

The manner in which we check if the job is a silo is by using a new
jobobject information class with QueryInformationJobObject that fails
unless the job is a silo.

@dcantah dcantah requested a review from a team as a code owner June 22, 2022 12:31
@dcantah dcantah force-pushed the jobobj-opensilo branch 2 times, most recently from a6700d3 to 66618c5 Compare June 24, 2022 14:58
@dcantah
Copy link
Contributor Author

dcantah commented Jun 24, 2022

Rebased to pickup CI fix for mingw

@dcantah
Copy link
Contributor Author

dcantah commented Jun 24, 2022

@kevpar ptal when you get a chance again

We don't set the silo field on Open of an existing job object today.
This is useful if once opening a job we want to bind a file that only
that silo can see as it relies on atomically checking the `silo` u32
field to determine if we can carry out the operation.

The manner in which we check if the job is a silo is by using a new
jobobject information class with QueryInformationJobObject that fails
unless the job is a silo.

Signed-off-by: Daniel Canter <dcanter@microsoft.com>
Change to isJobSilo from pointer receiver method as calling a method
on an object we made in the same function is a bit odd.

Signed-off-by: Daniel Canter <dcanter@microsoft.com>
@kevpar
Copy link
Member

kevpar commented Jun 29, 2022

As a side note, it seems weird to have Silo as a field on Options, given this struct is shared between Open and Create. This is the kind of thing we would want to fix before moving this package out of internal.

Copy link
Member

@kevpar kevpar left a comment

Choose a reason for hiding this comment

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

LGTM pending CI

@dcantah
Copy link
Contributor Author

dcantah commented Jun 29, 2022

As a side note, it seems weird to have Silo as a field on Options, given this struct is shared between Open and Create. This is the kind of thing we would want to fix before moving this package out of internal.

Agree, especially as it does nothing for Open

@dcantah dcantah merged commit 7049997 into microsoft:master Jun 29, 2022
kiashok pushed a commit to kiashok/hcsshim that referenced this pull request Jul 11, 2022
* Correctly set silo field when opening job object

We don't set the silo field on Open of an existing job object today.
This is useful if once opening a job we want to bind a file that only
that silo can see as it relies on atomically checking the `silo` u32
field to determine if we can carry out the operation.

The manner in which we check if the job is a silo is by using a new
jobobject information class with QueryInformationJobObject that fails
unless the job is a silo.

Signed-off-by: Daniel Canter <dcanter@microsoft.com>
anmaxvl pushed a commit that referenced this pull request Feb 7, 2023
princepereira pushed a commit to princepereira/hcsshim that referenced this pull request Aug 29, 2024
* Correctly set silo field when opening job object

We don't set the silo field on Open of an existing job object today.
This is useful if once opening a job we want to bind a file that only
that silo can see as it relies on atomically checking the `silo` u32
field to determine if we can carry out the operation.

The manner in which we check if the job is a silo is by using a new
jobobject information class with QueryInformationJobObject that fails
unless the job is a silo.

Signed-off-by: Daniel Canter <dcanter@microsoft.com>
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.

3 participants