[libvirt] [PATCH] virnetdevmacvlan.c: Introduce mutex for macvlan creation
by Michal Privoznik
Currently, after we removed the qemu driver lock, it may happen
that two or more threads will start up a machine with macvlan and
race over virNetDevMacVLanCreateWithVPortProfile(). However,
there's a racy section in which we are generating a sequence of
possible device names and detecting if they exits. If we found
one which doesn't we try to create a device with that name.
However, the other thread is doing just the same. Assume it will
succeed and we must therefore fail. If this happens more than 5
times (which in massive parallel startup surely will) we return
-1 without any error reported. This patch is a simple hack to
both of these problems. It introduces a mutex, so only one thread
will enter the section, and if it runs out of possibilities,
error is reported. Moreover, the number of retries is raised to 20.
---
This is just a quick hack which aim is to be as small as possible in order to
be well understood and hence included in the release. After the release, this
should be totally dropped in flavour of virNetDevNameAllocator.
src/util/virnetdevmacvlan.c | 38 ++++++++++++++++++++++++++++++++------
1 file changed, 32 insertions(+), 6 deletions(-)
diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
index a74db1e..bf5e7e0 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -31,6 +31,7 @@
#include "virmacaddr.h"
#include "virutil.h"
#include "virerror.h"
+#include "virthread.h"
#define VIR_FROM_THIS VIR_FROM_NET
@@ -71,6 +72,15 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, VIR_NETDEV_MACVLAN_MODE_LAST,
# define MACVLAN_NAME_PREFIX "macvlan"
# define MACVLAN_NAME_PATTERN "macvlan%d"
+virMutex virNetDevMacVLanCreateMutex;
+
+static int virNetDevMacVLanCreateMutexOnceInit(void)
+{
+ return virMutexInit(&virNetDevMacVLanCreateMutex);
+}
+
+VIR_ONCE_GLOBAL_INIT(virNetDevMacVLanCreateMutex);
+
/**
* virNetDevMacVLanCreate:
*
@@ -829,7 +839,7 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname,
char ifname[IFNAMSIZ];
int retries, do_retry = 0;
uint32_t macvtapMode;
- const char *cr_ifname;
+ const char *cr_ifname = NULL;
int ret;
int vf = -1;
@@ -870,23 +880,39 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname,
return -1;
} else {
create_name:
- retries = 5;
+ if (virNetDevMacVLanCreateMutexInitialize() < 0) {
+ virReportSystemError(errno, "%s", _("unable to init mutext"));
+ return -1;
+ }
+
+ retries = 20;
+ virMutexLock(&virNetDevMacVLanCreateMutex);
for (c = 0; c < 8192; c++) {
snprintf(ifname, sizeof(ifname), pattern, c);
- if ((ret = virNetDevExists(ifname)) < 0)
+ if ((ret = virNetDevExists(ifname)) < 0) {
+ virMutexUnlock(&virNetDevMacVLanCreateMutex);
return -1;
+ }
if (!ret) {
rc = virNetDevMacVLanCreate(ifname, type, macaddress, linkdev,
macvtapMode, &do_retry);
- if (rc == 0)
+ if (rc == 0) {
+ cr_ifname = ifname;
break;
+ }
if (do_retry && --retries)
continue;
- return -1;
+ break;
}
}
- cr_ifname = ifname;
+
+ virMutexUnlock(&virNetDevMacVLanCreateMutex);
+ if (!cr_ifname) {
+ virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("Unable to create macvlan device"));
+ return -1;
+ }
}
if (virNetDevVPortProfileAssociate(cr_ifname,
--
1.8.1.4
12 years, 1 month
[libvirt] [PATCH RFC 0/3] Keep original file label
by Michal Privoznik
Just sending out early to make sure this time I am going in acceptable
direction before digging into selinux. Hopefully, apparmor won't be any deal as
I don't see anything that should be restored on domain shut off process.
Michal Privoznik (3):
security_dac: Remember owner prior chown() and restore on relabel
security_manager: Introduce {Save,Load}Status
security_dac: Implement {save,load}Status
src/lxc/lxc_controller.c | 2 +-
src/lxc/lxc_driver.c | 1 +
src/qemu/qemu_driver.c | 3 +
src/security/security_dac.c | 465 ++++++++++++++++++++++++++++++++++-----
src/security/security_driver.h | 12 +
src/security/security_manager.c | 161 +++++++++++++-
src/security/security_manager.h | 2 +
tests/seclabeltest.c | 2 +-
tests/securityselinuxlabeltest.c | 3 +-
tests/securityselinuxtest.c | 3 +-
10 files changed, 590 insertions(+), 64 deletions(-)
--
1.8.1.4
12 years, 1 month
[libvirt] [PATCH 0/4]typo fix and codes improvement in generator.py
by Guannan Ren
This four patches try to fix various typoes, fd leaks and optimize codes
in generator.py script. This is the first round.
Guannan Ren(4)
[PATCH 1/4] python: global variable and debugging improvement for
[PATCH 2/4] python: fix typoes and repeated global vars references
[PATCH 3/4] python: optimize SAX xml parsing event handler
[PATCH 4/4] python: fix fd leak in generator.py
python/generator.py | 158 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------------------------------------------------------------------------
1 file changed, 70 insertions(+), 88 deletions(-)
12 years, 1 month
[libvirt] [PATCH] qemu: Make sure qemuProcessStart is run within a job
by Jiri Denemark
qemuProcessStart expects to be run with a job already set and every
caller except for qemuMigrationPrepareAny use it correctly. This bug can
be observed in libvirtd logs during incoming migration as
warning : qemuDomainObjEnterMonitorInternal:979 : This thread seems
to be the async job owner; entering monitor without asking for a
nested job is dangerous
---
src/qemu/qemu_domain.c | 35 ++++++++++++++++++++++++-----------
src/qemu/qemu_domain.h | 4 ++++
src/qemu/qemu_migration.c | 11 +++++++----
3 files changed, 35 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index eca85fc..0e56596 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -881,6 +881,29 @@ int qemuDomainObjBeginAsyncJob(virQEMUDriverPtr driver,
asyncJob);
}
+int
+qemuDomainObjBeginNestedJob(virQEMUDriverPtr driver,
+ virDomainObjPtr obj,
+ enum qemuDomainAsyncJob asyncJob)
+{
+ qemuDomainObjPrivatePtr priv = obj->privateData;
+
+ if (asyncJob != priv->job.asyncJob) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unexpected async job %d"), asyncJob);
+ return -1;
+ }
+
+ if (priv->job.asyncOwner != virThreadSelfID()) {
+ VIR_WARN("This thread doesn't seem to be the async job owner: %d",
+ priv->job.asyncOwner);
+ }
+
+ return qemuDomainObjBeginJobInternal(driver, obj,
+ QEMU_JOB_ASYNC_NESTED,
+ QEMU_ASYNC_JOB_NONE);
+}
+
/*
* obj must be locked before calling
@@ -955,17 +978,7 @@ qemuDomainObjEnterMonitorInternal(virQEMUDriverPtr driver,
qemuDomainObjPrivatePtr priv = obj->privateData;
if (asyncJob != QEMU_ASYNC_JOB_NONE) {
- if (asyncJob != priv->job.asyncJob) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unexpected async job %d"), asyncJob);
- return -1;
- }
- if (priv->job.asyncOwner != virThreadSelfID())
- VIR_WARN("This thread doesn't seem to be the async job owner: %d",
- priv->job.asyncOwner);
- if (qemuDomainObjBeginJobInternal(driver, obj,
- QEMU_JOB_ASYNC_NESTED,
- QEMU_ASYNC_JOB_NONE) < 0)
+ if (qemuDomainObjBeginNestedJob(driver, obj, asyncJob) < 0)
return -1;
if (!virDomainObjIsActive(obj)) {
virReportError(VIR_ERR_OPERATION_FAILED, "%s",
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 30e6b97..e114f89 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -192,6 +192,10 @@ int qemuDomainObjBeginAsyncJob(virQEMUDriverPtr driver,
virDomainObjPtr obj,
enum qemuDomainAsyncJob asyncJob)
ATTRIBUTE_RETURN_CHECK;
+int qemuDomainObjBeginNestedJob(virQEMUDriverPtr driver,
+ virDomainObjPtr obj,
+ enum qemuDomainAsyncJob asyncJob)
+ ATTRIBUTE_RETURN_CHECK;
bool qemuDomainObjEndJob(virQEMUDriverPtr driver,
virDomainObjPtr obj)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index a58a79d..4c6d7e1 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2118,6 +2118,10 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
goto endjob;
}
+ if (qemuDomainObjBeginNestedJob(driver, vm,
+ QEMU_ASYNC_JOB_MIGRATION_IN) < 0)
+ goto endjob;
+
/* Start the QEMU daemon, with the same command-line arguments plus
* -incoming $migrateFrom
*/
@@ -2126,9 +2130,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
VIR_QEMU_PROCESS_START_PAUSED |
VIR_QEMU_PROCESS_START_AUTODESTROY) < 0) {
virDomainAuditStart(vm, "migrated", false);
- /* Note that we don't set an error here because qemuProcessStart
- * should have already done that.
- */
+ if (qemuDomainObjEndJob(driver, vm) < 0)
+ vm = NULL;
goto endjob;
}
@@ -2235,7 +2238,7 @@ stop:
qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, 0);
endjob:
- if (!qemuMigrationJobFinish(driver, vm)) {
+ if (vm && !qemuMigrationJobFinish(driver, vm)) {
vm = NULL;
}
goto cleanup;
--
1.8.1.4
12 years, 1 month
