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

[Eric Blake]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120510/717c2cda/attachment-0001.sig>