Skip to content

Conversation

slp
Copy link
Collaborator

@slp slp commented Oct 10, 2025

Instead of generating a panic when rutabaga initialization fails, let's have virtio_gpu retry with a safe configuration.

slp added 2 commits October 10, 2025 12:51
It's only possible to call VirglRenderer::init() once to prevent
attempting to initialize virglrenderer twice. But this also prevents us
for retrying the initialization if the first time fails.

Let's only set the INIT_ONCE atomic when the call to virgl_renderer_init
has been successful.

Signed-off-by: Sergio Lopez <[email protected]>
So far, if the initialization of rutabaga fails, the GPU thread panics
with an "expect()". Instead, let's retry the initialization with safe
flags (NO_VIRGL).

Fixes: containers#418

Suggested-by: Val Packett <[email protected]>
Signed-off-by: Sergio Lopez <[email protected]>
@slp
Copy link
Collaborator Author

slp commented Oct 10, 2025

@valpackett PTAL

Comment on lines +327 to +338
if ret == 0 {
// virglrenderer is a global state backed library that uses thread bound OpenGL contexts.
// Initialize it only once and use the non-send/non-sync Renderer struct to keep things tied
// to whichever thread called this function first.
static INIT_ONCE: AtomicBool = AtomicBool::new(false);
if INIT_ONCE
.compare_exchange(false, true, Ordering::Acquire, Ordering::Acquire)
.is_err()
{
return Err(RutabagaError::AlreadyInUse);
}
}
Copy link
Collaborator

@mtjhrc mtjhrc Oct 10, 2025

Choose a reason for hiding this comment

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

I think the original intention of this was to prevent calling virgl_renderer_init when it was already initialized, no? But this change allows initializing virglrenderer a second time. I'm not sure if that is even a problem though...

If we want to keep the check working you can is still keep the check before virgl_renderer_init:

if INIT_ONCE.compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst).is_err() { 
    return `RutabagaError::AlreadyInUse` 
}

But after calling virgl_renderer_init, set it to the actual result:
INIT_ONCE = (virgl_renderer_init() == 0)

(Not sure about the memory ordering...)

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