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