On 05/10/2011 05:38 AM, Daniel P. Berrange wrote:
On Mon, May 09, 2011 at 09:28:49PM +0200, Michal Privoznik wrote:
> ---
> src/libvirt.c | 129 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 129 insertions(+), 0 deletions(-)
>
> diff --git a/src/libvirt.c b/src/libvirt.c
> index abacf85..d9b659d 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -7481,6 +7481,135 @@ virInterfaceFree(virInterfacePtr iface)
> return 0;
> }
>
> +/**
> + * virInterfaceChangeStart:
> + * @conn: pointer to hypervisor connection
> + * @flags: flags, not used yet
> + *
> + * This functions creates a restore point to which one can return
> + * later by calling virInterfaceChangeRollback. This function should
> + * be called before any transaction with interface configuration.
> + * Once knowing, new configuration works, it can be commited via
> + * virInterfaceChangeCommit, which frees restore point.
> + *
> + * Returns 0 in case of success and -1 in case of failure.
> + */
> +int
> +virInterfaceChangeStart(virConnectPtr conn, unsigned int flags)
> +{
> + int ret = -1;
> +
> + VIR_DEBUG("conn=%p, flags=%d", conn, flags);
> +
> + virResetLastError();
> +
> + if (!VIR_IS_CONNECT(conn)) {
> + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virDispatchError(NULL);
> + goto end;
> + }
> +
> + if (conn->flags& VIR_CONNECT_RO) {
> + virLibInterfaceError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
> + virDispatchError(conn);
> + goto end;
> + }
> +
> + if (conn->interfaceDriver&&
conn->interfaceDriver->interfaceChangeStart) {
> + ret = conn->interfaceDriver->interfaceChangeStart(conn, flags);
> + } else {
> + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
> + }
> +
> +end:
> + VIR_DEBUG("returning: %d", ret);
> + return ret;
> +}
> +
> +/**
> + * virInterfaceChangeCommit:
> + * @conn: pointer to hypervisor connection
> + * @flags: flags, not used yet
> + *
> + * This commits the changes made to interfaces and frees restore point
> + * created by virInterfaceChangeStart.
> + *
> + * Returns 0 in case of success and -1 in case of failure.
> + */
> +int
> +virInterfaceChangeCommit(virConnectPtr conn, unsigned int flags)
> +{
> + int ret = -1;
> +
> + VIR_DEBUG("conn=%p, flags=%d", conn, flags);
> +
> + virResetLastError();
> +
> + if (!VIR_IS_CONNECT(conn)) {
> + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virDispatchError(NULL);
> + goto end;
> + }
> +
> + if (conn->flags& VIR_CONNECT_RO) {
> + virLibInterfaceError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
> + virDispatchError(conn);
> + goto end;
> + }
> +
> + if (conn->interfaceDriver&&
conn->interfaceDriver->interfaceChangeCommit) {
> + ret = conn->interfaceDriver->interfaceChangeCommit(conn, flags);
> + } else {
> + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
> + }
> +
> +end:
> + VIR_DEBUG("returning: %d", ret);
> + return ret;
> +}
> +
> +/**
> + * virInterfaceChangeRollback:
> + * @conn: pointer to hypervisor connection
> + * @flags: flags, not used yet
> + *
> + * This cancels changes made to interfaces settings by restoring previous
> + * state created by virInterfaceChangeStart.
> + *
> + * Returns 0 in case of success and -1 in case of failure.
> + */
> +int
> +virInterfaceChangeRollback(virConnectPtr conn, unsigned int flags)
> +{
> + int ret = -1;
> +
> + VIR_DEBUG("conn=%p, flags=%d", conn, flags);
> +
> + virResetLastError();
> +
> + if (!VIR_IS_CONNECT(conn)) {
> + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virDispatchError(NULL);
> + goto end;
> + }
> +
> + if (conn->flags& VIR_CONNECT_RO) {
> + virLibInterfaceError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
> + virDispatchError(conn);
> + goto end;
> + }
> +
> + if (conn->interfaceDriver&&
> + conn->interfaceDriver->interfaceChangeRollback) {
> + ret = conn->interfaceDriver->interfaceChangeRollback(conn, flags);
> + } else {
> + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
> + }
> +
> +end:
> + VIR_DEBUG("returning: %d", ret);
> + return ret;
> +}
I think we also need to add comments to
virInterfaceDefineXML
virInterfaceUndefine
virInterfaceCreate
virInterfaceDestroy
about the difference in their behaviour if a transaction is open, vs
normal non-transactional usage.
They will behave the same at the moment they are called, but it will be
possible to later undo what was done, because "opening the transaction"
is really just saving off the original config state.
More importantly (and probably this is the part you were talking about)
if virInterfaceChangeCommit() isn't called before the next time the
system is rebooted, all the changes will be automatically reverted
during the boot process.
(I want to make sure that nobody thinks opening a transaction means that
all the changes are saved into a staging area and then later committed
to the live config all at once when the transaction is committed - doing
it this way would eliminate the ability to test the new config prior to
committing).