[libvirt] [PATCH 0/3] Fix various issues with libvirt networks

This patchset fixes various non-related issues with libvirt networking: Peter Krempa (3): network: bridge: Fix regression when defining persistent networks virsh: Reformat output of virsh net-list network: Report real error if addition of firewall rules fails src/network/bridge_driver.c | 10 ++++++++++ tools/virsh-network.c | 7 ++++--- 2 files changed, 14 insertions(+), 3 deletions(-) -- 1.8.1

Commit 0211fd6e04cdc402da20818df54299c6ded3d3cb introduced regression where newly defined networks were not made persistent. This patch makes the network persistent on each successful definition. --- src/network/bridge_driver.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 660c38d..975b7f6 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -3124,6 +3124,9 @@ static virNetworkPtr networkDefine(virConnectPtr conn, const char *xml) { goto cleanup; } + /* define makes the network persistent - always */ + network->persistent = 1; + /* def was asigned */ freeDef = false; -- 1.8.1

This patch changes whitespace and the length of the separation line from this format: $ virsh net-list --all Name State Autostart Persistent -------------------------------------------------- default inactive yes yes to $ virsh net-list --all Name State Autostart Persistent ---------------------------------------------------------- default inactive yes yes to match the output of virsh list. --- tools/virsh-network.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tools/virsh-network.c b/tools/virsh-network.c index c9cd15a..fc73a28 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -614,9 +614,10 @@ cmdNetworkList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) if (!(list = vshNetworkListCollect(ctl, flags))) return false; - vshPrintExtra(ctl, "%-20s %-10s %-13s %s\n", _("Name"), _("State"), + vshPrintExtra(ctl, " %-20s %-10s %-13s %s\n", _("Name"), _("State"), _("Autostart"), _("Persistent")); - vshPrintExtra(ctl, "--------------------------------------------------\n"); + vshPrintExtra(ctl, + "----------------------------------------------------------\n"); for (i = 0; i < list->nnets; i++) { virNetworkPtr network = list->nets[i]; @@ -628,7 +629,7 @@ cmdNetworkList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) else autostartStr = is_autostart ? _("yes") : _("no"); - vshPrint(ctl, "%-20s %-10s %-13s %s\n", + vshPrint(ctl, " %-20s %-10s %-13s %s\n", virNetworkGetName(network), virNetworkIsActive(network) ? _("active") : _("inactive"), autostartStr, -- 1.8.1

If addition of rules in networkAddIptablesRules() failed the real error was masked by error reported when trying to clean up the remaining rules. With this patch the original error message is saved and set back after the removal is complete. --- src/network/bridge_driver.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 975b7f6..f1be954 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2088,6 +2088,7 @@ networkAddIptablesRules(struct network_driver *driver, { int ii; virNetworkIpDefPtr ipdef; + virErrorPtr orig_error; /* Add "once per network" rules */ if (networkAddGeneralIptablesRules(driver, network) < 0) @@ -2104,6 +2105,9 @@ networkAddIptablesRules(struct network_driver *driver, return 0; err: + /* store the previous error message before attempting removal of rules */ + orig_error = virSaveLastError(); + /* The final failed call to networkAddIpSpecificIptablesRules will * have removed any rules it created, but we need to remove those * added for previous IP addresses. @@ -2113,6 +2117,9 @@ err: networkRemoveIpSpecificIptablesRules(driver, network, ipdef); } networkRemoveGeneralIptablesRules(driver, network); + + /* return the original error */ + virSetError(orig_error); return -1; } -- 1.8.1

On 11.01.2013 12:09, Peter Krempa wrote:
This patchset fixes various non-related issues with libvirt networking:
Peter Krempa (3): network: bridge: Fix regression when defining persistent networks virsh: Reformat output of virsh net-list network: Report real error if addition of firewall rules fails
src/network/bridge_driver.c | 10 ++++++++++ tools/virsh-network.c | 7 ++++--- 2 files changed, 14 insertions(+), 3 deletions(-)
ACK to all three patches. Although, reading the 2/3 made me realize maybe we need internal table formatter or something. Doing it the current way isn't optimal. Moreover, with fixed column width, we fail to format longer values than the width. With table formatter we would still format table nice. Michal

On 01/11/13 13:49, Michal Privoznik wrote:
On 11.01.2013 12:09, Peter Krempa wrote:
This patchset fixes various non-related issues with libvirt networking:
Peter Krempa (3): network: bridge: Fix regression when defining persistent networks virsh: Reformat output of virsh net-list network: Report real error if addition of firewall rules fails
src/network/bridge_driver.c | 10 ++++++++++ tools/virsh-network.c | 7 ++++--- 2 files changed, 14 insertions(+), 3 deletions(-)
ACK to all three patches.
Thanks; I've pushed these.
Although, reading the 2/3 made me realize maybe we need internal table formatter or something. Doing it the current way isn't optimal. Moreover, with fixed column width, we fail to format longer values than the width. With table formatter we would still format table nice.
I was considering this approach when I was trying to rework guest listing in virsh. The issue is even worse there. Peter
Michal
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (2)
-
Michal Privoznik
-
Peter Krempa