8:18 a.m.
On Fri, 2017-04-28 at 13:22 +0200, Michal Privoznik wrote:
When setting up mount namespace for a qemu domain the following
steps are executed:
1) get list of mountpoints under /dev/
2) move them to /var/run/libvirt/qemu/$domName.ext
3) start constructing new device tree under /var/run/libvirt/qemu/$domName.dev
4) move the mountpoint of the new device tree to /dev
5) restore original mountpoints from step 2)
Not the problem with this approach is that if some device in step
You may have wanted to write "Note" rather than "Not".
Otherwise ACK.
--
Cedric
3) requires access to a mountpoint from step 2) it will fail as
the mountpoint is not there anymore. For instance consider the
following domain disk configuration:
<disk type='file' device='disk'>
<driver name='qemu' type='raw'/>
<source file='/dev/shm/vhostmd0'/>
<target dev='vdb' bus='virtio'/>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x0a' function='0x0'/>
</disk>
In this case operation fails as we are unable to create vhostmd0
in the new device tree because after step 2) there is no /dev/shm
anymore. Leave aside fact that we shouldn't try to create devices
living in other mountpoints. That's a separate bug that will be
addressed later.
Currently, the order described above is rearranged to:
1) get list of mountpoints under /dev/
2) start constructing new device tree under /var/run/libvirt/qemu/$domName.dev
3) move them to /var/run/libvirt/qemu/$domName.ext
4) move the mountpoint of the new device tree to /dev
5) restore original mountpoints from step 3)
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_domain.c | 54 +++++++++++++++++++++++++-------------------------
1 file changed, 27 insertions(+), 27 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 00b0b4a..be02d54 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -7950,33 +7950,6 @@ qemuDomainBuildNamespace(virQEMUDriverConfigPtr cfg,
if (qemuDomainSetupDev(cfg, mgr, vm, devPath) < 0)
goto cleanup;
- /* Save some mount points because we want to share them with the host */
- for (i = 0; i < ndevMountsPath; i++) {
- struct stat sb;
-
- if (devMountsSavePath[i] == devPath)
- continue;
-
- if (stat(devMountsPath[i], &sb) < 0) {
- virReportSystemError(errno,
- _("Unable to stat: %s"),
- devMountsPath[i]);
- goto cleanup;
- }
-
- /* At this point, devMountsPath is either a regular file 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)) {
- virReportSystemError(errno,
- _("Failed to create %s"),
- devMountsSavePath[i]);
- goto cleanup;
- }
-
- if (virFileMoveMount(devMountsPath[i], devMountsSavePath[i]) < 0)
- goto cleanup;
- }
-
if (qemuDomainSetupAllDisks(cfg, vm, devPath) < 0)
goto cleanup;
@@ -8001,6 +7974,33 @@ qemuDomainBuildNamespace(virQEMUDriverConfigPtr cfg,
if (qemuDomainSetupAllRNGs(cfg, vm, devPath) < 0)
goto cleanup;
+ /* Save some mount points because we want to share them with the host */
+ for (i = 0; i < ndevMountsPath; i++) {
+ struct stat sb;
+
+ if (devMountsSavePath[i] == devPath)
+ continue;
+
+ if (stat(devMountsPath[i], &sb) < 0) {
+ virReportSystemError(errno,
+ _("Unable to stat: %s"),
+ devMountsPath[i]);
+ goto cleanup;
+ }
+
+ /* At this point, devMountsPath is either a regular file 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)) {
+ virReportSystemError(errno,
+ _("Failed to create %s"),
+ devMountsSavePath[i]);
+ goto cleanup;
+ }
+
+ if (virFileMoveMount(devMountsPath[i], devMountsSavePath[i]) < 0)
+ goto cleanup;
+ }
+
if (virFileMoveMount(devPath, "/dev") < 0)
goto cleanup;