Re: [PATCH] qemu: check for vm after starting a job

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

 



On 10/27/2010 08:08 AM, Daniel P. Berrange wrote:
On Tue, Oct 26, 2010 at 05:32:09PM -0600, Eric Blake wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=638285 - when migrating
a guest, it was very easy to provoke a race where an application
could query block information on a VM that had just been migrated
away.  Any time qemu code obtains a job lock, it must also check
that the VM was not taken down in the time where it was waiting
for the lock.

@@ -4948,6 +4949,12 @@ static int qemudDomainSetMemory(virDomainPtr dom, unsigned long newmem) {
      if (qemuDomainObjBeginJob(vm)<  0)
          goto cleanup;

+    if (!virDomainObjIsActive(vm)) {
+        qemuReportError(VIR_ERR_OPERATION_INVALID,
+                        "%s", _("domain is not running"));
+        goto endjob;
+    }
+

There is already an IsActive check in this method, it is simply in
the wrong place wrt to the BeginJob call, so it is better to just
move the existing call, rather than duplicate it i reckon.

Okay, I'll see about swapping things around.

@@ -10350,6 +10368,7 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom,
                                          &info->allocation);
          qemuDomainObjExitMonitor(vm);

+    endjob:
          if (qemuDomainObjEndJob(vm) == 0)
              vm = NULL;
      } else {

I'm not much of a fan of adding goto jump targets which are in the middle
of long methods. Could we just invert the IsActive checks in these two
methods&  thus avoid the 'endjob' in the middle of the method.

Yes, I'll submit a v2 that avoids extra goto labels (or at least sinks the endjob label to the end of the function rather than the middle).

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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]