[libvirt] [PATCH RFC 00/10] API && XML to control virtual link state of guest NICs
Hypervisors provide means for controling link state of virtual NICs. Ability to control this can be usefully used for various aplications. These patches add (or propose) an API for controling link states of these nics. API functions added: ----------------------------------------------------------------------- int virDomainInterfaceLinkSetState(virDomainPtr dom, const char *path, unsigned int state, unsigned int flags); This function sets link state for a NIC. A NIC is identified by the domain and path (which is MAC address of the NIC (in a string representation)). States are represented by the virInterfaceLinkStates enum. Flags determine manipulation impact of the command (live/persistent). int virDomainInterfaceLinkGetState(virDomainPtr dom, const char *path, unsigned int flags); This function requests the interface state and returns it as the return value. Peter Krempa (10): linkstate: XML for modification of interface's link state. linkstate: Public API for modifying link state on interfaces linkstate: Internal API for interface link state linkstate: Implementation of the public API linkstate: Wire up the remote protocol linkstate: Add parsing code for new XML element linkstate: Add usage of the new api to virsh linkstate: qemu: Add monitor support for setting link state linkstate: qemu: Add detection of link state setting capability. linkstate: qemu: Add link state modification support to qemu driver. docs/formatdomain.html.in | 21 ++++ - add examples for the new element docs/schemas/domain.rng | 11 ++ - add grammar for the new element include/libvirt/libvirt.h.in | 24 +++++ - add public API headers - add link states enum src/conf/domain_conf.c | 19 ++++ src/conf/domain_conf.h | 1 + - add new element and propertyto parsing and generating of domain configs src/driver.h | 14 +++- src/libvirt.c | 122 ++++++++++++++++++++++ src/libvirt_public.syms | 6 + - add implementation and expose public symbols src/qemu/qemu_capabilities.c | 5 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_driver.c | 228 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor.c | 19 ++++ src/qemu/qemu_monitor.h | 3 + src/qemu/qemu_monitor_json.c | 23 ++++ src/qemu/qemu_monitor_json.h | 4 + src/qemu/qemu_monitor_text.c | 47 +++++++++ src/qemu/qemu_monitor_text.h | 4 + src/qemu/qemu_process.c | 37 +++++++ - add support for qemu driver src/remote/remote_driver.c | 2 + src/remote/remote_protocol.x | 21 ++++- src/remote_protocol-structs | 16 +++ - remote protocol support for new API tools/virsh.c | 120 ++++++++++++++++++++++ tools/virsh.pod | 11 ++ - add comands to virsh and documentation 23 files changed, 757 insertions(+), 2 deletions(-) -- 1.7.6
Adds a optional element to XML definition of domains for modification of link state of network interfaces. --- docs/formatdomain.html.in | 21 +++++++++++++++++++++ docs/schemas/domain.rng | 11 +++++++++++ 2 files changed, 32 insertions(+), 0 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 7d2ba8a..dc4cecb 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1997,6 +1997,27 @@ qemu-kvm -net nic,model=? /dev/null <span class="since">Since 0.9.4</span> </p> + <h5><a name="elementLink">Modyfing virtual link state</a></h5> +<pre> + ... + <devices> + <interface type='network'> + <source network='default'/> + <target dev='vnet0'/> + <b><link state='down'/></b> + </interface> + <devices> + ...</pre> + + <p> + This element provides means of setting state of the virtual network link. + Possible values for atrribute <code>state</code> are <code>up</code> and + <code>down</code>. If <code>down</code> is specified as the value, the interface + behaves as if it had the network cable disconnected. Default behaviour, if this + element is unspecified is to have the link state <code>up</code>. + <span class="since">Since 0.9.5</span> + </p> + <h4><a name="elementsInput">Input devices</a></h4> <p> diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 6ccbeed..a4282eb 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -1192,6 +1192,17 @@ <optional> <ref name="bandwidth"/> </optional> + <optional> + <element name="link"> + <attribute name="state"> + <choice> + <value>up</value> + <value>down</value> + </choice> + </attribute> + <empty/> + </element> + </optional> </interleave> </define> <!-- -- 1.7.6
On Thu, Aug 11, 2011 at 05:27:39PM +0200, Peter Krempa wrote:
Adds a optional element to XML definition of domains for modification of link state of network interfaces. --- docs/formatdomain.html.in | 21 +++++++++++++++++++++ docs/schemas/domain.rng | 11 +++++++++++ 2 files changed, 32 insertions(+), 0 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 7d2ba8a..dc4cecb 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1997,6 +1997,27 @@ qemu-kvm -net nic,model=? /dev/null <span class="since">Since 0.9.4</span> </p>
+ <h5><a name="elementLink">Modyfing virtual link state</a></h5> +<pre> + ... + <devices> + <interface type='network'> + <source network='default'/> + <target dev='vnet0'/> + <b><link state='down'/></b> + </interface> + <devices> + ...</pre> + + <p> + This element provides means of setting state of the virtual network link. + Possible values for atrribute <code>state</code> are <code>up</code> and + <code>down</code>. If <code>down</code> is specified as the value, the interface + behaves as if it had the network cable disconnected. Default behaviour, if this + element is unspecified is to have the link state <code>up</code>. + <span class="since">Since 0.9.5</span> + </p> + <h4><a name="elementsInput">Input devices</a></h4>
<p> diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 6ccbeed..a4282eb 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -1192,6 +1192,17 @@ <optional> <ref name="bandwidth"/> </optional> + <optional> + <element name="link"> + <attribute name="state"> + <choice> + <value>up</value> + <value>down</value> + </choice> + </attribute> + <empty/> + </element> + </optional> </interleave> </define> <!--
Once we add link state to the XML, it ought to be possible to enable changes of the link state using the existing virDomainUpdateDevice() API call. We already use this for changing CDROM media, changing the VNC/SPICE graphics configuration, etc, so IMHO it should also be used for NIC device configuration changes. This lets you do more than just change link state. It will be possible to actually change the NIC device backend entirely. For example, you might want to take a guest with bridged networking and disconnect it entirely <interface type='bridge'> <source bridge='br0'/> <link state='up'/> </interface> By changing to type=none (does not exist yet, but could) issuing this to the virDomaniUpdateDevice: <interface type='none'> <link state='down'/> </interface> We'd do this by using the monitor to set the link state down, and then remove the network device backend, leaving only the guest device. Some time later we might want to connect the guest to a new LAN, but issuing virDomainUpdateDevice with: <interface type='network'> <source network='default'/> <link state='up'/> </interface> So I think it is more flexible todo this via virDomainUpdateDevice than to have special case APIs for link state. 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 08/11/2011 09:27 AM, Peter Krempa wrote:
Adds a optional element to XML definition of domains for modification of link state of network interfaces. --- docs/formatdomain.html.in | 21 +++++++++++++++++++++ docs/schemas/domain.rng | 11 +++++++++++ 2 files changed, 32 insertions(+), 0 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 7d2ba8a..dc4cecb 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1997,6 +1997,27 @@ qemu-kvm -net nic,model=? /dev/null <span class="since">Since 0.9.4</span> </p>
+<h5><a name="elementLink">Modyfing virtual link state</a></h5> +<pre> + ... +<devices> +<interface type='network'> +<source network='default'/> +<target dev='vnet0'/> +<b><link state='down'/></b> +</interface> +<devices> + ...</pre> + +<p> + This element provides means of setting state of the virtual network link. + Possible values for atrribute<code>state</code> are<code>up</code> and
s/atrribute/attribute/
+<code>down</code>. If<code>down</code> is specified as the value, the interface + behaves as if it had the network cable disconnected. Default behaviour, if this + element is unspecified is to have the link state<code>up</code>.
Mismatch of commas doesn't read well. Both of these approaches will work in English: Default behavior, if element is unspecified, is ... Default behavior if element is unspecified is ... although I tend to prefer the first (that is, add a comma after "unspecified"). ACK with those fixes, although no point in pushing the xml changes until we fix the rest of the series to use the new xml via existing api rather than inventing new special-purpose api. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
This API adds methods to modify link state of virtual interfaces of a domain. Idea of this API is to enable simulation of unplugging (virtual) cable from the machine to test networks or to allow propagation of topology changes on the host. --- include/libvirt/libvirt.h.in | 24 ++++++++++++++++++++++++ src/libvirt_public.syms | 6 ++++++ 2 files changed, 30 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index b1bda31..b149eb9 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1022,6 +1022,30 @@ int virDomainGetControlInfo (virDomainPtr domain, virDomainControlInfoPtr info, unsigned int flags); + +/* + * Domain network interface link state modification + */ + +int virDomainInterfaceLinkSetState(virDomainPtr dom, + const char *path, + unsigned int state, + unsigned int flags); +int virDomainInterfaceLinkGetState(virDomainPtr dom, + const char *path, + unsigned int flags); +/** + * virInterfaceLinkStates + * + * Link state options + */ +typedef enum { + VIR_LINK_STATE_DEFAULT = 0, /* Default link state (up) */ + VIR_LINK_STATE_UP, /* Link is up. ("cable" connected) */ + VIR_LINK_STATE_DOWN , /* Link is down. ("cable" disconnected) */ +} virInterfaceLinkStates; + + /* * Return scheduler type in effect 'sedf', 'credit', 'linux' */ diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index c2b6666..ac77c02 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -481,3 +481,9 @@ LIBVIRT_0.9.4 { } LIBVIRT_0.9.3; # .... define new API here using predicted next version number .... + +LIBVIRT_0.9.5 { + global: + virDomainInterfaceLinkSetState; + virDomainInterfaceLinkGetState; +} LIBVIRT_0.9.4; -- 1.7.6
On 08/11/2011 09:27 AM, Peter Krempa wrote:
This API adds methods to modify link state of virtual interfaces of a domain.
Idea of this API is to enable simulation of unplugging (virtual) cable from the machine to test networks or to allow propagation of topology changes on the host. --- include/libvirt/libvirt.h.in | 24 ++++++++++++++++++++++++ src/libvirt_public.syms | 6 ++++++ 2 files changed, 30 insertions(+), 0 deletions(-)
NACK. I agree with danpb that this is something that already fits quite nicely into virDomainUpdateDevice, without needing specialized API (however, we can still add a specialized virsh command that makes it easier to wrap virDomainUpdateDevice without making the user have to deal with quite as much xml). -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
--- src/driver.h | 14 +++++++++++++- 1 files changed, 13 insertions(+), 1 deletions(-) diff --git a/src/driver.h b/src/driver.h index 5136fb5..8101df3 100644 --- a/src/driver.h +++ b/src/driver.h @@ -351,7 +351,17 @@ typedef int (virDomainPtr domain, const char *path, struct _virDomainInterfaceStats *stats); - +typedef int + (*virDrvDomainInterfaceLinkSetState) + (virDomainPtr domain, + const char *path, + unsigned int state, + unsigned int flags); +typedef int + (*virDrvDomainInterfaceLinkGetState) + (virDomainPtr domain, + const char *path, + unsigned int flags); typedef int (*virDrvDomainMemoryStats) (virDomainPtr domain, @@ -801,6 +811,8 @@ struct _virDriver { virDrvDomainMigrateFinish domainMigrateFinish; virDrvDomainBlockStats domainBlockStats; virDrvDomainInterfaceStats domainInterfaceStats; + virDrvDomainInterfaceLinkSetState domainInterfaceLinkSetState; + virDrvDomainInterfaceLinkGetState domainInterfaceLinkGetState; virDrvDomainMemoryStats domainMemoryStats; virDrvDomainBlockPeek domainBlockPeek; virDrvDomainMemoryPeek domainMemoryPeek; -- 1.7.6
--- src/libvirt.c | 122 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 122 insertions(+), 0 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index c8af3e1..53b7a11 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -6421,6 +6421,128 @@ error: } /** + * virDomainInterfaceLinkSetState: + * @dom: pointer to the domain object + * @path: interface identification (MAC Address) + * @state: state of the link to be set (one of virInterfaceLinkStates) + * @flags: bitwise OR of virDomainModificationImpact + * + * This function sets a new link state for the interface. + * This function requires privileged access to the hypervisor. + * + * The path parameter is the name of the network interface. + * + * @flags may include VIR_DOMAIN_AFFECT_LIVE or VIR_DOMAIN_AFFECT_CONFIG. + * Both flags may be set. If VIR_DOMAIN_AFFECT_LIVE is set, the change + * affects a running domain and will fail if domain is not active. + * If VIR_DOMAIN_AFFECT_CONFIG is set, the change affects persistent state, + * and will fail for transient domains. + * If neither flag is specified (that is, @flags is VIR_DOMAIN_AFFECT_CURRENT), + * then an inactive domain modifies persistent setup, while an active domain is + * hypervisor-dependent on whether just live or both live and persistent state + * is changed. + * + * Returns: 0 in case of success or -1 in case of failure. + */ +int +virDomainInterfaceLinkSetState(virDomainPtr dom, const char *path, + unsigned int state, unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(dom, "path=%s, state=%u flags=%X", + path, state, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN (dom)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + if (!path || + !(state == VIR_LINK_STATE_UP || state == VIR_LINK_STATE_DOWN)) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + if (dom->conn->flags & VIR_CONNECT_RO) { + virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + conn = dom->conn; + + if (conn->driver->domainInterfaceLinkSetState) { + if (conn->driver->domainInterfaceLinkSetState (dom, path, state, flags) == -1) + goto error; + + return 0; + } + + virLibDomainError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(dom->conn); + return -1; +} + +/** + * virDomainInterfaceLinkGetState: + * @dom: pointer to the domain object + * @path: interface identification (MAC Address) + * @flags: one of virDomainModificationImpact + * + * This function gets link state for the interface. Current state for a running + * domain and persistent state for a inactive domain. + * + * If @flags includes VIR_DOMAIN_AFFECT_LIVE, this will query a + * running domain (which will fail if domain is not active); + * if it includes VIR_DOMAIN_AFFECT_CONFIG, this will query + * the XML description of the domain. It is an error to set + * both flags. If neither flag is set (that is, VIR_DOMAIN_AFFECT_CURRENT), + * then the configuration queried depends on whether the domain + * is currently running. + * + * Returns: One of virInterfaceLinkStates on success or -1 in case of failure. + */ +int +virDomainInterfaceLinkGetState(virDomainPtr dom, const char *path, + unsigned int flags) +{ + virConnectPtr conn; + int ret = -1; + + VIR_DOMAIN_DEBUG(dom, "path=%s, flags=%X", + path, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN (dom)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + if (!path) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + conn = dom->conn; + + if (conn->driver->domainInterfaceLinkGetState) { + if ((ret=conn->driver->domainInterfaceLinkGetState (dom, path, flags)) == -1) + goto error; + + return ret; + } + + virLibDomainError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(dom->conn); + return ret; +} + +/** * virDomainMemoryStats: * @dom: pointer to the domain object * @stats: nr_stats-sized array of stat structures (returned) -- 1.7.6
On 08/11/2011 09:27 AM, Peter Krempa wrote:
--- src/libvirt.c | 122 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 122 insertions(+), 0 deletions(-)
For new API, I've lately been squashing the definition (libvirt.h.in, patch 2/10), backend callback (driver.h 3/10), and public implementation (libvirt.c 4/10) into a single patch, since none of them is very big, and since having the libvirt.c docs alongside the libvirt.h.in declarations makes it easier to see what the new declarations are intended to do. At any rate, same NACK for this as for 2/10. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 08/11/2011 04:08 PM, Eric Blake wrote:
On 08/11/2011 09:27 AM, Peter Krempa wrote:
--- src/libvirt.c | 122 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 122 insertions(+), 0 deletions(-)
For new API, I've lately been squashing the definition (libvirt.h.in, patch 2/10), backend callback (driver.h 3/10), and public implementation (libvirt.c 4/10) into a single patch, since none of them is very big, and since having the libvirt.c docs alongside the libvirt.h.in declarations makes it easier to see what the new declarations are intended to do.
Maybe a change to http://libvirt.org/api_extension.html is in order? Volunteers? :-)
--- src/remote/remote_driver.c | 2 ++ src/remote/remote_protocol.x | 21 ++++++++++++++++++++- src/remote_protocol-structs | 16 ++++++++++++++++ 3 files changed, 38 insertions(+), 1 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index e5bfa4b..604b9fc 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -4300,6 +4300,8 @@ static virDriver remote_driver = { .domainMigrateFinish = remoteDomainMigrateFinish, /* 0.3.2 */ .domainBlockStats = remoteDomainBlockStats, /* 0.3.2 */ .domainInterfaceStats = remoteDomainInterfaceStats, /* 0.3.2 */ + .domainInterfaceLinkSetState = remoteDomainInterfaceLinkSetState, /* 0.9.5 */ + .domainInterfaceLinkGetState = remoteDomainInterfaceLinkGetState, /* 0.9.5 */ .domainMemoryStats = remoteDomainMemoryStats, /* 0.7.5 */ .domainBlockPeek = remoteDomainBlockPeek, /* 0.4.2 */ .domainMemoryPeek = remoteDomainMemoryPeek, /* 0.4.2 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 8f68808..20b9bdb 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -560,6 +560,23 @@ struct remote_domain_interface_stats_ret { /* insert@2 */ hyper tx_drop; }; +struct remote_domain_interface_link_set_state_args { + remote_nonnull_domain dom; + remote_nonnull_string path; + unsigned int state; + unsigned int flags; +}; + +struct remote_domain_interface_link_get_state_args { + remote_nonnull_domain dom; + remote_nonnull_string path; + unsigned int flags; +}; + +struct remote_domain_interface_link_get_state_ret { + int state; +}; + struct remote_domain_memory_stats_args { remote_nonnull_domain dom; unsigned int maxStats; @@ -2475,7 +2492,9 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_BLOCK_JOB_SET_SPEED = 239, /* autogen autogen */ REMOTE_PROC_DOMAIN_BLOCK_PULL = 240, /* autogen autogen */ - REMOTE_PROC_DOMAIN_EVENT_BLOCK_JOB = 241 /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_EVENT_BLOCK_JOB = 241, /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_INTERFACE_LINK_SET_STATE = 242, /* autogen autogen */ + REMOTE_PROC_DOMAIN_INTERFACE_LINK_GET_STATE = 243 /*autogen autogen */ /* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 91b3ca5..2f8b115 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -287,6 +287,20 @@ struct remote_domain_interface_stats_ret { int64_t tx_errs; int64_t tx_drop; }; +struct remote_domain_interface_link_set_state_args { + remote_nonnull_domain dom; + remote_nonnull_string path; + u_int state; + u_int flags; +}; +struct remote_domain_interface_link_get_state_args { + remote_nonnull_domain dom; + remote_nonnull_string path; + u_int flags; +}; +struct remote_domain_interface_link_get_state_ret { + int state; +}; struct remote_domain_memory_stats_args { remote_nonnull_domain dom; u_int maxStats; @@ -1936,4 +1950,6 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_BLOCK_JOB_SET_SPEED = 239, REMOTE_PROC_DOMAIN_BLOCK_PULL = 240, REMOTE_PROC_DOMAIN_EVENT_BLOCK_JOB = 241, + REMOTE_PROC_DOMAIN_INTERFACE_LINK_SET_STATE = 242, + REMOTE_PROC_DOMAIN_INTERFACE_LINK_GET_STATE = 243, }; -- 1.7.6
--- src/conf/domain_conf.c | 19 +++++++++++++++++++ src/conf/domain_conf.h | 1 + 2 files changed, 20 insertions(+), 0 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 010ce57..053f324 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2748,6 +2748,7 @@ virDomainNetDefParseXML(virCapsPtr caps, char *internal = NULL; char *devaddr = NULL; char *mode = NULL; + char *linkstate = NULL; virNWFilterHashTablePtr filterparams = NULL; virVirtualPortProfileParamsPtr virtPort = NULL; virDomainActualNetDefPtr actual = NULL; @@ -2824,6 +2825,9 @@ virDomainNetDefParseXML(virCapsPtr caps, /* An auto-generated target name, blank it out */ VIR_FREE(ifname); } + } else if ((linkstate == NULL) && + xmlStrEqual(cur->name, BAD_CAST "link")) { + linkstate = virXMLPropString(cur, "state"); } else if ((script == NULL) && (def->type == VIR_DOMAIN_NET_TYPE_ETHERNET || def->type == VIR_DOMAIN_NET_TYPE_BRIDGE) && @@ -3077,6 +3081,14 @@ virDomainNetDefParseXML(virCapsPtr caps, } } + def->linkstate = VIR_LINK_STATE_DEFAULT; + if (linkstate != NULL) { + if (STREQ(linkstate, "down")) + def->linkstate = VIR_LINK_STATE_DOWN; + else + def->linkstate = VIR_LINK_STATE_UP; + } + if (filter != NULL) { switch (def->type) { case VIR_DOMAIN_NET_TYPE_ETHERNET: @@ -3123,6 +3135,7 @@ cleanup: VIR_FREE(internal); VIR_FREE(devaddr); VIR_FREE(mode); + VIR_FREE(linkstate); virNWFilterHashTableFree(filterparams); return def; @@ -9019,6 +9032,12 @@ virDomainNetDefFormat(virBufferPtr buf, virBufferAddLit(buf, " </tune>\n"); } + + if (def->linkstate == VIR_LINK_STATE_DOWN) + virBufferAddLit(buf, " <link state='down'/>\n"); + if (def->linkstate == VIR_LINK_STATE_UP) + virBufferAddLit(buf, " <link state='up'/>\n"); + if (virBandwidthDefFormat(buf, def->bandwidth, " ") < 0) return -1; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index abf9cbd..4655563 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -427,6 +427,7 @@ struct _virDomainNetDef { char *filter; virNWFilterHashTablePtr filterparams; virBandwidthPtr bandwidth; + unsigned int linkstate; }; /* Used for prefix of ifname of any network name generated dynamically -- 1.7.6
On 08/11/2011 09:27 AM, Peter Krempa wrote:
--- src/conf/domain_conf.c | 19 +++++++++++++++++++ src/conf/domain_conf.h | 1 + 2 files changed, 20 insertions(+), 0 deletions(-)
This patch is useful, even without the new API.
@@ -3077,6 +3081,14 @@ virDomainNetDefParseXML(virCapsPtr caps, } }
+ def->linkstate = VIR_LINK_STATE_DEFAULT; + if (linkstate != NULL) { + if (STREQ(linkstate, "down")) + def->linkstate = VIR_LINK_STATE_DOWN; + else + def->linkstate = VIR_LINK_STATE_UP;
Do we really want to convert all other strings to STATE_UP, or should we specifically check for "up" and reject unknown strings?
@@ -9019,6 +9032,12 @@ virDomainNetDefFormat(virBufferPtr buf, virBufferAddLit(buf, "</tune>\n"); }
+ + if (def->linkstate == VIR_LINK_STATE_DOWN) + virBufferAddLit(buf, "<link state='down'/>\n"); + if (def->linkstate == VIR_LINK_STATE_UP)
This could be 'else if', but not a big deal to micro-optimize.
+ virBufferAddLit(buf, "<link state='up'/>\n"); + if (virBandwidthDefFormat(buf, def->bandwidth, " ")< 0) return -1;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index abf9cbd..4655563 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -427,6 +427,7 @@ struct _virDomainNetDef { char *filter; virNWFilterHashTablePtr filterparams; virBandwidthPtr bandwidth; + unsigned int linkstate;
Why unsigned? As currently used, you could get away with a bool. Would it be better to introduce a VIR_ENUM for link states, especially if we ever think it might be worth adding other link states? (I know that some 10/100/1000 NIC hardware has a state representing autonegotiation, where a remote end has been detected but where the link has not yet settled on which speed to use, although I have no idea if that hardware state is ever exposed to the userspace by the kernel, let alone something that can be emulated). If you do use VIR_ENUM, then our convention has been to use 'int', not 'unsigned int'. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
--- tools/virsh.c | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 11 +++++ 2 files changed, 131 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 1d660d0..b4c6e21 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1161,6 +1161,124 @@ cmdDomIfstat (vshControl *ctl, const vshCmd *cmd) return true; } +/* "domif-setlink" command + */ +static const vshCmdInfo info_domif_setlink[] = { + {"help", N_("set link state of a virtual interface")}, + {"desc", N_("Set link state of a domain's virtual interface.")}, + {NULL,NULL} +}; + +static const vshCmdOptDef opts_domif_setlink[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"interface", VSH_OT_DATA, VSH_OFLAG_REQ, N_("interface device (MAC Address)")}, + {"state", VSH_OT_DATA, VSH_OFLAG_REQ, N_("new state of the device")}, + {"persistent", VSH_OT_BOOL, 0, N_("persist interface state")}, + {NULL, 0, 0, NULL} +}; + +static bool +cmdDomIfSetLink (vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + const char *name = NULL, *device = NULL, *state_str = NULL; + int flags = VIR_DOMAIN_AFFECT_CURRENT; + int state = VIR_LINK_STATE_DEFAULT; + + + if (!vshConnectionUsability (ctl, ctl->conn)) + return false; + + if (!(dom = vshCommandOptDomain (ctl, cmd, &name))) + return false; + + if (vshCommandOptString (cmd, "interface", &device) <= 0) { + virDomainFree(dom); + return false; + } + + if (vshCommandOptString (cmd, "state", &state_str) <= 0) { + virDomainFree(dom); + return false; + } + + if (vshCommandOptBool(cmd, "persistent")) + flags = VIR_DOMAIN_AFFECT_CONFIG; + + /* parse state */ + if (STREQ(state_str, "up")) + state = VIR_LINK_STATE_UP; + else if (STREQ(state_str, "down")) + state = VIR_LINK_STATE_DOWN; + else { + vshError(ctl, _("Invalid state %s"), state_str); + virDomainFree(dom); + return false; + } + + if (virDomainInterfaceLinkSetState (dom, device, state, flags) == -1) { + vshError(ctl, _("Failed to set interface state %s: %s"), device, state_str); + virDomainFree(dom); + return false; + } + + virDomainFree(dom); + return true; +} + +/* "domif-getlink" command + */ +static const vshCmdInfo info_domif_getlink[] = { + {"help", N_("get link state of a virtual interface")}, + {"desc", N_("Get link state of a domain's virtual interface.")}, + {NULL,NULL} +}; + +static const vshCmdOptDef opts_domif_getlink[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"interface", VSH_OT_DATA, VSH_OFLAG_REQ, N_("interface device (MAC Address)")}, + {"persistent", VSH_OT_BOOL, 0, N_("Get persistent interface state")}, + {NULL, 0, 0, NULL} +}; + +static bool +cmdDomIfGetLink (vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + const char *name = NULL, *device = NULL; + int flags = VIR_DOMAIN_AFFECT_CURRENT; + int state; + + + if (!vshConnectionUsability (ctl, ctl->conn)) + return false; + + if (!(dom = vshCommandOptDomain (ctl, cmd, &name))) + return false; + + if (vshCommandOptString (cmd, "interface", &device) <= 0) { + virDomainFree(dom); + return false; + } + + if (vshCommandOptBool(cmd, "persistent")) + flags = VIR_DOMAIN_AFFECT_CONFIG; + + if ((state = virDomainInterfaceLinkGetState (dom, device, flags)) == -1) { + vshError(ctl, _("Failed to get interface state %s"), device); + virDomainFree(dom); + return false; + } + + if (state == VIR_LINK_STATE_DEFAULT || state == VIR_LINK_STATE_UP) + vshPrint(ctl, _("Interface %s is up.\n"), device); + else + vshPrint(ctl, _("Interface %s is down.\n"), device); + + virDomainFree(dom); + return true; +} + /* * "dommemstats" command */ @@ -12521,6 +12639,7 @@ static const vshCmdDef domManagementCmds[] = { {"detach-interface", cmdDetachInterface, opts_detach_interface, info_detach_interface, 0}, {"domid", cmdDomid, opts_domid, info_domid, 0}, + {"domif-setlink", cmdDomIfSetLink, opts_domif_setlink, info_domif_setlink, 0}, {"domjobabort", cmdDomjobabort, opts_domjobabort, info_domjobabort, 0}, {"domjobinfo", cmdDomjobinfo, opts_domjobinfo, info_domjobinfo, 0}, {"domname", cmdDomname, opts_domname, info_domname, 0}, @@ -12578,6 +12697,7 @@ static const vshCmdDef domMonitoringCmds[] = { {"domblkinfo", cmdDomblkinfo, opts_domblkinfo, info_domblkinfo, 0}, {"domblkstat", cmdDomblkstat, opts_domblkstat, info_domblkstat, 0}, {"domcontrol", cmdDomControl, opts_domcontrol, info_domcontrol, 0}, + {"domif-getlink", cmdDomIfGetLink, opts_domif_getlink, info_domif_getlink, 0}, {"domifstat", cmdDomIfstat, opts_domifstat, info_domifstat, 0}, {"dominfo", cmdDominfo, opts_dominfo, info_dominfo, 0}, {"dommemstat", cmdDomMemStat, opts_dommemstat, info_dommemstat, 0}, diff --git a/tools/virsh.pod b/tools/virsh.pod index a6af1e6..915fc6d 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -422,6 +422,17 @@ Get device block stats for a running domain. Get network interface stats for a running domain. +=item B<domif-setlink> I<domain> I<interface-MAC> I<state> I<--persistent> + +Modify link state of the domain's virtual interface. Possible values for +state are "up" and "down. If --persistent is specified, modify +persistent configuration of the domain. + +=item B<domif-setlink> I<domain> I<interface-MAC> I<--persistent> + +Query link state of the domain's virtual interface. If --persistent +is specified, query the persistent configuration. + =item B<dommemstat> I<domain> Get memory stats for a running domain. -- 1.7.6
On 08/11/2011 09:27 AM, Peter Krempa wrote:
--- tools/virsh.c | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 11 +++++ 2 files changed, 131 insertions(+), 0 deletions(-)
Again, I think the new virsh commands can be useful, but would need to map on top of virDomainUpdateDevice rather than new API, meaning this patch needs a v2.
+static const vshCmdOptDef opts_domif_setlink[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"interface", VSH_OT_DATA, VSH_OFLAG_REQ, N_("interface device (MAC Address)")}, + {"state", VSH_OT_DATA, VSH_OFLAG_REQ, N_("new state of the device")}, + {"persistent", VSH_OT_BOOL, 0, N_("persist interface state")},
Do we want to go with our usual --live/--config/--current? That is, I can change the link state of the running VM, of the next boot of the VM, or of whichever state the VM is in (whether running or inactive)? Especially since other virsh commands around virDomainUpdateDevice expose those three flags. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Adds support for QMP and text monitor access to qemu. --- src/qemu/qemu_monitor.c | 19 +++++++++++++++++ src/qemu/qemu_monitor.h | 3 ++ src/qemu/qemu_monitor_json.c | 23 ++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 4 +++ src/qemu/qemu_monitor_text.c | 47 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_text.h | 4 +++ 6 files changed, 100 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index db6107c..53a0ce3 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1135,6 +1135,25 @@ int qemuMonitorGetCPUInfo(qemuMonitorPtr mon, return ret; } +int qemuMonitorSetLink(qemuMonitorPtr mon, + const char *name, + unsigned int state) +{ + int ret; + VIR_DEBUG("mon=%p, name=%p:%s, state=%u", mon, name, name, state); + + if (!mon || !name) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor || name must not be NULL")); + return -1; + } + + if (mon->json) + ret = qemuMonitorJSONSetLink(mon, name, state); + else + ret = qemuMonitorTextSetLink(mon, name, state); + return ret; +} int qemuMonitorGetVirtType(qemuMonitorPtr mon, int *virtType) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index f241c9e..a42313c 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -145,6 +145,9 @@ void qemuMonitorUnlock(qemuMonitorPtr mon); int qemuMonitorRef(qemuMonitorPtr mon); int qemuMonitorUnref(qemuMonitorPtr mon) ATTRIBUTE_RETURN_CHECK; +int qemuMonitorSetLink(qemuMonitorPtr mon, const char *name, + unsigned int state) ; + /* These APIs are for use by the internal Text/JSON monitor impl code only */ char *qemuMonitorNextCommandID(qemuMonitorPtr mon); int qemuMonitorSend(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 2a9a078..f2f8dd5 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -955,6 +955,29 @@ int qemuMonitorJSONSystemPowerdown(qemuMonitorPtr mon) return ret; } +int qemuMonitorJSONSetLink(qemuMonitorPtr mon, + const char *name, + unsigned int state) +{ + + int ret; + virJSONValuePtr reply = NULL; + virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("set_link", + "s:name", name, + "b:up", state==VIR_LINK_STATE_DOWN?false:true, + NULL); + + if (!cmd) + return -1; + + if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + virJSONValueFree(cmd); + virJSONValueFree(reply); + + return ret; +} int qemuMonitorJSONSystemReset(qemuMonitorPtr mon) { diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 9512793..a029f5d 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -231,4 +231,8 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon, virDomainBlockJobInfoPtr info, int mode); +int qemuMonitorJSONSetLink(qemuMonitorPtr mon, + const char *name, + unsigned int state); + #endif /* QEMU_MONITOR_JSON_H */ diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 7bf733d..2739c50 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -416,6 +416,53 @@ int qemuMonitorTextSystemPowerdown(qemuMonitorPtr mon) { return 0; } +int qemuMonitorTextSetLink(qemuMonitorPtr mon, const char *name, unsigned int state) { + char *info = NULL; + char *cmd = NULL; + const char *st_str = NULL; + + /* determine state */ + if (state == VIR_LINK_STATE_DEFAULT || + state == VIR_LINK_STATE_UP) + st_str = "on"; + else + st_str = "off"; + + if (virAsprintf(&cmd, "set_link %s %s", name, st_str) < 0) { + virReportOOMError(); + goto error; + } + if (qemuMonitorHMPCommand(mon, cmd, &info) < 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + "%s", _("set_link operation failed")); + goto error; + } + + /* check if set_link command is supported */ + if (strstr(info, "\nunknown ")) { + qemuReportError(VIR_ERR_NO_SUPPORT, + "%s", + _("\'set_link\' not supported by this qemu")); + goto error; + } + + /* check if qemu didn't reject device name */ + if (strstr(info, "\nDevice ")) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + "%s", _("device name rejected")); + goto error; + } + + VIR_FREE(info); + VIR_FREE(cmd); + return 0; + +error: + VIR_FREE(info); + VIR_FREE(cmd); + + return -1; +} int qemuMonitorTextSystemReset(qemuMonitorPtr mon) { char *info; diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index b250738..43fc69d 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -224,4 +224,8 @@ int qemuMonitorTextBlockJob(qemuMonitorPtr mon, virDomainBlockJobInfoPtr info, int mode); +int qemuMonitorTextSetLink(qemuMonitorPtr mon, + const char *name, + unsigned int state); + #endif /* QEMU_MONITOR_TEXT_H */ -- 1.7.6
On 08/11/2011 09:27 AM, Peter Krempa wrote:
Adds support for QMP and text monitor access to qemu. --- src/qemu/qemu_monitor.c | 19 +++++++++++++++++ src/qemu/qemu_monitor.h | 3 ++ src/qemu/qemu_monitor_json.c | 23 ++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 4 +++ src/qemu/qemu_monitor_text.c | 47 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_text.h | 4 +++ 6 files changed, 100 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index db6107c..53a0ce3 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1135,6 +1135,25 @@ int qemuMonitorGetCPUInfo(qemuMonitorPtr mon, return ret; }
+int qemuMonitorSetLink(qemuMonitorPtr mon, + const char *name, + unsigned int state)
If you use VIR_ENUM in domain_conf.h to define the valid link states, then use that enum type here instead of unsigned int.
+int qemuMonitorJSONSetLink(qemuMonitorPtr mon, + const char *name, + unsigned int state) +{ + + int ret; + virJSONValuePtr reply = NULL; + virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("set_link", + "s:name", name, + "b:up", state==VIR_LINK_STATE_DOWN?false:true,
I'm not a fan of cond?false:true; why not just: "b:up", state == VIR_LINK_STATE_UP, Looks nice in general, though. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Adds a capability flag based on version number to detect if qemu supports setting link state. --- src/qemu/qemu_capabilities.c | 5 +++++ src/qemu/qemu_capabilities.h | 1 + 2 files changed, 6 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 3f36212..d1823d0 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -123,6 +123,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, "pci-multifunction", /* 60 */ "virtio-blk-pci.ioeventfd", "sga", + "link-state", ); struct qemu_feature_flags { @@ -1033,6 +1034,10 @@ qemuCapsComputeCmdFlags(const char *help, if (version >= 13000) qemuCapsSet(flags, QEMU_CAPS_PCI_MULTIFUNCTION); + + /* link state modification appeared before 0.11.0*/ + if (version >= 11000) + qemuCapsSet(flags, QEMU_CAPS_LINK_SET); } /* We parse the output of 'qemu -help' to get the QEMU diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index d251262..10631f7 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -98,6 +98,7 @@ enum qemuCapsFlags { QEMU_CAPS_PCI_MULTIFUNCTION = 60, /* -device multifunction=on|off */ QEMU_CAPS_VIRTIO_IOEVENTFD = 61, /* IOeventFD feature: virtio-{net|blk}-pci.ioeventfd=on/off */ QEMU_CAPS_SGA = 62, /* Serial Graphics Adapter */ + QEMU_CAPS_LINK_SET = 63, /* Modification of link state */ QEMU_CAPS_LAST, /* this must always be the last item */ }; -- 1.7.6
On 08/11/2011 09:27 AM, Peter Krempa wrote:
Adds a capability flag based on version number to detect if qemu supports setting link state. --- src/qemu/qemu_capabilities.c | 5 +++++ src/qemu/qemu_capabilities.h | 1 + 2 files changed, 6 insertions(+), 0 deletions(-)
No testsuite changes to validate that the new capability is detected?
@@ -1033,6 +1034,10 @@ qemuCapsComputeCmdFlags(const char *help,
if (version>= 13000) qemuCapsSet(flags, QEMU_CAPS_PCI_MULTIFUNCTION); + + /* link state modification appeared before 0.11.0*/ + if (version>= 11000) + qemuCapsSet(flags, QEMU_CAPS_LINK_SET);
Version checks are not nice (they are okay as a last resort, though). Is there is anything nicer that we could use? Also, how do you plan to set link=down at boot from the command line, or can link state only be changed via monitor command? -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Adds support for the new API. While starting a qemu link state cannot be set with an command line argument and therefore is done by entering monitor. --- src/qemu/qemu_driver.c | 228 +++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_process.c | 37 ++++++++ 2 files changed, 265 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 19e749f..a6a76be 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6887,6 +6887,232 @@ qemudDomainInterfaceStats (virDomainPtr dom, #endif static int +qemudDomainInterfaceLinkGetState (virDomainPtr dom, + const char *path, + unsigned int flags) +{ + struct qemud_driver *driver = dom->conn->privateData; + virDomainObjPtr vm; + int i; + int ret = -1; + unsigned char mac[VIR_MAC_BUFLEN]; + virDomainDefPtr def = NULL; + + virCheckFlags(VIR_DOMAIN_AFFECT_CURRENT | + VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + + qemuDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemuDriverUnlock(driver); + + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemuReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + if (virParseMacAddr(path, mac) < 0) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("Couldn't parse MAC address: %s"), path); + goto cleanup; + } + + if (virDomainObjIsActive(vm)) { + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + if (!(def = virDomainObjGetPersistentDef(driver->caps, vm))) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("Couldn't get persistent domain configuration")); + goto cleanup; + } + } else { + def = vm->def; + } + } else { + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + if (!(def = virDomainObjGetPersistentDef(driver->caps, vm))) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("Couldn't get persistent domain configuration")); + goto cleanup; + } + } + + /* Check the path is one of the domain's network interfaces. */ + /* Qemu does not support reading the link state, report saved state */ + for (i = 0 ; i < def->nnets ; i++) { + if (memcmp(def->nets[i]->mac, mac, VIR_MAC_BUFLEN) == 0) { + ret = def->nets[i]->linkstate; + break; + } + } + + if (ret < 0) + qemuReportError(VIR_ERR_INVALID_ARG, + _("invalid path, '%s' is not a known interface"), path); + +cleanup: + if (vm) + virDomainObjUnlock(vm); + return ret; + +} + +static int +qemudDomainInterfaceLinkSetState (virDomainPtr dom, + const char *path, + unsigned int state, + unsigned int flags) +{ + struct qemud_driver *driver = dom->conn->privateData; + qemuDomainObjPrivatePtr priv = NULL; + virDomainObjPtr vm; + virDomainDefPtr def = NULL; + virDomainDefPtr persistentDef = NULL; + + bool isActive; + int i; + int ret = -1; + bool found = false; + unsigned char mac[VIR_MAC_BUFLEN]; + + virCheckFlags(VIR_DOMAIN_AFFECT_CURRENT | + VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + + qemuDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemuDriverUnlock(driver); + + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemuReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + priv = vm->privateData; + + if (virParseMacAddr(path, mac) < 0) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("Couldn't parse MAC address: %s"), path); + goto cleanup; + } + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + + isActive = virDomainObjIsActive(vm); + + if (flags == VIR_DOMAIN_AFFECT_CURRENT) { + if (isActive) + flags |= VIR_DOMAIN_AFFECT_LIVE; + else + flags |= VIR_DOMAIN_AFFECT_CONFIG; + } + + if (!isActive && (flags & VIR_DOMAIN_AFFECT_LIVE)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + _("domain is not running")); + goto endjob; + } + + if (!vm->persistent && (flags & VIR_DOMAIN_AFFECT_CONFIG)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + _("cannot change persistent config of a transient domain")); + goto endjob; + } + + def = vm->def; + if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm))) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("Couldn't get persistent domain configuration")); + goto endjob; + } + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + for (i = 0; i < persistentDef->nnets; i++) { + if (memcmp(persistentDef->nets[i]->mac, mac, VIR_MAC_BUFLEN) == 0) { + found = true; + persistentDef->nets[i]->linkstate = state; + break; + } + } + + if (!found) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("Interface %s not found"), path); + goto endjob; + } + } + + found = false; + + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + for (i = 0; i < def->nnets; i++) { + if (memcmp(def->nets[i]->mac, mac, VIR_MAC_BUFLEN) == 0) { + if (def->nets[i]->info.alias) { + + if (!qemuCapsGet(priv->qemuCaps, QEMU_CAPS_LINK_SET)) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("This qemu doesn't support setting link state")); + goto endjob; + } + + qemuDomainObjEnterMonitor(driver, vm); + ret = qemuMonitorSetLink(priv->mon, def->nets[i]->info.alias, state); + qemuDomainObjExitMonitor(driver, vm); + } else { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("Device alias not found. (QEMU probably doesn't support device naming)")); + goto endjob; + } + + found = true; + if (ret == 0) + def->nets[i]->linkstate = state; + + break; + } + } + + if (!found) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("Interface %s not found"), path); + goto endjob; + } + + if (ret < 0) + goto endjob; + } + + /* one or both configurations successfuly altered */ + + if ((flags & VIR_DOMAIN_AFFECT_CONFIG) && + virDomainSaveConfig(driver->configDir, persistentDef) < 0) + goto endjob; + + /* everything went well */ + ret = 0; + +endjob: + if (qemuDomainObjEndJob(driver, vm) == 0) + vm = NULL; + +cleanup: + if (vm) + virDomainObjUnlock(vm); + + return ret; +} + +static int qemudDomainMemoryStats (virDomainPtr dom, struct _virDomainMemoryStat *stats, unsigned int nr_stats, @@ -9473,6 +9699,8 @@ static virDriver qemuDriver = { .domainMigratePerform = qemudDomainMigratePerform, /* 0.5.0 */ .domainBlockStats = qemudDomainBlockStats, /* 0.4.1 */ .domainInterfaceStats = qemudDomainInterfaceStats, /* 0.4.1 */ + .domainInterfaceLinkGetState = qemudDomainInterfaceLinkGetState, /* 0.9.5 */ + .domainInterfaceLinkSetState = qemudDomainInterfaceLinkSetState, /* 0.9.5 */ .domainMemoryStats = qemudDomainMemoryStats, /* 0.7.5 */ .domainBlockPeek = qemudDomainBlockPeek, /* 0.4.4 */ .domainMemoryPeek = qemudDomainMemoryPeek, /* 0.4.4 */ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 30c8b28..c0bf30b 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1441,6 +1441,29 @@ qemuProcessInitCpuAffinity(virDomainObjPtr vm) return 0; } +/* set link states to down on interfaces at qemu start */ +static int +qemuProcessSetLinkStates(virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainDefPtr def = vm->def; + int i; + int ret = 0; + + for (i = 0; i < def->nnets; i++) { + if (def->nets[i]->linkstate == VIR_LINK_STATE_DOWN) { + ret = qemuMonitorSetLink(priv->mon, + def->nets[i]->info.alias, + VIR_LINK_STATE_DOWN); + + if (ret != 0) + break; + } + } + + return ret; +} + /* Set CPU affinites for vcpus if vcpupin xml provided. */ static int qemuProcessSetVcpuAffinites(virConnectPtr conn, @@ -2980,6 +3003,20 @@ int qemuProcessStart(virConnectPtr conn, goto cleanup; } + /* set default link states */ + /* qemu doesn't support setting this on the command line, so + * enter the monitor */ + if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_LINK_SET)) { + VIR_DEBUG("Setting network link states"); + qemuDomainObjEnterMonitorWithDriver(driver, vm); + if (qemuProcessSetLinkStates(vm) < 0) { + qemuDomainObjExitMonitorWithDriver(driver, vm); + goto cleanup; + } + + qemuDomainObjExitMonitorWithDriver(driver, vm); + } + /* Technically, qemuProcessStart can be called from inside * QEMU_ASYNC_JOB_MIGRATION_IN, but we are okay treating this like * a sync job since no other job can call into the domain until -- 1.7.6
On 08/11/2011 09:27 AM, Peter Krempa wrote:
Adds support for the new API. While starting a qemu link state cannot be set with an command line argument and therefore is done by entering monitor. --- src/qemu/qemu_driver.c | 228 +++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_process.c | 37 ++++++++ 2 files changed, 265 insertions(+), 0 deletions(-)
Needs rework to be called as part of virDomainUpdateDevice, but you can reuse a lot of the non-boilerplate code for that purpose.
+ + /* Check the path is one of the domain's network interfaces. */ + /* Qemu does not support reading the link state, report saved state */
Don't you just love write-only interfaces :) How does link state fare across migration?
+ if (flags& VIR_DOMAIN_AFFECT_LIVE) { + for (i = 0; i< def->nnets; i++) { + if (memcmp(def->nets[i]->mac, mac, VIR_MAC_BUFLEN) == 0) { + if (def->nets[i]->info.alias) { + + if (!qemuCapsGet(priv->qemuCaps, QEMU_CAPS_LINK_SET)) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("This qemu doesn't support setting link state")); + goto endjob; + } + + qemuDomainObjEnterMonitor(driver, vm); + ret = qemuMonitorSetLink(priv->mon, def->nets[i]->info.alias, state); + qemuDomainObjExitMonitor(driver, vm); + } else { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("Device alias not found. (QEMU probably doesn't support device naming)"));
Don't we have a QEMU_CAPS that says whether we have -device (and therefore device naming)? In fact, if settable link state was introduced the same time as -device, then we might not need a new QEMU_CAPS_LINK_SET (but I'd have to research when things appeared).
+ + /* one or both configurations successfuly altered */
s/successfuly/successfully/; although if you hook into virDomainUpdateDevice code properly, this is probably boilerplate that you don't have to worry about.
+/* set link states to down on interfaces at qemu start */ +static int +qemuProcessSetLinkStates(virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainDefPtr def = vm->def; + int i; + int ret = 0; + + for (i = 0; i< def->nnets; i++) { + if (def->nets[i]->linkstate == VIR_LINK_STATE_DOWN) { + ret = qemuMonitorSetLink(priv->mon, + def->nets[i]->info.alias, + VIR_LINK_STATE_DOWN); + + if (ret != 0) + break; + } + } + + return ret; +} + /* Set CPU affinites for vcpus if vcpupin xml provided. */
As long as you are touching near this code: s/affinites/affinities/
+ /* set default link states */ + /* qemu doesn't support setting this on the command line, so + * enter the monitor */
Oh, well that answers my earlier question about why you didn't change qemu_command.c.
+ if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_LINK_SET)) { + VIR_DEBUG("Setting network link states"); + qemuDomainObjEnterMonitorWithDriver(driver, vm); + if (qemuProcessSetLinkStates(vm)< 0) { + qemuDomainObjExitMonitorWithDriver(driver, vm); + goto cleanup; + }
I don't think you quite got the logic right. If qemu lacks the capability, we still need to inspect all interfaces, and issue an error if any of them requested link down (if they all lacked <link>, or requested link up, then things are okay), rather than silently ignoring requests for link down when unsupported by qemu. Overall, I think the feature addition of being able to simulate link down will be nice to have, but the series needs some polish first :) -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Laine Stump -
Peter Krempa