-
Notifications
You must be signed in to change notification settings - Fork 100
govmm: add VhostUserFS vhost-user device type #86
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>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @stefanha
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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.
|
Okay, sounds good. |
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.