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