Re: [libvirt] [PATCH v3] Fix up basic migration.
On 05/25/2010 11:08 AM, Jim Meyering wrote:
Hi Chris,
This looks like a fine change, but I haven't delved into
it enough to give a formal ACK. However, one quick comment:
It was not at all obvious to me that those three lists of eight
VIR_* macros or'd together were identical. I used the editor to
confirm it. Considering that someone might be tempted to modify
one but miss the other two -- or add a 4th use, would it make sense
to define a new symbol, and then use that in those three calls?
#define VIR_MIGRATE_SOMETHING \
(VIR_MIGRATE_LIVE | \
VIR_MIGRATE_PEER2PEER | \
VIR_MIGRATE_TUNNELLED | \
VIR_MIGRATE_PERSIST_DEST | \
VIR_MIGRATE_UNDEFINE_SOURCE | \
VIR_MIGRATE_PAUSED | \
VIR_MIGRATE_NON_SHARED_DISK | \
VIR_MIGRATE_NON_SHARED_INC)
It's not a terrible idea. The good news is that from a testing
perspective, if anyone were to add a VIR_MIGRATE_FOO flag,
and put it into MigratePrepare and not MigratePerform, their
testing would fail and they would have to investigate. On the
other hand, it's much better to let them do things correctly
from the get-go. I'll look at making a more generic macro for this.
--
Chris Lalancette