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

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

 



On Fri, Jun 10, 2016 at 09:57:13AM -0400, Cole Robinson wrote:
> 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

Yep, I'm with Cole on this - having consistent naming for the different
types of objects is better IMHO, so we should *keep* the "Any" suffix on
these new methods

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

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