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

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



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 redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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