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

Re: [libvirt] [PATCH 1/5] qemu: Rename DEFAULT_JOB_MASK to QEMU_DEFAULT_JOB_MASK



On 09/03/2014 07:53 AM, Peter Krempa wrote:
> Be consistend with naming of private defines. Also line up code

s/consistend/consistent/

> correctly in few places where the macro is used.
> ---
>  src/qemu/qemu_domain.c    | 2 +-
>  src/qemu/qemu_domain.h    | 2 +-
>  src/qemu/qemu_driver.c    | 8 ++++----
>  src/qemu/qemu_migration.c | 6 +++---
>  4 files changed, 9 insertions(+), 9 deletions(-)
> 
> 
>  # define JOB_MASK(job)                  (1 << (job - 1))
> -# define DEFAULT_JOB_MASK               \
> +# define QEMU_DEFAULT_JOB_MASK          \
>      (JOB_MASK(QEMU_JOB_QUERY) |         \
>       JOB_MASK(QEMU_JOB_DESTROY) |       \
>       JOB_MASK(QEMU_JOB_ABORT))

I think your naming choice is okay, but it might look slightly better as
QEMU_JOB_DEFAULT_MASK, so that it shares the same prefix.

> +++ b/src/qemu/qemu_migration.c
> @@ -4907,9 +4907,9 @@ qemuMigrationJobStart(virQEMUDriverPtr driver,
>      if (job == QEMU_ASYNC_JOB_MIGRATION_IN) {
>          qemuDomainObjSetAsyncJobMask(vm, QEMU_JOB_NONE);
>      } else {
> -        qemuDomainObjSetAsyncJobMask(vm, DEFAULT_JOB_MASK |
> -                                     JOB_MASK(QEMU_JOB_SUSPEND) |
> -                                     JOB_MASK(QEMU_JOB_MIGRATION_OP));
> +        qemuDomainObjSetAsyncJobMask(vm, QEMU_DEFAULT_JOB_MASK |
> +                                         JOB_MASK(QEMU_JOB_SUSPEND) |
> +                                         JOB_MASK(QEMU_JOB_MIGRATION_OP));
>      }

At least in emacs, I find that lining up a split second argument by
using TAB to trigger automatic indentation is easier when done with
extra (), as in:

    qemuDomainObjSetAsyncJobMask(vm, (QEMU_DEFAULT_JOB_MASK |
                                      JOB_MASK(QEMU_JOB_SUSPEND) |
                                      JOB_MASK(QEMU_JOB_MIGRATION_OP)));

But that's purely cosmetic.

Whether or not you make those changes, ACK.

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