Re: [PATCH] util: do not take it as wrong if no PortData is found while getting migrateData

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Feb 26, 2015 at 10:25:45AM +0100, Martin Kletzander wrote:
On Thu, Feb 12, 2015 at 12:08:54PM +0800, Zhang Bo wrote:
The function virNetDevOpenvswitchGetMigrateData() uses the cmd ovs-vsctl to
get portdata. If no portdata is available, rather than failure in running
the cmd, we think we should just print a warning message here, rather than
taking it as wrong, because PortData is just optional for an ovs interface.
If we take it as wrong anyway, in function qemuMigrationBakeCookie() it will
terminate in the middle, and cookies such as NBD would not be baked, further
more errors would happen, such as nbd ports get overflowed, etc.
Thus, we add an if branch in virNetDevOpenvswitchGetMigrateData() to just warn
if portdata is unavailable.

Signed-off-by: Zhang Bo <oscar.zhangbo@xxxxxxxxxx>
Signed-off-by: Zhou Yimin <zhouyimin@xxxxxxxxxx>
---
src/util/virnetdevopenvswitch.c | 28 ++++++++++++++++++++++++----
1 file changed, 24 insertions(+), 4 deletions(-)


In my previous review I mixed what Set/Get functions do, so that's why
that made way less sense for me :)

Anyway, review v2:

diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
index e5c87bb..6116377 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
@@ -204,18 +207,29 @@ int virNetDevOpenvswitchRemovePort(const char *brname ATTRIBUTE_UNUSED, const ch
int virNetDevOpenvswitchGetMigrateData(char **migrate, const char *ifname)
{
   virCommandPtr cmd = NULL;
+    char *errbuf = NULL;
   int ret = -1;

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

Yet another idea, what if you used "--if-exists get Interface ..." and
then just checked for the empty line?  The code would be cleaner as well.

Attachment: pgpbBAdO7UUvw.pgp
Description: PGP signature

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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]