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

Add setup to emulate NVMe device #672

Merged
merged 1 commit into from
Sep 13, 2021
Merged

Conversation

alicefr
Copy link
Member

@alicefr alicefr commented Sep 2, 2021

The setup allows to create and emulate in the provisioned nodes NVMe
device. This can be particularly useful for testing scenarios with
NVMe like NVMe passthrough.

How to enable it during the cluster creation:

$ export KUBEVIRT_PROVIDER_EXTRA_ARGS="--nvme 1G --nvme 500M"
$ make cluster-up
[...]
$ cluster-up/ssh.sh node01
WARNING: KUBEVIRTCI_GOCLI_CONTAINER is set and will take precedence over the also set KUBEVIRTCI_TAG
selecting docker as container runtime
2021/09/02 10:34:58 Waiting for host: 192.168.66.101:22
2021/09/02 10:34:58 Connected to tcp://192.168.66.101:22
Last login: Thu Sep  2 10:24:50 2021 from 192.168.66.2
[vagrant@node01 ~]$ sudo lspci -nnD | grep NVM
0000:00:05.0 Non-Volatile memory controller [0108]: Intel Corporation QEMU NVM Express Controller [8086:5845] (rev 02)
[vagrant@node01 ~]$ lsblk
NAME    MAJ:MIN RM  SIZE RO TYPE MOUNTPOINT
sr0      11:0    1 1024M  0 rom  
vda     252:0    0   35G  0 disk 
└─vda1  252:1    0   35G  0 part /
nvme0n1 259:0    0    1G  0 disk 
nvme1n1 259:1    0  500M  0 disk 

@kubevirt-bot kubevirt-bot added the dco-signoff: yes Indicates the PR's author has DCO signed all their commits. label Sep 2, 2021
@kubevirt-bot kubevirt-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/S labels Sep 2, 2021
@kubevirt-bot kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 2, 2021
@vladikr
Copy link
Member

vladikr commented Sep 3, 2021

/test check-provision-k8s-1.20-cgroupsv2
/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Sep 3, 2021
@@ -119,4 +129,4 @@ exec qemu-system-x86_64 -enable-kvm -drive format=qcow2,file=${next},if=virtio,c
-serial pty -M q35,accel=kvm,kernel_irqchip=split \
-device intel-iommu,intremap=on,caching-mode=on -soundhw hda \
-uuid $(cat /proc/sys/kernel/random/uuid) \
${QEMU_ARGS}
${QEMU_ARGS} ${NVME_QEMU_ARG}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to touch vm.sh ? The NVMe qemu arguments must be after the other arguments ? If not this can be implemented just at gocli passing the NVMe arguments to QEMU_ARGS.

Copy link
Member Author

@alicefr alicefr Sep 6, 2021

Choose a reason for hiding this comment

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

@qinqon we need to create the image before calling qemu. For the rest, you are right I can pass the arguments as extra qemu args.

Copy link
Member

Choose a reason for hiding this comment

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

@alicefr I think there is a pretty simple way to still do this at cluster-up in contrast to cluster-provision while keeping most of your changes like they are. The vm.sh script is executed in both phases. You could move the gocli flag from provision to up and the disks would then only be created when vm.sh is executed for cluster-up. Similar like you can sets different memory for both phases.

Copy link
Member Author

Choose a reason for hiding this comment

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

sorry @rmohr I don't quite understand the comment. Do you mean moving the --nvme flag from gocli run to gocli provision?

Copy link
Member

Choose a reason for hiding this comment

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

No. Forget my comment. Everything is where it should be. I hod for some reason the impression that you provision the images during the provision phase. 👍

@kubevirt-bot kubevirt-bot removed the lgtm Indicates that a PR is ready to be merged. label Sep 6, 2021
The setup allows to create and emulate in the provisioned nodes an NVMe
device. This can be particularly useful for testing scenarios with
NVMe like NVMe passthrough.

Added in the gocli provision the new flag nvme in order to create the
device.

Signed-off-by: Alice Frosi <[email protected]>
@alicefr
Copy link
Member Author

alicefr commented Sep 6, 2021

@qinqon @vladikr I added the setup to create multiple NVMe devices and I moved the qemu argument in gocli.

@@ -482,6 +485,16 @@ func run(cmd *cobra.Command, args []string) (retErr error) {
nodeQemuArgs = fmt.Sprintf("%s -device vfio-pci,host=%s", nodeQemuArgs, gpuAddress)
}

var vmArgsNvmeDisks []string
if len(nvmeDisks) > 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be possible to execute qemu-img create command at gocli so we don't touch vm.sh ? maybe adding additional steps at the vmContainerConfig Cmd:.

We want to move as much bash code to gocli as possible.

Copy link
Member Author

@alicefr alicefr Sep 7, 2021

Choose a reason for hiding this comment

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

no because the gocli is executed in a separate container and it only sets the command to run in the node container. If we want to remove the script then we need some kind of program that executes the commands The current setup is not different than the way how the script creates the image for block devices (https://github.com/kubevirt/kubevirtci/blob/main/cluster-provision/centos8/scripts/vm.sh#L85)

Copy link
Member Author

Choose a reason for hiding this comment

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

qemu-img needs to be executed in the container that provisions the node

@qinqon
Copy link
Contributor

qinqon commented Sep 8, 2021

/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Sep 8, 2021
@qinqon
Copy link
Contributor

qinqon commented Sep 13, 2021

/approve

@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: qinqon

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:

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

@kubevirt-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 13, 2021
@kubevirt-bot kubevirt-bot merged commit 4e3eb21 into kubevirt:main Sep 13, 2021
kubevirt-bot added a commit to kubevirt-bot/kubevirt that referenced this pull request Sep 13, 2021
[1a96266 Update instruction to use K8s 1.21 since 1.16 already been deprecated.](kubevirt/kubevirtci#667)
[4e3eb21 Add setup to emulate NVMe device](kubevirt/kubevirtci#672)

Signed-off-by: kubevirt-bot <[email protected]>
kubevirt-bot added a commit to kubevirt-bot/kubevirt that referenced this pull request Sep 14, 2021
[1a96266 Update instruction to use K8s 1.21 since 1.16 already been deprecated.](kubevirt/kubevirtci#667)
[4e3eb21 Add setup to emulate NVMe device](kubevirt/kubevirtci#672)

Signed-off-by: kubevirt-bot <[email protected]>
@alicefr alicefr mentioned this pull request May 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged. size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants