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

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



On 05/21/2010 12:48 PM, Eric Blake wrote:
> On 05/21/2010 07:22 AM, Chris Lalancette wrote:
>>>> -    virCheckFlags(VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC,
>>>> -                  -1);
>>>> -
>>>
>>> Why are you removing the valid flag check altogether?  Is it a matter of
>>> adding more valid flags, or is this just a forwarding routine to other
>>> methods that also do a valid flag check?
>>
>> Yeah, I guess I should have been a bit more explicit here.  There's no hard-and-fast
>> rules about virCheckFlags(), but the general usage of it seems to be directly at
>> the entry points to the driver (i.e. in qemuDomainSuspend(), etc).  doNativeMigrate()
>> is not an entry point, it is a helper function.  In addition, there is a long list
>> of flags that migration supports, so the above list is far from complete.  Would it
>> be better if I added the check to qemuDomainMigratePerform() (with the complete
>> list of flags), which is actually the entry point for this function?
> 
> Sounds good to me - if all entry points filter on all accepted flags,
> then helper functions can assume that flags are already valid.  As long
> as the filtering gets done somewhere, we've left the door open for
> adding future flags while still correctly identifying situations where
> we are talking to older implementations that can't honor new flags.
> It's only when there is no flag filtering at all that we've locked
> ourselves out of easy-to-validate future extensions.

Unfortunately doing this caused a bit of churn in the qemu driver.  qemudDomainMigrate
takes an unsigned long as flags instead of unsigned int, which required me to create
two new macros: virCheckFlagsUI and virCheckFlagsUL.  The good news is that with this
patch in place we are doing more checking of the flags during migration, which is
probably a good thing.  A new patch is coming up.

-- 
Chris Lalancette


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