This repository was archived by the owner on Jan 20, 2022. It is now read-only.
govmm: add VhostUserFS vhost-user device type#86
Merged
markdryan merged 1 commit intokata-containers:masterfrom Feb 20, 2019
Merged
govmm: add VhostUserFS vhost-user device type#86markdryan merged 1 commit intokata-containers:masterfrom
markdryan merged 1 commit intokata-containers:masterfrom
Conversation
The QEMU vhost-user-fs-pci device provides virtio-fs host<->guest file system sharing (https://virtio-fs.gitlab.io/). The device is instantiated like this: $ qemu -chardev socket,path=/tmp/vhost-fs.sock,id=chr0 -device vhost-user-fs-pci,tag=myfs,chardev=chr0,cache-size=4G,versiontable=/dev/shm/fuse_shared_versions This patch adds the VhostUserFS DeviceDriver and command-line generation for this QEMU device. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
devimc
approved these changes
Feb 19, 2019
markdryan
approved these changes
Feb 19, 2019
Contributor
markdryan
left a comment
There was a problem hiding this comment.
Looks good to me as well. There seems to be some issue with coveralls, unrelated to this patch. I'll re-trigger the build to see whether that fixes it.
markdryan
reviewed
Feb 20, 2019
| devParams = append(devParams, string(driver)) | ||
| devParams = append(devParams, fmt.Sprintf("chardev=%s", vhostuserDev.CharDevID)) | ||
| devParams = append(devParams, fmt.Sprintf("tag=%s", vhostuserDev.Tag)) | ||
| devParams = append(devParams, fmt.Sprintf("cache-size=%dG", vhostuserDev.CacheSize)) |
Contributor
There was a problem hiding this comment.
coveralls seems to be working again, but actually I do have a question. Is 0 a valid value for vhostuserDev.CacheSize? I ask as we don't check this value in the Valid method, which led me to wonder whether cache-size=0G was a valid option.
Author
|
On Wed, Feb 20, 2019 at 8:32 AM Mark Ryan ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In qemu/qemu.go
<#86 (comment)>:
> @@ -809,6 +819,15 @@ func (vhostuserDev VhostUserDevice) QemuParams(config *Config) []string {
devParams = append(devParams, "logical_block_size=4096")
devParams = append(devParams, "size=512M")
devParams = append(devParams, fmt.Sprintf("chardev=%s", vhostuserDev.CharDevID))
+ case VhostUserFS:
+ driver = VhostUserFS
+ devParams = append(devParams, string(driver))
+ devParams = append(devParams, fmt.Sprintf("chardev=%s", vhostuserDev.CharDevID))
+ devParams = append(devParams, fmt.Sprintf("tag=%s", vhostuserDev.Tag))
+ devParams = append(devParams, fmt.Sprintf("cache-size=%dG", vhostuserDev.CacheSize))
coveralls seems to be working again, but actually I do have a question. Is
0 a valid value for vhostuserDev.CacheSize? I ask as we don't check this
value in the Valid method, which led me to wonder whether cache-size=0G was
a valid option.
QEMU currently prints an error for cache-size=0G but that will change
soon. cache-size=0G will disable the DAX feature on the QEMU side.
Probably best not to check vhostuserDev.CacheSize in Kata.
|
Contributor
Okay, sounds good. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The QEMU vhost-user-fs-pci device provides virtio-fs host<->guest file
system sharing (https://virtio-fs.gitlab.io/). The device is
instantiated like this:
$ qemu -chardev socket,path=/tmp/vhost-fs.sock,id=chr0
-device vhost-user-fs-pci,tag=myfs,chardev=chr0,cache-size=4G,versiontable=/dev/shm/fuse_shared_versions
This patch adds the VhostUserFS DeviceDriver and command-line generation
for this QEMU device.
Signed-off-by: Stefan Hajnoczi stefanha@redhat.com
Note that vhost-user-fs-pci is not yet available in qemu.git/master so it may be desirable to leave this pull request as "work in progress" until upstream QEMU merges the feature. This govmm patch is a prerequisite for Kata Containers virtio-fs support.