11:09 a.m.
After investigation, i found that ips and routes incorrectly assigning on device
deletion.
Also after libvirt create tap device it does not up.
Vasiliy Tolstov (1):
qemu: fix ethernet network type ip/route assign
src/qemu/qemu_interface.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
--
2.7.4
11:09 a.m.
New subject: [libvirt] [PATCH] qemu: fix ethernet network type ip/route assign
IP and routes assigenment incorrectly placed on device stop.
This is fixing it, also change device state according to xml.
Note that as i know in linux routes can't be created on device that does
not up.
Signed-off-by: Vasiliy Tolstov <v.tolstov(a)selfip.ru>
---
src/qemu/qemu_interface.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c
index e637d21fb77a..f80feff2d545 100644
--- a/src/qemu/qemu_interface.c
+++ b/src/qemu/qemu_interface.c
@@ -108,8 +108,25 @@ qemuInterfaceStartDevice(virDomainNetDefPtr net)
break;
}
- case VIR_DOMAIN_NET_TYPE_USER:
case VIR_DOMAIN_NET_TYPE_ETHERNET:
+ switch (dev->linkstate) {
+ case VIR_DOMAIN_NET_INTERFACE_LINK_STATE_UP:
+ case VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DEFAULT:
+ if ((ret = virNetDevSetOnline(dev->ifname, true)) < 0)
+ goto cleanup;
+ break;
+
+ case VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN:
+ if ((ret = virNetDevSetOnline(dev->ifname, false)) < 0)
+ goto cleanup;
+ break;
+ }
+ if (virNetDevIPInfoAddToDev(net->ifname, &net->hostIP) < 0)
+ goto cleanup;
+
+ break;
+
+ case VIR_DOMAIN_NET_TYPE_USER:
case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
case VIR_DOMAIN_NET_TYPE_SERVER:
case VIR_DOMAIN_NET_TYPE_CLIENT:
@@ -197,10 +214,6 @@ qemuInterfaceStopDevice(virDomainNetDefPtr net)
}
case VIR_DOMAIN_NET_TYPE_ETHERNET:
- if (virNetDevIPInfoAddToDev(net->ifname, &net->hostIP) < 0)
- goto cleanup;
- break;
-
case VIR_DOMAIN_NET_TYPE_USER:
case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
case VIR_DOMAIN_NET_TYPE_SERVER:
--
2.7.4
8:35 a.m.
New subject: [libvirt] [PATCH] qemu: fix ethernet network type ip/route assign
2016-08-24 19:09 GMT+03:00 Vasiliy Tolstov <v.tolstov(a)selfip.ru>:
IP and routes assigenment incorrectly placed on device stop.
This is fixing it, also change device state according to xml.
Note that as i know in linux routes can't be created on device that does
not up.
Forget please, incorrect version switch (dev->linkstate) -> switch
(net->linkstate) {
--
Vasiliy Tolstov,
e-mail: v.tolstov(a)selfip.ru
1:32 p.m.
New subject: [libvirt] [PATCH] qemu: fix ethernet network type ip/route assign
On 08/24/2016 12:09 PM, Vasiliy Tolstov wrote:
IP and routes assigenment incorrectly placed on device stop.
This is fixing it, also change device state according to xml.
Note that as i know in linux routes can't be created on device that does
not up.
Signed-off-by: Vasiliy Tolstov <v.tolstov(a)selfip.ru>
---
src/qemu/qemu_interface.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c
index e637d21fb77a..f80feff2d545 100644
--- a/src/qemu/qemu_interface.c
+++ b/src/qemu/qemu_interface.c
@@ -108,8 +108,25 @@ qemuInterfaceStartDevice(virDomainNetDefPtr net)
break;
}
- case VIR_DOMAIN_NET_TYPE_USER:
case VIR_DOMAIN_NET_TYPE_ETHERNET:
+ switch (dev->linkstate) {
+ case VIR_DOMAIN_NET_INTERFACE_LINK_STATE_UP:
+ case VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DEFAULT:
+ if ((ret = virNetDevSetOnline(dev->ifname, true)) < 0)
+ goto cleanup;
+ break;
+
+ case VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN:
+ if ((ret = virNetDevSetOnline(dev->ifname, false)) < 0)
+ goto cleanup;
+ break;
+ }
The Online/Offline handling of the tap device for ethernet devices
should be identical to that used for network/bridge network devices. If
something is necessary (and it may be), it should be in a separate patch.
(Note that macvtap devices are set online in this function because
they're artificially kept offline when the macvtap device is created -->
this is necessary because the macvtap and guest interface use the same
MAC address, and setting it online early could cause unwanted traffic
with that MAC address, thus prematurely directing traffic for the MAC
address to this host. See this BZ for more details:
https://bugzilla.redhat.com/show_bug.cgi?id=1113474
+ if (virNetDevIPInfoAddToDev(net->ifname,
&net->hostIP) < 0)
+ goto cleanup;
ACK to moving this.
+
+ break;
+
+ case VIR_DOMAIN_NET_TYPE_USER:
case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
case VIR_DOMAIN_NET_TYPE_SERVER:
case VIR_DOMAIN_NET_TYPE_CLIENT:
@@ -197,10 +214,6 @@ qemuInterfaceStopDevice(virDomainNetDefPtr net)
}
case VIR_DOMAIN_NET_TYPE_ETHERNET:
- if (virNetDevIPInfoAddToDev(net->ifname, &net->hostIP) < 0)
- goto cleanup;
- break;
-
case VIR_DOMAIN_NET_TYPE_USER:
case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
case VIR_DOMAIN_NET_TYPE_SERVER:
1 a.m.
New subject: [libvirt] [PATCH] qemu: fix ethernet network type ip/route assign
On 08/24/2016 02:32 PM, Laine Stump wrote:
On 08/24/2016 12:09 PM, Vasiliy Tolstov wrote:
> IP and routes assigenment incorrectly placed on device stop.
> This is fixing it, also change device state according to xml.
> Note that as i know in linux routes can't be created on device that does
> not up.
>
> Signed-off-by: Vasiliy Tolstov <v.tolstov(a)selfip.ru>
> ---
> src/qemu/qemu_interface.c | 23 ++++++++++++++++++-----
> 1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c
> index e637d21fb77a..f80feff2d545 100644
> --- a/src/qemu/qemu_interface.c
> +++ b/src/qemu/qemu_interface.c
> @@ -108,8 +108,25 @@ qemuInterfaceStartDevice(virDomainNetDefPtr net)
> break;
> }
> - case VIR_DOMAIN_NET_TYPE_USER:
> case VIR_DOMAIN_NET_TYPE_ETHERNET:
> + switch (dev->linkstate) {
> + case VIR_DOMAIN_NET_INTERFACE_LINK_STATE_UP:
> + case VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DEFAULT:
> + if ((ret = virNetDevSetOnline(dev->ifname, true)) < 0)
> + goto cleanup;
> + break;
> +
> + case VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN:
> + if ((ret = virNetDevSetOnline(dev->ifname, false)) < 0)
> + goto cleanup;
> + break;
> + }
The Online/Offline handling of the tap device for ethernet devices
should be identical to that used for network/bridge network devices.
If something is necessary (and it may be), it should be in a separate
patch.
More on this - aside from code that you added in your patch 9717d6
"autocreate tap device for ethernet network type", the linkstate of a
device is only used to send qemu commands controlling the linkstate of
the *guest* device, but never to set the online status of the host side
of the tap device (several of us missed this during the review of
multiple versions of patches leading to 9717d6; heck, I didn't even
realize it when looking at this new patch until I went back and read
through all the code again)
This (plus a bit of investigation about when tap devices for bridge
networks are set online) leads to the following:
1) The above code is erroneous. It shouldn't be there at all.
2) The code in qemuDomainChangeNetLinkState() that modifies the online
status of the tap device when the interface is type='ethernet' also
should not be there.
3) The reason the tap interface isn't IFF_UP is that
virNetDevTapCreate() (called from qemuInterfaceEthernetConnect() in the
same patch 9717d6) *ignores the VIR_NETDEV_TAP_CREATE_IFUP flag* (and it
is documented this way). That's never been noticed before because the
only previous call to virNetDevTapCreate() is from
virNetDevTapCreateInBridgePort(), which sets IFF_UP on the tap device
itself after calling virNetDevTapCreate().
My assumption is that it was done this way possibly because the tap
device needed to be offline during some part of the initialization (or
possibly because that's just the way some function was split up during a
refactoring or something).
Anyway, I think there are 3 patches needed:
1) The part of your patch that just moves the virNetDevIPInfoAddToDev()
call.
2) A patch to remove the virNetDevSetOnline() from
qemuDomainChangeNetLinkState()
3) A patch to call virNetDevSetOnline() after calling virNetDevTapCreate()
I just modified your patch, created the other two patches, and sent the
resulting patch series to the list.
If you can try it out and verify that it works for you (it works for
me), then I'll push it in the morning ( US EST) so it's in before the
freeze.
1:52 a.m.
New subject: [libvirt] [PATCH] qemu: fix ethernet network type ip/route assign
25 Авг 2016 г. 9:00 пользователь "Laine Stump" <laine(a)laine.org> написал:
On 08/24/2016 02:32 PM, Laine Stump wrote:
>
> On 08/24/2016 12:09 PM, Vasiliy Tolstov wrote:
>>
>> IP and routes assigenment incorrectly placed on device stop.
>> This is fixing it, also change device state according to xml.
>> Note that as i know in linux routes can't be created on device that does
>> not up.
>>
>> Signed-off-by: Vasiliy Tolstov <v.tolstov(a)selfip.ru>
>> ---
>> src/qemu/qemu_interface.c | 23 ++++++++++++++++++-----
>> 1 file changed, 18 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c
>> index e637d21fb77a..f80feff2d545 100644
>> --- a/src/qemu/qemu_interface.c
>> +++ b/src/qemu/qemu_interface.c
>> @@ -108,8 +108,25 @@ qemuInterfaceStartDevice(virDomainNetDefPtr net)
>> break;
>> }
>> - case VIR_DOMAIN_NET_TYPE_USER:
>> case VIR_DOMAIN_NET_TYPE_ETHERNET:
>> + switch (dev->linkstate) {
>> + case VIR_DOMAIN_NET_INTERFACE_LINK_STATE_UP:
>> + case VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DEFAULT:
>> + if ((ret = virNetDevSetOnline(dev->ifname, true)) < 0)
>> + goto cleanup;
>> + break;
>> +
>> + case VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN:
>> + if ((ret = virNetDevSetOnline(dev->ifname, false)) < 0)
>> + goto cleanup;
>> + break;
>> + }
>
>
> The Online/Offline handling of the tap device for ethernet devices
should be
identical to that used for network/bridge network devices. If
something is necessary (and it may be), it should be in a separate patch.
More on this - aside from code that you added in your patch 9717d6
"autocreate
tap device for ethernet network type", the linkstate of a
device is only used to send qemu commands controlling the linkstate of the
*guest* device, but never to set the online status of the host side of the
tap device (several of us missed this during the review of multiple
versions of patches leading to 9717d6; heck, I didn't even realize it when
looking at this new patch until I went back and read through all the code
again)
This (plus a bit of investigation about when tap devices for bridge
networks are
set online) leads to the following:
1) The above code is erroneous. It shouldn't be there at all.
2) The code in qemuDomainChangeNetLinkState() that modifies the online
status of
the tap device when the interface is type='ethernet' also should
not be there.
3) The reason the tap interface isn't IFF_UP is that virNetDevTapCreate()
(called from qemuInterfaceEthernetConnect() in the same patch 9717d6)
*ignores the VIR_NETDEV_TAP_CREATE_IFUP flag* (and it is documented this
way). That's never been noticed before because the only previous call to
virNetDevTapCreate() is from virNetDevTapCreateInBridgePort(), which sets
IFF_UP on the tap device itself after calling virNetDevTapCreate().
My assumption is that it was done this way possibly because the tap
device needed
to be offline during some part of the initialization (or
possibly because that's just the way some function was split up during a
refactoring or something).
Anyway, I think there are 3 patches needed:
1) The part of your patch that just moves the virNetDevIPInfoAddToDev()
call.
2) A patch to remove the virNetDevSetOnline() from
qemuDomainChangeNetLinkState()
3) A patch to call virNetDevSetOnline() after calling virNetDevTapCreate()
I just modified your patch, created the other two patches, and sent the
resulting
patch series to the list.
If you can try it out and verify that it works for you (it works for me),
then
I'll push it in the morning ( US EST) so it's in before the freeze.
Big thanks! I'm test it today.
1:59 a.m.
New subject: [libvirt] [PATCH] qemu: fix ethernet network type ip/route assign
25 Авг 2016 г. 9:00 пользователь "Laine Stump" <laine(a)laine.org> написал:
On 08/24/2016 02:32 PM, Laine Stump wrote:
>
> On 08/24/2016 12:09 PM, Vasiliy Tolstov wrote:
>>
>> IP and routes assigenment incorrectly placed on device stop.
>> This is fixing it, also change device state according to xml.
>> Note that as i know in linux routes can't be created on device that does
>> not up.
>>
>> Signed-off-by: Vasiliy Tolstov <v.tolstov(a)selfip.ru>
>> ---
>> src/qemu/qemu_interface.c | 23 ++++++++++++++++++-----
>> 1 file changed, 18 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c
>> index e637d21fb77a..f80feff2d545 100644
>> --- a/src/qemu/qemu_interface.c
>> +++ b/src/qemu/qemu_interface.c
>> @@ -108,8 +108,25 @@ qemuInterfaceStartDevice(virDomainNetDefPtr net)
>> break;
>> }
>> - case VIR_DOMAIN_NET_TYPE_USER:
>> case VIR_DOMAIN_NET_TYPE_ETHERNET:
>> + switch (dev->linkstate) {
>> + case VIR_DOMAIN_NET_INTERFACE_LINK_STATE_UP:
>> + case VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DEFAULT:
>> + if ((ret = virNetDevSetOnline(dev->ifname, true)) < 0)
>> + goto cleanup;
>> + break;
>> +
>> + case VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN:
>> + if ((ret = virNetDevSetOnline(dev->ifname, false)) < 0)
>> + goto cleanup;
>> + break;
>> + }
>
>
> The Online/Offline handling of the tap device for ethernet devices
should be
identical to that used for network/bridge network devices. If
something is necessary (and it may be), it should be in a separate patch.
More on this - aside from code that you added in your patch 9717d6
"autocreate
tap device for ethernet network type", the linkstate of a
device is only used to send qemu commands controlling the linkstate of the
*guest* device, but never to set the online status of the host side of the
tap device (several of us missed this during the review of multiple
versions of patches leading to 9717d6; heck, I didn't even realize it when
looking at this new patch until I went back and read through all the code
again)
This (plus a bit of investigation about when tap devices for bridge
networks are
set online) leads to the following:
1) The above code is erroneous. It shouldn't be there at all.
2) The code in qemuDomainChangeNetLinkState() that modifies the online
status of
the tap device when the interface is type='ethernet' also should
not be there.
3) The reason the tap interface isn't IFF_UP is that virNetDevTapCreate()
(called from qemuInterfaceEthernetConnect() in the same patch 9717d6)
*ignores the VIR_NETDEV_TAP_CREATE_IFUP flag* (and it is documented this
way). That's never been noticed before because the only previous call to
virNetDevTapCreate() is from virNetDevTapCreateInBridgePort(), which sets
IFF_UP on the tap device itself after calling virNetDevTapCreate().
My assumption is that it was done this way possibly because the tap
device needed
to be offline during some part of the initialization (or
possibly because that's just the way some function was split up during a
refactoring or something).
Anyway, I think there are 3 patches needed:
1) The part of your patch that just moves the virNetDevIPInfoAddToDev()
call.
2) A patch to remove the virNetDevSetOnline() from
qemuDomainChangeNetLinkState()
3) A patch to call virNetDevSetOnline() after calling virNetDevTapCreate()
I just modified your patch, created the other two patches, and sent the
resulting
patch series to the list.
If you can try it out and verify that it works for you (it works for me),
then
I'll push it in the morning ( US EST) so it's in before the freeze.
May be I miss something but in you patch you always up tap device, but if I
create domain with link down in XML this not right.
What do you think?
10:17 a.m.
New subject: [libvirt] [PATCH] qemu: fix ethernet network type ip/route assign
On 08/25/2016 02:59 AM, Vasiliy Tolstov wrote:
May be I miss something but in you patch you always up tap device, but
if I create domain with link down in XML this not right.
What do you think?
Until now, linkstate has always been used to control the online status
of the emulated NIC in the guest, and the tap device has always been
IFF_UP (and for a good reason. See my reply to you in the thread for my
patches for more details...)
10:31 a.m.
New subject: [libvirt] [PATCH] qemu: fix ethernet network type ip/route assign
2016-08-25 18:17 GMT+03:00 Laine Stump <laine(a)laine.org>:
Until now, linkstate has always been used to control the online
status of
the emulated NIC in the guest, and the tap device has always been IFF_UP
(and for a good reason. See my reply to you in the thread for my patches for
more details...)
Yes, i commented with proposed solution to get that i need =)
--
Vasiliy Tolstov,
e-mail: v.tolstov(a)selfip.ru
3011
days inactive
3012
days old
8 comments
2 participants
participants (2)
-
Laine Stump -
Vasiliy Tolstov