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

On Thu, May 19, 2011 at 04:51:28PM -0400, Laine Stump wrote: > From: Michal Privoznik<mprivozn(a)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.

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