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

Re: [libvirt] [PATCH v3] Fix up basic migration.



Chris Lalancette wrote:
> Basic live migration was broken by the commit that added
> non-shared block support in two ways:
>
> 1)  It added a virCheckFlags() to doNativeMigrate().  Besides
...
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 77e71cc..941b482 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -10175,6 +10175,15 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn,
>      int ret = -1;
>      int internalret;
>
> +    virCheckFlags(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, -1);

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)

...
> +    virCheckFlags(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, -1);
...
> +    virCheckFlags(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, NULL);


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