Re: [PATCH] virtiofsd: Show submounts

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2644 bytes --]

On 27.04.20 19:59, Dr. David Alan Gilbert wrote:
> * Max Reitz (mreitz@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.

>> Signed-off-by: Max Reitz <mreitz@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.

> 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.)

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]