Re: [libvirt] [PATCH v3] util: don't fail if no PortData is found while getting migrateData

2015-03-13 Thread John Ferlan


On 03/04/2015 09:01 PM, zhang bo wrote:
 
 Introduced by f6a2f97e
 
 Problem Description:
 After multiple times of migrating a domain, which has an ovs interface with 
 no portData set,
 with non-shared disk, nbd ports got overflowed.
 
 The steps to reproduce the problem:
 1 define and start a domain with its network configured as:
 interface type='bridge'
   source bridge='br0'/
   virtualport type='openvswitch'
   /virtualport
   model type='virtio'/
   driver name='vhost' queues='4'/
 /interface
 2 do not set the network's portData.
 3 migrate(ToURI2) it with flag 91(1011011), which means:
   VIR_MIGRATE_LIVE
   VIR_MIGRATE_PEER2PEER
   VIR_MIGRATE_PERSIST_DEST
   VIR_MIGRATE_UNDEFINE_SOURCE
   VIR_MIGRATE_NON_SHARED_DISK
 4 migrate success, but we got an error log in libvirtd.log:
   error : virCommandWait:2423 : internal error: Child process (ovs-vsctl 
 --timeout=5 get Interface
   vnet1 external_ids:PortData) unexpected exit status 1: ovs-vsctl: no key 
 PortData in Interface
   record vnet1 column external_ids
 5 migrate it back, migrate it , migrate it back, ...
 6 nbd port got overflowed.
 
 The reasons for the problem is :
 1 virNetDevOpenvswitchGetMigrateData() takes it as wrong if no portData is 
 available for  the ovs
  interface of a domain. (We think it's not appropriate, as portData is just 
 OPTIONAL)
 2 in func qemuMigrationBakeCookie(), it fails in 
 qemuMigrationCookieAddNetwork(), and returns with -1.
  qemuMigrationCookieAddNBD() is not called thereafter, and mig-nbd is still 
 NULL.
 3 However, qemuMigrationRun() just *WARN* if qemuMigrationBakeCookie() fails, 
 migration still successes.
  cookie is NULL, it's not baked on the src side.
 4 On the destination side, it would alloc a port first and then free the nbd 
 port in COOKIE.
  But the cookie is NULL due to qemuMigrationCookieAddNetwork() failure at src 
 side. thus the nbd port
  is not freed.
 
 In this patch, we add --if-exists option to make ovs-vsctl not raise error 
 if there's no portData available.
 Further more, because portData may be NULL in the cookie at the dest side, 
 check it before setting portData.
 
 
 Signed-off-by: Zhou Yimin zhouyi...@huawei.com
 Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
 ---
 V1 here:
  https://www.redhat.com/archives/libvir-list/2015-February/msg00388.html
 V2:
Add --if-exists option to ovs-vsctl cmd,  making ovs-vsctl not raise 
 error if there's no portData
available. Suggested by Martin.
We Tested the patch, it works.
link: https://www.redhat.com/archives/libvir-list/2015-March/msg00104.html
 V3:
move V1/V2/V3 description from above the patch to here. It did not show 
 the detailed description above
after apply PATCH v2.
 ---
  src/util/virnetdevopenvswitch.c | 11 ++-
  1 file changed, 10 insertions(+), 1 deletion(-)
 

Looks like this is a stray... Seems to me you followed Martin's advice
from his 2nd/3rd pass on your v1... Had some issues attempting to 'git
am' the patch, but eventually found that the last diff stanza was off by
a line as if someone edited something before sending... If I change:

 @@ -241,6 +244,12 @@ int virNetDevOpenvswitchSetMigrateData(char
*migrate, const char *ifname)

to

 @@ -241,6 +244,11 @@ int virNetDevOpenvswitchSetMigrateData(char
*migrate, const char *ifname)

Then the patch applied...

In any case, ACK and I've pushed.

