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@xxxxxxxxxx> >>> --- >>> 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@xxxxxxxxxx> 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 > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
