[libvirt] [PATCH 2/8] qemuDomainBuildNamespace: Handle special file mount points
John Ferlan
jferlan at redhat.com
Tue Jul 11 10:05:39 UTC 2017
On 07/11/2017 01:14 AM, Michal Privoznik wrote:
> On 06/27/2017 11: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);
>
> Not really. mount --bind makes the target to be the correct type. IOW:
>
OK - I wasn't sure what all those other things were and going with
!IS_DIR just seemed to open the door to unsurety for me. Since there
was other code that was more restrictive to decide "ifReg", I figured
using that same list would be fine, but I'm not sure if I ever check
where in the scheme of the path the other check is make.
If you're fine with how things are, then fine consider it...
Reviewed-by: John Ferlan <jferlan at redhat.com>
John
> # create a regular file
> touch /tmp/blah
>
> # here, assume /source/socket is a socket
> mount --bind /source/socket /tmp/blah
>
> # now, /tmp/blah will be type of socket too
>
> The only problem here is that for file based 'devices' (or things in
> general) we have to create the file whereas for directories we have to
> create directories. Just like in the example.
>
> BTW, what type of file are NAM, MPB, MPC, NWK?
>
> Michal
>
More information about the libvir-list
mailing list