Skip to content

Support setting volume identifier#38

Merged
p14c31355 merged 7 commits intop14c31355:mainfrom
iamjpotts:volume-label
Apr 2, 2026
Merged

Support setting volume identifier#38
p14c31355 merged 7 commits intop14c31355:mainfrom
iamjpotts:volume-label

Conversation

@iamjpotts
Copy link
Copy Markdown
Contributor

@iamjpotts iamjpotts commented Nov 6, 2025

Overview

Makes a breaking change to support volume identifiers by adding an optional field IsoImage::volume_id which when not set defaults to the current volume identifier of ISOBEMAKI.

ISO images used with cloud init in "no cloud" scenarios (such as with VirtualBox) must have cidata as their volume identifier.

Change details

  • New feature
  • Refactoring
  • Bug fix
  • CI / Build settings correction
  • Documentation update

Build / Test Results

Adds a new integration test basic_iso::test_sets_volume_label

@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello @iamjpotts, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the ISO image generation utility by adding the capability to specify a custom volume identifier. This feature is particularly important for compatibility with systems like cloud-init in 'no cloud' scenarios, which often require a specific volume ID (e.g., 'cidata') for proper operation. The changes involve modifying the core IsoImage structure and the underlying ISO building logic to incorporate this new optional parameter, ensuring flexibility while maintaining a default behavior.

Highlights

  • Volume Identifier Support: Introduced an optional volume_id field to the IsoImage configuration, allowing users to specify a custom volume identifier for the generated ISO 9660 image. If not provided, it defaults to ISOBEMAK.
  • API Changes: The IsoImage struct now includes pub volume_id: Option<String>. This is a breaking change for direct struct instantiation, requiring existing code to explicitly set volume_id: None or a custom value.
  • Integration with ISO Builder: The internal IsoBuilder and related functions (write_descriptors, write_primary_volume_descriptor, write_volume_descriptors) have been updated to accept and correctly apply the provided volume_id during ISO creation.
  • Documentation and Testing: The README.md has been updated with examples demonstrating how to set the volume_id. A new integration test, test_sets_volume_label, was added to verify the functionality of setting a custom volume identifier.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds support for custom volume identifiers, which is a great feature, especially for use cases like cloud-init. The implementation is mostly solid, with good test coverage for the new functionality.

I've found a couple of issues:

  • A potential panic if the provided volume ID is longer than 32 characters. I've suggested a fix to truncate it instead.
  • A minor typo in the documentation for the default volume ID.

After addressing these points, the PR should be in good shape.

Comment on lines +68 to 75
let project_name = match volume_id {
None => b"ISOBEMAKI",
Some(id) => id.as_bytes(),
};

let mut volume_id = [b' '; 32];
volume_id[..project_name.len()].copy_from_slice(project_name);
pvd[PVD_VOLUME_ID_OFFSET..PVD_VOLUME_ID_OFFSET + 32].copy_from_slice(&volume_id);
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.

high

The current implementation for setting the volume identifier can panic if the provided volume_id is longer than 32 bytes, as copy_from_slice will fail. The documentation mentions a 32-character limit, but it's not enforced. It's better to truncate the string to 32 bytes to prevent a panic.

This suggestion also makes the code more concise by using unwrap_or and renames the buffer variable to avoid shadowing the function parameter.

Suggested change
let project_name = match volume_id {
None => b"ISOBEMAKI",
Some(id) => id.as_bytes(),
};
let mut volume_id = [b' '; 32];
volume_id[..project_name.len()].copy_from_slice(project_name);
pvd[PVD_VOLUME_ID_OFFSET..PVD_VOLUME_ID_OFFSET + 32].copy_from_slice(&volume_id);
let mut volume_id_buf = [b' '; 32];
let project_name = volume_id.unwrap_or("ISOBEMAKI").as_bytes();
let len = project_name.len().min(volume_id_buf.len());
volume_id_buf[..len].copy_from_slice(&project_name[..len]);
pvd[PVD_VOLUME_ID_OFFSET..PVD_VOLUME_ID_OFFSET + 32].copy_from_slice(&volume_id_buf);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

pre-existing issue

Comment thread src/iso/iso_image.rs Outdated
/// Configuration for the entire ISO image to be built.
#[derive(Clone, Debug)]
pub struct IsoImage {
/// Defaults to ISOBEMAK. Maximum length is 32 characters.
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.

medium

There's a small typo in the documentation. The default volume ID is ISOBEMAKI, not ISOBEMAK.

Suggested change
/// Defaults to ISOBEMAK. Maximum length is 32 characters.
/// Defaults to ISOBEMAKI. Maximum length is 32 characters.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

@p14c31355
Copy link
Copy Markdown
Owner

Hello @iamjpotts !
I sincerely apologize for the long delay due to my busy personal life.

@p14c31355
Copy link
Copy Markdown
Owner

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds support for custom volume IDs in ISO images by updating the IsoImage and IsoBuilder structures and modifying the volume descriptor logic. Feedback includes addressing compilation errors caused by the removal of required macro imports, implementing bounds checking for the volume ID to prevent potential panics, and clarifying documentation regarding the 32-byte limit for the volume identifier.

Comment thread src/iso/builder.rs
Comment thread src/iso/builder_utils.rs
Comment thread src/iso/iso_writer.rs
Comment thread src/iso/volume_descriptor.rs
Comment thread src/iso/iso_image.rs Outdated
p14c31355 and others added 5 commits April 3, 2026 07:40
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@p14c31355
Copy link
Copy Markdown
Owner

/gemini summary

@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

This pull request introduces the ability to configure a custom volume identifier for generated ISO images. By allowing users to specify this identifier, the utility now supports specific deployment scenarios, such as 'no cloud' configurations for cloud-init, where a precise volume label is required. The changes involve updating the core configuration structure, modifying the ISO writing logic to handle the new parameter, and ensuring backward compatibility through default values.

Highlights

  • Volume Identifier Support: Added an optional volume_id field to the IsoImage configuration, enabling custom volume identifiers for ISO 9660 images, which is essential for compatibility with tools like cloud-init.
  • API Update: Updated the IsoImage struct to include pub volume_id: Option<String>, which serves as a breaking change requiring updates to existing struct instantiations.
  • Builder Logic: Refactored internal ISO building functions, including write_descriptors and write_primary_volume_descriptor, to accept and apply the custom volume identifier.
  • Testing and Documentation: Updated documentation with usage examples and added a new integration test, test_sets_volume_label, to verify the correct application of custom volume labels.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Activity
  • Pull request created by iamjpotts.
  • Initial summary and code review provided by Gemini Code Assist.
  • Author addressed documentation feedback regarding the default volume ID.
  • Reviewer p14c31355 requested a new review and summary.
  • Gemini Code Assist identified several potential compilation issues related to removed macro imports and provided further feedback on string handling.

@p14c31355 p14c31355 merged commit 976a618 into p14c31355:main Apr 2, 2026
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