-
Notifications
You must be signed in to change notification settings - Fork 374
virtiofs: fix virtiofs crash when cache=none #1924
virtiofs: fix virtiofs crash when cache=none #1924
Conversation
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 @ganeshmaharaj
kataVirtioFSDevType = "virtio-fs" | ||
sharedDir9pOptions = []string{"trans=virtio,version=9p2000.L,cache=mmap", "nodev"} | ||
sharedDirVirtioFSOptions = []string{"default_permissions,allow_other,rootmode=040000,user_id=0,group_id=0,tag=" + mountGuest9pTag, "nodev"} | ||
sharedDirVirtioFSDaxOptions = "dax" |
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.
really hard to identify what was the change here
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.
I know. I wish golang wouldn't keep moving every line to match every other line. The change is an one line addition.
sharedDirVirtioFSDaxOptions = "dax"
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.
and remove dax
from sharedDirVirtioFSOptions
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.
ohh hehe.. yes.. 😜
When virtio_fs_cache is set to none, the mount options for the folder inside the guest should not contain the dax option else it leads to invalid address errors and a crash of the daemon on the host. Fixes: kata-containers#1907 Signed-off-by: Ganesh Maharaj Mahalingam <[email protected]>
1d70418
to
6e1e6a2
Compare
/test |
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.
@ganeshmaharaj thanks
@jcvenegas @devimc the initrd fails is the one a revert is going to fix, right? |
Codecov Report
@@ Coverage Diff @@
## master #1924 +/- ##
==========================================
- Coverage 52.19% 52.19% -0.01%
==========================================
Files 108 108
Lines 14196 14198 +2
==========================================
+ Hits 7410 7411 +1
- Misses 5907 5909 +2
+ Partials 879 878 -1 |
// the guest directory can be mounted with option 'dax' allowing it to | ||
// directly map contents from the host. When set to 'none', the mount | ||
// options should not contain 'dax' lest the virtio-fs daemon crashing | ||
// with an invalid address reference. |
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.
@ganeshmaharaj Do we know why we get the invalid address reference?
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.
@amshinde if I understand right, cause the guest thinks the memory region is shared between the host daemon and the guest and expects the host to know memory mapping to that region but the host process was started with cache=none
and does not understand the translation.
metrics CI seems to be failing with below error.
@chavafg any chance you have seen this in the recent past? |
not really, maybe a network issue in the infra? I sent a restart, hopefully it doesn't fail this time |
When virtio_fs_cache is set to none, the mount options for the folder
inside the guest should not contain the dax option else it leads to
invalid address errors and a crash of the daemon on the host.
Fixes: #1907
Signed-off-by: Ganesh Maharaj Mahalingam [email protected]