[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv2 3/7] interface: implement public APIs for libvirt transactional network changes



On Thu, May 19, 2011 at 04:51:25PM -0400, Laine Stump wrote:
> From: Michal Privoznik <mprivozn redhat com>
> 
> ---
>  src/libvirt.c |  141 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 139 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libvirt.c b/src/libvirt.c
> index ff16c48..786ce15 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -8143,7 +8143,9 @@ error:
>   * @xml: the XML description for the interface, preferably in UTF-8
>   * @flags: and OR'ed set of extraction flags, not used yet
>   *
> - * Define an interface (or modify existing interface configuration)
> + * Define an interface (or modify existing interface configuration).
> + * This can, however be affected by virInterfaceChangeBegin
> + * and/or friends.
>   *
>   * Returns NULL in case of error, a pointer to the interface otherwise
>   */
> @@ -8189,6 +8191,8 @@ error:
>   *
>   * Undefine an interface, ie remove it from the config.
>   * This does not free the associated virInterfacePtr object.
> + * This can, however be affected by virInterfaceChangeBegin
> + * and/or friends.
>   *
>   * Returns 0 in case of success, -1 in case of error
>   */
> @@ -8230,7 +8234,9 @@ error:
>   * @iface: pointer to a defined interface
>   * @flags: and OR'ed set of extraction flags, not used yet
>   *
> - * Activate an interface (ie call "ifup")
> + * Activate an interface (ie call "ifup").
> + * This can, however be affected by virInterfaceChangeBegin
> + * and/or friends.
>   *
>   * Returns 0 in case of success, -1 in case of error
>   */
> @@ -8276,6 +8282,8 @@ error:
>   * deactivate an interface (ie call "ifdown")
>   * This does not remove the interface from the config, and
>   * does not free the associated virInterfacePtr object.
> + * This can, however be affected by virInterfaceChangeBegin
> + * and/or friends.
>   *
>   * Returns 0 in case of success and -1 in case of failure.
>   */
> @@ -8374,6 +8382,135 @@ virInterfaceFree(virInterfacePtr iface)
>      return 0;
>  }
>  
> +/**
> + * virInterfaceChangeBegin:
> + * @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
> +virInterfaceChangeBegin(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->interfaceChangeBegin) {
> +        ret = conn->interfaceDriver->interfaceChangeBegin(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 virInterfaceChangeBegin.
> + *
> + * 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 virInterfaceChangeBegin.
> + *
> + * 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;
> +}

The code pattern / flow control in these 3 APIs is completely different
from all the other public APIs, for no obvious benefit. It needs to be
updated to follow the same code pattern as the existing APIs impls.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]