<br><tt><font size=2>Eric Blake <eblake@redhat.com> wrote on 03/18/2010
03:25:25 PM:<br>
<br>
> On 03/18/2010 09:16 AM, Stefan Berger wrote:<br>
> > This patch adds the implementation of the public API for the
network<br>
> > filtering (ACL) extensions to libvirt.c .<br>
> > <br>
> > Signed-off-by: Stefan Berger <stefanb@us.ibm.com><br>
> <br>
> Some nits (again, leaving the content review to those more knowledgeable<br>
> about API additions):<br>
> <br>
> > +virRegisterNWFilterDriver(virNWFilterDriverPtr driver)<br>
> > +{<br>
> > +    if (virInitialize() < 0)<br>
> > +      return -1;<br>
> > +<br>
> > +    if (driver == NULL) {<br>
> > +        virLibConnError(NULL, VIR_ERR_INVALID_ARG,
__FUNCTION__);<br>
> > +        return(-1);<br>
> <br>
> Why the two different styles of returning -1?  A quick grep shows
that<br>
> the former style (return -1) is used nearly 9x more frequently than
the<br>
> latter (return(-1)).</font></tt>
<br>
<br><tt><font size=2>Parts have been recycled from the storage driver and
that's likely where that comes from.</font></tt>
<br><tt><font size=2><br>
> <br>
> > +        DEBUG("nwfilter driver %d %s
returned %s",<br>
> > +              i, virNWFilterDriverTab[i]->name,<br>
> > +              res == VIR_DRV_OPEN_SUCCESS
? "SUCCESS" :<br>
> > +              (res == VIR_DRV_OPEN_DECLINED
? "DECLINED" :<br>
> > +               (res == VIR_DRV_OPEN_ERROR
? "ERROR" : "unknown status")));<br>
> > +        if (res == VIR_DRV_OPEN_ERROR) {<br>
> > +            if (STREQ(virNWFilterDriverTab[i]->name,
"remote")) {<br>
> > +                virLibConnWarning
(NULL, VIR_WAR_NO_NWFILTER,<br>
> > +                  
                "Is the daemon
running ?");<br>
> <br>
> Do DEBUG messages need to be marked for translation?  Even if
not, the<br>
> virLibConnWarning probably should be.</font></tt>
<br>
<br><tt><font size=2>Correct. I will fix the error message.</font></tt>
<br>
<br><tt><font size=2> Thanks and regards,</font></tt>
<br><tt><font size=2>   Stefan</font></tt>
<br>
<br>
<br><tt><font size=2><br>
> <br>
> -- <br>
> Eric Blake   eblake@redhat.com    +1-801-349-2682<br>
> Libvirt virtualization library </font></tt><a href=http://libvirt.org/><tt><font size=2>http://libvirt.org</font></tt></a><tt><font size=2><br>
> <br>
> [attachment "signature.asc" deleted by Stefan Berger/Watson/IBM]
</font></tt>