9:20 a.m.
v2
Michal Privoznik (4):
libvirt_private.syms: Expose virDomainNetTypeFromString
virsh attach-interface: Use enum instead of arbitrary integers
virsh attach-interface: Use virDomainNetType{From,To}String()
virsh attach-interface: Allow macvtap hotplug
src/libvirt_private.syms | 1 +
tools/virsh-domain.c | 36 ++++++++++++++++++++++++++----------
tools/virsh.pod | 14 ++++++++------
3 files changed, 35 insertions(+), 16 deletions(-)
--
2.0.5
9:20 a.m.
New subject: [libvirt] [PATCH v2 1/4] libvirt_private.syms: Expose virDomainNetTypeFromString
The enum converters are defined in the domain_conf.h (so
accessible widely across the code), but on the symbol layer, only
virDomainNetTypeToString was exposed. However, FromString variant
is going to be needed shortly.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/libvirt_private.syms | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 376c69b..9cb81dd 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -343,6 +343,7 @@ virDomainNetGetActualVlan;
virDomainNetInsert;
virDomainNetRemove;
virDomainNetRemoveHostdev;
+virDomainNetTypeFromString;
virDomainNetTypeToString;
virDomainNostateReasonTypeFromString;
virDomainNostateReasonTypeToString;
--
2.0.5
9:20 a.m.
New subject: [libvirt] [PATCH v2 2/4] virsh attach-interface: Use enum instead of arbitrary integers
The type of interface to attach is held in the variable 'typ'.
Depending on interface type selected by user, the variable is set
either to 1 (network), or 2 (bridge). Lets use already existing
enum from domain_conf.h instead: virDomainNetType.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tools/virsh-domain.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 358d61c..201a1b1 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -906,7 +906,7 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
*type = NULL, *source = NULL, *model = NULL,
*inboundStr = NULL, *outboundStr = NULL;
virNetDevBandwidthRate inbound, outbound;
- int typ;
+ virDomainNetType typ;
int ret;
bool functionReturn = false;
virBuffer buf = VIR_BUFFER_INITIALIZER;
@@ -946,9 +946,9 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
/* check interface type */
if (STREQ(type, "network")) {
- typ = 1;
+ typ = VIR_DOMAIN_NET_TYPE_NETWORK;
} else if (STREQ(type, "bridge")) {
- typ = 2;
+ typ = VIR_DOMAIN_NET_TYPE_BRIDGE;
} else {
vshError(ctl, _("No support for %s in command
'attach-interface'"),
type);
@@ -982,9 +982,9 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
virBufferAsprintf(&buf, "<interface type='%s'>\n",
type);
virBufferAdjustIndent(&buf, 2);
- if (typ == 1)
+ if (typ == VIR_DOMAIN_NET_TYPE_NETWORK)
virBufferAsprintf(&buf, "<source network='%s'/>\n",
source);
- else if (typ == 2)
+ else if (typ == VIR_DOMAIN_NET_TYPE_BRIDGE)
virBufferAsprintf(&buf, "<source bridge='%s'/>\n",
source);
if (target != NULL)
--
2.0.5
9:20 a.m.
New subject: [libvirt] [PATCH v2 3/4] virsh attach-interface: Use virDomainNetType{From, To}String()
Instead of verbose string to enum conversion (if STREQ() else if
STREQ() else if STREQ() ...) lets use virDomainNetType{From,To}String.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tools/virsh-domain.c | 32 +++++++++++++++++++++++---------
1 file changed, 23 insertions(+), 9 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 201a1b1..117c52b 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -945,11 +945,7 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
/* check interface type */
- if (STREQ(type, "network")) {
- typ = VIR_DOMAIN_NET_TYPE_NETWORK;
- } else if (STREQ(type, "bridge")) {
- typ = VIR_DOMAIN_NET_TYPE_BRIDGE;
- } else {
+ if ((int)(typ = virDomainNetTypeFromString(type)) < 0) {
vshError(ctl, _("No support for %s in command
'attach-interface'"),
type);
goto cleanup;
@@ -982,10 +978,28 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
virBufferAsprintf(&buf, "<interface type='%s'>\n",
type);
virBufferAdjustIndent(&buf, 2);
- if (typ == VIR_DOMAIN_NET_TYPE_NETWORK)
- virBufferAsprintf(&buf, "<source network='%s'/>\n",
source);
- else if (typ == VIR_DOMAIN_NET_TYPE_BRIDGE)
- virBufferAsprintf(&buf, "<source bridge='%s'/>\n",
source);
+ switch (typ) {
+ case VIR_DOMAIN_NET_TYPE_NETWORK:
+ case VIR_DOMAIN_NET_TYPE_BRIDGE:
+ virBufferAsprintf(&buf, "<source %s='%s'/>\n",
+ virDomainNetTypeToString(typ), source);
+ break;
+
+ case VIR_DOMAIN_NET_TYPE_USER:
+ case VIR_DOMAIN_NET_TYPE_ETHERNET:
+ case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
+ case VIR_DOMAIN_NET_TYPE_SERVER:
+ case VIR_DOMAIN_NET_TYPE_CLIENT:
+ case VIR_DOMAIN_NET_TYPE_MCAST:
+ case VIR_DOMAIN_NET_TYPE_INTERNAL:
+ case VIR_DOMAIN_NET_TYPE_DIRECT:
+ case VIR_DOMAIN_NET_TYPE_HOSTDEV:
+ case VIR_DOMAIN_NET_TYPE_LAST:
+ vshError(ctl, _("No support for %s in command
'attach-interface'"),
+ type);
+ goto cleanup;
+ break;
+ }
if (target != NULL)
virBufferAsprintf(&buf, "<target dev='%s'/>\n",
target);
--
2.0.5
9:20 a.m.
New subject: [libvirt] [PATCH v2 4/4] virsh attach-interface: Allow macvtap hotplug
Our hotplug code supports macvtap insertion to guests. However, we
somehow forgot about 'attach-interface' (which tries to build XML from
passed arguments and use virDomainAttachDeviceFlags()).
New type is accessible under 'direct' type, to keep the same type as
used in domain XML.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tools/virsh-domain.c | 4 +++-
tools/virsh.pod | 14 ++++++++------
2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 117c52b..13c0b83 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -984,6 +984,9 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
virBufferAsprintf(&buf, "<source %s='%s'/>\n",
virDomainNetTypeToString(typ), source);
break;
+ case VIR_DOMAIN_NET_TYPE_DIRECT:
+ virBufferAsprintf(&buf, "<source dev='%s'/>\n",
source);
+ break;
case VIR_DOMAIN_NET_TYPE_USER:
case VIR_DOMAIN_NET_TYPE_ETHERNET:
@@ -992,7 +995,6 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
case VIR_DOMAIN_NET_TYPE_CLIENT:
case VIR_DOMAIN_NET_TYPE_MCAST:
case VIR_DOMAIN_NET_TYPE_INTERNAL:
- case VIR_DOMAIN_NET_TYPE_DIRECT:
case VIR_DOMAIN_NET_TYPE_HOSTDEV:
case VIR_DOMAIN_NET_TYPE_LAST:
vshError(ctl, _("No support for %s in command
'attach-interface'"),
diff --git a/tools/virsh.pod b/tools/virsh.pod
index a3f527f..50de32c 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2391,12 +2391,14 @@ Likewise, I<--shareable> is an alias for I<--mode
shareable>.
[I<--target target>] [I<--mac mac>] [I<--script script>] [I<--model
model>]
[I<--config>] [I<--inbound average,peak,burst>] [I<--outbound
average,peak,burst>]
-Attach a new network interface to the domain. I<type> can be either
-I<network> to indicate connection via a libvirt virtual network or
-I<bridge> to indicate connection via a bridge device on the host.
-I<source> indicates the source of the connection (either the name of a
-network, or of a bridge device). I<target> is used to specify the
-tap/macvtap device to be used to connect the domain to the
+Attach a new network interface to the domain. I<type> can be
+I<network> to indicate connection via a libvirt virtual network, or
+I<bridge> to indicate connection via a bridge device on the host, or
+I<direct> to indicate connection directly to one of the host's network
+interfaces or bridges. I<source> indicates the source of the
+connection (the name of a network, or of a bridge device, or the
+host's network interfaces or bridges). I<target> is used to specify
+the tap/macvtap device to be used to connect the domain to the
source. Names starting with 'vnet' are considered as auto-generated
and are blanked out/regenerated each time the interface is attached.
I<mac> specifies the MAC address of the network interface; if a MAC
--
2.0.5
3:50 p.m.
On 02/09/2015 10:20 AM, Michal Privoznik wrote:
v2
Michal Privoznik (4):
libvirt_private.syms: Expose virDomainNetTypeFromString
virsh attach-interface: Use enum instead of arbitrary integers
virsh attach-interface: Use virDomainNetType{From,To}String()
virsh attach-interface: Allow macvtap hotplug
src/libvirt_private.syms | 1 +
tools/virsh-domain.c | 36 ++++++++++++++++++++++++++----------
tools/virsh.pod | 14 ++++++++------
3 files changed, 35 insertions(+), 16 deletions(-)
ACK series...
If you want to merge 1-3 into just one patch that'd be fine too... At
the very least 1&3 are related since the From API isn't used until 3.
John
6:55 a.m.
On 02/12/2015 04:50 PM, John Ferlan wrote:
On 02/09/2015 10:20 AM, Michal Privoznik wrote:
> v2
>
> Michal Privoznik (4):
> libvirt_private.syms: Expose virDomainNetTypeFromString
> virsh attach-interface: Use enum instead of arbitrary integers
> virsh attach-interface: Use virDomainNetType{From,To}String()
> virsh attach-interface: Allow macvtap hotplug
>
> src/libvirt_private.syms | 1 +
> tools/virsh-domain.c | 36 ++++++++++++++++++++++++++----------
> tools/virsh.pod | 14 ++++++++------
> 3 files changed, 35 insertions(+), 16 deletions(-)
>
ACK series...
I had thought that during V1 of this patch series there had been enough
sentiment about not pulling in private parts of libvirt internals that
this V2 (posted 2 minutes before the last comment in V1, so very
literally crossing in the inter-tubes) would be discarded in favor of
something that only used public APIs - the last word on the subject was
this:
https://www.redhat.com/archives/libvir-list/2015-February/msg00284.html
(Yes, I know there are other places where it's already done, but it
seems like a better idea to clean those up than to compound the pollution.)
8:23 a.m.
On Fri, Feb 13, 2015 at 07:55:48AM -0500, Laine Stump wrote:
On 02/12/2015 04:50 PM, John Ferlan wrote:
>
> On 02/09/2015 10:20 AM, Michal Privoznik wrote:
>> v2
>>
>> Michal Privoznik (4):
>> libvirt_private.syms: Expose virDomainNetTypeFromString
>> virsh attach-interface: Use enum instead of arbitrary integers
>> virsh attach-interface: Use virDomainNetType{From,To}String()
>> virsh attach-interface: Allow macvtap hotplug
>>
>> src/libvirt_private.syms | 1 +
>> tools/virsh-domain.c | 36 ++++++++++++++++++++++++++----------
>> tools/virsh.pod | 14 ++++++++------
>> 3 files changed, 35 insertions(+), 16 deletions(-)
>>
> ACK series...
I had thought that during V1 of this patch series there had been enough
sentiment about not pulling in private parts of libvirt internals that
this V2 (posted 2 minutes before the last comment in V1, so very
literally crossing in the inter-tubes) would be discarded in favor of
something that only used public APIs - the last word on the subject was
this:
https://www.redhat.com/archives/libvir-list/2015-February/msg00284.html
(Yes, I know there are other places where it's already done, but it
seems like a better idea to clean those up than to compound the pollution.)
Yeah, I'd prefer that in general, but this particular patch is only using
the trivial VIR_ENUM string/int conversion, so I'm willing to overlook it
for this. If the internal API being used were non-trivial then I'd say
we should look at the public API support.
Regards,
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 :|
3570
days inactive
3574
days old
7 comments
4 participants
participants (4)
-
Daniel P. Berrange -
John Ferlan -
Laine Stump -
Michal Privoznik