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

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



2010/5/25 Chris Lalancette <clalance redhat com>:
> On 05/24/2010 04:34 PM, Eric Blake wrote:
>> On 05/24/2010 02:20 PM, Chris Lalancette wrote:
>>>> 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.
>>
>> Hmm, since virCheckFlags() is already a macro in the first place, can we
>> use some sizeof(flags) magic to get it to polymorphically do the right
>> thing rather than having to invent an alternate spelling?
>
> We discussed this on IRC.  Unfortunately I don't think you can use sizeof()
> directly, since on 32-bit sizeof(unsigned int) == sizeof(unsigned long).  However,
> what I've done instead is to unconditionally promote flags to an unsigned long
> and then use %lx.  That seems to work.  I'll post v3 of the patch in a moment.
>

Well, the actual question is why does qemudDomainMigrate take an
unsigned long? If it's because it should be able to take more then 32
flags then unsigned long is the wrong type obviously, because unsigned
long is just 32 bit width on 32 bit platforms. For that case
qemudDomainMigrate has to have an unsigned long long flags parameter.
If qemudDomainMigrate doesn't need to support more than 32 flags then
lets just change unsigned long to unsigned int there.

If we want to extend virCheckFlags to cover up to 64 flags properly we
have to use unsigned long long (64 bit width on 32 and 64 bit
platforms) instead of unsigned long.

Unfortunately the public API member virDomainMigrate takes a unsigned
long flags parameter, but effectively limited to a 32 flags maximum,
and we are stuck with that.

Matthias


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