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

Re: [libvirt] [PATCH v2 3/4] lxc, libxl: reuse virDomainObjUpdateModificationImpact




On 02/24/2016 03:38 AM, Nikolay Shirokovskiy wrote:
> First fortunately all versions of *_CURRENT, *_CONFIG and *_LIVE flags
> are numerically equal. Second libxl functions that don't use masks
> for flag expansion forbid extra flags via virCheckFlags before.
> 
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy virtuozzo com>
> ---
>  src/libxl/libxl_driver.c | 63 ++++------------------------------------
>  src/lxc/lxc_driver.c     | 75 ++++--------------------------------------------
>  2 files changed, 12 insertions(+), 126 deletions(-)
> 

This should be two separate patches and use of singular libxl: and lxc:
prefixes on the subject line.

#1 For libxl driver changes (there's a bug there too)
#2 for lxc changes

This one I'll ask you to fix the bug and repost as two separate patches

> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 06feea5..7e65ba2 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -3660,25 +3660,8 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml,
>      if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
>          goto cleanup;
>  
> -    if (virDomainObjIsActive(vm)) {
> -        if (flags == VIR_DOMAIN_DEVICE_MODIFY_CURRENT)
> -            flags |= VIR_DOMAIN_DEVICE_MODIFY_LIVE;
> -    } else {
> -        if (flags == VIR_DOMAIN_DEVICE_MODIFY_CURRENT)
> -            flags |= VIR_DOMAIN_DEVICE_MODIFY_CONFIG;
> -        /* check consistency between flags and the vm state */
> -        if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) {
> -            virReportError(VIR_ERR_OPERATION_INVALID,
> -                           "%s", _("Domain is not running"));
> -            goto endjob;
> -        }
> -    }
> -
> -    if ((flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) && !vm->persistent) {
> -         virReportError(VIR_ERR_OPERATION_INVALID,
> -                        "%s", _("cannot modify device on transient domain"));
> -         goto endjob;
> -    }
> +    if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
> +        goto endjob;
>  
>      if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
>          if (!(dev = virDomainDeviceDefParse(xml, vm->def,
> @@ -3768,25 +3751,8 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml,
>      if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
>          goto cleanup;
>  
> -    if (virDomainObjIsActive(vm)) {
> -        if (flags == VIR_DOMAIN_DEVICE_MODIFY_CURRENT)
> -            flags |= VIR_DOMAIN_DEVICE_MODIFY_LIVE;
> -    } else {
> -        if (flags == VIR_DOMAIN_DEVICE_MODIFY_CURRENT)
> -            flags |= VIR_DOMAIN_DEVICE_MODIFY_CONFIG;
> -        /* check consistency between flags and the vm state */
> -        if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) {
> -            virReportError(VIR_ERR_OPERATION_INVALID,
> -                           "%s", _("Domain is not running"));
> -            goto endjob;
> -        }
> -    }
> -
> -    if ((flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) && !vm->persistent) {
> -         virReportError(VIR_ERR_OPERATION_INVALID,
> -                        "%s", _("cannot modify device on transient domain"));
> -         goto endjob;
> -    }
> +    if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
> +        goto endjob;
>  
>      if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
>          if (!(dev = virDomainDeviceDefParse(xml, vm->def,
> @@ -3873,25 +3839,8 @@ libxlDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml,
>      if (virDomainUpdateDeviceFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
>          goto cleanup;
>  
> -    if (virDomainObjIsActive(vm)) {
> -        if (flags == VIR_DOMAIN_DEVICE_MODIFY_CURRENT)
> -            flags |= VIR_DOMAIN_DEVICE_MODIFY_LIVE;
> -    } else {
> -        if (flags == VIR_DOMAIN_DEVICE_MODIFY_CURRENT)
> -            flags |= VIR_DOMAIN_DEVICE_MODIFY_CONFIG;
> -        /* check consistency between flags and the vm state */
> -        if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) {
> -            virReportError(VIR_ERR_OPERATION_INVALID,
> -                           "%s", _("Domain is not running"));
> -            goto cleanup;
> -        }
> -    }
> -
> -    if ((flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) && !vm->persistent) {
> -         virReportError(VIR_ERR_OPERATION_INVALID,
> -                        "%s", _("cannot modify device on transient domain"));
> -         goto cleanup;
> -    }
> +    if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
> +        goto endjob;

This fails to compile since endjob doesn't exist in this context.


John
>  
>      if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
>          if (!(dev = virDomainDeviceDefParse(xml, vm->def,
> diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
> index 41639ac..316c60c 100644
> --- a/src/lxc/lxc_driver.c
> +++ b/src/lxc/lxc_driver.c
> @@ -4993,43 +4993,22 @@ static int lxcDomainAttachDeviceFlags(virDomainPtr dom,
>      virDomainDefPtr vmdef = NULL;
>      virDomainDeviceDefPtr dev = NULL, dev_copy = NULL;
>      int ret = -1;
> -    unsigned int affect;
>      virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver);
>  
>      virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
>                    VIR_DOMAIN_AFFECT_CONFIG, -1);
>  
> -    affect = flags & (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG);
> -
>      if (!(vm = lxcDomObjFromDomain(dom)))
>          goto cleanup;
>  
>      if (virDomainAttachDeviceFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
>          goto cleanup;
>  
> -    if (virDomainObjIsActive(vm)) {
> -        if (affect == VIR_DOMAIN_AFFECT_CURRENT)
> -            flags |= VIR_DOMAIN_AFFECT_LIVE;
> -    } else {
> -        if (affect == VIR_DOMAIN_AFFECT_CURRENT)
> -            flags |= VIR_DOMAIN_AFFECT_CONFIG;
> -        /* check consistency between flags and the vm state */
> -        if (flags & VIR_DOMAIN_AFFECT_LIVE) {
> -            virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> -                           _("cannot do live update a device on "
> -                             "inactive domain"));
> -            goto cleanup;
> -        }
> -    }
> -
>      if (!(caps = virLXCDriverGetCapabilities(driver, false)))
>          goto cleanup;
>  
> -    if ((flags & VIR_DOMAIN_AFFECT_CONFIG) && !vm->persistent) {
> -         virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> -                        _("cannot modify device on transient domain"));
> -         goto cleanup;
> -    }
> +    if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
> +        goto cleanup;
>  
>      dev = dev_copy = virDomainDeviceDefParse(xml, vm->def,
>                                               caps, driver->xmlopt,
> @@ -5121,41 +5100,20 @@ static int lxcDomainUpdateDeviceFlags(virDomainPtr dom,
>      virDomainDefPtr vmdef = NULL;
>      virDomainDeviceDefPtr dev = NULL, dev_copy = NULL;
>      int ret = -1;
> -    unsigned int affect;
>      virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver);
>  
>      virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
>                    VIR_DOMAIN_AFFECT_CONFIG |
>                    VIR_DOMAIN_DEVICE_MODIFY_FORCE, -1);
>  
> -    affect = flags & (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG);
> -
>      if (!(vm = lxcDomObjFromDomain(dom)))
>          goto cleanup;
>  
>      if (virDomainUpdateDeviceFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
>          goto cleanup;
>  
> -    if (virDomainObjIsActive(vm)) {
> -        if (affect == VIR_DOMAIN_AFFECT_CURRENT)
> -            flags |= VIR_DOMAIN_AFFECT_LIVE;
> -    } else {
> -        if (affect == VIR_DOMAIN_AFFECT_CURRENT)
> -            flags |= VIR_DOMAIN_AFFECT_CONFIG;
> -        /* check consistency between flags and the vm state */
> -        if (flags & VIR_DOMAIN_AFFECT_LIVE) {
> -            virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> -                           _("cannot do live update a device on "
> -                             "inactive domain"));
> -            goto cleanup;
> -        }
> -    }
> -
> -    if ((flags & VIR_DOMAIN_AFFECT_CONFIG) && !vm->persistent) {
> -         virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> -                        _("cannot modify device on transient domain"));
> -         goto cleanup;
> -    }
> +    if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
> +        goto cleanup;
>  
>      if (!(caps = virLXCDriverGetCapabilities(driver, false)))
>          goto cleanup;
> @@ -5235,40 +5193,19 @@ static int lxcDomainDetachDeviceFlags(virDomainPtr dom,
>      virDomainDefPtr vmdef = NULL;
>      virDomainDeviceDefPtr dev = NULL, dev_copy = NULL;
>      int ret = -1;
> -    unsigned int affect;
>      virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver);
>  
>      virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
>                    VIR_DOMAIN_AFFECT_CONFIG, -1);
>  
> -    affect = flags & (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG);
> -
>      if (!(vm = lxcDomObjFromDomain(dom)))
>          goto cleanup;
>  
>      if (virDomainDetachDeviceFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
>          goto cleanup;
>  
> -    if (virDomainObjIsActive(vm)) {
> -        if (affect == VIR_DOMAIN_AFFECT_CURRENT)
> -            flags |= VIR_DOMAIN_AFFECT_LIVE;
> -    } else {
> -        if (affect == VIR_DOMAIN_AFFECT_CURRENT)
> -            flags |= VIR_DOMAIN_AFFECT_CONFIG;
> -        /* check consistency between flags and the vm state */
> -        if (flags & VIR_DOMAIN_AFFECT_LIVE) {
> -            virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> -                           _("cannot do live update a device on "
> -                             "inactive domain"));
> -            goto cleanup;
> -        }
> -    }
> -
> -    if ((flags & VIR_DOMAIN_AFFECT_CONFIG) && !vm->persistent) {
> -         virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> -                        _("cannot modify device on transient domain"));
> -         goto cleanup;
> -    }
> +    if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
> +        goto cleanup;
>  
>      if (!(caps = virLXCDriverGetCapabilities(driver, false)))
>          goto cleanup;
> 


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