[libvirt] [PATCH 2/8] qemuDomainBuildNamespace: Handle special file mount points

[John Ferlan]

On 06/27/2017 05:37 PM, John Ferlan wrote:
> 
> 
> On 06/22/2017 12:18 PM, Michal Privoznik wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1459592
>>
>> In 290a00e41d I've tried to fix the process of building a
>> qemu namespace when dealing with file mount points. What I
>> haven't realized then is that we might be dealing not with just
>> regular files but also special files (like sockets). Indeed, try
>> the following:
>>
>> 1) socat unix-listen:/tmp/soket stdio
>> 2) touch /dev/socket
>> 3) mount --bind /tmp/socket /dev/socket
>> 4) virsh start anyDomain
>>
>> Problem with my previous approach is that I wasn't creating the
>> temporary location (where mount points under /dev are moved) for
>> anything but directories and regular files.
>>
>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>> ---
>>  src/qemu/qemu_domain.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>> index 8e7404da6..212717c80 100644
>> --- a/src/qemu/qemu_domain.c
>> +++ b/src/qemu/qemu_domain.c
>> @@ -8356,9 +8356,11 @@ qemuDomainBuildNamespace(virQEMUDriverConfigPtr cfg,
>>              goto cleanup;
>>          }
>>  
>> -        /* At this point, devMountsPath is either a regular file or a directory. */
>> +        /* At this point, devMountsPath is either:
>> +         * a file (regular or special), or
>> +         * a directory. */
>>          if ((S_ISDIR(sb.st_mode) && virFileMakePath(devMountsSavePath[i]) < 0) ||
>> -            (S_ISREG(sb.st_mode) && virFileTouch(devMountsSavePath[i], sb.st_mode) < 0)) {
>> +            (!S_ISDIR(sb.st_mode) && virFileTouch(devMountsSavePath[i], sb.st_mode) < 0)) {
> 
> It would seem to me that this would open Pandora's box to all different
> types of things (BLK, CHR, FIFO, LNK, NAM, MPB, MPC, NWK) - some of
> which it may not be so popular to perform a touch on.
> 
> I think you should keep it specific...  Perhaps use the list from
> qemuDomainCreateDeviceRecursive:
> 
>    isReg = S_ISREG(sb.st_mode) || S_ISFIFO(sb.st_mode) ||
> S_ISSOCK(sb.st_mode);
> 
> John
> 

I guess it's obvious now that I've actually paged in patches 4-8 that I
used the same sandbox I'm reviewing in order to make this comment!

I think the same concern applies though...

John

> (FWIW: I used a cscope search on S_ISSOCK...)
> 
> 
>>              virReportSystemError(errno,
>>                                   _("Failed to create %s"),
>>                                   devMountsSavePath[i]);
>>
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>