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

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


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