[libvirt] [PATCH v2] qemu: add reporting of vCPU wait time
by Daniel P. Berrange
The VIR_DOMAIN_STATS_VCPU flag to virDomainListGetStats
enables reporting of stats about vCPUs. Currently we
only report the cumulative CPU running time and the
execution state.
This adds reporting of the wait time - time the vCPU
wants to run, but the host schedular has something else
running ahead of it.
The data is reported per-vCPU eg
$ virsh domstats --vcpu demo
Domain: 'demo'
vcpu.current=4
vcpu.maximum=4
vcpu.0.state=1
vcpu.0.time=1420000000
vcpu.0.wait=18403928
vcpu.1.state=1
vcpu.1.time=130000000
vcpu.1.wait=10612111
vcpu.2.state=1
vcpu.2.time=110000000
vcpu.2.wait=12759501
vcpu.3.state=1
vcpu.3.time=90000000
vcpu.3.wait=21825087
In implementing this I notice our reporting of CPU execute
time has very poor granularity, since we are getting it
from /proc/$PID/stat. As a future enhancement we should
prefer to get CPU execute time from /proc/$PID/schedstat
or /proc/$PID/sched (if either exist on the running kernel)
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/qemu/qemu_driver.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 96 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 36fb047..40c52d4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1361,6 +1361,80 @@ static char *qemuConnectGetCapabilities(virConnectPtr conn) {
static int
+qemuGetSchedInfo(unsigned long long *cpuWait,
+ pid_t pid, pid_t tid)
+{
+ char *proc = NULL;
+ char *data = NULL;
+ char **lines = NULL;
+ size_t i;
+ int ret = -1;
+ double val;
+
+ *cpuWait = 0;
+
+ /* In general, we cannot assume pid_t fits in int; but /proc parsing
+ * is specific to Linux where int works fine. */
+ if (tid)
+ ret = virAsprintf(&proc, "/proc/%d/task/%d/sched", (int)pid, (int)tid);
+ else
+ ret = virAsprintf(&proc, "/proc/%d/sched", (int)pid);
+ if (ret < 0)
+ goto cleanup;
+ ret = -1;
+
+ /* The file is not guaranteed to exist (needs CONFIG_SCHED_DEBUG) */
+ if (access(proc, R_OK) < 0)
+ return 0;
+
+ if (virFileReadAll(proc, (1<<16), &data) < 0)
+ goto cleanup;
+
+ lines = virStringSplit(data, "\n", 0);
+ if (!lines)
+ goto cleanup;
+
+ for (i = 0; lines[i] != NULL; i++) {
+ const char *line = lines[i];
+
+ /* Needs CONFIG_SCHEDSTATS. The second check
+ * is the old name the kernel used in past */
+ if (STRPREFIX(line, "se.statistics.wait_sum") ||
+ STRPREFIX(line, "se.wait_sum")) {
+ line = strchr(line, ':');
+ if (!line) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Missing separate in sched info '%s'"),
+ lines[i]);
+ goto cleanup;
+ }
+ line++;
+ while (*line == ' ')
+ line++;
+
+ if (virStrToDouble(line, NULL, &val) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unable to parse sched info value '%s'"),
+ line);
+ goto cleanup;
+ }
+
+ *cpuWait = (unsigned long long)(val * 1000000);
+ break;
+ }
+ }
+
+ ret = 0;
+
+ cleanup:
+ VIR_FREE(data);
+ VIR_FREE(proc);
+ VIR_FREE(lines);
+ return ret;
+}
+
+
+static int
qemuGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, long *vm_rss,
pid_t pid, int tid)
{
@@ -1424,6 +1498,7 @@ qemuGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, long *vm_rss,
static int
qemuDomainHelperGetVcpus(virDomainObjPtr vm,
virVcpuInfoPtr info,
+ unsigned long long *cpuwait,
int maxinfo,
unsigned char *cpumaps,
int maplen)
@@ -1476,6 +1551,11 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm,
virBitmapFree(map);
}
+ if (cpuwait) {
+ if (qemuGetSchedInfo(&(cpuwait[i]), vm->pid, vcpupid) < 0)
+ return -1;
+ }
+
ncpuinfo++;
}
@@ -5490,7 +5570,7 @@ qemuDomainGetVcpus(virDomainPtr dom,
goto cleanup;
}
- ret = qemuDomainHelperGetVcpus(vm, info, maxinfo, cpumaps, maplen);
+ ret = qemuDomainHelperGetVcpus(vm, info, NULL, maxinfo, cpumaps, maplen);
cleanup:
virDomainObjEndAPI(&vm);
@@ -19036,6 +19116,7 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
int ret = -1;
char param_name[VIR_TYPED_PARAM_FIELD_LENGTH];
virVcpuInfoPtr cpuinfo = NULL;
+ unsigned long long *cpuwait = NULL;
if (virTypedParamsAddUInt(&record->params,
&record->nparams,
@@ -19051,10 +19132,12 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
virDomainDefGetVcpusMax(dom->def)) < 0)
return -1;
- if (VIR_ALLOC_N(cpuinfo, virDomainDefGetVcpus(dom->def)) < 0)
- return -1;
+ if (VIR_ALLOC_N(cpuinfo, virDomainDefGetVcpus(dom->def)) < 0 ||
+ VIR_ALLOC_N(cpuwait, virDomainDefGetVcpus(dom->def)) < 0)
+ goto cleanup;
- if (qemuDomainHelperGetVcpus(dom, cpuinfo, virDomainDefGetVcpus(dom->def),
+ if (qemuDomainHelperGetVcpus(dom, cpuinfo, cpuwait,
+ virDomainDefGetVcpus(dom->def),
NULL, 0) < 0) {
virResetLastError();
ret = 0; /* it's ok to be silent and go ahead */
@@ -19083,12 +19166,21 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
param_name,
cpuinfo[i].cpuTime) < 0)
goto cleanup;
+ snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "vcpu.%zu.wait", i);
+ if (virTypedParamsAddULLong(&record->params,
+ &record->nparams,
+ maxparams,
+ param_name,
+ cpuwait[i]) < 0)
+ goto cleanup;
}
ret = 0;
cleanup:
VIR_FREE(cpuinfo);
+ VIR_FREE(cpuwait);
return ret;
}
--
2.5.0
9 years, 2 months
[libvirt] [PATCH] qemu: qemuDomainRename and virDomainObjListNumOfDomains ABBA deadlock fix
by Maxim Nestratov
A pretty nasty deadlock occurs while trying to rename a VM in parallel
with virDomainObjListNumOfDomains.
The short description of the problem is as follows:
Thread #1:
qemuDomainRename:
------> aquires domain lock by qemuDomObjFromDomain
---------> waits for domain list lock in any of the listed functions:
- virDomainObjListFindByName
- virDomainObjListRenameAddNew
- virDomainObjListRenameRemove
Thread #2:
virDomainObjListNumOfDomains:
------> aquires domain list lock
---------> waits for domain lock in virDomainObjListCount
The patch establishes a single order of taking locks: driver->domains list
first, then a particular VM. This is done by implementing a set of
virDomainObjListXxxLocked functions working with driver->domains that assume
that list lock is already aquired by calling functions.
Signed-off-by: Maxim Nestratov <mnestratov(a)virtuozzo.com>
---
src/conf/virdomainobjlist.c | 74 +++++++++++++++++++++++++++++++++++++--------
src/conf/virdomainobjlist.h | 9 ++++++
src/libvirt_private.syms | 4 +++
src/qemu/qemu_driver.c | 40 +++++++++++++++++++++---
4 files changed, 110 insertions(+), 17 deletions(-)
diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c
index 7568f93..89e28a8 100644
--- a/src/conf/virdomainobjlist.c
+++ b/src/conf/virdomainobjlist.c
@@ -132,21 +132,19 @@ virDomainObjPtr virDomainObjListFindByID(virDomainObjListPtr doms,
static virDomainObjPtr
-virDomainObjListFindByUUIDInternal(virDomainObjListPtr doms,
+virDomainObjListFindByUUIDInternalLocked(virDomainObjListPtr doms,
const unsigned char *uuid,
bool ref)
{
char uuidstr[VIR_UUID_STRING_BUFLEN];
virDomainObjPtr obj;
- virObjectLock(doms);
virUUIDFormat(uuid, uuidstr);
obj = virHashLookup(doms->objs, uuidstr);
- if (ref) {
+ if (ref)
virObjectRef(obj);
- virObjectUnlock(doms);
- }
+
if (obj) {
virObjectLock(obj);
if (obj->removing) {
@@ -156,8 +154,19 @@ virDomainObjListFindByUUIDInternal(virDomainObjListPtr doms,
obj = NULL;
}
}
- if (!ref)
- virObjectUnlock(doms);
+ return obj;
+}
+
+static virDomainObjPtr
+virDomainObjListFindByUUIDInternal(virDomainObjListPtr doms,
+ const unsigned char *uuid,
+ bool ref)
+{
+ virDomainObjPtr obj;
+
+ virObjectLock(doms);
+ obj = virDomainObjListFindByUUIDInternalLocked(doms, uuid, ref);
+ virObjectUnlock(doms);
return obj;
}
@@ -169,6 +178,13 @@ virDomainObjListFindByUUID(virDomainObjListPtr doms,
return virDomainObjListFindByUUIDInternal(doms, uuid, false);
}
+virDomainObjPtr
+virDomainObjListFindByUUIDRefLocked(virDomainObjListPtr doms,
+ const unsigned char *uuid)
+{
+ return virDomainObjListFindByUUIDInternalLocked(doms, uuid, true);
+}
+
virDomainObjPtr
virDomainObjListFindByUUIDRef(virDomainObjListPtr doms,
@@ -177,6 +193,23 @@ virDomainObjListFindByUUIDRef(virDomainObjListPtr doms,
return virDomainObjListFindByUUIDInternal(doms, uuid, true);
}
+virDomainObjPtr virDomainObjListFindByNameLocked(virDomainObjListPtr doms,
+ const char *name)
+{
+ virDomainObjPtr obj;
+
+ obj = virHashLookup(doms->objsName, name);
+ virObjectRef(obj);
+ if (obj) {
+ virObjectLock(obj);
+ if (obj->removing) {
+ virObjectUnlock(obj);
+ virObjectUnref(obj);
+ obj = NULL;
+ }
+ }
+ return obj;
+}
virDomainObjPtr virDomainObjListFindByName(virDomainObjListPtr doms,
const char *name)
@@ -312,14 +345,12 @@ virDomainObjPtr virDomainObjListAdd(virDomainObjListPtr doms,
return ret;
}
-
int
-virDomainObjListRenameAddNew(virDomainObjListPtr doms,
+virDomainObjListRenameAddNewLocked(virDomainObjListPtr doms,
virDomainObjPtr vm,
const char *name)
{
int ret = -1;
- virObjectLock(doms);
/* Add new name into the hash table of domain names. */
if (virHashAddEntry(doms->objsName, name, vm) < 0)
@@ -332,21 +363,38 @@ virDomainObjListRenameAddNew(virDomainObjListPtr doms,
ret = 0;
cleanup:
+ return ret;
+}
+
+int
+virDomainObjListRenameAddNew(virDomainObjListPtr doms,
+ virDomainObjPtr vm,
+ const char *name)
+{
+ int ret = -1;
+ virObjectLock(doms);
+ ret = virDomainObjListRenameAddNewLocked(doms, vm, name);
virObjectUnlock(doms);
return ret;
}
+int
+virDomainObjListRenameRemoveLocked(virDomainObjListPtr doms, const char *name)
+{
+ virHashRemoveEntry(doms->objsName, name);
+ return 0;
+}
int
virDomainObjListRenameRemove(virDomainObjListPtr doms, const char *name)
{
+ int ret;
virObjectLock(doms);
- virHashRemoveEntry(doms->objsName, name);
+ ret = virDomainObjListRenameRemoveLocked(doms, name);
virObjectUnlock(doms);
- return 0;
+ return ret;
}
-
/*
* The caller must hold a lock on the driver owning 'doms',
* and must also have locked 'dom', to ensure no one else
diff --git a/src/conf/virdomainobjlist.h b/src/conf/virdomainobjlist.h
index f479598..bca31cf 100644
--- a/src/conf/virdomainobjlist.h
+++ b/src/conf/virdomainobjlist.h
@@ -38,8 +38,12 @@ virDomainObjPtr virDomainObjListFindByUUID(virDomainObjListPtr doms,
const unsigned char *uuid);
virDomainObjPtr virDomainObjListFindByUUIDRef(virDomainObjListPtr doms,
const unsigned char *uuid);
+virDomainObjPtr virDomainObjListFindByUUIDRefLocked(virDomainObjListPtr doms,
+ const unsigned char *uuid);
virDomainObjPtr virDomainObjListFindByName(virDomainObjListPtr doms,
const char *name);
+virDomainObjPtr virDomainObjListFindByNameLocked(virDomainObjListPtr doms,
+ const char *name);
enum {
VIR_DOMAIN_OBJ_LIST_ADD_LIVE = (1 << 0),
@@ -54,8 +58,13 @@ virDomainObjPtr virDomainObjListAdd(virDomainObjListPtr doms,
int virDomainObjListRenameAddNew(virDomainObjListPtr doms,
virDomainObjPtr vm,
const char *name);
+int virDomainObjListRenameAddNewLocked(virDomainObjListPtr doms,
+ virDomainObjPtr vm,
+ const char *name);
int virDomainObjListRenameRemove(virDomainObjListPtr doms,
const char *name);
+int virDomainObjListRenameRemoveLocked(virDomainObjListPtr doms,
+ const char *name);
void virDomainObjListRemove(virDomainObjListPtr doms,
virDomainObjPtr dom);
void virDomainObjListRemoveLocked(virDomainObjListPtr doms,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 5e05a98..b05d6cc 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -881,8 +881,10 @@ virDomainObjListConvert;
virDomainObjListExport;
virDomainObjListFindByID;
virDomainObjListFindByName;
+virDomainObjListFindByNameLocked;
virDomainObjListFindByUUID;
virDomainObjListFindByUUIDRef;
+virDomainObjListFindByUUIDRefLocked;
virDomainObjListForEach;
virDomainObjListGetActiveIDs;
virDomainObjListGetInactiveNames;
@@ -892,7 +894,9 @@ virDomainObjListNumOfDomains;
virDomainObjListRemove;
virDomainObjListRemoveLocked;
virDomainObjListRenameAddNew;
+virDomainObjListRenameAddNewLocked;
virDomainObjListRenameRemove;
+virDomainObjListRenameRemoveLocked;
# cpu/cpu.h
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e46404b..b012516 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -206,6 +206,36 @@ struct qemuAutostartData {
virConnectPtr conn;
};
+/**
+ * qemuDomObjFromDomainLocked:
+ * @domain: Domain pointer that has to be looked up
+ *
+ * This function looks up @domain and returns the appropriate virDomainObjPtr
+ * that has to be released by calling virDomainObjEndAPI().
+ * It assumes that driver->domains list is locked already, thus it uses Locked
+ * variant of virDomainObjListFindByUUIDRef function.
+ *
+ * Returns the domain object with incremented reference counter which is locked
+ * on success, NULL otherwise.
+ */
+static virDomainObjPtr
+qemuDomObjFromDomainLocked(virDomainPtr domain)
+{
+ virDomainObjPtr vm;
+ virQEMUDriverPtr driver = domain->conn->privateData;
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+
+ vm = virDomainObjListFindByUUIDRefLocked(driver->domains, domain->uuid);
+ if (!vm) {
+ virUUIDFormat(domain->uuid, uuidstr);
+ virReportError(VIR_ERR_NO_DOMAIN,
+ _("no domain with matching uuid '%s' (%s)"),
+ uuidstr, domain->name);
+ return NULL;
+ }
+
+ return vm;
+}
/**
* qemuDomObjFromDomain:
@@ -19892,7 +19922,8 @@ static int qemuDomainRename(virDomainPtr dom,
virCheckFlags(0, ret);
- if (!(vm = qemuDomObjFromDomain(dom)))
+ virObjectLock(driver->domains);
+ if (!(vm = qemuDomObjFromDomainLocked(dom)))
goto cleanup;
if (virDomainRenameEnsureACL(dom->conn, vm->def) < 0)
@@ -19938,7 +19969,7 @@ static int qemuDomainRename(virDomainPtr dom,
* internal error. And since new_name != name here, there's no
* deadlock imminent.
*/
- tmp_dom = virDomainObjListFindByName(driver->domains, new_name);
+ tmp_dom = virDomainObjListFindByNameLocked(driver->domains, new_name);
if (tmp_dom) {
virObjectUnlock(tmp_dom);
virObjectUnref(tmp_dom);
@@ -19956,7 +19987,7 @@ static int qemuDomainRename(virDomainPtr dom,
goto endjob;
}
- if (virDomainObjListRenameAddNew(driver->domains, vm, new_name) < 0)
+ if (virDomainObjListRenameAddNewLocked(driver->domains, vm, new_name) < 0)
goto endjob;
event_old = virDomainEventLifecycleNewFromObj(vm,
@@ -19980,7 +20011,7 @@ static int qemuDomainRename(virDomainPtr dom,
}
/* Remove old domain name from table. */
- virDomainObjListRenameRemove(driver->domains, old_dom_name);
+ virDomainObjListRenameRemoveLocked(driver->domains, old_dom_name);
event_new = virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_DEFINED,
@@ -20000,6 +20031,7 @@ static int qemuDomainRename(virDomainPtr dom,
qemuDomainEventQueue(driver, event_old);
qemuDomainEventQueue(driver, event_new);
virObjectUnref(cfg);
+ virObjectUnlock(driver->domains);
return ret;
rollback:
--
1.8.3.1
9 years, 2 months
[libvirt] Ability to add arbitrary data to snapshots
by Alexander Burluka
Hello!
Currently our Virtuozzo company is working on some kind of management
tool for libvirt. We encountered some problems with snapshots while
development.
So I would like to ask community a few questions:
1) Would you mind if I extend current libvirt snapshot with ability to
add arbitrary data (blob or some config files)?
2) Would this feature be useful for your users?
I see it as something like:
virsh snapshot-create VM1 --blob /path/to/some-file
virsh snapshot-revert VM1 <number>
and /path/to/some-file appeared again.
Looking forward for your comments and help.
Thanks!
--
Regards,
Alexander Burluka
9 years, 2 months
[libvirt] [PATCH v2] virsh: improve waiting for block job readiness
by Michael Chapman
After a block job hits 100%, we only need to apply a timeout waiting for
a block job event if exactly one of the BLOCK_JOB or BLOCK_JOB_2
callbacks were able to be registered.
If neither callback could be registered, there's clearly no need for a
timeout.
If both callbacks were registered, then we're guaranteed to eventually
get one of the events. The path being used by virsh must be exactly the
source path or target device in the domain's disk definition, and these
are the respective strings sent back in these two events.
Signed-off-by: Michael Chapman <mike(a)very.puzzling.org>
---
v1 discussion at:
http://www.redhat.com/archives/libvir-list/2016-January/msg00031.html
Changes since v1:
- Fixed bugs in cb_id/cb_id2 conditionals
- Consistently used break to exit loop, dropped cleanup label
- Clarified the logic and behaviour in comments
- Improved commit message
tools/virsh-domain.c | 60 ++++++++++++++++++++++++++++------------------------
1 file changed, 32 insertions(+), 28 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index edbbc34..853416c 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1875,14 +1875,17 @@ virshBlockJobWaitFree(virshBlockJobWaitDataPtr data)
* virshBlockJobWait:
* @data: private data initialized by virshBlockJobWaitInit
*
- * Waits for the block job to complete. This function prefers to get an event
- * from libvirt but still has fallback means if the device name can't be matched
+ * Waits for the block job to complete. This function prefers to wait for a
+ * matching VIR_DOMAIN_EVENT_ID_BLOCK_JOB or VIR_DOMAIN_EVENT_ID_BLOCK_JOB_2
+ * event from libvirt, however it has a fallback mode should either of these
+ * events not be available.
*
- * This function returns values from the virConnectDomainEventBlockJobStatus enum
- * or -1 in case of a internal error. Fallback states if a block job vanishes
- * without triggering the event is VIR_DOMAIN_BLOCK_JOB_COMPLETED. For two phase
- * jobs after the retry count for waiting for the event expires is
- * VIR_DOMAIN_BLOCK_JOB_READY.
+ * This function returns values from the virConnectDomainEventBlockJobStatus
+ * enum or -1 in case of a internal error.
+ *
+ * If the fallback mode is activated the returned event is
+ * VIR_DOMAIN_BLOCK_JOB_COMPLETED if the block job vanishes, or
+ * VIR_DOMAIN_BLOCK_JOB_READY if the block job reaches 100%.
*/
static int
virshBlockJobWait(virshBlockJobWaitDataPtr data)
@@ -1932,28 +1935,32 @@ virshBlockJobWait(virshBlockJobWaitDataPtr data)
if (result < 0) {
vshError(data->ctl, _("failed to query job for disk %s"), data->dev);
- goto cleanup;
+ break;
}
- /* if we've got an event for the device we are waiting for we can end
- * the waiting loop */
+ /* If either callback could be registered and we've got an event, we can
+ * can end the waiting loop */
if ((data->cb_id >= 0 || data->cb_id2 >= 0) && data->status != -1) {
ret = data->status;
- goto cleanup;
+ break;
}
- /* since virsh can't guarantee that the path provided by the user will
- * later be matched in the event we will need to keep the fallback
- * approach and claim success if the block job finishes or vanishes. */
- if (result == 0)
- break;
+ /* Fallback behaviour is only needed if one or both callbacks could not
+ * be registered */
+ if (data->cb_id < 0 || data->cb_id2 < 0) {
+ /* If the block job vanishes, synthesize a COMPLETED event */
+ if (result == 0) {
+ ret = VIR_DOMAIN_BLOCK_JOB_COMPLETED;
+ break;
+ }
- /* for two-phase jobs we will try to wait in the synchronized phase
- * for event arrival since 100% completion doesn't necessarily mean that
- * the block job has finished and can be terminated with success */
- if (info.end == info.cur && --retries == 0) {
- ret = VIR_DOMAIN_BLOCK_JOB_READY;
- goto cleanup;
+ /* If the block job hits 100%, wait a little while for a possible
+ * event from libvirt, else synthesize our own READY event */
+ if (info.end == info.cur &&
+ ((data->cb_id < 0 && data->cb_id2 < 0) || --retries == 0)) {
+ ret = VIR_DOMAIN_BLOCK_JOB_READY;
+ break;
+ }
}
if (data->verbose)
@@ -1962,26 +1969,23 @@ virshBlockJobWait(virshBlockJobWaitDataPtr data)
if (data->timeout && virTimeMillisNow(&curr) < 0) {
vshSaveLibvirtError();
- goto cleanup;
+ break;
}
if (intCaught || (data->timeout && (curr - start > data->timeout))) {
if (virDomainBlockJobAbort(data->dom, data->dev, abort_flags) < 0) {
vshError(data->ctl, _("failed to abort job for disk '%s'"),
data->dev);
- goto cleanup;
+ break;
}
ret = VIR_DOMAIN_BLOCK_JOB_CANCELED;
- goto cleanup;
+ break;
}
usleep(500 * 1000);
}
- ret = VIR_DOMAIN_BLOCK_JOB_COMPLETED;
-
- cleanup:
/* print 100% completed */
if (data->verbose &&
(ret == VIR_DOMAIN_BLOCK_JOB_COMPLETED ||
--
2.4.3
9 years, 2 months
[libvirt] [PATCH] virsh: fix cpu-stats command output format issue
by Luyao Huang
After commit 57177f1, the cpu-stats command format change to:
CPU0:
cpu_time 14401.507878990 seconds
vcpu_time 14378732785511
vcpu_time is not user friendly. After this patch, it will
change back:
CPU0:
cpu_time 14401.507878990 seconds
vcpu_time 14378.732785511 seconds
https://bugzilla.redhat.com/show_bug.cgi?id=1301807
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
tools/virsh-domain.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 1089a4d..c2146d2 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -7243,6 +7243,7 @@ vshCPUStatsPrintField(vshControl *ctl,
{
vshPrint(ctl, "\t%-12s ", param->field);
if ((STREQ(param->field, VIR_DOMAIN_CPU_STATS_CPUTIME) ||
+ STREQ(param->field, VIR_DOMAIN_CPU_STATS_VCPUTIME) ||
STREQ(param->field, VIR_DOMAIN_CPU_STATS_USERTIME) ||
STREQ(param->field, VIR_DOMAIN_CPU_STATS_SYSTEMTIME)) &&
param->type == VIR_TYPED_PARAM_ULLONG) {
--
1.8.3.1
9 years, 2 months
[libvirt] [PATCHv2] util: keep/use a bitmap of in-use macvtap devices
by Laine Stump
This patch creates two bitmaps, one for macvlan devicenames and one
for macvtap. The bitmap position is used to indicate that libvirt is
currently using a device with the name macvtap%d/macvlan%d, where %d
is the position in the bitmap. When requested to create a new
macvtap/macvlan device, libvirt will now look for the first clear bit
in the appropriate bitmap and derive the device name from that rather
than just starting at 0 and counting up until one works.
When libvirtd is restarted, qemu calls the appropriate function to
"re-reserve" the device names as it is scanning the status of running
domains.
Note that it may seem strange that the retry counter now starts at
8192 instead of 5. This is because we now don't do a "pre-check" for
the existence of a device once we've reserved it in the bitmap - we
move straight to creating it; although very unlikely, it's possible
that someone has a running system where they have a large number of
network devices *created outside libvirt* named "macvtap%d" or
"macvlan%d" - such a setup would still allow creating more devices
with the old code, while a low retry max in the new code would cause a
failure. Since the objective of the retry max is just to prevent an
infinite loop, and it's highly unlikely to do more than 1 iteration
anyway, having a high max is a reasonable concession in order to
prevent lots of new failures.
---
Changes from V1:
* combine virNetDevMacVLanReserveNextFreeID() and virNetDevMacVLanReserveID()
* put "macvlan" or "macvtap" into the "unable to create [blah] device
"%s" as appropriate
* eliminate redundant message when all device names are in use.
(Background info that doesn't need to be in commit log:
Tony Krowiak <akrowiak(a)linux.vnet.ibm.com> had pointed out here:
https://www.redhat.com/archives/libvir-list/2015-October/msg00877.html
that libvirt takes a *very long time* to creat macvtap/macvlan devices
when there are a lot of them already in use. This is because the
algorithm simply starts at macvtap0 and iterates calling
virNetDevExists() for each possible name until it finds a free one. He
had said at the time that he had tried keeping track of in-use
macvtap/macvlan numbers in a bitmap, and had returns start time for a
domain by 1m46s (from 2m to 14s) when there were 82 macvtap devices
already in use. There was also an implication that he would post some
patches for this to the list but, like all of us, apparently got busy
with something else.
Last week I ran into a situation where libvirt had created a macvtap
device for a domain, and the domain was still using it, but for some
reason the kernel (2.6.32-something on RHEL6) had lost track of that
macvtap device, so when libvirt was asked to create a new macvtap
device for another domain, it found that name to be "unused", and
created a new device. The device creation was successful, but 802.1Qbh
negotion failed. This failure was possibly because of the re-use of
the macvtap name, possibly not, but the only way to find out was to
not re-use the name. So lacking Tony's patches, I made my own and am
posting them here.
)
src/libvirt_private.syms | 2 +
src/qemu/qemu_process.c | 10 +-
src/util/virnetdevmacvlan.c | 380 ++++++++++++++++++++++++++++++++++++--------
src/util/virnetdevmacvlan.h | 5 +-
4 files changed, 332 insertions(+), 65 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 5e05a98..f08bd8c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1851,6 +1851,8 @@ virNetDevMacVLanCreate;
virNetDevMacVLanCreateWithVPortProfile;
virNetDevMacVLanDelete;
virNetDevMacVLanDeleteWithVPortProfile;
+virNetDevMacVLanReleaseName;
+virNetDevMacVLanReserveName;
virNetDevMacVLanRestartWithVPortProfile;
virNetDevMacVLanVPortProfileRegisterCallback;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index d465b4f..31812f6 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1,7 +1,7 @@
/*
* qemu_process.c: QEMU process management
*
- * Copyright (C) 2006-2015 Red Hat, Inc.
+ * Copyright (C) 2006-2016 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -3161,6 +3161,14 @@ qemuProcessNotifyNets(virDomainDefPtr def)
for (i = 0; i < def->nnets; i++) {
virDomainNetDefPtr net = def->nets[i];
+ /* keep others from trying to use the macvtap device name, but
+ * don't return error if this happens, since that causes the
+ * domain to be unceremoniously killed, which would be *very*
+ * impolite.
+ */
+ if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_DIRECT)
+ ignore_value(virNetDevMacVLanReserveName(net->ifname, false));
+
if (networkNotifyActualDevice(def, net) < 0)
return -1;
}
diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
index 98da009..fd168b7 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2010-2015 Red Hat, Inc.
+ * Copyright (C) 2010-2016 Red Hat, Inc.
* Copyright (C) 2010-2012 IBM Corporation
*
* This library is free software; you can redistribute it and/or
@@ -64,6 +64,7 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, VIR_NETDEV_MACVLAN_MODE_LAST,
# include "virnetlink.h"
# include "virnetdev.h"
# include "virpidfile.h"
+# include "virbitmap.h"
VIR_LOG_INIT("util.netdevmacvlan");
@@ -73,7 +74,225 @@ VIR_LOG_INIT("util.netdevmacvlan");
# define MACVLAN_NAME_PREFIX "macvlan"
# define MACVLAN_NAME_PATTERN "macvlan%d"
+# define MACVLAN_MAX_ID 8191
+
virMutex virNetDevMacVLanCreateMutex = VIR_MUTEX_INITIALIZER;
+virBitmapPtr macvtapIDs = NULL;
+virBitmapPtr macvlanIDs = NULL;
+
+static int
+virNetDevMacVLanOnceInit(void)
+{
+
+ if (!macvtapIDs &&
+ !(macvtapIDs = virBitmapNew(MACVLAN_MAX_ID + 1)))
+ return -1;
+ if (!macvlanIDs &&
+ !(macvlanIDs = virBitmapNew(MACVLAN_MAX_ID + 1)))
+ return -1;
+ return 0;
+}
+
+VIR_ONCE_GLOBAL_INIT(virNetDevMacVLan);
+
+
+/**
+ * virNetDevMacVLanReserveID:
+ *
+ * @id: id 0 - MACVLAN_MAX_ID+1 to reserve (or -1 for "first free")
+ * @flags: set VIR_NETDEV_MACVLAN_CREATE_WITH_TAP for macvtapN else macvlanN
+ * @quietFail: don't log an error if this name is already in-use
+ * @nextFree: reserve the next free ID *after* @id rather than @id itself
+ *
+ * Reserve the indicated ID in the appropriate bitmap, or find the
+ * first free ID if @id is -1.
+ *
+ * returns id or -1 to indicate failure
+ */
+static int
+virNetDevMacVLanReserveID(int id, unsigned int flags,
+ bool quietFail, bool nextFree)
+{
+ virBitmapPtr bitmap;
+
+ if (virNetDevMacVLanInitialize() < 0)
+ return -1;
+
+ bitmap = (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?
+ macvtapIDs : macvlanIDs;
+
+ if (id > MACVLAN_MAX_ID) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("can't use name %s%d - out of range 0-%d"),
+ (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?
+ MACVTAP_NAME_PREFIX : MACVLAN_NAME_PREFIX,
+ id, MACVLAN_MAX_ID);
+ return -1;
+ }
+
+ if ((id < 0 || nextFree) &&
+ (id = virBitmapNextClearBit(bitmap, 0)) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("no unused %s names available"),
+ (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?
+ MACVTAP_NAME_PREFIX : MACVLAN_NAME_PREFIX);
+ return -1;
+ }
+
+ if (virBitmapIsBitSet(bitmap, id)) {
+ if (quietFail) {
+ VIR_INFO("couldn't reserve name %s%d - already in use",
+ (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?
+ MACVTAP_NAME_PREFIX : MACVLAN_NAME_PREFIX, id);
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("couldn't reserve name %s%d - already in use"),
+ (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?
+ MACVTAP_NAME_PREFIX : MACVLAN_NAME_PREFIX, id);
+ }
+ return -1;
+ }
+
+ if (virBitmapSetBit(bitmap, id) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("couldn't mark %s%d as used"),
+ (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?
+ MACVTAP_NAME_PREFIX : MACVLAN_NAME_PREFIX, id);
+ return -1;
+ }
+
+ VIR_INFO("reserving device %s%d",
+ (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?
+ MACVTAP_NAME_PREFIX : MACVLAN_NAME_PREFIX, id);
+ return id;
+}
+
+
+/**
+ * virNetDevMacVLanReleaseID:
+ * @id: id 0 - MACVLAN_MAX_ID+1 to release
+ *
+ * returns 0 for success or -1 for failure
+ */
+static int
+virNetDevMacVLanReleaseID(int id, unsigned int flags)
+{
+ virBitmapPtr bitmap;
+
+ if (virNetDevMacVLanInitialize() < 0)
+ return 0;
+
+ bitmap = (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?
+ macvtapIDs : macvlanIDs;
+
+ if (id > MACVLAN_MAX_ID) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("can't free name %s%d - out of range 0-%d"),
+ (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?
+ MACVTAP_NAME_PREFIX : MACVLAN_NAME_PREFIX,
+ id, MACVLAN_MAX_ID);
+ return -1;
+ }
+
+ if (id < 0)
+ return 0;
+
+ VIR_INFO("releasing %sdevice %s%d",
+ virBitmapIsBitSet(bitmap, id) ? "" : "unreserved",
+ (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?
+ MACVTAP_NAME_PREFIX : MACVLAN_NAME_PREFIX, id);
+
+ if (virBitmapClearBit(bitmap, id) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("couldn't mark %s%d as unused"),
+ (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?
+ MACVTAP_NAME_PREFIX : MACVLAN_NAME_PREFIX, id);
+ return -1;
+ }
+ return 0;
+}
+
+
+/**
+ * virNetDevMacVLanReserveName:
+ *
+ * @name: already-known name of device
+ * @quietFail: don't log an error if this name is already in-use
+ *
+ * Extract the device type and id from a macvtap/macvlan device name
+ * and mark the appropriate position as in-use in the appropriate
+ * bitmap.
+ *
+ * returns 0 on success, -1 on failure, -2 if the name doesn't fit the auto-pattern
+ */
+int
+virNetDevMacVLanReserveName(const char *name, bool quietFail)
+{
+ unsigned int id;
+ unsigned int flags = 0;
+ const char *idstr = NULL;
+
+ if (virNetDevMacVLanInitialize() < 0)
+ return -1;
+
+ if (STRPREFIX(name, MACVTAP_NAME_PREFIX)) {
+ idstr = name + strlen(MACVTAP_NAME_PREFIX);
+ flags |= VIR_NETDEV_MACVLAN_CREATE_WITH_TAP;
+ } else if (STRPREFIX(name, MACVLAN_NAME_PREFIX)) {
+ idstr = name + strlen(MACVLAN_NAME_PREFIX);
+ } else {
+ return -2;
+ }
+
+ if (virStrToLong_ui(idstr, NULL, 10, &id) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("couldn't get id value from macvtap device name %s"),
+ name);
+ return -1;
+ }
+ return virNetDevMacVLanReserveID(id, flags, quietFail, false);
+}
+
+
+/**
+ * virNetDevMacVLanReleaseName:
+ *
+ * @name: already-known name of device
+ *
+ * Extract the device type and id from a macvtap/macvlan device name
+ * and mark the appropriate position as in-use in the appropriate
+ * bitmap.
+ *
+ * returns 0 on success, -1 on failure
+ */
+int
+virNetDevMacVLanReleaseName(const char *name)
+{
+ unsigned int id;
+ unsigned int flags = 0;
+ const char *idstr = NULL;
+
+ if (virNetDevMacVLanInitialize() < 0)
+ return -1;
+
+ if (STRPREFIX(name, MACVTAP_NAME_PREFIX)) {
+ idstr = name + strlen(MACVTAP_NAME_PREFIX);
+ flags |= VIR_NETDEV_MACVLAN_CREATE_WITH_TAP;
+ } else if (STRPREFIX(name, MACVLAN_NAME_PREFIX)) {
+ idstr = name + strlen(MACVLAN_NAME_PREFIX);
+ } else {
+ return 0;
+ }
+
+ if (virStrToLong_ui(idstr, NULL, 10, &id) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("couldn't get id value from macvtap device name %s"),
+ name);
+ return -1;
+ }
+ return virNetDevMacVLanReleaseID(id, flags);
+}
+
/**
* virNetDevMacVLanCreate:
@@ -724,14 +943,20 @@ virNetDevMacVLanVPortProfileRegisterCallback(const char *ifname,
* virNetDevMacVLanCreateWithVPortProfile:
* Create an instance of a macvtap device and open its tap character
* device.
- * @tgifname: Interface name that the macvtap is supposed to have. May
- * be NULL if this function is supposed to choose a name
+
+ * @ifnameRequested: Interface name that the caller wants the macvtap
+ * device to have, or NULL to pick the first available name
+ * appropriate for the type (macvlan%d or macvtap%d). If the
+ * suggested name fits one of those patterns, but is already in
+ * use, we will fallback to finding the first available. If the
+ * suggested name *doesn't* fit a pattern and the name is in use,
+ * we will fail.
* @macaddress: The MAC address for the macvtap device
* @linkdev: The interface name of the NIC to connect to the external bridge
- * @mode: int describing the mode for 'bridge', 'vepa', 'private' or 'passthru'.
+ * @mode: macvtap mode (VIR_NETDEV_MACVLAN_MODE_(BRIDGE|VEPA|PRIVATE|PASSTHRU)
* @vmuuid: The UUID of the VM the macvtap belongs to
* @virtPortProfile: pointer to object holding the virtual port profile data
- * @res_ifname: Pointer to a string pointer where the actual name of the
+ * @ifnameResult: Pointer to a string pointer where the actual name of the
* interface will be stored into if everything succeeded. It is up
* to the caller to free the string.
* @tapfd: array of file descriptor return value for the new tap device
@@ -744,39 +969,36 @@ virNetDevMacVLanVPortProfileRegisterCallback(const char *ifname,
*
* Return 0 on success, -1 on error.
*/
-int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname,
- const virMacAddr *macaddress,
- const char *linkdev,
- virNetDevMacVLanMode mode,
- const unsigned char *vmuuid,
- virNetDevVPortProfilePtr virtPortProfile,
- char **res_ifname,
- virNetDevVPortProfileOp vmOp,
- char *stateDir,
- int *tapfd,
- size_t tapfdSize,
- unsigned int flags)
+int
+virNetDevMacVLanCreateWithVPortProfile(const char *ifnameRequested,
+ const virMacAddr *macaddress,
+ const char *linkdev,
+ virNetDevMacVLanMode mode,
+ const unsigned char *vmuuid,
+ virNetDevVPortProfilePtr virtPortProfile,
+ char **ifnameResult,
+ virNetDevVPortProfileOp vmOp,
+ char *stateDir,
+ int *tapfd,
+ size_t tapfdSize,
+ unsigned int flags)
{
const char *type = (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?
"macvtap" : "macvlan";
- const char *prefix = (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?
- MACVTAP_NAME_PREFIX : MACVLAN_NAME_PREFIX;
const char *pattern = (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?
MACVTAP_NAME_PATTERN : MACVLAN_NAME_PATTERN;
- int c, rc;
+ int rc, reservedID = -1;
char ifname[IFNAMSIZ];
int retries, do_retry = 0;
uint32_t macvtapMode;
- const char *cr_ifname = NULL;
+ const char *ifnameCreated = NULL;
int ret;
int vf = -1;
bool vnet_hdr = flags & VIR_NETDEV_MACVLAN_VNET_HDR;
macvtapMode = modeMap[mode];
- *res_ifname = NULL;
-
- VIR_DEBUG("%s: VM OPERATION: %s", __FUNCTION__, virNetDevVPortProfileOpTypeToString(vmOp));
+ *ifnameResult = NULL;
/** Note: When using PASSTHROUGH mode with MACVTAP devices the link
* device's MAC address must be set to the VMs MAC address. In
@@ -803,52 +1025,82 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname,
}
}
- if (tgifname) {
- if ((ret = virNetDevExists(tgifname)) < 0)
- return -1;
+ if (ifnameRequested) {
+ bool isAutoName
+ = (STRPREFIX(ifnameRequested, MACVTAP_NAME_PREFIX) ||
+ STRPREFIX(ifnameRequested, MACVLAN_NAME_PREFIX));
+
+ VIR_INFO("Requested macvtap device name: %s", ifnameRequested);
+ virMutexLock(&virNetDevMacVLanCreateMutex);
+ if ((ret = virNetDevExists(ifnameRequested)) < 0) {
+ virMutexUnlock(&virNetDevMacVLanCreateMutex);
+ return -1;
+ }
if (ret) {
- if (STRPREFIX(tgifname, prefix))
+ if (isAutoName)
goto create_name;
virReportSystemError(EEXIST,
- _("Unable to create macvlan device %s"), tgifname);
+ _("Unable to create %s device %s"),
+ type, ifnameRequested);
+ virMutexUnlock(&virNetDevMacVLanCreateMutex);
return -1;
}
- cr_ifname = tgifname;
- rc = virNetDevMacVLanCreate(tgifname, type, macaddress, linkdev,
- macvtapMode, &do_retry);
- if (rc < 0)
+ if (isAutoName &&
+ (reservedID = virNetDevMacVLanReserveName(ifnameRequested, true)) < 0) {
+ reservedID = -1;
+ goto create_name;
+ }
+
+ if (virNetDevMacVLanCreate(ifnameRequested, type, macaddress,
+ linkdev, macvtapMode, &do_retry) < 0) {
+ if (isAutoName) {
+ virNetDevMacVLanReleaseName(ifnameRequested);
+ reservedID = -1;
+ goto create_name;
+ }
+ virMutexUnlock(&virNetDevMacVLanCreateMutex);
return -1;
- } else {
+ }
+ /* virNetDevMacVLanCreate() was successful - use this name */
+ ifnameCreated = ifnameRequested;
create_name:
- retries = 5;
+ virMutexUnlock(&virNetDevMacVLanCreateMutex);
+ }
+
+ retries = MACVLAN_MAX_ID + 1;
+ while (!ifnameCreated && retries) {
virMutexLock(&virNetDevMacVLanCreateMutex);
- for (c = 0; c < 8192; c++) {
- snprintf(ifname, sizeof(ifname), pattern, c);
- if ((ret = virNetDevExists(ifname)) < 0) {
- virMutexUnlock(&virNetDevMacVLanCreateMutex);
+ reservedID = virNetDevMacVLanReserveID(reservedID, flags, false, true);
+ if (reservedID < 0) {
+ virMutexUnlock(&virNetDevMacVLanCreateMutex);
+ return -1;
+ }
+ snprintf(ifname, sizeof(ifname), pattern, reservedID);
+ rc = virNetDevMacVLanCreate(ifname, type, macaddress, linkdev,
+ macvtapMode, &do_retry);
+ if (rc < 0) {
+ virNetDevMacVLanReleaseID(reservedID, flags);
+ virMutexUnlock(&virNetDevMacVLanCreateMutex);
+ if (!do_retry)
return -1;
- }
- if (!ret) {
- rc = virNetDevMacVLanCreate(ifname, type, macaddress, linkdev,
- macvtapMode, &do_retry);
- if (rc == 0) {
- cr_ifname = ifname;
- break;
- }
-
- if (do_retry && --retries)
- continue;
- break;
- }
+ VIR_INFO("Device %s wasn't reserved but already existed, skipping",
+ ifname);
+ retries--;
+ continue;
}
-
+ ifnameCreated = ifname;
virMutexUnlock(&virNetDevMacVLanCreateMutex);
- if (!cr_ifname)
- return -1;
}
- if (virNetDevVPortProfileAssociate(cr_ifname,
+ if (!ifnameCreated) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Too many unreserved %s devices in use"),
+ type);
+ return -1;
+ }
+
+ if (virNetDevVPortProfileAssociate(ifnameCreated,
virtPortProfile,
macaddress,
linkdev,
@@ -859,26 +1111,26 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname,
}
if (flags & VIR_NETDEV_MACVLAN_CREATE_IFUP) {
- if (virNetDevSetOnline(cr_ifname, true) < 0) {
+ if (virNetDevSetOnline(ifnameCreated, true) < 0) {
rc = -1;
goto disassociate_exit;
}
}
if (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) {
- if (virNetDevMacVLanTapOpen(cr_ifname, tapfd, tapfdSize, 10) < 0)
+ if (virNetDevMacVLanTapOpen(ifnameCreated, tapfd, tapfdSize, 10) < 0)
goto disassociate_exit;
if (virNetDevMacVLanTapSetup(tapfd, tapfdSize, vnet_hdr) < 0) {
VIR_FORCE_CLOSE(rc); /* sets rc to -1 */
goto disassociate_exit;
}
- if (VIR_STRDUP(*res_ifname, cr_ifname) < 0) {
+ if (VIR_STRDUP(*ifnameResult, ifnameCreated) < 0) {
VIR_FORCE_CLOSE(rc); /* sets rc to -1 */
goto disassociate_exit;
}
} else {
- if (VIR_STRDUP(*res_ifname, cr_ifname) < 0)
+ if (VIR_STRDUP(*ifnameResult, ifnameCreated) < 0)
goto disassociate_exit;
rc = 0;
}
@@ -889,7 +1141,7 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname,
* a saved image) - migration and libvirtd restart are handled
* elsewhere.
*/
- if (virNetDevMacVLanVPortProfileRegisterCallback(cr_ifname, macaddress,
+ if (virNetDevMacVLanVPortProfileRegisterCallback(ifnameCreated, macaddress,
linkdev, vmuuid,
virtPortProfile,
vmOp) < 0)
@@ -899,7 +1151,7 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname,
return rc;
disassociate_exit:
- ignore_value(virNetDevVPortProfileDisassociate(cr_ifname,
+ ignore_value(virNetDevVPortProfileDisassociate(ifnameCreated,
virtPortProfile,
macaddress,
linkdev,
@@ -909,7 +1161,8 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname,
VIR_FORCE_CLOSE(tapfd[tapfdSize]);
link_del_exit:
- ignore_value(virNetDevMacVLanDelete(cr_ifname));
+ ignore_value(virNetDevMacVLanDelete(ifnameCreated));
+ virNetDevMacVLanReleaseName(ifnameCreated);
return rc;
}
@@ -945,6 +1198,7 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char *ifname,
ret = -1;
if (virNetDevMacVLanDelete(ifname) < 0)
ret = -1;
+ virNetDevMacVLanReleaseName(ifname);
}
if (mode == VIR_NETDEV_MACVLAN_MODE_PASSTHRU) {
diff --git a/src/util/virnetdevmacvlan.h b/src/util/virnetdevmacvlan.h
index 70e3b01..560593e 100644
--- a/src/util/virnetdevmacvlan.h
+++ b/src/util/virnetdevmacvlan.h
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2011, 2013 Red Hat, Inc.
+ * Copyright (C) 2011, 2013, 2016 Red Hat, Inc.
* Copyright (C) 2010 IBM Corporation
*
* This library is free software; you can redistribute it and/or
@@ -50,6 +50,9 @@ typedef enum {
VIR_NETDEV_MACVLAN_VNET_HDR = 1 << 2,
} virNetDevMacVLanCreateFlags;
+int virNetDevMacVLanReserveName(const char *name, bool quietfail);
+int virNetDevMacVLanReleaseName(const char *name);
+
int virNetDevMacVLanCreate(const char *ifname,
const char *type,
const virMacAddr *macaddress,
--
2.5.0
9 years, 2 months
[libvirt] [PATCH v5 0/7] Per domain bandwidth settings
by Alexander Burluka
We decide to make a global per domain bandwidth setting
as were discussed in mailing list earlier.
This patchset implements hierarchy top level cpu.cfs_period_us
and cpu.cfs_quota_us control knob. I've named this parameters
as global_period and global_quota.
Changes in v2: add XML validation test
Changes in v3: remove unneccessary cgroup copying
Changes in v4: fix little rebase error
Changes in v5: rebase to version 1.3.1
Alexander Burluka (7):
Add global period definitions
Add global quota parameter necessary definitions
Add error checking on global quota and period
Add global_period and global_quota XML validation test
Rename qemuSetupCgroupVcpuBW to qemuSetupBandwidthCgroup
Implement qemuSetupGlobalCpuCgroup
Implement handling of per-domain bandwidth settings
docs/schemas/domaincommon.rng | 10 +++
include/libvirt/libvirt-domain.h | 32 ++++++++
src/conf/domain_conf.c | 37 +++++++++
src/conf/domain_conf.h | 2 +
src/qemu/qemu_cgroup.c | 68 ++++++++++++++--
src/qemu/qemu_cgroup.h | 7 +-
src/qemu/qemu_command.c | 3 +-
src/qemu/qemu_driver.c | 102 ++++++++++++++++++++++--
src/qemu/qemu_process.c | 4 +
tests/qemuxml2argvdata/qemuxml2argv-cputune.xml | 2 +
10 files changed, 251 insertions(+), 16 deletions(-)
--
1.8.3.1
9 years, 2 months
[libvirt] [PATCH] Dont set ABI update flag during migration cookie parsing for persistent copy
by Shivaprasad G Bhat
The migration would fail if the checks in virDomainDefPostParseMemory() fail
after ABI update. The ABI update normally done during the guest start and not
during define. If someone is using --persistent with the virsh
migrate it should be treated equivalent to domain define and not start. The
furture restart would/should anyway do the ABI updates and flag correct errors.
Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com>
---
src/qemu/qemu_migration.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 51e7125..39e259a 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1284,8 +1284,7 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig,
}
mig->persistent = virDomainDefParseNode(doc, nodes[0],
caps, driver->xmlopt,
- VIR_DOMAIN_DEF_PARSE_INACTIVE |
- VIR_DOMAIN_DEF_PARSE_ABI_UPDATE);
+ VIR_DOMAIN_DEF_PARSE_INACTIVE);
if (!mig->persistent) {
/* virDomainDefParseNode already reported
* an error for us */
9 years, 2 months
[libvirt] [PATCH 0/2] Couple of virt-host-validate fixes
by Michal Privoznik
While preparing my talk for DevConf, where I want to present this awesome tool
I've found couple of bugs in it.
Michal Privoznik (2):
virt-host-validate: Check those CGroups that we actually use
virt-host-validate: Fix error level for user namespace check
tools/virt-host-validate-lxc.c | 18 +++++++++---------
tools/virt-host-validate-qemu.c | 10 +++++-----
2 files changed, 14 insertions(+), 14 deletions(-)
--
2.4.10
9 years, 2 months
[libvirt] [PATCH] virsh: Correctly detect inserted media in change-media command
by Michal Privoznik
https://bugzilla.redhat.com/show_bug.cgi?id=1250331
It all works like this. The change-media command dumps domain
XML, finds the corresponding cdrom device we want to change media
in and returns it in the xmlNodePtr form. This way we don't have
to bother with keeping all the subelements or attributes that we
don't care about in the XML that is fed back to libvirt for the
update API.
Now, the problem is we try to be clever here and detect if disk
already has a source (indicated by <source/> subelement).
However, bare fact that the element is there does not mean disk
has source. The element has some attributes and only if @file or
@dev is within them disk has source. Any other attribute is
meaningless for our purpose now. Make our clever check better.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tools/virsh-domain.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index d239ba8..8fb4204 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11055,6 +11055,7 @@ virshUpdateDiskXML(xmlNodePtr disk_node,
char *device_type = NULL;
char *ret = NULL;
char *startupPolicy = NULL;
+ char *source_path = NULL;
if (!disk_node)
return NULL;
@@ -11111,7 +11112,10 @@ virshUpdateDiskXML(xmlNodePtr disk_node,
}
if (source) {
- if (type == VIRSH_UPDATE_DISK_XML_INSERT) {
+ if (!(source_path = virXMLPropString(source, "file")))
+ source_path = virXMLPropString(source, "dev");
+
+ if (source_path && type == VIRSH_UPDATE_DISK_XML_INSERT) {
vshError(NULL, _("The disk device '%s' already has media"), target);
goto cleanup;
}
@@ -11170,6 +11174,7 @@ virshUpdateDiskXML(xmlNodePtr disk_node,
cleanup:
VIR_FREE(device_type);
VIR_FREE(startupPolicy);
+ VIR_FREE(source_path);
return ret;
}
--
2.4.10
9 years, 2 months