Re: [PATCH] qemu: Jump to cleanup label on umount failure
On 2/6/23 19:08, Jim Fehlig wrote:
Similar to other error paths in qemuDomainUnshareNamespace(), jump
to
the cleanup label on umount error instead of directly returning -1.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
I noticed this while looking at a bug report containing the error. ATM I'm not
sure why the umount failed, but have asked for more info in the bug
https://bugzilla.opensuse.org/show_bug.cgi?id=1207889
src/qemu/qemu_namespace.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c
index 5769a4dfe0..833313d5a6 100644
--- a/src/qemu/qemu_namespace.c
+++ b/src/qemu/qemu_namespace.c
@@ -779,7 +779,7 @@ qemuDomainUnshareNamespace(virQEMUDriverConfig *cfg,
#if defined(__linux__)
if (umount("/dev") < 0) {
virReportSystemError(errno, "%s", _("failed to umount devfs on
/dev"));
- return -1;
+ goto cleanup;
}
#endif /* !defined(__linux__) */
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
And huge thanks to Martin Kletzander who actually came up with the root
cause for this problem. My code does not handle multiple mounts on the
same target. For instance:
# mount | grep /dev
udev on /dev type devtmpfs
(rw,nosuid,noexec,relatime,size=10240k,nr_inodes=4061238,mode=755)
devpts on /dev/pts type devpts (rw,relatime,gid=5,mode=620,ptmxmode=000)
tmpfs on /dev/shm type tmpfs (rw,nosuid,nodev)
mqueue on /dev/mqueue type mqueue (rw,nosuid,nodev,noexec,relatime)
tmpfs2 on /dev/shm type tmpfs (rw,relatime)
I've mounted /dev/shm for the second time as:
# mount -t tmpfs tmpfs2 /dev/shm/
The problem is that qemuDomainGetPreservedMounts() drops the one of the
/dev/shm entries, therefore, only one instance is moved to
/var/run/libvirt/... leaving the other behind. Hence, failing umount().
I'll try to come up with a fix, unless somebody else beats me to it.
Michal