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

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

 



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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]