Re: [PATCH 1/3] qemu: Refactor qemuDomainSetVcpusFlags

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

 



On 05/07/2012 06:16 AM, Peter Krempa wrote:

Mentioning a summary of what you refactored might be nice for someone
reading 'git log'

> ---
>  src/qemu/qemu_driver.c |   24 ++++++++----------------
>  1 files changed, 8 insertions(+), 16 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index c100a1a..7b1d1b6 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -3463,8 +3463,7 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
>          goto endjob;
>      }
> 
> -    switch (flags) {
> -    case VIR_DOMAIN_AFFECT_CONFIG:
> +    if (flags & VIR_DOMAIN_AFFECT_CONFIG) {

Thank you for cleaning this up.  I have no idea what I was thinking when
I first coded this switch statement (it was back in the days when I
wasn't quite sure how live vs. persistent definitions worked).

>          if (maximum) {
>              persistentDef->maxvcpus = nvcpus;
>              if (nvcpus < persistentDef->vcpus)
> @@ -3472,24 +3471,17 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
>          } else {
>              persistentDef->vcpus = nvcpus;
>          }
> -        ret = 0;
> -        break;
> 
> -    case VIR_DOMAIN_AFFECT_LIVE:
> -        ret = qemudDomainHotplugVcpus(driver, vm, nvcpus);
> -        break;
> +        if (virDomainSaveConfig(driver->configDir, persistentDef) < 0)
> +            goto endjob;
> +    }
> 
> -    case VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG:
> -        ret = qemudDomainHotplugVcpus(driver, vm, nvcpus);
> -        if (ret == 0) {
> -            persistentDef->vcpus = nvcpus;
> -        }
> -        break;
> +    if (flags & VIR_DOMAIN_AFFECT_LIVE) {
> +        if (qemudDomainHotplugVcpus(driver, vm, nvcpus) < 0)
> +            goto endjob;
>      }

Alas, I see a flaw in your refactoring.  You want to affect live before
config, otherwise, if the live change fails, you would have to undo the
config change (or, at the bare minimum, sink the
virDomainSaveConfig(persistentDef) until after the affect_live portion,
even if the rest of the persistent code is done first).

> 
> -    /* Save the persistent config to disk */
> -    if (flags & VIR_DOMAIN_AFFECT_CONFIG)
> -        ret = virDomainSaveConfig(driver->configDir, persistentDef);
> +    ret = 0;
> 
>  endjob:
>      if (qemuDomainObjEndJob(driver, vm) == 0)

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