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
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list