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.

Sign up for GitHub

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

Jump to bottom

support different architectures in machine init #12430

Closed
wants to merge 1 commit into from
Closed

support different architectures in machine init #12430

wants to merge 1 commit into from

Conversation

vladdy
Copy link

@vladdy vladdy commented Nov 28, 2021 •
edited

(WIP to get early feedback and track the progress)

TODOs:

  • refactor code to support the arch command line option in machine init
  • (optional) consider the arch to be an enum instead of string
  • review and test amd64 options when on arm64 (MacOS)
  • review and test amd64 options when on arm64 (Linux)
  • review arm64 options when on amd64 (MacOS)
  • review arm64 options when on amd64 (Linux)

What this PR does / why we need it:

This PR adds the arch parameter to podman machine init to provide an ability to create a virtual machine with different architecture type than the host machine. By default the parameter is set to the host machine's architecture so there is not change in the default behavior.

Closes: #11458

How to verify it

podman machine init --arch=x86_64 x86_machine

Which issue(s) this PR fixes:

None

Special notes for your reviewer:

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 28, 2021
@rhatdan
Copy link
Member

rhatdan commented Nov 29, 2021

@baude PTAL

baude
baude reviewed Nov 29, 2021
View reviewed changes
pkg/machine/qemu/options_darwin.go
// TODO
}

panic(fmt.Sprintf("unsupported combination of host and VM architectures: host: %s, vm: %s", fromArch, toArch))
Copy link
Member

Choose a reason for hiding this comment

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

this will need to be an error return.

@baude
Copy link
Member

baude commented Nov 29, 2021

looks ok so far, one nit

@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 19, 2021
@github-actions
Copy link

A friendly reminder that this PR had no activity for 30 days.

@rhatdan
Copy link
Member

rhatdan commented Jan 19, 2022

This PR requires a rebase.

@vladdy
Copy link
Author

vladdy commented Jan 19, 2022

Will do this this week as well as push the progress I've got.

@github-actions github-actions bot removed the stale-pr label Jan 20, 2022
@rhatdan rhatdan added the 4.1 label Jan 27, 2022
@github-actions
Copy link

A friendly reminder that this PR had no activity for 30 days.

@rhatdan
Copy link
Member

rhatdan commented Feb 28, 2022

@vladdy Anything going on here?

@github-actions github-actions bot removed the stale-pr label Mar 1, 2022
@vladdy
Copy link
Author

vladdy commented Mar 4, 2022

WIll be looking into this this weekend.

@vladdy
initial refactoring to support different architectures in machine init
6cf595f 
Signed-off-by: Vlad Artamonov <perestrelka@gmail.com>
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 8, 2022

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: vladdy
To complete the pull request process, please assign ashley-cui after the PR has been reviewed.
You can assign the PR to them by writing /assign @ashley-cui in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 8, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 27, 2022

@vladdy: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 27, 2022
@github-actions
Copy link

A friendly reminder that this PR had no activity for 30 days.

@rhatdan
Copy link
Member

rhatdan commented Apr 27, 2022

@vladdy still working on this?

@baude
Copy link
Member

baude commented Apr 27, 2022

i really want to have input on this ... because doing this without being step with @dustymabe /fcos folks, im worried about consistently being able to reproduce the same results.

@dustymabe we continue to see high demand for this function ... and to do so, we need qemu-static. what say you about us lobbying to have it added?

@github-actions github-actions bot removed the stale-pr label Apr 28, 2022
@dustymabe
Copy link
Contributor

i really want to have input on this ... because doing this without being step with @dustymabe /fcos folks, im worried about consistently being able to reproduce the same results.

@dustymabe we continue to see high demand for this function ... and to do so, we need qemu-static. what say you about us lobbying to have it added?

I'd refer to this ongoing discussion about the topic: coreos/fedora-coreos-tracker#1088

@dustymabe
Copy link
Contributor

Though there isn't any reason users can't package layer it today and get what they want.

@umohnani8
Copy link
Member

Don't see this making it into 4.1, removing the 4.1 label from it.

@umohnani8 umohnani8 removed the 4.1 label May 4, 2022
@github-actions
Copy link

github-actions bot commented Jun 4, 2022

A friendly reminder that this PR had no activity for 30 days.

@rhatdan
Copy link
Member

rhatdan commented Jun 21, 2022

Since qemu-user-static packages are in fedora coreos now (or soon) and this PR feels like it is abandoned, I am going to close. Feel free to reopen if this is something you want in.

@rhatdan rhatdan closed this Jun 21, 2022
@Luap99 Luap99 mentioned this pull request Jul 28, 2022
Open
@piers-walter-ibm
Copy link

Hi @rhatdan, not sure how close this was to being implemented but I can see it still being useful. My team was trying to run icr.io/ibm-messaging/mq:latest and could only get it to work using an x86_64 Fedora CoreOS VM, not on the aarch64 one. The option to flag an architecture to use for the base VM would definitely improve compatibility although at the cost of performance

@dustymabe
Copy link
Contributor

Assuming that is a x86_64 container image it should work on an aarch64 instance in latest Fedora CoreOS releases:

  • stable stream 36.20220703.3.1 and newer
  • testing stream 36.20220703.2.1 and newer
  • next stream 36.20220703.1.1 and newer

@piers-walter-ibm
Copy link

The container itself launches, but crashes. The container can be tested by running podman run --rm --env LICENSE=accept --env MQ_QMGR_NAME=QM1 icr.io/ibm-messaging/mq:latest. I am using the 36.20220716.2.0 CoreOS release. Other x86_64 containers run ok so it's only specific ones which seem to generate issues

@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 20, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@baude baude baude left review comments

Assignees
No one assigned
Labels
do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. stale-pr
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

podman machine init Support emulating different architectures
6 participants
@vladdy @rhatdan @baude @dustymabe @umohnani8 @piers-walter-ibm