[libvirt] [PATCH 0/2] bhyve: fix crashes when running without networking
Craig Rodrigues reported a problem on freebsd-virtualization list when libvirtd with bhyve driver crashes when the bridge does not exist [1]. 1: http://docs.freebsd.org/cgi/getmsg.cgi?fetch=1431191+0+current/freebsd-virtu... These two patches addresses the problem. Roman Bogorodskiy (2): bhyve: fix crash in bhyveBuildNetArgStr bhyve: do not cleanup unallocated networks on fail src/bhyve/bhyve_command.c | 3 +-- src/bhyve/bhyve_process.c | 10 ++++++---- 2 files changed, 7 insertions(+), 6 deletions(-) -- 1.9.0
bhyveBuildNetArgStr() calls virNetDevTapCreateInBridgePort() and passes tapfd = NULL, but tapfdSize = 1. That is wrong, because if virNetDevTapCreateInBridgePort() crashes after successfully creating a TAP device, it'll jump to 'error' label, that loops over tapfd and calls VIR_FORCE_CLOSE: for (i = 0; i < tapfdSize && tapfd[i] >= 0; i++) In that case we get a segfault. As the bhyve code doesn't use tapfd, pass NULL and set tapfdSize to 0. --- src/bhyve/bhyve_command.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index d3b3f69..f1862fe 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -46,7 +46,6 @@ bhyveBuildNetArgStr(const virDomainDef *def, { char macaddr[VIR_MAC_STRING_BUFLEN]; char *realifname = NULL; - int *tapfd = NULL; char *brname = NULL; int actualType = virDomainNetGetActualType(net); @@ -72,7 +71,7 @@ bhyveBuildNetArgStr(const virDomainDef *def, if (!dryRun) { if (virNetDevTapCreateInBridgePort(brname, &net->ifname, &net->mac, - def->uuid, tapfd, 1, + def->uuid, NULL, 0, virDomainNetGetActualVirtPortProfile(net), virDomainNetGetActualVlan(net), VIR_NETDEV_TAP_CREATE_IFUP | VIR_NETDEV_TAP_CREATE_PERSIST) < 0) { -- 1.9.0
On 06/13/2014 10:48 AM, Roman Bogorodskiy wrote:
bhyveBuildNetArgStr() calls virNetDevTapCreateInBridgePort() and passes tapfd = NULL, but tapfdSize = 1. That is wrong, because if virNetDevTapCreateInBridgePort() crashes after successfully creating a TAP device, it'll jump to 'error' label, that loops over tapfd and calls VIR_FORCE_CLOSE:
for (i = 0; i < tapfdSize && tapfd[i] >= 0; i++)
In that case we get a segfault.
As the bhyve code doesn't use tapfd, pass NULL and set tapfdSize to 0. --- src/bhyve/bhyve_command.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Eric Blake wrote:
On 06/13/2014 10:48 AM, Roman Bogorodskiy wrote:
bhyveBuildNetArgStr() calls virNetDevTapCreateInBridgePort() and passes tapfd = NULL, but tapfdSize = 1. That is wrong, because if virNetDevTapCreateInBridgePort() crashes after successfully creating a TAP device, it'll jump to 'error' label, that loops over tapfd and calls VIR_FORCE_CLOSE:
for (i = 0; i < tapfdSize && tapfd[i] >= 0; i++)
In that case we get a segfault.
As the bhyve code doesn't use tapfd, pass NULL and set tapfdSize to 0. --- src/bhyve/bhyve_command.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
ACK.
Thanks; pushed. Roman Bogorodskiy
virBhyveProcessStart() calls bhyveNetCleanup() if it fails. However, it might fail earlier than networks are allocated, so modify bhyveNetCleanup() to check if net->ifname is not NULL before going further with the cleanup. --- src/bhyve/bhyve_process.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c index a5ad68d..b8db076 100644 --- a/src/bhyve/bhyve_process.c +++ b/src/bhyve/bhyve_process.c @@ -78,10 +78,12 @@ bhyveNetCleanup(virDomainObjPtr vm) int actualType = virDomainNetGetActualType(net); if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) { - ignore_value(virNetDevBridgeRemovePort( - virDomainNetGetActualBridgeName(net), - net->ifname)); - ignore_value(virNetDevTapDelete(net->ifname)); + if (net->ifname) { + ignore_value(virNetDevBridgeRemovePort( + virDomainNetGetActualBridgeName(net), + net->ifname)); + ignore_value(virNetDevTapDelete(net->ifname)); + } } } } -- 1.9.0
On 06/13/2014 10:48 AM, Roman Bogorodskiy wrote:
virBhyveProcessStart() calls bhyveNetCleanup() if it fails. However, it might fail earlier than networks are allocated, so modify bhyveNetCleanup() to check if net->ifname is not NULL before going further with the cleanup. --- src/bhyve/bhyve_process.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Eric Blake wrote:
On 06/13/2014 10:48 AM, Roman Bogorodskiy wrote:
virBhyveProcessStart() calls bhyveNetCleanup() if it fails. However, it might fail earlier than networks are allocated, so modify bhyveNetCleanup() to check if net->ifname is not NULL before going further with the cleanup. --- src/bhyve/bhyve_process.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
ACK.
Thanks; pushed. Roman Bogorodskiy
participants (2)
-
Eric Blake -
Roman Bogorodskiy