[libvirt] [PATCH 5/14] Implementation of the public API

[Stefan Berger]

Eric Blake <eblake at redhat.com> wrote on 03/18/2010 03:25:25 PM:

> On 03/18/2010 09:16 AM, Stefan Berger wrote:
> > This patch adds the implementation of the public API for the network
> > filtering (ACL) extensions to libvirt.c .
> > 
> > Signed-off-by: Stefan Berger <stefanb at us.ibm.com>
> 
> Some nits (again, leaving the content review to those more knowledgeable
> about API additions):
> 
> > +virRegisterNWFilterDriver(virNWFilterDriverPtr driver)
> > +{
> > +    if (virInitialize() < 0)
> > +      return -1;
> > +
> > +    if (driver == NULL) {
> > +        virLibConnError(NULL, VIR_ERR_INVALID_ARG, __FUNCTION__);
> > +        return(-1);
> 
> Why the two different styles of returning -1?  A quick grep shows that
> the former style (return -1) is used nearly 9x more frequently than the
> latter (return(-1)).

Parts have been recycled from the storage driver and that's likely where 
that comes from.

> 
> > +        DEBUG("nwfilter driver %d %s returned %s",
> > +              i, virNWFilterDriverTab[i]->name,
> > +              res == VIR_DRV_OPEN_SUCCESS ? "SUCCESS" :
> > +              (res == VIR_DRV_OPEN_DECLINED ? "DECLINED" :
> > +               (res == VIR_DRV_OPEN_ERROR ? "ERROR" : "unknown 
status")));
> > +        if (res == VIR_DRV_OPEN_ERROR) {
> > +            if (STREQ(virNWFilterDriverTab[i]->name, "remote")) {
> > +                virLibConnWarning (NULL, VIR_WAR_NO_NWFILTER,
> > +                                   "Is the daemon running ?");
> 
> Do DEBUG messages need to be marked for translation?  Even if not, the
> virLibConnWarning probably should be.

Correct. I will fix the error message.

 Thanks and regards,
   Stefan



> 
> -- 
> Eric Blake   eblake at redhat.com    +1-801-349-2682
> Libvirt virtualization library http://libvirt.org
> 
> [attachment "signature.asc" deleted by Stefan Berger/Watson/IBM] 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20100318/89ce9805/attachment-0001.htm>