5:15 p.m.
Back to old thread with Laine Stump with message title:
"qemu: remove unnecessary setting of tap device online state"
I'm not tested ip and route assign in case of up/down link and
device update on the fly. But host side link status tested and worked
fine.
Vasiliy Tolstov (1):
qemu: allow to control host side link status of network device
docs/formatdomain.html.in | 21 +++++++++++++++++++++
docs/schemas/domaincommon.rng | 11 +++++++++++
src/conf/domain_conf.c | 28 ++++++++++++++++++++++++++++
src/conf/domain_conf.h | 1 +
src/qemu/qemu_hotplug.c | 17 +++++++++++++++++
src/qemu/qemu_interface.c | 8 ++++----
6 files changed, 82 insertions(+), 4 deletions(-)
--
2.9.3
5:15 p.m.
New subject: [libvirt] [PATCH] qemu: allow to control host side link status of network device
Signed-off-by: Vasiliy Tolstov <v.tolstov(a)selfip.ru>
---
docs/formatdomain.html.in | 21 +++++++++++++++++++++
docs/schemas/domaincommon.rng | 11 +++++++++++
src/conf/domain_conf.c | 28 ++++++++++++++++++++++++++++
src/conf/domain_conf.h | 1 +
src/qemu/qemu_hotplug.c | 17 +++++++++++++++++
src/qemu/qemu_interface.c | 8 ++++----
6 files changed, 82 insertions(+), 4 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 8c884f4af9cb..dd8e6a4afa99 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -5421,6 +5421,27 @@ qemu-kvm -net nic,model=? /dev/null
<span class="since">Since 0.9.5</span>
</p>
+ <h5><a name="elementHostLink">Modifying phisical link
state</a></h5>
+<pre>
+...
+<devices>
+ <interface type='ethernet'>
+ <source>
+ <b><link state='down'/></b>
+ <target dev='vnet0'/>
+ </interface>
+</devices>
+...</pre>
+
+ <p>
+ This element provides means of setting state of the phisical network interface.
+ Possible values for attribute <code>state</code> are
<code>up</code> and
+ <code>down</code>. If <code>down</code> is specified as the
value, the interface
+ put in down state. Default behavior if this element is unspecified is to have the
+ link state <code>up</code>.
+ <span class="since">Since 3.3.2</span>
+ </p>
+
<h5><a name="mtu">MTU configuration</a></h5>
<pre>
...
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 281309ec09da..89213d63b6e9 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2798,6 +2798,17 @@
All ip-related info for either the host or guest side of an interface
-->
<define name="interface-ip-info">
+ <optional>
+ <element name="link">
+ <attribute name="state">
+ <choice>
+ <value>up</value>
+ <value>down</value>
+ </choice>
+ </attribute>
+ <empty/>
+ </element>
+ </optional>
<zeroOrMore>
<element name="ip">
<attribute name="address">
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0ff216e3a373..b7398276af57 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -9606,6 +9606,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
char *devaddr = NULL;
char *mode = NULL;
char *linkstate = NULL;
+ char *hostlinkstate = NULL;
char *addrtype = NULL;
char *domain_name = NULL;
char *vhostuser_mode = NULL;
@@ -9654,6 +9655,10 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
if (virDomainNetIPInfoParseXML(_("interface host IP"),
ctxt, &def->hostIP) < 0)
goto error;
+
+ if (!hostlinkstate)
+ hostlinkstate = virXPathString("string(./link/@state)",
ctxt);
+
ctxt->node = tmpnode;
}
if (!macaddr && xmlStrEqual(cur->name, BAD_CAST "mac"))
{
@@ -10303,6 +10308,16 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
}
}
+ def->hostlinkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DEFAULT;
+ if (hostlinkstate != NULL) {
+ if ((def->hostlinkstate =
virDomainNetInterfaceLinkStateTypeFromString(hostlinkstate)) <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown interface link state '%s'"),
+ hostlinkstate);
+ goto error;
+ }
+ }
+
if (filter != NULL) {
switch (def->type) {
case VIR_DOMAIN_NET_TYPE_ETHERNET:
@@ -10371,6 +10386,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
VIR_FREE(devaddr);
VIR_FREE(mode);
VIR_FREE(linkstate);
+ VIR_FREE(hostlinkstate);
VIR_FREE(addrtype);
VIR_FREE(domain_name);
VIR_FREE(trustGuestRxFilters);
@@ -22113,6 +22129,18 @@ virDomainNetDefFormat(virBufferPtr buf,
break;
}
+ if (def->hostlinkstate) {
+ if (sourceLines == 0) {
+ virBufferAddLit(buf, "<source>\n");
+ sourceLines += 2;
+ }
+ virBufferAdjustIndent(buf, 2);
+ virBufferAsprintf(buf, "<link state='%s'/>\n",
+ virDomainNetInterfaceLinkStateTypeToString(def->hostlinkstate));
+ virBufferAdjustIndent(buf, -2);
+ sourceLines += 2;
+ }
+
/* if sourceLines == 0 - no <source> info at all so far
* sourceLines == 1 - first line written, no terminating ">"
* sourceLines > 1 - multiple lines, including subelements
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 09fb7aada4b2..71e12a30c2c1 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1037,6 +1037,7 @@ struct _virDomainNetDef {
virNetDevVlan vlan;
int trustGuestRxFilters; /* enum virTristateBool */
int linkstate;
+ int hostlinkstate;
unsigned int mtu;
virNetDevCoalescePtr coalesce;
};
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index e8d29186eb32..7fc41b28d9f8 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2978,6 +2978,7 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
bool needBridgeChange = false;
bool needFilterChange = false;
bool needLinkStateChange = false;
+ bool needHostLinkStateChange = false;
bool needReplaceDevDef = false;
bool needBandwidthSet = false;
int ret = -1;
@@ -3264,6 +3265,9 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
if (olddev->linkstate != newdev->linkstate)
needLinkStateChange = true;
+ if (olddev->hostlinkstate != newdev->hostlinkstate)
+ needHostLinkStateChange = true;
+
if (!virNetDevBandwidthEqual(virDomainNetGetActualBandwidth(olddev),
virDomainNetGetActualBandwidth(newdev)))
needBandwidthSet = true;
@@ -3308,6 +3312,19 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
goto cleanup;
}
+ if (needHostLinkStateChange) {
+ if (newdev->hostlinkstate == VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN) {
+ if (virNetDevSetOnline(newdev->ifname, false) < 0)
+ goto cleanup;
+ } else {
+ if (virNetDevSetOnline(newdev->ifname, true) < 0)
+ goto cleanup;
+ if (virNetDevIPInfoAddToDev(newdev->ifname, &newdev->hostIP) <
0)
+ goto cleanup;
+ }
+ needReplaceDevDef = true;
+ }
+
if (needReplaceDevDef) {
/* the changes above warrant replacing olddev with newdev in
* the domain's nets list.
diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c
index d8a678b4ab13..f3afbdae4009 100644
--- a/src/qemu/qemu_interface.c
+++ b/src/qemu/qemu_interface.c
@@ -413,7 +413,7 @@ qemuInterfaceEthernetConnect(virDomainDefPtr def,
{
virMacAddr tapmac;
int ret = -1;
- unsigned int tap_create_flags = VIR_NETDEV_TAP_CREATE_IFUP;
+ unsigned int tap_create_flags = 0;
bool template_ifname = false;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
const char *tunpath = "/dev/net/tun";
@@ -427,6 +427,9 @@ qemuInterfaceEthernetConnect(virDomainDefPtr def,
}
}
+ if (net->hostlinkstate != VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN)
+ tap_create_flags |= VIR_NETDEV_TAP_CREATE_IFUP;
+
if (!net->ifname ||
STRPREFIX(net->ifname, VIR_NET_GENERATED_TAP_PREFIX) ||
strchr(net->ifname, '%')) {
@@ -453,9 +456,6 @@ qemuInterfaceEthernetConnect(virDomainDefPtr def,
if (virNetDevSetMAC(net->ifname, &tapmac) < 0)
goto cleanup;
- if (virNetDevSetOnline(net->ifname, true) < 0)
- goto cleanup;
-
if (net->script &&
virNetDevRunEthernetScript(net->ifname, net->script) < 0)
goto cleanup;
--
2.9.3
1:32 a.m.
New subject: [libvirt] [PATCH] qemu: allow to control host side link status of network device
On Tue, May 16, 2017 at 01:15:59 +0300, Vasiliy Tolstov wrote:
Signed-off-by: Vasiliy Tolstov <v.tolstov(a)selfip.ru>
---
docs/formatdomain.html.in | 21 +++++++++++++++++++++
docs/schemas/domaincommon.rng | 11 +++++++++++
src/conf/domain_conf.c | 28 ++++++++++++++++++++++++++++
src/conf/domain_conf.h | 1 +
src/qemu/qemu_hotplug.c | 17 +++++++++++++++++
src/qemu/qemu_interface.c | 8 ++++----
6 files changed, 82 insertions(+), 4 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 8c884f4af9cb..dd8e6a4afa99 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -5421,6 +5421,27 @@ qemu-kvm -net nic,model=? /dev/null
<span class="since">Since 0.9.5</span>
</p>
+ <h5><a name="elementHostLink">Modifying phisical link
state</a></h5>
+<pre>
+...
+<devices>
+ <interface type='ethernet'>
+ <source>
+ <b><link state='down'/></b>
+ <target dev='vnet0'/>
+ </interface>
+</devices>
+...</pre>
+
+ <p>
+ This element provides means of setting state of the phisical network interface.
+ Possible values for attribute <code>state</code> are
<code>up</code> and
+ <code>down</code>. If <code>down</code> is specified as
the value, the interface
+ put in down state. Default behavior if this element is unspecified is to have the
+ link state <code>up</code>.
+ <span class="since">Since 3.3.2</span>
3.3.2? We dropped micro versions some time ago.
+ </p>
+
<h5><a name="mtu">MTU configuration</a></h5>
<pre>
There already is an element called <link> with attribute "state" which
modifies the link state in qemu. So that maps to guest link state. So
this patch has a naming collision going on.
I also don't really see how modifying the host side link state would be
useful. Could you elaborate why you think it would be useful?
At any rate, if you provide a justification, you also need to modify the
XML portion to have a separate element and provide a test case as it's
required with XML changes.
2:28 a.m.
New subject: [libvirt] [PATCH] qemu: allow to control host side link status of network device
2017-05-16 9:32 GMT+03:00 Peter Krempa <pkrempa(a)redhat.com>:
3.3.2? We dropped micro versions some time ago.
This is copy/paste from link element, sorry.
> + </p>
> +
> <h5><a name="mtu">MTU configuration</a></h5>
> <pre>
There already is an element called <link> with attribute "state" which
modifies the link state in qemu. So that maps to guest link state. So
this patch has a naming collision going on.
Yes, but some times ago Laine Stump says that for host side link
status we can use source element with link state.
I also don't really see how modifying the host side link state would be
useful. Could you elaborate why you think it would be useful?
OSPF - tap device have addresses assigned to it and bird/quagga create
routing based on interface link status and addresses on device.
So if i have host side device in up state no matter that have guest -
host system forward traffic to it. So i need to modify host side link
status too.
At any rate, if you provide a justification, you also need to modify the
XML portion to have a separate element and provide a test case as it's
required with XML changes.
Can you gave me example test for xml changes?
Thanks!
--
Vasiliy Tolstov,
e-mail: v.tolstov(a)selfip.ru
3:16 a.m.
New subject: [libvirt] [PATCH] qemu: allow to control host side link status of network device
On Tue, May 16, 2017 at 10:28:27AM +0300, Vasiliy Tolstov wrote:
2017-05-16 9:32 GMT+03:00 Peter Krempa <pkrempa(a)redhat.com>:
> 3.3.2? We dropped micro versions some time ago.
>
This is copy/paste from link element, sorry.
>> + </p>
>> +
>> <h5><a name="mtu">MTU
configuration</a></h5>
>> <pre>
>
>
> There already is an element called <link> with attribute "state"
which
> modifies the link state in qemu. So that maps to guest link state. So
> this patch has a naming collision going on.
Yes, but some times ago Laine Stump says that for host side link
status we can use source element with link state.
>
> I also don't really see how modifying the host side link state would be
> useful. Could you elaborate why you think it would be useful?
OSPF - tap device have addresses assigned to it and bird/quagga create
routing based on interface link status and addresses on device.
So if i have host side device in up state no matter that have guest -
host system forward traffic to it. So i need to modify host side link
status too.
Shouldn't we just tie the host & guest link state together then. This
doesn't seem like enough of a reason to introduce new XML elements.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
3:21 a.m.
New subject: [libvirt] [PATCH] qemu: allow to control host side link status of network device
2017-05-16 11:16 GMT+03:00 Daniel P. Berrange <berrange(a)redhat.com>:
Shouldn't we just tie the host & guest link state together
then. This
doesn't seem like enough of a reason to introduce new XML elements.
Back to half year ago i'm already have such patch, but it reverted
with message :
link state only for guest side, for host side we need different element in xml.
--
Vasiliy Tolstov,
e-mail: v.tolstov(a)selfip.ru
5:40 p.m.
New subject: [libvirt] [PATCH] qemu: allow to control host side link status of network device
On 05/16/2017 04:21 AM, Vasiliy Tolstov wrote:
2017-05-16 11:16 GMT+03:00 Daniel P. Berrange
<berrange(a)redhat.com>:
> Shouldn't we just tie the host & guest link state together then. This
> doesn't seem like enough of a reason to introduce new XML elements.
Back to half year ago i'm already have such patch, but it reverted
with message :
link state only for guest side, for host side we need different element in xml.
I guess I should have been working through my backlog in reverse order.
(I just responded to the message Vasiliy sent last week about this. At
the time of the original discussion, I intended to send a patch to
implement <source><link state='blah'/></source> as he's done
here).
Setting an interface online/offline can have further reaching
sonsequences than just turning carrier on and off. It also removes and
adds routes, IP addresses, and can trigger DHCP to request a new
address, etc. Since we have other things separated for host vs guest
side, it makes sense to do this for the link state as well. (tying two
functions to a single knob often leads to problems, and once they're
tied together, they can't be untied once you discover one of those
problems. If there are two separate options, modifying both of them may
be a bit more verbose, but it can still be done).
8:25 p.m.
New subject: [libvirt] [PATCH] qemu: allow to control host side link status of network device
On 05/16/2017 06:40 PM, Laine Stump wrote:
On 05/16/2017 04:21 AM, Vasiliy Tolstov wrote:
> 2017-05-16 11:16 GMT+03:00 Daniel P. Berrange <berrange(a)redhat.com>:
>> Shouldn't we just tie the host & guest link state together then. This
>> doesn't seem like enough of a reason to introduce new XML elements.
>
>
> Back to half year ago i'm already have such patch, but it reverted
> with message :
> link state only for guest side, for host side we need different element in xml.
>
I guess I should have been working through my backlog in reverse order.
(I just responded to the message Vasiliy sent last week about this. At
the time of the original discussion, I intended to send a patch to
implement <source><link state='blah'/></source> as he's
done here).
Setting an interface online/offline can have further reaching
sonsequences than just turning carrier on and off. It also removes and
adds routes, IP addresses, and can trigger DHCP to request a new
address, etc. Since we have other things separated for host vs guest
side, it makes sense to do this for the link state as well. (tying two
functions to a single knob often leads to problems, and once they're
tied together, they can't be untied once you discover one of those
problems. If there are two separate options, modifying both of them may
be a bit more verbose, but it can still be done).
Oh, and I forgot to point out here (although I did in the original
thread last fall) that a toplevel <link state='blah'/> historically only
affects the state of the interface in the guest. If it began affecting
the state of the tap device in the host as well, there would be
unexpected (and undesireable) consequences.
For example, if a tap device is attached to a bridge that has STP
enabled, and you set the tap device link state down and then back up,
the port will revert to "learning" mode, and won't begin forwarding
packets again until the STP forward delay timer has expired. Likewise,
if there is an IP address configured on the host side of the tap device
and you set link state to down, any route associated with that device
will also disappear, leading to route flapping if you have any sort of
routing protocol running.
Both of these would be undesirable side effects if, for example, all you
wanted to do was trigger the guest to renegotiate an address with dhcp
(by toggling the guest NIC's link state) (I'm sure there are other
reasons someone might want to set the guest-side link state to down for
a period without encountering the side effects I listed above, that's
just what comes to mind first.)
So even if we weren't concerned about the precendent of setting <link
state='down'/> not affecting the tap device, it still makes sense to
keep the two operations separate.
2:51 a.m.
New subject: [libvirt] [PATCH] qemu: allow to control host side link status of network device
2017-05-17 4:25 GMT+03:00 Laine Stump <laine(a)laine.org>:
Oh, and I forgot to point out here (although I did in the original
thread last fall) that a toplevel <link state='blah'/> historically only
affects the state of the interface in the guest. If it began affecting
the state of the tap device in the host as well, there would be
unexpected (and undesireable) consequences.
So as i understand xml for this feature looks right. I found one minor
issue, and send v2 shortly,
but i want to know, does this will be accepted by upstream?
--
Vasiliy Tolstov,
e-mail: v.tolstov(a)selfip.ru
7:16 a.m.
New subject: [libvirt] [PATCH] qemu: allow to control host side link status of network device
On 05/17/2017 03:51 AM, Vasiliy Tolstov wrote:
2017-05-17 4:25 GMT+03:00 Laine Stump <laine(a)laine.org>:
> Oh, and I forgot to point out here (although I did in the original
> thread last fall) that a toplevel <link state='blah'/> historically
only
> affects the state of the interface in the guest. If it began affecting
> the state of the tap device in the host as well, there would be
> unexpected (and undesireable) consequences.
So as i understand xml for this feature looks right. I found one minor
issue, and send v2 shortly,
but i want to know, does this will be accepted by upstream?
Well, *I* think I've given sufficient reasons for having the two link
states controlled separately, but since Dan and Peter had questioned its
usefulness, we should see whether or not I've swayed their opinions :-)
(BTW, sorry about dropping / completely forgetting about my patch for
this back in August - this could have been resolved much sooner if I was
better about keeping track of open branches.)
7:28 a.m.
New subject: [libvirt] [PATCH] qemu: allow to control host side link status of network device
On Wed, May 17, 2017 at 08:16:58 -0400, Laine Stump wrote:
On 05/17/2017 03:51 AM, Vasiliy Tolstov wrote:
> 2017-05-17 4:25 GMT+03:00 Laine Stump <laine(a)laine.org>:
>> Oh, and I forgot to point out here (although I did in the original
>> thread last fall) that a toplevel <link state='blah'/>
historically only
>> affects the state of the interface in the guest. If it began affecting
>> the state of the tap device in the host as well, there would be
>> unexpected (and undesireable) consequences.
>
>
> So as i understand xml for this feature looks right. I found one minor
> issue, and send v2 shortly,
> but i want to know, does this will be accepted by upstream?
>
Well, *I* think I've given sufficient reasons for having the two link
states controlled separately, but since Dan and Peter had questioned its
usefulness, we should see whether or not I've swayed their opinions :-)
I think we should have two elements in this case. I just don't consider
setting the host link side to be very useful.
3:32 p.m.
New subject: [libvirt] [PATCH] qemu: allow to control host side link status of network device
2017-05-17 15:28 GMT+03:00 Peter Krempa <pkrempa(a)redhat.com>:
>
> Well, *I* think I've given sufficient reasons for having the two link
> states controlled separately, but since Dan and Peter had questioned its
> usefulness, we should see whether or not I've swayed their opinions :-)
I think we should have two elements in this case. I just don't consider
setting the host link side to be very useful.
Sorry, Laine. As i understand you already have alternative patch for
such feature, can you share it?
I'm really interesting how much it different from my.
--
Vasiliy Tolstov,
e-mail: v.tolstov(a)selfip.ru
4:39 p.m.
New subject: [libvirt] [PATCH] qemu: allow to control host side link status of network device
On 05/18/2017 04:32 PM, Vasiliy Tolstov wrote:
2017-05-17 15:28 GMT+03:00 Peter Krempa <pkrempa(a)redhat.com>:
>>
>> Well, *I* think I've given sufficient reasons for having the two link
>> states controlled separately, but since Dan and Peter had questioned its
>> usefulness, we should see whether or not I've swayed their opinions :-)
>
> I think we should have two elements in this case. I just don't consider
> setting the host link side to be very useful.
>
Sorry, Laine. As i understand you already have alternative patch for
such feature, can you share it?
I'm really interesting how much it different from my.
After much searching, I finally found my patch, and learned that it was
incomplete (I'd suspected that) so not of much use. The one thing I
would say is that I had a prerequisite patch that changed the name of
"linkstate" both in the virDomainNetDef and the temp local in the
parsing function to "guestLinkState" so that it was more difficult to
confuse the two.
Aside from that, I looked through your patch and have a few comments the
I will post. (it's not a thorough review, because I have a hard
deadlines of tomorrow AM for a couple of completely unrelated (to
libvirt, or even software development) tasks. (Protip - *never*
volunteer to be the "parent advisor" for your kid's yearbook if you will
be moving during the time that it all must be done. Also never store
anything in heavy glass bottles directly over an electric stovetop that
is itself made of glass).
5:11 p.m.
New subject: [libvirt] [PATCH] qemu: allow to control host side link status of network device
On 05/15/2017 06:15 PM, Vasiliy Tolstov wrote:
Signed-off-by: Vasiliy Tolstov <v.tolstov(a)selfip.ru>
---
docs/formatdomain.html.in | 21 +++++++++++++++++++++
docs/schemas/domaincommon.rng | 11 +++++++++++
src/conf/domain_conf.c | 28 ++++++++++++++++++++++++++++
src/conf/domain_conf.h | 1 +
src/qemu/qemu_hotplug.c | 17 +++++++++++++++++
src/qemu/qemu_interface.c | 8 ++++----
6 files changed, 82 insertions(+), 4 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 8c884f4af9cb..dd8e6a4afa99 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -5421,6 +5421,27 @@ qemu-kvm -net nic,model=? /dev/null
<span class="since">Since 0.9.5</span>
</p>
+ <h5><a name="elementHostLink">Modifying phisical link
state</a></h5>
+<pre>
+...
+<devices>
+ <interface type='ethernet'>
+ <source>
+ <b><link state='down'/></b>
+ <target dev='vnet0'/>
+ </interface>
+</devices>
+...</pre>
+
+ <p>
+ This element provides means of setting state of the phisical network interface.
It's not the physical device, it is the host side of the tap device that
connects the guest to the network.
+ Possible values for attribute <code>state</code>
are <code>up</code> and
+ <code>down</code>. If <code>down</code> is specified as
the value, the interface
+ put in down state. Default behavior if this element is unspecified is to have the
+ link state <code>up</code>.
+ <span class="since">Since 3.3.2</span>
What pkrempa said. This will now be 3.4.0 (right? I can never keep track
and I'm too lazy to look)
+ </p>
+
<h5><a name="mtu">MTU configuration</a></h5>
<pre>
...
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 281309ec09da..89213d63b6e9 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2798,6 +2798,17 @@
All ip-related info for either the host or guest side of an interface
-->
<define name="interface-ip-info">
+ <optional>
+ <element name="link">
+ <attribute name="state">
+ <choice>
+ <value>up</value>
+ <value>down</value>
+ </choice>
+ </attribute>
+ <empty/>
+ </element>
+ </optional>
<zeroOrMore>
<element name="ip">
<attribute name="address">
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0ff216e3a373..b7398276af57 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -9606,6 +9606,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
char *devaddr = NULL;
char *mode = NULL;
char *linkstate = NULL;
As I said in my other mail, I had renamed "linkstate" to
"guestLinkState" in a separate patch to make it clearer which was for
what. If you do that, do it in a separate patch.
+ char *hostlinkstate = NULL;
I prefer hostLinkState, but the naming in this function is all mixed up
anyway...
char *addrtype = NULL;
char *domain_name = NULL;
char *vhostuser_mode = NULL;
@@ -9654,6 +9655,10 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
if (virDomainNetIPInfoParseXML(_("interface host IP"),
ctxt, &def->hostIP) < 0)
goto error;
+
+ if (!hostlinkstate)
+ hostlinkstate = virXPathString("string(./link/@state)",
ctxt);
The parsing for guest link state uses virXMLPropString() (as does the
parsing for just about everything else inside <source>), but I like this
better anyway (virXPathBlah() may be less efficient, but I think we
should use it more).
+
ctxt->node = tmpnode;
}
if (!macaddr && xmlStrEqual(cur->name, BAD_CAST "mac"))
{
@@ -10303,6 +10308,16 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
}
}
+ def->hostlinkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DEFAULT;
DEFAULT is 0, so there isn't any need to explicitly set it.
+ if (hostlinkstate != NULL) {
+ if ((def->hostlinkstate =
virDomainNetInterfaceLinkStateTypeFromString(hostlinkstate)) <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown interface link state '%s'"),
+ hostlinkstate);
+ goto error;
+ }
+ }
+
You need to validate that it isn't set for interface type='hostdev' (and
maybe some other types?
if (filter != NULL) {
switch (def->type) {
case VIR_DOMAIN_NET_TYPE_ETHERNET:
@@ -10371,6 +10386,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
VIR_FREE(devaddr);
VIR_FREE(mode);
VIR_FREE(linkstate);
+ VIR_FREE(hostlinkstate);
VIR_FREE(addrtype);
VIR_FREE(domain_name);
VIR_FREE(trustGuestRxFilters);
@@ -22113,6 +22129,18 @@ virDomainNetDefFormat(virBufferPtr buf,
break;
}
+ if (def->hostlinkstate) {
+ if (sourceLines == 0) {
+ virBufferAddLit(buf, "<source>\n");
+ sourceLines += 2;
+ }
+ virBufferAdjustIndent(buf, 2);
+ virBufferAsprintf(buf, "<link state='%s'/>\n",
+ virDomainNetInterfaceLinkStateTypeToString(def->hostlinkstate));
+ virBufferAdjustIndent(buf, -2);
+ sourceLines += 2;
+ }
+
Maybe put this *below* this following comment instead of above it
/* if sourceLines == 0 - no <source> info at all so
far
* sourceLines == 1 - first line written, no terminating ">"
* sourceLines > 1 - multiple lines, including subelements
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 09fb7aada4b2..71e12a30c2c1 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1037,6 +1037,7 @@ struct _virDomainNetDef {
virNetDevVlan vlan;
int trustGuestRxFilters; /* enum virTristateBool */
int linkstate;
+ int hostlinkstate;
Again, I like hostLinkState (I looked in HACKING and don't see any
opinion of all lowercase vs camelCase vs underscores for struct members
or locals though).
unsigned int mtu;
virNetDevCoalescePtr coalesce;
};
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index e8d29186eb32..7fc41b28d9f8 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2978,6 +2978,7 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
bool needBridgeChange = false;
bool needFilterChange = false;
bool needLinkStateChange = false;
+ bool needHostLinkStateChange = false;
bool needReplaceDevDef = false;
bool needBandwidthSet = false;
int ret = -1;
@@ -3264,6 +3265,9 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
if (olddev->linkstate != newdev->linkstate)
needLinkStateChange = true;
+ if (olddev->hostlinkstate != newdev->hostlinkstate)
+ needHostLinkStateChange = true;
+
if (!virNetDevBandwidthEqual(virDomainNetGetActualBandwidth(olddev),
virDomainNetGetActualBandwidth(newdev)))
needBandwidthSet = true;
@@ -3308,6 +3312,19 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
goto cleanup;
}
+ if (needHostLinkStateChange) {
+ if (newdev->hostlinkstate == VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN) {
+ if (virNetDevSetOnline(newdev->ifname, false) < 0)
+ goto cleanup;
+ } else {
+ if (virNetDevSetOnline(newdev->ifname, true) < 0)
+ goto cleanup;
+ if (virNetDevIPInfoAddToDev(newdev->ifname, &newdev->hostIP) <
0)
+ goto cleanup;
(I probably would have forgotten to do this :-P)
+ }
+ needReplaceDevDef = true;
+ }
+
if (needReplaceDevDef) {
/* the changes above warrant replacing olddev with newdev in
* the domain's nets list.
diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c
index d8a678b4ab13..f3afbdae4009 100644
--- a/src/qemu/qemu_interface.c
+++ b/src/qemu/qemu_interface.c
@@ -413,7 +413,7 @@ qemuInterfaceEthernetConnect(virDomainDefPtr def,
{
virMacAddr tapmac;
int ret = -1;
- unsigned int tap_create_flags = VIR_NETDEV_TAP_CREATE_IFUP;
+ unsigned int tap_create_flags = 0;
bool template_ifname = false;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
const char *tunpath = "/dev/net/tun";
@@ -427,6 +427,9 @@ qemuInterfaceEthernetConnect(virDomainDefPtr def,
}
}
+ if (net->hostlinkstate != VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN)
+ tap_create_flags |= VIR_NETDEV_TAP_CREATE_IFUP;
+
if (!net->ifname ||
STRPREFIX(net->ifname, VIR_NET_GENERATED_TAP_PREFIX) ||
strchr(net->ifname, '%')) {
@@ -453,9 +456,6 @@ qemuInterfaceEthernetConnect(virDomainDefPtr def,
if (virNetDevSetMAC(net->ifname, &tapmac) < 0)
goto cleanup;
- if (virNetDevSetOnline(net->ifname, true) < 0)
- goto cleanup;
-
You need to look at the commit log message for the commit that added the
previous two lines - 07262221 - virNetdevTapCreate() doesn't honor the
VIR_NETDEV_TAP_CREATE_IFUP flag (only virNetdevTapCreateInBridgePort()
does). I don't recall why I decided to fix the bug by just setting the
device online explicitly in qemuInterfaceEthernetConnect() rather than
changing virNetdevTapCreate(); my guess is that modifying
virNetdevTapCreate() directly would have caused side effects elsewhere
(but when I look now, I see the only other place it's called from is
virNetdevTapCreateInBridgePort(), which could easily be modified to send
"flags & ~VIR_NETDEV_TAP_CREATE_IFUP" to virNetdevTapCreate(), then set
the device online itself. (All of that fixup should be in a separate
prerequisite patch though, not in this one)
if (net->script &&
virNetDevRunEthernetScript(net->ifname, net->script) < 0)
goto cleanup;
You haven't done anything to honor the host link state for tap devices
that are connected to bridges, or for macvtap devices. Both of those
need similar support.
BTW, Cc me on v2 so I'll notice it sooner :-)
2742
days inactive
2748
days old
13 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Laine Stump -
Peter Krempa -
Vasiliy Tolstov