Re: [libvirt] [PATCHv2 6/7] interface: Implement driver methods for network config transaction API

[Laine Stump <laine laine org>]

On 05/26/2011 04:35 AM, Daniel P. Berrange wrote:
> On Thu, May 19, 2011 at 04:51:28PM -0400, Laine Stump wrote:
> > From: Michal Privoznik<mprivozn redhat com>
> >
> > This is the functionality at the end of the libvirt part of the call
> > chain - for each function, the corresponding netcf API is called.
> > ---
> >   configure.ac                 |    5 +++
> >   src/interface/netcf_driver.c |   76 ++++++++++++++++++++++++++++++++++++++++++
> >   2 files changed, 81 insertions(+), 0 deletions(-)
> >
> > diff --git a/configure.ac b/configure.ac
> > index 8db3226..dbec54c 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -1491,6 +1491,11 @@ if test "$with_netcf" = "yes" || test "$with_netcf" = "check"; then
> >     if test "$with_netcf" = "yes" ; then
> >       AC_DEFINE_UNQUOTED([WITH_NETCF], 1,
> >         [whether libnetcf is available to configure physical host network interfaces])
> > +    AC_CHECK_LIB([netcf], [ncf_change_begin], [netcf_transactions=1], [netcf_transactions=0])
> > +    if test "$netcf_transactions" = "1" ; then
> > +        AC_DEFINE_UNQUOTED([HAVE_NETCF_TRANSACTIONS], ["1"],
> > +          [we have sufficiently new version of netcf for transaction network API])
> > +    fi
> >     fi
> >   fi
> >   AM_CONDITIONAL([WITH_NETCF], [test "$with_netcf" = "yes"])
> > diff --git a/src/interface/netcf_driver.c b/src/interface/netcf_driver.c
> > index d5b401a..c94cbd5 100644
> > --- a/src/interface/netcf_driver.c
> > +++ b/src/interface/netcf_driver.c
> > @@ -540,6 +540,77 @@ cleanup:
> >       return ret;
> >   }
> >
> > +#ifdef HAVE_NETCF_TRANSACTIONS
> > +static int interfaceChangeBegin(virConnectPtr conn ATTRIBUTE_UNUSED,
> > +                                unsigned int flags ATTRIBUTE_UNUSED)
> Both conn and flags *are* used in this method, and the 2 following
> methods.

Oops. yeah, fixed.

> +
> > +    interfaceDriverLock(driver);
> > +
> > +    ret = ncf_change_begin(driver->netcf, 0);
> > +    if (ret<  0) {
> > +        const char *errmsg, *details;
> > +        int errcode = ncf_error(driver->netcf,&errmsg,&details);
> > +        interfaceReportError(netcf_to_vir_err(errcode),
> > +                             _("failed to begin transaction (netcf: %s - %s)"),
> > +                            errmsg, details ? details : "");
> I don't think including the word 'netcf' in the error message here
> is relevant. If 'details' is "", then this also adds a trailing ' - ' to
> the error string. eg it needs to be
>
>         interfaceReportError(netcf_to_vir_err(errcode),
>                              _("failed to begin transaction: %s%s%s)"),
>                              errmsg, details ? " - " : "", details ? details : "");
>
>
> Likewise in the other methods

Okay, I've changed that in these three new methods, and will submit a 
followup patch to change it in the existing methods as well..