On 28.04.20 11:59, Dr. David Alan Gilbert wrote: > * Max Reitz (mre...@redhat.com) wrote: >> On 27.04.20 19:59, Dr. David Alan Gilbert wrote: >>> * Max Reitz (mre...@redhat.com) wrote: >>>> Currently, setup_mounts() bind-mounts the shared directory without >>>> MS_REC. This makes all submounts disappear. >>>> >>>> Pass MS_REC so that the guest can see submounts again. >>> >>> Thanks! >>> >>>> Fixes: 3ca8a2b1c83eb185c232a4e87abbb65495263756 >>> >>> Should this actually be 5baa3b8e95064c2434bd9e2f312edd5e9ae275dc ? >> >> Well, I bisected it and landed at 3ca8a2b1. So while the problematic >> line may have been introduced by 5baa3b8e, it wasn’t used until 3ca8a2b1. > > OK, I'd rather stick with the Fixes: for the patch that was actually > wrong.
Why not both? :) >>>> Signed-off-by: Max Reitz <mre...@redhat.com> >>>> --- >>>> tools/virtiofsd/passthrough_ll.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/tools/virtiofsd/passthrough_ll.c >>>> b/tools/virtiofsd/passthrough_ll.c >>>> index 4c35c95b25..9d7f863e66 100644 >>>> --- a/tools/virtiofsd/passthrough_ll.c >>>> +++ b/tools/virtiofsd/passthrough_ll.c >>>> @@ -2643,7 +2643,7 @@ static void setup_mounts(const char *source) >>>> int oldroot; >>>> int newroot; >>>> >>>> - if (mount(source, source, NULL, MS_BIND, NULL) < 0) { >>>> + if (mount(source, source, NULL, MS_BIND | MS_REC, NULL) < 0) { >>>> fuse_log(FUSE_LOG_ERR, "mount(%s, %s, MS_BIND): %m\n", source, >>>> source); >>>> exit(1); >>>> } >>> >>> Do we want MS_SLAVE to pick up future mounts that might happenf rom the >>> host? >> >> Hm. So first it looks to me from the man page like one shouldn’t give >> MS_SLAVE on the first mount() call but kind of only use it for remounts >> (in the list at the start, “Create a bind mount” is separate from >> “Change the propagation type of an existing mount”, and the man page >> later says “The only other flags that can be specified while changing >> the propagation type are MS_REC (described below) and MS_SILENT (which >> is ignored).”). >> >> Second, even if I do change the propagation type to MS_SLAVE in a second >> call, mounts done after qemu has been started don’t show up in the guest >> (for me). >> >> So while it sounds correct, I can’t see it having an effect, actually. > > That's unfortunate; but I guess we can debug that separately > >>> What's the interaction between this and the MS_REC|MS_SLAVE that we have >>> a few lines above for / ? >> >> Good question. It would seem to me that there isn’t any. That previous >> mount call just sets MS_REC | MS_SLAVE for the whole mount namespace, >> and then we do a new mount here (by default from / to /) that needs its >> own flags. >> >> (More interesting is perhaps why we have that other mount() call below, >> which again sets MS_REC | MS_SLAVE for the old (not-yet-bind-mounted) /. >> I can’t imagine that to have any effect.) > > Is that just trying to be careful before the umount2 so it doesn't try > to unmount something useful? Perhaps, but still, it shouldn’t matter. I rather suspect that setup_namespaces() and setup_mounts() were developed (or taken from elsewhere) independently, so they both have to work independently, and thus they do overlapping stuff. Max
signature.asc
Description: OpenPGP digital signature