Re: [PATCHv6 4/7] Implement virDomain{Set, Get}BlockIoTune for the qemu driver

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

 



On 11/23/2011 02:44 PM, Eric Blake wrote:
> From: Lei Li <lilei@xxxxxxxxxxxxxxxxxx>
> 
> Implement the block I/O throttle setting and getting support to qemu
> driver.
> 
> Signed-off-by: Lei Li <lilei@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Zhi Yong Wu <wuzhy@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>

> +qemuDomainSetBlockIoTune(virDomainPtr dom,
> +                         const char *disk,
> +                         virTypedParameterPtr params,
> +                         int nparams,
> +                         unsigned int flags)
> +{

> +
> +    if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
> +        if (!vm->persistent) {
> +            qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                            _("cannot change persistent config of a transient domain"));
> +            goto endjob;
> +        }
> +        if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm)))
> +            goto endjob;
> +    }
> +

> +
> +    if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
> +        sa_assert(persistentDef);
> +        int idx = virDomainDiskIndexByName(vm->def, disk, true);

Oops - this should be on persistentDef, not vm->def.

> +        if (i < 0)
> +            goto endjob;
> +        persistentDef->disks[idx]->blkdeviotune = info;

And this assignment should be delayed...

> +    }
> +
> +    if (flags & VIR_DOMAIN_AFFECT_LIVE) {
> +        priv = vm->privateData;
> +        qemuDomainObjEnterMonitorWithDriver(driver, vm);
> +        ret = qemuMonitorSetBlockIoThrottle(priv->mon, device, &info);
> +        qemuDomainObjExitMonitorWithDriver(driver, vm);
> +    }
> +
> +    if (flags & VIR_DOMAIN_AFFECT_CONFIG) {

to here, after we know the live change (if any) took place.  Not to
mention that we must not get here if the live change failed.  Here's
what I'm squashing in:

diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index 698a961..ce4cba1 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -11080,6 +11080,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
     int ret = -1;
     int i;
     bool isActive;
+    int idx = -1;

     virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
                   VIR_DOMAIN_AFFECT_CONFIG, -1);
@@ -11126,6 +11127,9 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
         }
         if (!(persistentDef =
virDomainObjGetPersistentDef(driver->caps, vm)))
             goto endjob;
+        idx = virDomainDiskIndexByName(persistentDef, disk, true);
+        if (i < 0)
+            goto endjob;
     }

     for (i = 0; i < nparams; i++) {
@@ -11177,22 +11181,18 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
         goto endjob;
     }

-    if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
-        sa_assert(persistentDef);
-        int idx = virDomainDiskIndexByName(vm->def, disk, true);
-        if (i < 0)
-            goto endjob;
-        persistentDef->disks[idx]->blkdeviotune = info;
-    }
-
     if (flags & VIR_DOMAIN_AFFECT_LIVE) {
         priv = vm->privateData;
         qemuDomainObjEnterMonitorWithDriver(driver, vm);
         ret = qemuMonitorSetBlockIoThrottle(priv->mon, device, &info);
         qemuDomainObjExitMonitorWithDriver(driver, vm);
     }
+    if (ret < 0)
+        goto endjob;

     if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+        sa_assert(persistentDef && idx >= 0);
+        persistentDef->disks[idx]->blkdeviotune = info;
         ret = virDomainSaveConfig(driver->configDir, persistentDef);
         if (ret < 0) {
             qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",

-- 
Eric Blake   eblake@xxxxxxxxxx    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
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]