On 05/20/2011 12:59 PM, Laine Stump wrote: > This patch provides three new public API functions that allow > reverting the network configuration back to a previous "known good" > state. The three new functions are: > > ncf_change_begin() - save a snapshot of current network config > ncf_change_rollback() - revert to the config previously saved > ncf_change_commit() - delete the saved snapshot, making the new > config permanent
I've let this slip a whole week without review...
> +++ b/configure.ac
> @@ -6,7 +6,7 @@ AM_CONFIG_HEADER([config.h])
> AM_INIT_AUTOMAKE([-Wno-portability 1.11 color-tests parallel-tests])
> AM_SILENT_RULES([yes]) # make --enable-silent-rules the default.
>
> -AC_SUBST([LIBNETCF_VERSION_INFO], [4:3:3])
> +AC_SUBST([LIBNETCF_VERSION_INFO], [5:0:4])
Yep, this matches libtool conventions (we are adding an API, so the
first digit must change; this is the first use of the new API so the
second digit starts at 0, and we are still backwards compatible to all
code compiled against version [5-4==] 1 or later.
>
> +/* Functions to take a snapshot of network config (change_begin), and
> + * later either revert to that config (change_rollback), or make the
> + * new config permanent (change_commit).
> + */
> +int
> +drv_change_begin(struct netcf *ncf ATTRIBUTE_UNUSED,
ncf is used...
> + unsigned int flags ATTRIBUTE_UNUSED)
> +{
> + int result = -1;
> +
> + run1(ncf, NETCF_TRANSACTION, "change-begin");
unless run1 is a macro that might be compiled out. Also, to make future
changes easier, you MUST validate that flags == 0 (and error out if
non-zero). Otherwise, if a newer netcf assigns a meaning to flags, and
a client passes a non-zero flag, but you don't honor that assigned
meaning, you are breaking the client.
> +int
> +drv_change_rollback(struct netcf *ncf ATTRIBUTE_UNUSED,
> + unsigned int flags ATTRIBUTE_UNUSED)
> +{
Same comment on flags.
> +int
> +drv_change_commit(struct netcf *ncf ATTRIBUTE_UNUSED,
> + unsigned int flags ATTRIBUTE_UNUSED)
> +{
> + int result = -1;
And again. I sense a pattern here :)
> +
> +static const struct command_def cmd_change_begin_def = {
> + .name = "change-begin",
> + .opts = cmd_change_begin_opts,
> + .handler = cmd_change_begin,
> + .synopsis = "mark the beginning of a set of revertable network config
> changes",
s/revertable/revertible/ throughout
> +++ b/src/netcf.h
> @@ -169,6 +169,28 @@ char *ncf_if_xml_state(struct netcf_if *);
> */
> int ncf_if_status(struct netcf_if *nif, unsigned int *flags);
>
> +/* Mark the beginning of a sequence of revertable changes to the
> + * network interface config by saving a snapshot of all relevant
> + * config information.
For public documentation, I prefer s/config/configuration/ - config is
an abbreviation great for coding, but a bit hackish for docs.
--
Eric Blake [email protected] +1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
_______________________________________________ netcf-devel mailing list [email protected] https://fedorahosted.org/mailman/listinfo/netcf-devel
