[libvirt] [PATCH 1/2] virsh: extend domif-{get, set}link command to accept target name as a parameter
Other virsh domifXXX commands can accept target name as a parameter to specify interface. From viewpoint of consistency, virsh domif-getlink command should accept target name as a parameter. This patch achieves this. Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com> --- tools/virsh.c | 25 ++++++++++++++++++------- tools/virsh.pod | 3 ++- 2 files changed, 20 insertions(+), 8 deletions(-) Index: libvirt/tools/virsh.c =================================================================== --- libvirt.orig/tools/virsh.c +++ libvirt/tools/virsh.c @@ -1509,7 +1509,10 @@ cmdDomIfGetLink (vshControl *ctl, const const char *iface = NULL; int flags = 0; char *state = NULL; - char *mac = NULL; + char *value = NULL; + unsigned char macaddr[VIR_MAC_BUFLEN]; + const char *element; + const char *attribute; bool ret = false; int i; char *desc; @@ -1552,27 +1555,35 @@ cmdDomIfGetLink (vshControl *ctl, const goto cleanup; } + if (virParseMacAddr(iface, macaddr) == 0) { + element = "mac"; + attribute = "address"; + } else { + element = "target"; + attribute = "dev"; + } + /* find interface with matching mac addr */ for (i = 0; i < obj->nodesetval->nodeNr; i++) { cur = obj->nodesetval->nodeTab[i]->children; while (cur) { if (cur->type == XML_ELEMENT_NODE && - xmlStrEqual(cur->name, BAD_CAST "mac")) { + xmlStrEqual(cur->name, BAD_CAST element)) { - mac = virXMLPropString(cur, "address"); + value = virXMLPropString(cur, attribute); - if (STRCASEEQ(mac, iface)){ - VIR_FREE(mac); + if (STRCASEEQ(value, iface)){ + VIR_FREE(value); goto hit; } - VIR_FREE(mac); + VIR_FREE(value); } cur = cur->next; } } - vshError(ctl, _("Interface with address '%s' not found."), iface); + vshError(ctl, _("Interface (%s: %s) not found."), element, iface); goto cleanup; hit: Index: libvirt/tools/virsh.pod =================================================================== --- libvirt.orig/tools/virsh.pod +++ libvirt/tools/virsh.pod @@ -475,10 +475,11 @@ Modify link state of the domain's virtua state are "up" and "down. If --persistent is specified, only the persistent configuration of the domain is modified. -=item B<domif-getlink> I<domain> I<interface-MAC> I<--persistent> +=item B<domif-getlink> I<domain> I<interface-device> I<--persistent> Query link state of the domain's virtual interface. If --persistent is specified, query the persistent configuration. +I<interface-device> can be the interface's target name or the MAC address. =item B<domiftune> I<domain> I<interface-device> [[I<--config>] [I<--live>] | [I<--current>]]
Other virsh domifXXX commands can accept target name as a parameter to specify interface. From viewpoint of consistency, virsh domif-setlink command should accept target name as a parameter. This patch achieves this. Signd-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com> --- tools/virsh.c | 25 ++++++++++++++++++------- tools/virsh.pod | 3 ++- 2 files changed, 20 insertions(+), 8 deletions(-) Index: libvirt/tools/virsh.c =================================================================== --- libvirt.orig/tools/virsh.c +++ libvirt/tools/virsh.c @@ -1344,8 +1344,11 @@ cmdDomIfSetLink (vshControl *ctl, const virDomainPtr dom; const char *iface; const char *state; - const char *mac; + const char *value; const char *desc; + unsigned char macaddr[VIR_MAC_BUFLEN]; + const char *element; + const char *attribute; bool persistent; bool ret = false; unsigned int flags = 0; @@ -1405,26 +1408,34 @@ cmdDomIfSetLink (vshControl *ctl, const goto cleanup; } + if (virParseMacAddr(iface, macaddr) == 0) { + element = "mac"; + attribute = "address"; + } else { + element = "target"; + attribute = "dev"; + } + /* find interface with matching mac addr */ for (i = 0; i < obj->nodesetval->nodeNr; i++) { cur = obj->nodesetval->nodeTab[i]->children; while (cur) { if (cur->type == XML_ELEMENT_NODE && - xmlStrEqual(cur->name, BAD_CAST "mac")) { - mac = virXMLPropString(cur, "address"); + xmlStrEqual(cur->name, BAD_CAST element)) { + value = virXMLPropString(cur, attribute); - if (STRCASEEQ(mac, iface)) { - VIR_FREE(mac); + if (STRCASEEQ(value, iface)) { + VIR_FREE(value); goto hit; } - VIR_FREE(mac); + VIR_FREE(value); } cur = cur->next; } } - vshError(ctl, _("interface with address '%s' not found"), iface); + vshError(ctl, _("interface (%s: %s) not found"), element, iface); goto cleanup; hit: Index: libvirt/tools/virsh.pod =================================================================== --- libvirt.orig/tools/virsh.pod +++ libvirt/tools/virsh.pod @@ -469,11 +469,12 @@ B<Explanation of fields> (fields appear Get network interface stats for a running domain. -=item B<domif-setlink> I<domain> I<interface-MAC> I<state> I<--persistent> +=item B<domif-setlink> I<domain> I<interface-device> 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, only the persistent configuration of the domain is modified. +I<interface-device> can be the interface's target name or the MAC address. =item B<domif-getlink> I<domain> I<interface-device> I<--persistent>
On 01/19/2012 11:38 PM, Taku Izumi wrote:
Other virsh domifXXX commands can accept target name as a parameter to specify interface. From viewpoint of consistency, virsh domif-setlink command should accept target name as a parameter. This patch achieves this.
Signd-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com> --- tools/virsh.c | 25 ++++++++++++++++++------- tools/virsh.pod | 3 ++- 2 files changed, 20 insertions(+), 8 deletions(-)
Index: libvirt/tools/virsh.c =================================================================== --- libvirt.orig/tools/virsh.c +++ libvirt/tools/virsh.c @@ -1344,8 +1344,11 @@ cmdDomIfSetLink (vshControl *ctl, const virDomainPtr dom; const char *iface; const char *state; - const char *mac; + const char *value; const char *desc; + unsigned char macaddr[VIR_MAC_BUFLEN]; + const char *element; + const char *attribute;
Same renaming exercise as in 1/2, and same ACK and push. Actually, I merged these into one patch, since the subject line of both patches implied that both functions were being modified at once. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
long subject line; I trimmed it to: virsh: let domif-{get,set}link take target name On 01/19/2012 11:30 PM, Taku Izumi wrote:
Other virsh domifXXX commands can accept target name as a parameter to specify interface. From viewpoint of consistency, virsh domif-getlink command should accept target name as a parameter. This patch achieves this.
+ unsigned char macaddr[VIR_MAC_BUFLEN]; + const char *element; + const char *attribute;
The libxml2 writers should be shot for polluting the global namespace with nice-to-use names: virsh.c: In function 'cmdDomIfGetLink': virsh.c:1515:17: error: declaration of 'attribute' shadows a global declaration [-Werror=shadow] /usr/include/libxml2/libxml/SAX.h:104:3: error: shadowed declaration is here [-Werror=shadow] libxml2-devel-2.7.8-6.fc16.x86_64 (and yes, I know that DV reads this list - the real problem is that we must still compile on RHEL 5, and therefore must use LIBXML_LEGACY_ENABLED - using the upstream library without back-compat does not suffer from pollution ;) I renamed the virsh variable to attr to get around that stupidity.
- if (STRCASEEQ(mac, iface)){ - VIR_FREE(mac); + if (STRCASEEQ(value, iface)){ + VIR_FREE(value);
As long as we're touching this, I added the missing space before {. ACK and pushed. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Taku Izumi