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

Re: [libvirt] [PATCH v2 1/2] make qemu dump memory in kdump-compressed format



On 02/26/2014 01:08 AM, Qiao Nuohan wrote:

>>>    int                     virDomainCoreDump       (virDomainPtr domain,
>>>                                                     const char *to,
>>> -                                                 unsigned int flags);
>>> +                                                 unsigned int flags,
>>> +                                                 const unsigned int
>>> memory_dump_format);
>>
>> This is not allowed and I believe "make syntax-check" should fail on the
>> corresponding change to remote_protocol.x. If you want to add a new

I don't know if syntax-check would catch it, but the fact that you would
have had to modify src/remote_protocol-structs is indeed a red flag that
reviewers will catch.

>> parameter to an existing API, you have keep the API as is and instead
>> create a new API with a different name that will have all the parameters
>> you need.
> 
> Hello Jirka,
> 
> Thanks for pointing it out for me, I finally found it's libvirt's policy
> never
> to change a committed public API. But driver handling methods are
> allowed to be
> changed, am I right?

Changing driver handling methods is fine.  That said, how are you going
to provide the extra parameter to the driver handler without also having
a public API?  We have a script that validates that all the driver
function names are close derivatives of the public API names, because
that's the only way to programmatically enforce that we have proper ACL
checks in all APIs.  So really, the easiest would be adding a new API
and new driver methods, with a better signature.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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