Skip to content

run_qemu.sh: fix --rw option just broken by switch to -blockdev #213

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

Merged
merged 1 commit into from
Jun 13, 2025

Conversation

marc-hb
Copy link
Collaborator

@marc-hb marc-hb commented Jun 12, 2025

Use a qcow2 overlay to fix the --rw regression just added by commit dfa1d3f ("run_qemu.sh: replace -drive rootfs with -blockdev + -device")

The QEMU -snapshot option is silently ignored by -blockdev (this is documented in the qemu-system man page)

Use a qcow2 overlay to fix the --rw regression just added by commit
dfa1d3f ("run_qemu.sh: replace -drive rootfs with -blockdev +
-device")

The QEMU -snapshot option is silently ignored by -blockdev (this is
documented in the qemu-system man page)

Signed-off-by: Marc Herbert <[email protected]>
@marc-hb marc-hb marked this pull request as ready for review June 12, 2025 18:35
@marc-hb marc-hb requested a review from stellarhopper as a code owner June 12, 2025 18:35
@@ -1903,6 +1914,7 @@ prepare_qcmd()
fi

if [[ $_arg_rw == "off" ]]; then
# Note this is only for the (deprecated) -drive and ignored by -blockdev
Copy link
Member

Choose a reason for hiding this comment

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

Does it make sense to just remove this block - considering we've fully moved away from -drive..

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We still use -drive in a couple other places.

else
local _overlay=${_arg_rootfs}-overlay.qcow2
rm -f "${_overlay}"
qemu-img create -F raw -b "${_arg_rootfs}" -f qcow2 "${_overlay}"
Copy link
Member

Choose a reason for hiding this comment

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

Just out of curiosity, is there a way to 'apply' the overlay onto the raw image?

A workflow I've used in the past is to create an image using run_qemu with my kernel and anything else I need, and then use that raw image elsewhere, say an emulator or even dd it to a drive and use it on a real system. I'm not sure if something like this is frequently used/needed by others, but these changes (starting with -blockdev) would preclude doing this..

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

is there a way to 'apply' the overlay onto the raw image?

You can "apply" and many other things using qemu-img but I think that's out of scope for run_qemu.sh. I mean, run_qemu.sh is already a wrapper of many other tools, I don't think we should add qemu-img to that long list.

The overlay is left behind (until the next run) so anyone is free to change their mind and apply it outside run_qemu.sh.

but these changes (starting with -blockdev) would preclude doing this..

This PR is designed to fix --rw which I just broke and bring it back exactly where it was. I don't see what it would preclude, can you elaborate? If nothing else I feel like it's opening more possibilities, including... becoming a qemu-img wrapper, which I think is not a good idea.

Copy link
Member

@stellarhopper stellarhopper Jun 12, 2025

Choose a reason for hiding this comment

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

Oh yes I was not suggesting run_qemu should grow to provide this apply overlay functionality. More that if we ever wanted to do this kind of an "export a customized image" workflow, there was still /some/ way to accomplish that with the overlay. No issues merging this by me!

Copy link
Collaborator Author

@marc-hb marc-hb left a comment

Choose a reason for hiding this comment

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

@jic23 you're likely the most expert in QEMU around here, can you help review with this very small, QCOW overlay fix? Which fixes a regression coming indirectly from you :-P

else
local _overlay=${_arg_rootfs}-overlay.qcow2
rm -f "${_overlay}"
qemu-img create -F raw -b "${_arg_rootfs}" -f qcow2 "${_overlay}"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

is there a way to 'apply' the overlay onto the raw image?

You can "apply" and many other things using qemu-img but I think that's out of scope for run_qemu.sh. I mean, run_qemu.sh is already a wrapper of many other tools, I don't think we should add qemu-img to that long list.

The overlay is left behind (until the next run) so anyone is free to change their mind and apply it outside run_qemu.sh.

but these changes (starting with -blockdev) would preclude doing this..

This PR is designed to fix --rw which I just broke and bring it back exactly where it was. I don't see what it would preclude, can you elaborate? If nothing else I feel like it's opening more possibilities, including... becoming a qemu-img wrapper, which I think is not a good idea.

@@ -1903,6 +1914,7 @@ prepare_qcmd()
fi

if [[ $_arg_rw == "off" ]]; then
# Note this is only for the (deprecated) -drive and ignored by -blockdev
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We still use -drive in a couple other places.

@marc-hb marc-hb merged commit 2bcc508 into pmem:main Jun 13, 2025
2 checks passed
@marc-hb marc-hb deleted the overlay-fix-rw branch June 13, 2025 14:30
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