On Thu, May 26, 2011 at 12:17:39PM -0400, Laine Stump wrote:

From: Michal Privoznik <mprivozn@redhat.com>

No change from previously ACKed patch.

This is the API agreed on in:

https://www.redhat.com/archives/libvir-list/2011-May/msg00026.html

(with a slight name change to use "...begin" rather than "...start"). This implements transactional changes to the host network config. When a transaction is begun with ncf_change_begin(), all other netcf APIs will continue to work as they always have, but a snapshot of the existing config will be taken, allowing reversion (rollback, using ncf_change_rollback()) to the exact state of config at the time ncf_change_begin() was called. Alternately, if it's determined that the new changes are acceptable, ncf_change_commit() can be called, which will eliminate the snapshot and make the changes permanent.

As a failsafe measure, if neither ncf_change_commit() or ncf_change_rollback() is called by the next time the system reboots, the netcf-transaction initscript will be automatically called to rollback the changes. --- include/libvirt/libvirt.h.in | 7 +++++++ src/libvirt_public.syms | 3 +++ 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 7cd6e13..4a45390 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1405,6 +1405,13 @@ int virInterfaceDestroy (virInterfacePtr iface, int virInterfaceRef (virInterfacePtr iface); int virInterfaceFree (virInterfacePtr iface);

+int virInterfaceChangeBegin (virConnectPtr conn, + unsigned int flags); +int virInterfaceChangeCommit (virConnectPtr conn, + unsigned int flags); +int virInterfaceChangeRollback(virConnectPtr conn, + unsigned int flags); + /** * virStoragePool: * diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 0590535..f4f8262 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -442,6 +442,9 @@ LIBVIRT_0.9.2 { virDomainInjectNMI; virDomainScreenshot; virDomainSetSchedulerParametersFlags; + virInterfaceChangeBegin; + virInterfaceChangeCommit; + virInterfaceChangeRollback; } LIBVIRT_0.9.0;

# .... define new API here using predicted next version number ....

ACK 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 :|

On Thu, May 26, 2011 at 12:17:40PM -0400, Laine Stump wrote:

From: Michal Privoznik <mprivozn@redhat.com>

No change from previous version of ACKed patch. --- src/driver.h | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/src/driver.h b/src/driver.h index 450dd53..c9747aa 100644 --- a/src/driver.h +++ b/src/driver.h @@ -865,6 +865,15 @@ typedef int typedef int (*virDrvInterfaceIsActive)(virInterfacePtr iface);

+typedef int + (*virDrvInterfaceChangeBegin) (virConnectPtr conn, + unsigned int flags); +typedef int + (*virDrvInterfaceChangeCommit) (virConnectPtr conn, + unsigned int flags); +typedef int + (*virDrvInterfaceChangeRollback)(virConnectPtr conn, + unsigned int flags);

typedef struct _virInterfaceDriver virInterfaceDriver; typedef virInterfaceDriver *virInterfaceDriverPtr; @@ -895,6 +904,9 @@ struct _virInterfaceDriver { virDrvInterfaceCreate interfaceCreate; virDrvInterfaceDestroy interfaceDestroy; virDrvInterfaceIsActive interfaceIsActive; + virDrvInterfaceChangeBegin interfaceChangeBegin; + virDrvInterfaceChangeCommit interfaceChangeCommit; + virDrvInterfaceChangeRollback interfaceChangeRollback; };

ACK 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 :|

On Thu, May 26, 2011 at 12:17:41PM -0400, Laine Stump wrote:

From: Michal Privoznik <mprivozn@redhat.com>

change from previous version: modified to be the same code style as other libvirt public API functions. --- src/libvirt.c | 182 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 180 insertions(+), 2 deletions(-)

diff --git a/src/libvirt.c b/src/libvirt.c index ff16c48..f850665 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -8143,7 +8143,18 @@ 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). + * + * Normally this change in the interface configuration is immediately + * permanent/persistent, but if virInterfaceChangeBegin() has been + * previously called (i.e. if an interface config transaction is + * open), the new interface definition will only become permanent if + * virInterfaceChangeCommit() is called prior to the next reboot of + * the system running libvirtd. Prior to that time, it can be + * explicitly removed using virInterfaceChangeRollback(), or will be + * automatically removed during the next reboot of the system running + * libvirtd. * * Returns NULL in case of error, a pointer to the interface otherwise */ @@ -8190,6 +8201,16 @@ error: * Undefine an interface, ie remove it from the config. * This does not free the associated virInterfacePtr object. * + * Normally this change in the interface configuration is + * permanent/persistent, but if virInterfaceChangeBegin() has been + * previously called (i.e. if an interface config transaction is + * open), the removal of the interface definition will only become + * permanent if virInterfaceChangeCommit() is called prior to the next + * reboot of the system running libvirtd. Prior to that time, the + * definition can be explicitly restored using + * virInterfaceChangeRollback(), or will be automatically restored + * during the next reboot of the system running libvirtd. + * * Returns 0 in case of success, -1 in case of error */ int @@ -8230,8 +8251,13 @@ 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 (i.e. call "ifup"). * + * If there was an open network config transaction at the time this + * interface was defined (that is, if virInterfaceChangeBegin() had + * been called), the interface will be brought back down (and then + * undefined) if virInterfaceChangeRollback() is called. +p * * Returns 0 in case of success, -1 in case of error */ int @@ -8277,6 +8303,13 @@ error: * This does not remove the interface from the config, and * does not free the associated virInterfacePtr object. * + + * If there is an open network config transaction at the time this + * interface is destroyed (that is, if virInterfaceChangeBegin() had + * been called), and if the interface is later undefined and then + * virInterfaceChangeRollback() is called, the restoral of the + * interface definition will also bring the interface back up. + * * Returns 0 in case of success and -1 in case of failure. */ int @@ -8374,6 +8407,151 @@ 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 the restore point. + * + * If virInterfaceChangeBegin() is called when a transaction is + * already opened, this function will fail, and a + * VIR_ERR_INVALID_OPERATION will be logged. + * + * Returns 0 in case of success and -1 in case of failure. + */ +int +virInterfaceChangeBegin(virConnectPtr conn, unsigned int flags) +{ + VIR_DEBUG("conn=%p, flags=%d", conn, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (conn->flags & VIR_CONNECT_RO) { + virLibInterfaceError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + if (conn->interfaceDriver && conn->interfaceDriver->interfaceChangeBegin) { + int ret; + ret = conn->interfaceDriver->interfaceChangeBegin(conn, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(conn); + return -1; +} + +/** + * virInterfaceChangeCommit: + * @conn: pointer to hypervisor connection + * @flags: flags, not used yet + * + * This commits the changes made to interfaces and frees the restore point + * created by virInterfaceChangeBegin(). + * + * If virInterfaceChangeCommit() is called when a transaction is not + * opened, this function will fail, and a VIR_ERR_INVALID_OPERATION + * will be logged. + * + * Returns 0 in case of success and -1 in case of failure. + */ +int +virInterfaceChangeCommit(virConnectPtr conn, unsigned int flags) +{ + VIR_DEBUG("conn=%p, flags=%d", conn, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (conn->flags & VIR_CONNECT_RO) { + virLibInterfaceError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + if (conn->interfaceDriver && conn->interfaceDriver->interfaceChangeCommit) { + int ret; + ret = conn->interfaceDriver->interfaceChangeCommit(conn, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(conn); + return -1; +} + +/** + * 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(). + * + * If virInterfaceChangeRollback() is called when a transaction is not + * opened, this function will fail, and a VIR_ERR_INVALID_OPERATION + * will be logged. + * + * Returns 0 in case of success and -1 in case of failure. + */ +int +virInterfaceChangeRollback(virConnectPtr conn, unsigned int flags) +{ + VIR_DEBUG("conn=%p, flags=%d", conn, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (conn->flags & VIR_CONNECT_RO) { + virLibInterfaceError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + if (conn->interfaceDriver && + conn->interfaceDriver->interfaceChangeRollback) { + int ret; + ret = conn->interfaceDriver->interfaceChangeRollback(conn, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(conn); + return -1; +} +

ACK 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 :|

On Thu, May 26, 2011 at 12:17:42PM -0400, Laine Stump wrote:

From: Michal Privoznik <mprivozn@redhat.com>

No change from previously ACKed patch --- src/remote/remote_driver.c | 3 +++ src/remote/remote_protocol.x | 17 ++++++++++++++++- src/remote_protocol-structs | 9 +++++++++ 3 files changed, 28 insertions(+), 1 deletions(-)

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 1691dab..075fb56 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -6909,6 +6909,9 @@ static virInterfaceDriver interface_driver = { .interfaceCreate = remoteInterfaceCreate, /* 0.7.2 */ .interfaceDestroy = remoteInterfaceDestroy, /* 0.7.2 */ .interfaceIsActive = remoteInterfaceIsActive, /* 0.7.3 */ + .interfaceChangeBegin = remoteInterfaceChangeBegin, /* 0.9.2 */ + .interfaceChangeCommit = remoteInterfaceChangeCommit, /* 0.9.2 */ + .interfaceChangeRollback = remoteInterfaceChangeRollback, /* 0.9.2 */ };

static virStorageDriver storage_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index f0da95d..a663b25 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -1164,6 +1164,18 @@ struct remote_interface_destroy_args { unsigned int flags; };

+struct remote_interface_change_begin_args { + unsigned int flags; +}; + +struct remote_interface_change_commit_args { + unsigned int flags; +}; + +struct remote_interface_change_rollback_args { + unsigned int flags; +}; +

/* Auth calls: */

@@ -2291,8 +2303,11 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_MIGRATE_PERFORM3 = 216, /* skipgen skipgen */ REMOTE_PROC_DOMAIN_MIGRATE_FINISH3 = 217, /* skipgen skipgen */ REMOTE_PROC_DOMAIN_MIGRATE_CONFIRM3 = 218, /* skipgen skipgen */ - REMOTE_PROC_DOMAIN_SET_SCHEDULER_PARAMETERS_FLAGS = 219 /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_SET_SCHEDULER_PARAMETERS_FLAGS = 219, /* skipgen skipgen */ + REMOTE_PROC_INTERFACE_CHANGE_BEGIN = 220, /* autogen autogen */

+ REMOTE_PROC_INTERFACE_CHANGE_COMMIT = 221, /* autogen autogen */ + REMOTE_PROC_INTERFACE_CHANGE_ROLLBACK = 222 /* autogen autogen */ /* * Notice how the entries are grouped in sets of 10 ? * Nice isn't it. Please keep it this way when adding more. diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 414b4d5..a28cd13 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -803,6 +803,15 @@ struct remote_interface_destroy_args { remote_nonnull_interface iface; u_int flags; }; +struct remote_interface_change_begin_args { + unsigned int flags; +}; +struct remote_interface_change_commit_args { + unsigned int flags; +}; +struct remote_interface_change_rollback_args { + unsigned int flags; +}; struct remote_auth_list_ret { struct { u_int types_len;

ACK 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 :|

I intentionally set things up so 'virsh help interface' lists commands in alphabetical order, but 'man virsh' lists them in topical order; this matches our practice on some other commands. * tools/virsh.pod: Document all iface commands. * tools/virsh.c (ifaceCmds): Sort. --- Dan was partially right:

Though we will be wanting a followup patch for virsh.pod to add docs for this

but missed the fact that _none_ of the iface-* commands were documented, so Laine's initial patch wasn't making the situation any worse. At any rate, this patch was heavily copy-and-pasted from existing net-* commands. tools/virsh.c | 12 ++++---- tools/virsh.pod | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 95 insertions(+), 7 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index dfd5bd2..b9f1101 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -11063,6 +11063,10 @@ static const vshCmdDef nodedevCmds[] = { }; static const vshCmdDef ifaceCmds[] = { + {"iface-begin", cmdInterfaceBegin, opts_interface_begin, + info_interface_begin, 0}, + {"iface-commit", cmdInterfaceCommit, opts_interface_commit, + info_interface_commit, 0}, {"iface-define", cmdInterfaceDefine, opts_interface_define, info_interface_define, 0}, {"iface-destroy", cmdInterfaceDestroy, opts_interface_destroy, @@ -11077,16 +11081,12 @@ static const vshCmdDef ifaceCmds[] = { info_interface_mac, 0}, {"iface-name", cmdInterfaceName, opts_interface_name, info_interface_name, 0}, + {"iface-rollback", cmdInterfaceRollback, opts_interface_rollback, + info_interface_rollback, 0}, {"iface-start", cmdInterfaceStart, opts_interface_start, info_interface_start, 0}, {"iface-undefine", cmdInterfaceUndefine, opts_interface_undefine, info_interface_undefine, 0}, - {"iface-begin", cmdInterfaceBegin, opts_interface_begin, - info_interface_begin, 0}, - {"iface-commit", cmdInterfaceCommit, opts_interface_commit, - info_interface_commit, 0}, - {"iface-rollback", cmdInterfaceRollback, opts_interface_rollback, - info_interface_rollback, 0}, {NULL, NULL, NULL, NULL, 0} }; diff --git a/tools/virsh.pod b/tools/virsh.pod index 9251db6..4ef518f 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -865,7 +865,7 @@ The following commands manipulate networks. Libvirt has the capability to define virtual networks which can then be used by domains and linked to actual network devices. For more detailed information about this feature see the documentation at L<http://libvirt.org/formatnetwork.html> . A lot -of the command for virtual networks are similar to the one used for domains, +of the commands for virtual networks are similar to the ones used for domains, but the way to name a virtual network is either by its name or UUID. =over 4 @@ -937,6 +937,94 @@ Convert a network name to network UUID. =back +=head1 INTERFACE COMMANDS + +The following commands manipulate host interfaces to be used by +virtual networks. A lot of the commands for host interfaces are +similar to the ones used for domains, and the way to name an interface +is either by its name or its MAC address. + +=over 4 + +=item B<iface-define> I<file> + +Define a host interface from an XML I<file>, the interface is just defined but +not started. + +=item B<iface-destroy> I<interface> + +Destroy a given host interface, such as by running "if-down" to +disable that interface from active use. This takes effect immediately. + +=item B<iface-dumpxml> I<interface> optional I<--inactive> + +Output the host interface information as an XML dump to stdout. If +I<--inactive> is specified, then the output reflects the persistent +state of the interface that will be used the next time it is started. + +=item B<iface-edit> I<interface> + +Edit the XML configuration file for a host interface. + +This is equivalent to: + + virsh iface-dumpxml iface > iface.xml + vi iface.xml (or make changes with your other text editor) + virsh iface-define iface.xml + +except that it does some error checking. + +The editor used can be supplied by the C<$VISUAL> or C<$EDITOR> environment +variables, and defaults to C<vi>. + +=item B<iface-list> optional I<--inactive> or I<--all> + +Returns the list of active host interfaces. If I<--all> is specified +this will also include defined but inactive interfaces. If +I<--inactive> is specified only the inactive ones will be listed. + +=item B<iface-name> I<iface-MAC> + +Convert a host interface MAC to interface name. + +=item B<iface-name> I<iface-MAC> + +Convert a host interface name to MAC address. + +=item B<iface-start> I<iface> + +Start a (previously defined) host interface. + +=item B<iface-undefine> I<iface> + +Undefine the configuration for an inactive host interface. + +=item B<iface-begin> + +Create a snapshot of current host interface settings, which can later +be committed (I<iface-commit>) or restored (I<iface-rollback>). If a +snapshot already exists, then this command will fail until the +previous snapshot has been committed or restored. Undefined behavior +results if any external changes are made to host interfaces outside of +the libvirt API between the beginning of a snapshot and its eventual +commit or rollback. + +=item B<iface-commit> + +Confirm that all changes since the last I<iface-begin> are working, +and delete the rollback point. If no interface snapshot has already +been started, then this command will fail. + +=item B<iface-rollback> + +Revert all host interface settings back to the state recorded in the +last I<iface-begin>. If no interface snapshot has already been +started, then this command will fail. If appropriate netcf init +scripts are also installed, then rebooting the host may serve as an +implicit rollback point. + +=back + =head1 STORAGE POOL COMMANDS The following commands manipulate storage pools. Libvirt has the -- 1.7.4.4

I intentionally set things up so 'virsh help interface' lists commands in alphabetical order, but 'man virsh' lists them in topical order; this matches our practice on some other commands. * tools/virsh.pod: Document all iface commands. * tools/virsh.c (ifaceCmds): Sort. --- v2: fix points raised by Laine. Interdiff from v1 before the actual patch: =head1 INTERFACE COMMANDS -The following commands manipulate host interfaces to be used by -virtual networks. A lot of the commands for host interfaces are -similar to the ones used for domains, and the way to name an interface -is either by its name or its MAC address. +The following commands manipulate host interfaces. Often, these host +interfaces can then be used by name within domain <interface> elements +(such as a system-created bridge interface), but there is no +requirement that host interfaces be tied to any particular guest +configuration XML at all. + +A lot of the commands for host interfaces are similar to the ones used +for domains, and the way to name an interface is either by its name or +its MAC address. However, using a MAC address for an I<iface> +argument only works when that address is unique (if an interface and a +bridge share the same MAC address, which is often the case, then using +that MAC address results in an error due to ambiguity, and you must +resort to a name instead). =over 4 @@ -985,7 +994,8 @@ I<--inactive> is specified only the inactive ones will be listed. =item B<iface-name> I<iface-MAC> -Convert a host interface MAC to interface name. +Convert a host interface MAC to interface name, if the I<iface-MAC> is +unique among the host's interfaces. =item B<iface-name> I<iface-MAC> @@ -993,7 +1003,7 @@ Convert a host interface name to MAC address. =item B<iface-start> I<iface> -Start a (previously defined) host interface. +Start a (previously defined) host interface, such as by running "if-up". =item B<iface-undefine> I<iface> @@ -1011,17 +1021,16 @@ commit or rollback. =item B<iface-commit> -Confirm that all changes since the last I<iface-begin> are working, -and delete the rollback point. If no interface snapshot has already -been started, then this command will fail. +Declare all changes since the last I<iface-begin> as working, and +delete the rollback point. If no interface snapshot has already been +started, then this command will fail. =item B<iface-rollback> Revert all host interface settings back to the state recorded in the last I<iface-begin>. If no interface snapshot has already been -started, then this command will fail. If appropriate netcf init -scripts are also installed, then rebooting the host may serve as an -implicit rollback point. +started, then this command will fail. Rebooting the host also serves +as an implicit rollback point. =back tools/virsh.c | 12 +++--- tools/virsh.pod | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 104 insertions(+), 7 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index dfd5bd2..b9f1101 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -11063,6 +11063,10 @@ static const vshCmdDef nodedevCmds[] = { }; static const vshCmdDef ifaceCmds[] = { + {"iface-begin", cmdInterfaceBegin, opts_interface_begin, + info_interface_begin, 0}, + {"iface-commit", cmdInterfaceCommit, opts_interface_commit, + info_interface_commit, 0}, {"iface-define", cmdInterfaceDefine, opts_interface_define, info_interface_define, 0}, {"iface-destroy", cmdInterfaceDestroy, opts_interface_destroy, @@ -11077,16 +11081,12 @@ static const vshCmdDef ifaceCmds[] = { info_interface_mac, 0}, {"iface-name", cmdInterfaceName, opts_interface_name, info_interface_name, 0}, + {"iface-rollback", cmdInterfaceRollback, opts_interface_rollback, + info_interface_rollback, 0}, {"iface-start", cmdInterfaceStart, opts_interface_start, info_interface_start, 0}, {"iface-undefine", cmdInterfaceUndefine, opts_interface_undefine, info_interface_undefine, 0}, - {"iface-begin", cmdInterfaceBegin, opts_interface_begin, - info_interface_begin, 0}, - {"iface-commit", cmdInterfaceCommit, opts_interface_commit, - info_interface_commit, 0}, - {"iface-rollback", cmdInterfaceRollback, opts_interface_rollback, - info_interface_rollback, 0}, {NULL, NULL, NULL, NULL, 0} }; diff --git a/tools/virsh.pod b/tools/virsh.pod index 9251db6..ae0c887 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -865,7 +865,7 @@ The following commands manipulate networks. Libvirt has the capability to define virtual networks which can then be used by domains and linked to actual network devices. For more detailed information about this feature see the documentation at L<http://libvirt.org/formatnetwork.html> . A lot -of the command for virtual networks are similar to the one used for domains, +of the commands for virtual networks are similar to the ones used for domains, but the way to name a virtual network is either by its name or UUID. =over 4 @@ -937,6 +937,103 @@ Convert a network name to network UUID. =back +=head1 INTERFACE COMMANDS + +The following commands manipulate host interfaces. Often, these host +interfaces can then be used by name within domain <interface> elements +(such as a system-created bridge interface), but there is no +requirement that host interfaces be tied to any particular guest +configuration XML at all. + +A lot of the commands for host interfaces are similar to the ones used +for domains, and the way to name an interface is either by its name or +its MAC address. However, using a MAC address for an I<iface> +argument only works when that address is unique (if an interface and a +bridge share the same MAC address, which is often the case, then using +that MAC address results in an error due to ambiguity, and you must +resort to a name instead). + +=over 4 + +=item B<iface-define> I<file> + +Define a host interface from an XML I<file>, the interface is just defined but +not started. + +=item B<iface-destroy> I<interface> + +Destroy a given host interface, such as by running "if-down" to +disable that interface from active use. This takes effect immediately. + +=item B<iface-dumpxml> I<interface> optional I<--inactive> + +Output the host interface information as an XML dump to stdout. If +I<--inactive> is specified, then the output reflects the persistent +state of the interface that will be used the next time it is started. + +=item B<iface-edit> I<interface> + +Edit the XML configuration file for a host interface. + +This is equivalent to: + + virsh iface-dumpxml iface > iface.xml + vi iface.xml (or make changes with your other text editor) + virsh iface-define iface.xml + +except that it does some error checking. + +The editor used can be supplied by the C<$VISUAL> or C<$EDITOR> environment +variables, and defaults to C<vi>. + +=item B<iface-list> optional I<--inactive> or I<--all> + +Returns the list of active host interfaces. If I<--all> is specified +this will also include defined but inactive interfaces. If +I<--inactive> is specified only the inactive ones will be listed. + +=item B<iface-name> I<iface-MAC> + +Convert a host interface MAC to interface name, if the I<iface-MAC> is +unique among the host's interfaces. + +=item B<iface-name> I<iface-MAC> + +Convert a host interface name to MAC address. + +=item B<iface-start> I<iface> + +Start a (previously defined) host interface, such as by running "if-up". + +=item B<iface-undefine> I<iface> + +Undefine the configuration for an inactive host interface. + +=item B<iface-begin> + +Create a snapshot of current host interface settings, which can later +be committed (I<iface-commit>) or restored (I<iface-rollback>). If a +snapshot already exists, then this command will fail until the +previous snapshot has been committed or restored. Undefined behavior +results if any external changes are made to host interfaces outside of +the libvirt API between the beginning of a snapshot and its eventual +commit or rollback. + +=item B<iface-commit> + +Declare all changes since the last I<iface-begin> as working, and +delete the rollback point. If no interface snapshot has already been +started, then this command will fail. + +=item B<iface-rollback> + +Revert all host interface settings back to the state recorded in the +last I<iface-begin>. If no interface snapshot has already been +started, then this command will fail. Rebooting the host also serves +as an implicit rollback point. + +=back + =head1 STORAGE POOL COMMANDS The following commands manipulate storage pools. Libvirt has the -- 1.7.4.4

On Thu, May 26, 2011 at 12:17:44PM -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.

Change from previous version: modify the error log format as suggested by danpb. --- configure.ac | 5 +++ src/interface/netcf_driver.c | 78 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 0 deletions(-)

diff --git a/configure.ac b/configure.ac index e17e7af..58c7733 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..b5b4f79 100644 --- a/src/interface/netcf_driver.c +++ b/src/interface/netcf_driver.c @@ -86,6 +86,11 @@ static int netcf_to_vir_err(int netcf_errcode) case NETCF_EEXEC: /* external program execution failed or returned non-0 */ return VIR_ERR_INTERNAL_ERROR; +#ifdef NETCF_EINVALIDOP + case NETCF_EINVALIDOP: + /* attempted operation is invalid while the system is in the current state. */ + return VIR_ERR_OPERATION_INVALID; +#endif default: return VIR_ERR_INTERNAL_ERROR; } @@ -540,6 +545,74 @@ cleanup: return ret; }

+#ifdef HAVE_NETCF_TRANSACTIONS +static int interfaceChangeBegin(virConnectPtr conn, unsigned int flags) +{ + struct interface_driver *driver = conn->interfacePrivateData; + int ret; + + virCheckFlags(0, -1); /* currently flags must be 0 */ + + 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: %s%s%s)"), + errmsg, details ? " - " : "", details ? details : ""); + } + + interfaceDriverUnlock(driver); + return ret; +} + +static int interfaceChangeCommit(virConnectPtr conn, unsigned int flags) +{ + struct interface_driver *driver = conn->interfacePrivateData; + int ret; + + virCheckFlags(0, -1); /* currently flags must be 0 */ + + interfaceDriverLock(driver); + + ret = ncf_change_commit(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 commit transaction: %s%s%s)"), + errmsg, details ? " - " : "", details ? details : ""); + } + + interfaceDriverUnlock(driver); + return ret; +} + +static int interfaceChangeRollback(virConnectPtr conn, unsigned int flags) +{ + struct interface_driver *driver = conn->interfacePrivateData; + int ret; + + virCheckFlags(0, -1); /* currently flags must be 0 */ + + interfaceDriverLock(driver); + + ret = ncf_change_rollback(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 rollback transaction: %s%s%s)"), + errmsg, details ? " - " : "", details ? details : ""); + } + + interfaceDriverUnlock(driver); + return ret; +} +#endif /* HAVE_NETCF_TRANSACTIONS */ + static virInterfaceDriver interfaceDriver = { "Interface", .open = interfaceOpenInterface, /* 0.7.0 */ @@ -556,6 +629,11 @@ static virInterfaceDriver interfaceDriver = { .interfaceCreate = interfaceCreate, /* 0.7.0 */ .interfaceDestroy = interfaceDestroy, /* 0.7.0 */ .interfaceIsActive = interfaceIsActive, /* 0.7.3 */ +#ifdef HAVE_NETCF_TRANSACTIONS + .interfaceChangeBegin = interfaceChangeBegin, /* 0.9.2 */ + .interfaceChangeCommit = interfaceChangeCommit, /* 0.9.2 */ + .interfaceChangeRollback = interfaceChangeRollback, /* 0.9.2 */ +#endif /* HAVE_NETCF_TRANSACTIONS */ };

int interfaceRegister(void) {

ACK 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 :|

On Thu, May 26, 2011 at 12:17:45PM -0400, Laine Stump wrote:

From: Michal Privoznik <mprivozn@redhat.com>

Changes from previous version:

1) don't print extra/incorrect error on format failure. 2) use cleanup: instead of no_memory: and end: 3) always clear out the destination list at the start. 4) remove bogus "conn->ref++" --- src/conf/interface_conf.c | 38 ++++++++++++++++++++ src/conf/interface_conf.h | 3 ++ src/libvirt_private.syms | 1 + src/test/test_driver.c | 83 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 125 insertions(+), 0 deletions(-)

diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index f3848bd..10377e7 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -1227,6 +1227,44 @@ void virInterfaceObjListFree(virInterfaceObjListPtr interfaces) interfaces->count = 0; }

+int virInterfaceObjListClone(virInterfaceObjListPtr src, + virInterfaceObjListPtr dest) +{ + int ret = -1; + unsigned int i, cnt; + + if (!src || !dest) + goto cleanup; + + virInterfaceObjListFree(dest); /* start with an empty list */ + cnt = src->count; + for (i = 0; i < cnt; i++) { + virInterfaceDefPtr def = src->objs[i]->def; + virInterfaceDefPtr backup; + virInterfaceObjPtr iface; + char *xml = virInterfaceDefFormat(def); + + if (!xml) + goto cleanup; + + if ((backup = virInterfaceDefParseString(xml)) == NULL) { + VIR_FREE(xml); + goto cleanup; + } + + VIR_FREE(xml); + if ((iface = virInterfaceAssignDef(dest, backup)) == NULL) + goto cleanup; + virInterfaceObjUnlock(iface); /* was locked by virInterfaceAssignDef */ + } + + ret = cnt; +cleanup: + if ((ret < 0) && dest) + virInterfaceObjListFree(dest); + return ret; +} + virInterfaceObjPtr virInterfaceAssignDef(virInterfaceObjListPtr interfaces, const virInterfaceDefPtr def) { diff --git a/src/conf/interface_conf.h b/src/conf/interface_conf.h index 6073b49..c5630d4 100644 --- a/src/conf/interface_conf.h +++ b/src/conf/interface_conf.h @@ -192,6 +192,9 @@ virInterfaceObjPtr virInterfaceFindByName(const virInterfaceObjListPtr void virInterfaceDefFree(virInterfaceDefPtr def); void virInterfaceObjFree(virInterfaceObjPtr iface); void virInterfaceObjListFree(virInterfaceObjListPtr vms); +int virInterfaceObjListClone(virInterfaceObjListPtr src, + virInterfaceObjListPtr dest); +

virInterfaceObjPtr virInterfaceAssignDef(virInterfaceObjListPtr interfaces, const virInterfaceDefPtr def); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 4cb8dda..5f7fdb2 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -485,6 +485,7 @@ virInterfaceDefParseNode; virInterfaceDefParseString; virInterfaceFindByMACString; virInterfaceFindByName; +virInterfaceObjListClone; virInterfaceObjListFree; virInterfaceObjLock; virInterfaceObjUnlock; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index e86bc4e..b703e9b 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -85,6 +85,8 @@ struct _testConn { virDomainObjList domains; virNetworkObjList networks; virInterfaceObjList ifaces; + bool transaction_running; + virInterfaceObjList backupIfaces; virStoragePoolObjList pools; virNodeDeviceObjList devs; int numCells; @@ -3455,6 +3457,84 @@ cleanup: return ret; }

+static int testInterfaceChangeBegin(virConnectPtr conn, + unsigned int flags ATTRIBUTE_UNUSED) +{ + testConnPtr privconn = conn->privateData; + int ret = -1; + + testDriverLock(privconn); + if (privconn->transaction_running) { + testError(VIR_ERR_OPERATION_INVALID, + _("there is another transaction running.")); + goto cleanup; + } + + privconn->transaction_running = true; + + if (virInterfaceObjListClone(&privconn->ifaces, + &privconn->backupIfaces) < 0) + goto cleanup; + + ret = 0; +cleanup: + testDriverUnlock(privconn); + return ret; +} + +static int testInterfaceChangeCommit(virConnectPtr conn, + unsigned int flags ATTRIBUTE_UNUSED) +{ + testConnPtr privconn = conn->privateData; + int ret = -1; + + testDriverLock(privconn); + + if (!privconn->transaction_running) { + testError(VIR_ERR_OPERATION_INVALID, _("no transaction running, " + "nothing to be commited.")); + goto cleanup; + } + + virInterfaceObjListFree(&privconn->backupIfaces); + privconn->transaction_running = false; + + ret = 0; + +cleanup: + testDriverUnlock(privconn); + + return ret; +} + +static int testInterfaceChangeRollback(virConnectPtr conn, + unsigned int flags ATTRIBUTE_UNUSED) +{ + testConnPtr privconn = conn->privateData; + int ret = -1; + + testDriverLock(privconn); + + if (!privconn->transaction_running) { + testError(VIR_ERR_OPERATION_INVALID, _("no transaction running, " + "nothing to rollback.")); + goto cleanup; + } + + virInterfaceObjListFree(&privconn->ifaces); + privconn->ifaces.count = privconn->backupIfaces.count; + privconn->ifaces.objs = privconn->backupIfaces.objs; + privconn->backupIfaces.count = 0; + privconn->backupIfaces.objs = NULL; + + privconn->transaction_running = false; + + ret = 0; + +cleanup: + testDriverUnlock(privconn); + return ret; +}

static char *testInterfaceGetXMLDesc(virInterfacePtr iface, unsigned int flags ATTRIBUTE_UNUSED) @@ -5428,6 +5508,9 @@ static virInterfaceDriver testInterfaceDriver = { .interfaceCreate = testInterfaceCreate, /* 0.7.0 */ .interfaceDestroy = testInterfaceDestroy, /* 0.7.0 */ .interfaceIsActive = testInterfaceIsActive, /* 0.7.3 */ + .interfaceChangeBegin = testInterfaceChangeBegin, /* 0.9.2 */ + .interfaceChangeCommit = testInterfaceChangeCommit, /* 0.9.2 */ + .interfaceChangeRollback = testInterfaceChangeRollback, /* 0.9.2 */ };

ACK 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 :|