[libvirt] [PATCH v3 5/8] qemu: snapshots: Simplify REDEFINE flag check

[Michal Privoznik]

On 25.09.2013 21:15, Cole Robinson wrote:
> Makes things more readable IMO
> ---
>  src/qemu/qemu_driver.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 346a8f9..0dc975b 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -12273,6 +12273,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
>      char uuidstr[VIR_UUID_STRING_BUFLEN];
>      virDomainSnapshotDefPtr def = NULL;
>      bool update_current = true;
> +    bool redefine = flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE;
>      unsigned int parse_flags = VIR_DOMAIN_SNAPSHOT_PARSE_DISKS;
>      virDomainSnapshotObjPtr other = NULL;
>      int align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL;
> @@ -12297,11 +12298,10 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
>          return NULL;
>      }
>  
> -    if (((flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) &&
> -         !(flags & VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT)) ||
> +    if ((redefine && !(flags & VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT)) ||
>          (flags & VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA))
>          update_current = false;
> -    if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE)
> +    if (redefine)
>          parse_flags |= VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE;
>  
>      virUUIDFormat(domain->uuid, uuidstr);
> @@ -12366,14 +12366,14 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
>      if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_LIVE &&
>          (!virDomainObjIsActive(vm) ||
>           def->memory != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL ||
> -         flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE)) {
> +         redefine)) {
>          virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
>                         _("live snapshot creation is supported only "
>                           "with external checkpoints"));
>          goto cleanup;
>      }
>  
> -    if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) {
> +    if (redefine) {
>          /* Prevent circular chains */
>          if (def->parent) {
>              if (STREQ(def->name, def->parent)) {
> @@ -12544,7 +12544,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
>      if (update_current)
>          snap->def->current = true;
>      if (vm->current_snapshot) {
> -        if (!(flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) &&
> +        if (!redefine &&
>              VIR_STRDUP(snap->def->parent, vm->current_snapshot->def->name) < 0)
>                  goto cleanup;
>          if (update_current) {
> @@ -12557,7 +12557,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
>      }
>  
>      /* actually do the snapshot */
> -    if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) {
> +    if (redefine) {
>          /* XXX Should we validate that the redefined snapshot even
>           * makes sense, such as checking that qemu-img recognizes the
>           * snapshot name in at least one of the domain's disks?  */
> 

ACK. While this is safe for 1.1.3 I'd rather wait and push it after the
freeze. You know, the freeze is supposed to be for bug fixing and not
code refactoring.

Moreover, if you'd move this somewhere at the beginning of you set, the
test driver can use the more readable code too.

Michal

(This is the last patch I've time to review today, I'll continue tomorrow)