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

doc: Add quickstart document #66

Merged
merged 2 commits into from
Nov 10, 2023
Merged

doc: Add quickstart document #66

merged 2 commits into from
Nov 10, 2023

Conversation

cfergeau
Copy link
Collaborator

No description provided.

@cfergeau
Copy link
Collaborator Author

Direct link to the quickstart document https://github.com/cfergeau/vfkit/blob/quickstart/doc/quickstart.md
I need to fix a few links.

doc/quickstart.md Outdated Show resolved Hide resolved
doc/quickstart.md Outdated Show resolved Hide resolved
# For Intel Macs
$ curl -L https://download.fedoraproject.org/pub/fedora/linux/releases/38/Cloud/x86_64/images/Fedora-Cloud-Base-38-1.6.x86_64.raw.xz
$ xz -d ./Fedora-Cloud-Base-38-1.6.x86_64.raw.xz
$ cp -c Fedora-Cloud-Base-38-1.6.x86_64.raw Fedora-Cloud-Base-38.raw
Copy link

Choose a reason for hiding this comment

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

why not mv ? the resulting copy is only to use a consistent name in the next paragraphs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's a good point that cp -c might be "too advanced" for this guide. However, to me it's a very neat feature to be aware of, it's macOS-specific so users are likely not to be aware of it, and it allows to have the equivalent of qcow2 backing files. You keep a base image, and you have an overlay with your changes on top of it. Maybe move this to its own paragraph with more details? Or to the reference documentation ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have left this unchanged in the update I just made, but will do more changes now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've addressed this now, only using mv in the quick start document, and pointing at a more detailed doc about cp -c and truncate and corresponding syscalls.

$ cp -c Fedora-Cloud-Base-38-1.6.x86_64.raw Fedora-Cloud-Base-38.raw
```

`cp -c` will create a copy on write image, its initial content is the same as the downloaded file, but all modifications will only be done in the copy.
Copy link

Choose a reason for hiding this comment

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

this seems unnecessary information, as you explain what a 'copy' of a virtual disk image represents. It would most likely be easier to just call the copy in that case simply something like: fedora-vfkit-test.raw or whatever.

Copy link

Choose a reason for hiding this comment

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

you do not include a cleanup for 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.

No cleanup for the downloaded file either, I would leave that up to the reader?

doc/quickstart.md Outdated Show resolved Hide resolved
doc/quickstart.md Outdated Show resolved Hide resolved
doc/quickstart.md Outdated Show resolved Hide resolved
Copy link

@gbraad gbraad left a comment

Choose a reason for hiding this comment

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

several comments.

  • cp needed, as you use aan unpacked image anyway. might make the name clearly indicate a test, and cleanup
  • fedora or Fedora?
  • other small suggestions

@openshift-ci openshift-ci bot removed the lgtm label Nov 1, 2023
doc/quickstart.md Outdated Show resolved Hide resolved
doc/quickstart.md Outdated Show resolved Hide resolved
doc/quickstart.md Outdated Show resolved Hide resolved
@cfergeau cfergeau force-pushed the quickstart branch 2 times, most recently from d03ad2a to c923127 Compare November 2, 2023 09:53
@cfergeau
Copy link
Collaborator Author

cfergeau commented Nov 2, 2023

One big unsanswered question in this document is how to inject a ssh key into the image, or get shell access to the guest, or do anything useful with the guest. This is an image which uses cloud-init, and I don't know how to use this with vfkit, so it will stay unanswered for now.

You start a virtual machine by running vfkit with a set of arguments describing the virtual machine configuration/hardware.
When vfkit stops, the virtual machine stops running.
It requires macOS 11 or newer, and runs on both x86_64 and aarch64 Macs.
Some features are only available on macOS 13 or newer.
Copy link

Choose a reason for hiding this comment

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

some features? if EFI is not available < 13 it might be wise to explain this.

Copy link
Collaborator Author

@cfergeau cfergeau Nov 7, 2023

Choose a reason for hiding this comment

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

EFI, virtio-fs, ...

### Getting a disk image

Your virtual machine will need an operating system to run, so you need to download a disk image first.
The image needs to be in the raw or iso format. qcow2 or VirtualBox images cannot be used by vfkit.
Copy link

Choose a reason for hiding this comment

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

Please note that qcow to avoid starting the sentence without a capital.


The virtual machine will be running until you hit Ctrl+C or kill the vfkit process.
If you are using an image which does not support UEFI boot, you can use the [`linux` bootloader](https://github.com/crc-org/vfkit/blob/main/doc/usage.md#linux-bootloader).
This requires separate kernel, initrd file and kernel command-line arguments.
Copy link

Choose a reason for hiding this comment

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

I would suggest to use linux bootloader as a regular part of sentence and moving the usage.md to the end of the following sentence.

This requires a .... Details about the use can be found in the usage instructions.

See also [vz/CreateDiskImage](https://pkg.go.dev/github.com/Code-Hex/vz/v3#CreateDiskImage).


#### Thin images
Copy link

Choose a reason for hiding this comment

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

👍

Copy link

@gbraad gbraad left a comment

Choose a reason for hiding this comment

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

Small suggestions ...
Overall LGTM, so could be merged.

@cfergeau
Copy link
Collaborator Author

cfergeau commented Nov 7, 2023

Small suggestions ... Overall LGTM, so could be merged.

Updated, I'll merge it tomorrow if no more comments.

@openshift-ci openshift-ci bot added the lgtm label Nov 10, 2023
Copy link

openshift-ci bot commented Nov 10, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: anjannath, gbraad
Once this PR has been reviewed and has the lgtm label, please ask for approval from cfergeau. For more information see the Kubernetes Code Review Process.

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

@gbraad gbraad merged commit 2a4d725 into crc-org:main Nov 10, 2023
4 of 5 checks passed
@cfergeau cfergeau deleted the quickstart branch January 23, 2024 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants