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

mantle: Rework testiso/metal to use new qemu channel API #1307

Merged

Conversation

cgwalters
Copy link
Member

@cgwalters cgwalters commented Apr 1, 2020

First, add an API for creating channels that give a pipe
to the Go side, so we don't need to write to temp files that
we then read.

Now, I'm still trying to fine-tune the interface between the
"metal" API and the more hardcoded "testiso".

Tweak things so that the install path supports being provided
a builder object, but add an API that creates a default one
that mostly just sets up the default max memory.

(Also, I stopped trying to special case a lower memory for
the legacy install because that's going to go away anyways)

@cgwalters cgwalters force-pushed the testiso-provide-builder branch 2 times, most recently from a580247 to 5513ba8 Compare April 1, 2020 21:09
@cgwalters
Copy link
Member Author

Rebased 🏄‍♂️

@cgwalters cgwalters changed the title mantle: Rework testiso/metal to support passing a QemuBuilder mantle: Rework testiso/metal to use new qemu channel API Apr 2, 2020
First, add an API for creating channels that give a pipe
to the Go side, so we don't need to write to temp files that
we then read.

Now, I'm still trying to fine-tune the interface between the
"metal" API and the more hardcoded "testiso".

Tweak things so that the install path supports being provided
a builder object, but add an API that creates a default one
that mostly just sets up the default max memory.

(Also, I stopped trying to special case a lower memory for
 the legacy install because that's going to go away anyways)
Copy link
Contributor

@darkmuggle darkmuggle left a comment

Choose a reason for hiding this comment

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

This is pretty slick. I'm a fan of using virtio-serial as a channel.

@darkmuggle
Copy link
Contributor

/approve LGTM

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters, darkmuggle

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

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [cgwalters,darkmuggle]

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

@darkmuggle darkmuggle added the lgtm label Apr 2, 2020
@openshift-merge-robot openshift-merge-robot merged commit a34488c into coreos:master Apr 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants