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

Run disk format detection also for CD/DVD discs #1124

Merged
merged 1 commit into from
Jul 12, 2023

Conversation

skobyda
Copy link
Contributor

@skobyda skobyda commented Jun 27, 2023

@skobyda skobyda requested a review from jelly June 27, 2023 10:12
@skobyda
Copy link
Contributor Author

skobyda commented Jun 27, 2023

So there is a bug where it's appareant we just automatically set every CD/DVD disc to qcow2.
But a while ago, @jelly added format autodetection at #646 . But I see the autodetection is only limited to Disk image files:
Screenshot from 2023-06-27 12-13-08

But doesn't run for CD/DVD discs:
Screenshot from 2023-06-27 12-13-08

My naive approach was to also extend autodetection to CD/DVD discs, expecting to run into some issues. But it seems to work fine.

@jelly you wrote that code. Do you remember if there was any reason why you limited it only for Disk images, and didn't extend it to CD/DVD discs?

@jelly
Copy link
Member

jelly commented Jun 27, 2023

@skobyda I can't recall why, but I think it was because I never considered the cd/dvd image option. The option did exist back then, I simply did not take it into account :)

@skobyda skobyda changed the title WIP: Run disk format detection also for CD/DVD discs Run disk format detection also for CD/DVD discs Jun 27, 2023
test/check-machines-disks Outdated Show resolved Hide resolved
Copy link
Member

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

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

Ah, so an ISO file counts as "raw" format -- that's only relevant in the sense of "is it qcow or not"? Then this is straightforward indeed. Thanks!

@martinpitt martinpitt merged commit 15d065c into cockpit-project:main Jul 12, 2023
12 of 13 checks passed
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