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