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