John
 diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
 index e5c87bb..722d0dd 100644
 --- a/src/util/virnetdevopenvswitch.c
 +++ b/src/util/virnetdevopenvswitch.c
 @@ -30,9 +30,12 @@
  #include virerror.h
  #include virmacaddr.h
  #include virstring.h
 +#include virlog.h
 
  #define VIR_FROM_THIS VIR_FROM_NONE
 
 +VIR_LOG_INIT(util.netdevopenvswitch);
 +
  /**
   * virNetDevOpenvswitchAddPort:
   * @brname: the bridge name
 @@ -206,7 +209,7 @@ int virNetDevOpenvswitchGetMigrateData(char **migrate, 
 const char *ifname)
  virCommandPtr cmd = NULL;
  int ret = -1;
 
 -cmd = virCommandNewArgList(OVSVSCTL, --timeout=5, get, Interface,
 +cmd = virCommandNewArgList(OVSVSCTL, --timeout=5, --if-exists, 
 get, Interface,
 ifname, external_ids:PortData, NULL);
 
  virCommandSetOutputBuffer(cmd, migrate);
 @@ -241,6 +244,12 @@ int virNetDevOpenvswitchSetMigrateData(char *migrate, 
 const char *ifname)
  virCommandPtr cmd = NULL;
  int ret = -1;
 
 +if (!migrate) {
 +VIR_DEBUG(No OVS port data for interface %s, ifname);
 +return 0;
 +}
 +
  cmd = virCommandNewArgList(OVSVSCTL, --timeout=5, set,
 Interface, ifname, NULL);
  virCommandAddArgFormat(cmd, external_ids:PortData=%s, migrate);
 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v3] util: don't fail if no PortData is found while getting migrateData

2015-03-04 Thread zhang bo

Introduced by f6a2f97e

Problem Description:
After multiple times of migrating a domain, which has an ovs interface with no 
portData set,
with non-shared disk, nbd ports got overflowed.

The steps to reproduce the problem:
1 define and start a domain with its network configured as:
interface type='bridge'
  source bridge='br0'/
  virtualport type='openvswitch'
  /virtualport
  model type='virtio'/
  driver name='vhost' queues='4'/
/interface
2 do not set the network's portData.
3 migrate(ToURI2) it with flag 91(1011011), which means:
  VIR_MIGRATE_LIVE
  VIR_MIGRATE_PEER2PEER
  VIR_MIGRATE_PERSIST_DEST
  VIR_MIGRATE_UNDEFINE_SOURCE
  VIR_MIGRATE_NON_SHARED_DISK
4 migrate success, but we got an error log in libvirtd.log:
  error : virCommandWait:2423 : internal error: Child process (ovs-vsctl 
--timeout=5 get Interface
  vnet1 external_ids:PortData) unexpected exit status 1: ovs-vsctl: no key 
PortData in Interface
  record vnet1 column external_ids
5 migrate it back, migrate it , migrate it back, ...
6 nbd port got overflowed.

The reasons for the problem is :
1 virNetDevOpenvswitchGetMigrateData() takes it as wrong if no portData is 
available for  the ovs
 interface of a domain. (We think it's not appropriate, as portData is just 
OPTIONAL)
2 in func qemuMigrationBakeCookie(), it fails in 
qemuMigrationCookieAddNetwork(), and returns with -1.
 qemuMigrationCookieAddNBD() is not called thereafter, and mig-nbd is still 
NULL.
3 However, qemuMigrationRun() just *WARN* if qemuMigrationBakeCookie() fails, 
migration still successes.
 cookie is NULL, it's not baked on the src side.
4 On the destination side, it would alloc a port first and then free the nbd 
port in COOKIE.
 But the cookie is NULL due to qemuMigrationCookieAddNetwork() failure at src 
side. thus the nbd port
 is not freed.

In this patch, we add --if-exists option to make ovs-vsctl not raise error if 
there's no portData available.
Further more, because portData may be NULL in the cookie at the dest side, 
check it before setting portData.


Signed-off-by: Zhou Yimin zhouyi...@huawei.com
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
---
V1 here:
 https://www.redhat.com/archives/libvir-list/2015-February/msg00388.html
V2:
   Add --if-exists option to ovs-vsctl cmd,  making ovs-vsctl not raise error 
if there's no portData
   available. Suggested by Martin.
   We Tested the patch, it works.
   link: https://www.redhat.com/archives/libvir-list/2015-March/msg00104.html
V3:
   move V1/V2/V3 description from above the patch to here. It did not show the 
detailed description above
   after apply PATCH v2.
---
 src/util/virnetdevopenvswitch.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
index e5c87bb..722d0dd 100644
--- a/src/util/virnetdevopenvswitch.c
+++ b/src/util/virnetdevopenvswitch.c
@@ -30,9 +30,12 @@
 #include virerror.h
 #include virmacaddr.h
 #include virstring.h
+#include virlog.h

 #define VIR_FROM_THIS VIR_FROM_NONE

+VIR_LOG_INIT(util.netdevopenvswitch);
+
 /**
  * virNetDevOpenvswitchAddPort:
  * @brname: the bridge name
@@ -206,7 +209,7 @@ int virNetDevOpenvswitchGetMigrateData(char **migrate, 
const char *ifname)
 virCommandPtr cmd = NULL;
 int ret = -1;

-cmd = virCommandNewArgList(OVSVSCTL, --timeout=5, get, Interface,
+cmd = virCommandNewArgList(OVSVSCTL, --timeout=5, --if-exists, get, 
Interface,
ifname, external_ids:PortData, NULL);

 virCommandSetOutputBuffer(cmd, migrate);
@@ -241,6 +244,12 @@ int virNetDevOpenvswitchSetMigrateData(char *migrate, 
const char *ifname)
 virCommandPtr cmd = NULL;
 int ret = -1;

+if (!migrate) {
+VIR_DEBUG(No OVS port data for interface %s, ifname);
+return 0;
+}
+
 cmd = virCommandNewArgList(OVSVSCTL, --timeout=5, set,
Interface, ifname, NULL);
 virCommandAddArgFormat(cmd, external_ids:PortData=%s, migrate);
-- 
1.7.12.4


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list