[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] qemu: Correctly label migration TCP socket passed to qemu



On 08/25/2011 08:30 AM, Jiri Denemark wrote:
-        VIR_FORCE_CLOSE(spec->dest.fd.qemu);

Unconditional, changed to...

+        if (virSecurityManagerSetImageFDLabel(driver->securityManager, vm,
+                                              spec->dest.fd.qemu) == 0) {
+            ret = qemuMonitorMigrateToFd(priv->mon, migrate_flags,
+                                         spec->dest.fd.qemu);
+            VIR_FORCE_CLOSE(spec->dest.fd.qemu);

...conditional.  Oops - you leak the fd if changing the label fails.

No, there's no leak here. The fd is closed by the caller in any case. Note,
we don't close it here if we fail before we ever get to
qemuMonitorMigrateToFd. This additional close after qemuMonitorMigrateToFd()
is just for better feeling in case migration started to avoid having an fd
open which doesn't technically belong to us anymore for the whole time of
migration.

Ah, makes sense. Until a child process is using the fd, it doesn't matter if we close early, and you've convinced me that the caller mops up in the cases where we don't close early. Meanwhile, once a child is using the fd, then we don't want to get in the way (especially important on pipes, where having multiple processes open as writers on the same pipe can prevent readers from seeing EOF), so only closing early after spawning the child makes sense. So,

ACK to this patch.

--
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]