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

feat: add a small collection of requested features and fixes #1199

Merged
merged 15 commits into from May 12, 2024

Conversation

flexiondotorg
Copy link
Member

No description provided.

Copy link
Contributor

@philclifford philclifford left a comment

Choose a reason for hiding this comment

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

  • --kill works - but still seeing the qemu-img output 😕
  • --offline works

LGTM but I have uneasy feelings that more testing is needed (and I can't 'til tonight)

exit 1
fi

if [ -e "${disk_img}" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

It must be somewhere else still:

quickemu --vm alpine-latest.conf --kill
qemu-img: Could not open 'alpine-latest/disk.qcow2': Failed to get shared "write" lock
Is another process using the image [alpine-latest/disk.qcow2]?
VM already started!
Killing alpine-latest
 - alpine-latest (6798) killed.

@philclifford
Copy link
Contributor

 quickemu --vm alpine-latest.conf --monitor-cmd quit
qemu-img: Could not open 'alpine-latest/disk.qcow2': Failed to get shared "write" lock
Is another process using the image [alpine-latest/disk.qcow2]?
VM already started!
Processing line: ssh,22220
 - Sending:  quit

we send the monitor command now, and quit works OK

I think maybe an apt update has been though, because now system_powerdown does nothing (system_reset works still)

@popey
Copy link
Collaborator

popey commented May 11, 2024

    elif [ -e "${disk_img}" ]; then
        # Check there isn't already a process attached to the disk image.
        if ! ${QEMU_IMG} info "${disk_img}" >/dev/null; then
            echo "             Failed to get \"write\" lock. Is another process using the disk?"
            exit 1

This looks like the culprit for this:

./quickemu --vm ./ubuntu-24.04.conf --kill                                                                                
qemu-img: Could not open 'ubuntu-24.04/disk.qcow2': Failed to get shared "write" lock                                                                                                         
Is another process using the image [ubuntu-24.04/disk.qcow2]?                                                                                                                                 
VM already started!                                                                                                                                                                           
Killing ubuntu-24.04                                                                                                                                                                          
 - ubuntu-24.04 (1539308) killed.                                           

Agreed offline works though 👍
Good on the resolution output too!

@flexiondotorg
Copy link
Member Author

@philclifford and @popey Thanks for the feedback, there was quite a few logic bombs in there. Please test this branch again and let me know how it goes.

Copy link
Contributor

@philclifford philclifford left a comment

Choose a reason for hiding this comment

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

LGTM
New features all good and --monitor-cmd works normally again.
A little casual testing with assistance of Old Peculiar seems satisfactory. 🤞

@flexiondotorg flexiondotorg merged commit d2b5ee4 into master May 12, 2024
2 checks passed
@flexiondotorg flexiondotorg deleted the features branch May 12, 2024 10:40
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.

None yet

3 participants