Re: [PATCH 1/6] Introduce storage lifecycle event APIs

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

 



On 06/10/2016 09:50 AM, Ján Tomko wrote:
> On Fri, Jun 10, 2016 at 08:30:33AM -0400, Cole Robinson wrote:
>> On 06/10/2016 07:05 AM, Ján Tomko wrote:
>>> On Thu, Jun 09, 2016 at 05:43:05PM -0400, Cole Robinson wrote:
>>>> On 06/09/2016 02:25 PM, Jovanka Gulicoska wrote:
>>>>> +        virReportInvalidArg(eventID,
>>>>> +                            _("eventID in %s must be less than %d"),
>>>>> +                            __FUNCTION__, VIR_STORAGE_POOL_EVENT_ID_LAST);
>>>>> +        goto error;
>>>>> +    }
>>>>> +
>>>>> +    if (conn->storageDriver &&
>>>>> conn->storageDriver->connectStoragePoolEventRegisterAny) {
>>>>
>>>> Long line, split it after the &&
>>>>
>>>>> +        int ret;
>>>>> +        ret =
>>>>> conn->storageDriver->connectStoragePoolEventRegisterAny(conn, pool,
>>>>> +                                                                     
>>>>> eventID,
>>>>> +                                                                     
>>>>> cb, opaque,
>>>>> +                                                                     
>>>>> freecb);
>>>>
>>>> If you put pool and opaque on their own lines this breaks some over 80 column
>>>> lines, but I don't know if that's better or worse
>>>>
>>>
>>> The 'Any' suffix can also be dropped.
>>> There is no legacy virConnectStoragePoolEventRegister API so we don't
>>> need to add suffixes like we had to for virConnectDomainEventRegister.
>>>
>>
>> You mean the internal driver name 'connectStoragePoolEventRegisterAny', not
>> the public API?. If so that sounds fine to me, but I'm not sure how much
>> precedent there is for having driver function names differ from the public API
>> names
> 
> I meant both the public API and the internal driver name.
> 

I disagree, that will make the public API inconsistent across object types. We
would end up with basically

DomainEventRegister(conn, cb)
DomainEventRegisterAny(conn, obj, eventID, cb)
NetworkEventRegisterAny(conn, obj, eventID, cb)
StoragePoolEventRegister(conn, obj, eventID, cb)

So not only would the preferred API not have consistent naming, the similarly
named APIs would have different signatures. I don't think that's worth it to
to get slightly nicer named APIs for storage events

- Cole

--
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]