10:33 a.m.
libvirt docs says that network type ethernet not fully
managed by libvirt, so don't check for ip.
This fixes issue when no ip assigned to interface in libvirt domain xml
and somebody want to change link state of interface.
Signed-off-by: Vasiliy Tolstov <v.tolstov(a)selfip.ru>
---
src/qemu/qemu_hotplug.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index aabdb78..db4fdda 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2394,10 +2394,7 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
case VIR_DOMAIN_NET_TYPE_ETHERNET:
if (STRNEQ_NULLABLE(olddev->data.ethernet.dev,
- newdev->data.ethernet.dev) ||
- olddev->nips == 0 || newdev->nips == 0 ||
- !virSocketAddrEqual(&olddev->ips[0]->address,
- &newdev->ips[0]->address)) {
+ newdev->data.ethernet.dev)) {
needReconnect = true;
}
break;
--
2.5.0
5:15 a.m.
2015-08-22 18:33 GMT+03:00 Vasiliy Tolstov <v.tolstov(a)selfip.ru>:
libvirt docs says that network type ethernet not fully
managed by libvirt, so don't check for ip.
This fixes issue when no ip assigned to interface in libvirt domain xml
and somebody want to change link state of interface.
And does it possible to add this (if patch ok) to upcoming libvirt release?
--
Vasiliy Tolstov,
e-mail: v.tolstov(a)selfip.ru
6:45 a.m.
On 08/24/2015 06:15 AM, Vasiliy Tolstov wrote:
2015-08-22 18:33 GMT+03:00 Vasiliy Tolstov
<v.tolstov(a)selfip.ru>:
> libvirt docs says that network type ethernet not fully
> managed by libvirt, so don't check for ip.
> This fixes issue when no ip assigned to interface in libvirt domain xml
> and somebody want to change link state of interface.
And does it possible to add this (if patch ok) to upcoming libvirt release?
I've been looking back at the history of why this code was added in the
first place to make sure we're not inadvertently breaking something else.
It looks like originally that code did check that ipaddr hadn't changed
(when there was a single IP address), which wasn't needed but also
didn't hurt anything (since ipaddr would always be 0 for type='ethernet'
in the qemu driver. Then recently (commit aa2cc7) support for multiple
IP addresses with type='ethernet' was added (apparently for LXC, but
also added for openvz), any code that had touched ethernet.ipaddr was
changed to look at nips and the ip array instead, but the code that
replaced the unnecessary but innocuous check in qemu_hotplug.c was done
incorrectly. So the correct thing *is* to simply remove the check.
All of that is a long winded way of saying ACK. I'll push it in awhile.
6:56 a.m.
2015-08-24 14:45 GMT+03:00 Laine Stump <laine(a)laine.org>:
I've been looking back at the history of why this code was added
in the
first place to make sure we're not inadvertently breaking something else.
It looks like originally that code did check that ipaddr hadn't changed
(when there was a single IP address), which wasn't needed but also
didn't hurt anything (since ipaddr would always be 0 for type='ethernet'
in the qemu driver. Then recently (commit aa2cc7) support for multiple
IP addresses with type='ethernet' was added (apparently for LXC, but
also added for openvz), any code that had touched ethernet.ipaddr was
changed to look at nips and the ip array instead, but the code that
replaced the unnecessary but innocuous check in qemu_hotplug.c was done
incorrectly. So the correct thing *is* to simply remove the check.
All of that is a long winded way of saying ACK. I'll push it in awhile.
Thanks!
--
Vasiliy Tolstov,
e-mail: v.tolstov(a)selfip.ru
5:42 a.m.
2015-08-24 14:56 GMT+03:00 Vasiliy Tolstov <v.tolstov(a)selfip.ru>:
2015-08-24 14:45 GMT+03:00 Laine Stump <laine(a)laine.org>:
> I've been looking back at the history of why this code was added in the
> first place to make sure we're not inadvertently breaking something else.
>
> It looks like originally that code did check that ipaddr hadn't changed
> (when there was a single IP address), which wasn't needed but also
> didn't hurt anything (since ipaddr would always be 0 for type='ethernet'
> in the qemu driver. Then recently (commit aa2cc7) support for multiple
> IP addresses with type='ethernet' was added (apparently for LXC, but
> also added for openvz), any code that had touched ethernet.ipaddr was
> changed to look at nips and the ip array instead, but the code that
> replaced the unnecessary but innocuous check in qemu_hotplug.c was done
> incorrectly. So the correct thing *is* to simply remove the check.
>
> All of that is a long winded way of saying ACK. I'll push it in awhile.
Thanks!
I don't see this change in current master. Does it goes to 1.2.19 or to 1.2.20?
--
Vasiliy Tolstov,
e-mail: v.tolstov(a)selfip.ru
9:03 a.m.
On 08/28/2015 06:42 AM, Vasiliy Tolstov wrote:
2015-08-24 14:56 GMT+03:00 Vasiliy Tolstov
<v.tolstov(a)selfip.ru>:
> 2015-08-24 14:45 GMT+03:00 Laine Stump <laine(a)laine.org>:
>> I've been looking back at the history of why this code was added in the
>> first place to make sure we're not inadvertently breaking something else.
>>
>> It looks like originally that code did check that ipaddr hadn't changed
>> (when there was a single IP address), which wasn't needed but also
>> didn't hurt anything (since ipaddr would always be 0 for
type='ethernet'
>> in the qemu driver. Then recently (commit aa2cc7) support for multiple
>> IP addresses with type='ethernet' was added (apparently for LXC, but
>> also added for openvz), any code that had touched ethernet.ipaddr was
>> changed to look at nips and the ip array instead, but the code that
>> replaced the unnecessary but innocuous check in qemu_hotplug.c was done
>> incorrectly. So the correct thing *is* to simply remove the check.
>>
>> All of that is a long winded way of saying ACK. I'll push it in awhile.
>
> Thanks!
I don't see this change in current master. Does it goes to 1.2.19 or to 1.2.20?
It was pushed on on Monday, commit id
bbc705d1309b8ec2eb4cdae011dc702802c96d11. It will be in 1.2.19. I took
the liberty of expanding the commit log message, so you may not have
recognized it:
commit bbc705d1309b8ec2eb4cdae011dc702802c96d11
Author: Vasiliy Tolstov <v.tolstov(a)selfip.ru>
Date: Sat Aug 22 15:33:15 2015 +0000
Eliminate incorrect and unnecessary check for changed IP address
Commit aa2cc7 modified a previously unnecessary but innocuous check
for interface IP address during interface update incorrectly, causing
all attempted updates (e.g. changing link state) to interfaces of
type='ethernet' for QEMU to fail.
This patch fixes the issue by completely removing the check for IP
address, which is pointless since QEMU doesn't support setting
interface IP addresses from the domain interface XML anyway.
10:43 a.m.
2015-08-28 17:03 GMT+03:00 Laine Stump <laine(a)laine.org>:
It was pushed on on Monday, commit id
bbc705d1309b8ec2eb4cdae011dc702802c96d11. It will be in 1.2.19. I took the
liberty of expanding the commit log message, so you may not have recognized
it:
Thanks! =)
--
Vasiliy Tolstov,
e-mail: v.tolstov(a)selfip.ru
3374
days inactive
3380
days old
6 comments
2 participants
participants (2)
-
Laine Stump -
Vasiliy Tolstov