Eric Blake <eblake@xxxxxxxxxx> 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@xxxxxxxxxx>
>
> 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@xxxxxxxxxx +1-801-349-2682
> Libvirt virtualization library http://libvirt.org
>
> [attachment "signature.asc" deleted by Stefan Berger/Watson/IBM]
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list