- Devel - libvirt List Archives

[libvirt] how to adjust the value passed by xml dynamicly
by yue 07 Feb '14

07 Feb '14

hi,all we can pass value throught xml when start vm. how to adjust the value dynamicly as needed while vm is running via API calls? for example , change from <filterref filter='hcount'> <parameter name='HC' value='6'/> <parameter name='HC' value='10'/> </filterref> to <filterref filter='hcount'> <parameter name='HC' value='16'/> <parameter name='HC' value='20'/> </filterref> i do not want to restart or interrupt the running state of vms(via api, not command line tools).

3 2

[libvirt] [PATCH] qemu: keep pre-migration domain state after failed migration
by Martin Kletzander 07 Feb '14

07 Feb '14

Couple of codepaths shared the same code which can be moved out to a function and on one of such places, qemuMigrationConfirmPhase(), the domain was resumed even if it wasn't running before the migration started. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1057407 Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- src/qemu/qemu_domain.h | 3 +- src/qemu/qemu_migration.c | 117 +++++++++++++++++++++++++--------------------- 2 files changed, 66 insertions(+), 54 deletions(-) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 6a92351..84624f9 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -1,7 +1,7 @@ /* * qemu_domain.h: QEMU domain private state * - * Copyright (C) 2006-2013 Red Hat, Inc. + * Copyright (C) 2006-2014 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -161,6 +161,7 @@ struct _qemuDomainObjPrivate { char *origname; int nbdPort; /* Port used for migration with NBD */ unsigned short migrationPort; + int preMigrationState; virChrdevsPtr devs; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 407fb70..1a29efa 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1075,6 +1075,53 @@ error: return NULL; } +static void +qemuDomainObjStorePreMigrationState(virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + priv->preMigrationState = virDomainObjGetState(vm, NULL); + + VIR_DEBUG("Storing pre-migration state=%d domain=%p", + priv->preMigrationState, vm); +} + +/* Returns true if the domain was resumed, false otherwise */ +static bool +qemuDomainObjRestorePreMigrationState(virConnectPtr conn, virDomainObjPtr vm) +{ + virQEMUDriverPtr driver = conn->privateData; + qemuDomainObjPrivatePtr priv = vm->privateData; + int state = virDomainObjGetState(vm, NULL); + bool ret = false; + + VIR_DEBUG("driver=%p, vm=%p, pre-mig-state=%d, state=%d", + driver, vm, priv->preMigrationState, state); + + if (state == VIR_DOMAIN_PAUSED && + priv->preMigrationState == VIR_DOMAIN_RUNNING) { + /* This is basically the only restore possibility that's safe + * and we should attemt to do */ + + VIR_DEBUG("Restoring pre-migration state due to migration error"); + + /* we got here through some sort of failure; start the domain again */ + if (qemuProcessStartCPUs(driver, vm, conn, + VIR_DOMAIN_RUNNING_MIGRATION_CANCELED, + QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) { + /* Hm, we already know we are in error here. We don't want to + * overwrite the previous error, though, so we just throw something + * to the logs and hope for the best */ + VIR_ERROR(_("Failed to resume guest %s after failure"), vm->def->name); + goto cleanup; + } + ret = true; + } + + cleanup: + priv->preMigrationState = VIR_DOMAIN_NOSTATE; + return ret; +} + /** * qemuMigrationStartNBDServer: * @driver: qemu driver @@ -1952,8 +1999,8 @@ cleanup: /* The caller is supposed to lock the vm and start a migration job. */ -static char -*qemuMigrationBeginPhase(virQEMUDriverPtr driver, +static char * +qemuMigrationBeginPhase(virQEMUDriverPtr driver, virDomainObjPtr vm, const char *xmlin, const char *dname, @@ -2075,6 +2122,8 @@ qemuMigrationBegin(virConnectPtr conn, asyncJob = QEMU_ASYNC_JOB_NONE; } + qemuDomainObjStorePreMigrationState(vm); + if (!virDomainObjIsActive(vm) && !(flags & VIR_MIGRATE_OFFLINE)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); @@ -2744,22 +2793,12 @@ qemuMigrationConfirmPhase(virQEMUDriverPtr driver, /* cancel any outstanding NBD jobs */ qemuMigrationCancelDriveMirror(mig, driver, vm); - /* run 'cont' on the destination, which allows migration on qemu - * >= 0.10.6 to work properly. This isn't strictly necessary on - * older qemu's, but it also doesn't hurt anything there - */ - if (qemuProcessStartCPUs(driver, vm, conn, - VIR_DOMAIN_RUNNING_MIGRATED, - QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) { - if (virGetLastError() == NULL) - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("resume operation failed")); - goto cleanup; + if (qemuDomainObjRestorePreMigrationState(conn, vm)) { + event = virDomainEventLifecycleNewFromObj(vm, + VIR_DOMAIN_EVENT_RESUMED, + VIR_DOMAIN_EVENT_RESUMED_MIGRATED); } - event = virDomainEventLifecycleNewFromObj(vm, - VIR_DOMAIN_EVENT_RESUMED, - VIR_DOMAIN_EVENT_RESUMED_MIGRATED); if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) { VIR_WARN("Failed to save status on vm %s", vm->def->name); goto cleanup; @@ -4065,7 +4104,6 @@ qemuMigrationPerformJob(virQEMUDriverPtr driver, { virObjectEventPtr event = NULL; int ret = -1; - int resume = 0; virErrorPtr orig_err = NULL; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); bool abort_on_error = !!(flags & VIR_MIGRATE_ABORT_ON_ERROR); @@ -4085,7 +4123,7 @@ qemuMigrationPerformJob(virQEMUDriverPtr driver, if (!(flags & VIR_MIGRATE_UNSAFE) && !qemuMigrationIsSafe(vm->def)) goto endjob; - resume = virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING; + qemuDomainObjStorePreMigrationState(vm); if ((flags & (VIR_MIGRATE_TUNNELLED | VIR_MIGRATE_PEER2PEER))) { ret = doPeer2PeerMigrate(driver, conn, vm, xmlin, @@ -4112,25 +4150,13 @@ qemuMigrationPerformJob(virQEMUDriverPtr driver, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_MIGRATED); } - resume = 0; endjob: if (ret < 0) orig_err = virSaveLastError(); - if (resume && virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) { - /* we got here through some sort of failure; start the domain again */ - if (qemuProcessStartCPUs(driver, vm, conn, - VIR_DOMAIN_RUNNING_MIGRATION_CANCELED, - QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) { - /* Hm, we already know we are in error here. We don't want to - * overwrite the previous error, though, so we just throw something - * to the logs and hope for the best - */ - VIR_ERROR(_("Failed to resume guest %s after failure"), - vm->def->name); - } - + if (!v3proto && + qemuDomainObjRestorePreMigrationState(conn, vm)) { event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_RESUMED, VIR_DOMAIN_EVENT_RESUMED_MIGRATED); @@ -4179,7 +4205,6 @@ qemuMigrationPerformPhase(virQEMUDriverPtr driver, { virObjectEventPtr event = NULL; int ret = -1; - bool resume; bool hasrefs; /* If we didn't start the job in the begin phase, start it now. */ @@ -4194,32 +4219,18 @@ qemuMigrationPerformPhase(virQEMUDriverPtr driver, virCloseCallbacksUnset(driver->closeCallbacks, vm, qemuMigrationCleanup); - resume = virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING; ret = doNativeMigrate(driver, vm, uri, cookiein, cookieinlen, cookieout, cookieoutlen, flags, resource, NULL, graphicsuri); - if (ret < 0 && resume && - virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) { - /* we got here through some sort of failure; start the domain again */ - if (qemuProcessStartCPUs(driver, vm, conn, - VIR_DOMAIN_RUNNING_MIGRATION_CANCELED, - QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) { - /* Hm, we already know we are in error here. We don't want to - * overwrite the previous error, though, so we just throw something - * to the logs and hope for the best - */ - VIR_ERROR(_("Failed to resume guest %s after failure"), - vm->def->name); + if (ret < 0) { + if (qemuDomainObjRestorePreMigrationState(conn, vm)) { + event = virDomainEventLifecycleNewFromObj(vm, + VIR_DOMAIN_EVENT_RESUMED, + VIR_DOMAIN_EVENT_RESUMED_MIGRATED); } - - event = virDomainEventLifecycleNewFromObj(vm, - VIR_DOMAIN_EVENT_RESUMED, - VIR_DOMAIN_EVENT_RESUMED_MIGRATED); - } - - if (ret < 0) goto endjob; + } qemuMigrationJobSetPhase(driver, vm, QEMU_MIGRATION_PHASE_PERFORM3_DONE); -- 1.8.5.3

3 3

[libvirt] [PATCH v2 0/2] small gluster fixes
by Christophe Fergeau 07 Feb '14

07 Feb '14

Hi, I noticed that "virsh pool-list --type gluster" is not giving me an empty list as expected, but rather the list of active pools. This is happening because virConnectListAllStoragePools() is returning VIR_ERROR_INVALID_ARG because VIR_CONNECT_LIST_STORAGE_POOLS_GLUSTER is unrecognized. vshStoragePoolListCollect then goes to retry with only VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE | VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE in the flags, which is questionable, but explains the result I got. Adding VIR_CONNECT_LIST_STORAGE_POOLS_GLUSTER to VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE is enough to fix that. I've played a bit with adding some compile time checks to avoid this in the future, but while they work, they are a bit ugly-looking, and they would require some changes in docs/apibuild.py to teach it to parse 1 << N, or to make it ignore values in #ifdef VIR_ENUM_SENTINELS, so before spending too much time on this, I'd like to know if these checks would be a welcome/useful addition. The checks are: commit 2aee2a1827497ebde19738c88d2ababfe2dbb13d Author: Christophe Fergeau <cfergeau(a)redhat.com> Date: Thu Feb 6 17:39:43 2014 +0100 add pool related compile-time checks diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index b3ce000..9d79ae6 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3104,6 +3104,10 @@ typedef enum { VIR_CONNECT_LIST_STORAGE_POOLS_RBD = 1 << 14, VIR_CONNECT_LIST_STORAGE_POOLS_SHEEPDOG = 1 << 15, VIR_CONNECT_LIST_STORAGE_POOLS_GLUSTER = 1 << 16, + +#ifdef VIR_ENUM_SENTINELS + VIR_CONNECT_LIST_STORAGE_POOLS_LAST +#endif } virConnectListAllStoragePoolsFlags; int virConnectListAllStoragePools(virConnectPtr conn, diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index eaa9325..c8d4df4 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -36,6 +36,7 @@ #include "virerror.h" #include "datatypes.h" #include "storage_conf.h" +#include "verify.h" #include "virstoragefile.h" #include "virxml.h" @@ -51,6 +52,16 @@ #define DEFAULT_POOL_PERM_MODE 0755 #define DEFAULT_VOL_PERM_MODE 0600 +/* Check if VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE seems to + * contain all virConnectListAllStoragePoolsFlags elements + */ +verify(VIR_CONNECT_LIST_STORAGE_POOLS_LAST < VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE); + +/* Check if virConnectListAllStorageFlags has one flag per known storage + * pool type */ +verify(1 << (5+VIR_STORAGE_POOL_LAST) == VIR_CONNECT_LIST_STORAGE_POOLS_LAST - 1); + + VIR_ENUM_IMPL(virStorageVol, VIR_STORAGE_VOL_LAST, "file", "block", "dir", "network", "netdir") @@ -281,6 +292,8 @@ static virStoragePoolTypeInfo poolTypeInfo[] = { } }; +/* Check if poolTypeInfo has one entry per known storage pool type */ +verify(ARRAY_CARDINALITY(poolTypeInfo) == VIR_STORAGE_POOL_LAST); static virStoragePoolTypeInfoPtr virStoragePoolTypeInfoLookup(int type) -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

2 6

[libvirt] [PATCH 0/2] glusterfs related fixes
by Christophe Fergeau 06 Feb '14

06 Feb '14

Hi, I noticed that "virsh pool-list --type gluster" is not giving me an empty list as expected, but rather the list of active pools. This is happening because virConnectListAllStoragePools() is returning VIR_ERROR_INVALID_ARG because VIR_CONNECT_LIST_STORAGE_POOLS_GLUSTER is unrecognized. vshStoragePoolListCollect then goes to retry with only VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE | VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE in the flags, which is questionable, but explains the result I got. Adding VIR_CONNECT_LIST_STORAGE_POOLS_GLUSTER to VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE is enough to fix that. I've played a bit with adding some compile time checks to avoid this in the future, but while they work, they are a bit ugly-looking, and they would require some changes in docs/apibuild.py to teach it to parse 1 << N, or to make it ignore values in #ifdef VIR_ENUM_SENTINELS, so before spending too much time on this, I'd like to know if these checks would be a welcome/useful addition. The checks are: commit 2aee2a1827497ebde19738c88d2ababfe2dbb13d Author: Christophe Fergeau <cfergeau(a)redhat.com> Date: Thu Feb 6 17:39:43 2014 +0100 add pool related compile-time checks diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index b3ce000..9d79ae6 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3104,6 +3104,10 @@ typedef enum { VIR_CONNECT_LIST_STORAGE_POOLS_RBD = 1 << 14, VIR_CONNECT_LIST_STORAGE_POOLS_SHEEPDOG = 1 << 15, VIR_CONNECT_LIST_STORAGE_POOLS_GLUSTER = 1 << 16, + +#ifdef VIR_ENUM_SENTINELS + VIR_CONNECT_LIST_STORAGE_POOLS_LAST +#endif } virConnectListAllStoragePoolsFlags; int virConnectListAllStoragePools(virConnectPtr conn, diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index eaa9325..c8d4df4 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -36,6 +36,7 @@ #include "virerror.h" #include "datatypes.h" #include "storage_conf.h" +#include "verify.h" #include "virstoragefile.h" #include "virxml.h" @@ -51,6 +52,16 @@ #define DEFAULT_POOL_PERM_MODE 0755 #define DEFAULT_VOL_PERM_MODE 0600 +/* Check if VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE seems to + * contain all virConnectListAllStoragePoolsFlags elements + */ +verify(VIR_CONNECT_LIST_STORAGE_POOLS_LAST < VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE); + +/* Check if virConnectListAllStorageFlags has one flag per known storage + * pool type */ +verify(1 << (5+VIR_STORAGE_POOL_LAST) == VIR_CONNECT_LIST_STORAGE_POOLS_LAST - 1); + + VIR_ENUM_IMPL(virStorageVol, VIR_STORAGE_VOL_LAST, "file", "block", "dir", "network", "netdir") @@ -281,6 +292,8 @@ static virStoragePoolTypeInfo poolTypeInfo[] = { } }; +/* Check if poolTypeInfo has one entry per known storage pool type */ +verify(ARRAY_CARDINALITY(poolTypeInfo) == VIR_STORAGE_POOL_LAST); static virStoragePoolTypeInfoPtr virStoragePoolTypeInfoLookup(int type)

1 2

[libvirt] [PATCH] libxl: remove unneeded locking of driver when restoring
by Jim Fehlig 06 Feb '14

06 Feb '14

libxlDomainRestoreFlags acquires the driver lock while reading the domain config from the save file and adding it to libxlDriverPrivatePtr->domains. But virDomainObjList provides self-locking APIs, so remove the needless driver locking. Signed-off-by: Jim Fehlig <jfehlig(a)suse.com> --- I noticed this while working on the recent series to improve event handling in the libxl driver, but it got lost when I reshuffled the patches a bit before sending to the list. src/libxl/libxl_driver.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index b852279..03aa463 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1956,11 +1956,6 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from, return -1; } - /* Lock the driver until domain def is read from the saved - image and a virDomainObj is created and locked. - */ - libxlDriverLock(driver); - fd = libxlSaveImageOpen(driver, cfg, from, &def, &hdr); if (fd < 0) goto cleanup_unlock; @@ -1975,7 +1970,6 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from, NULL))) goto cleanup_unlock; - libxlDriverUnlock(driver); def = NULL; ret = libxlVmStart(driver, vm, (flags & VIR_DOMAIN_SAVE_PAUSED) != 0, fd); -- 1.8.1.4

2 2

[libvirt] [PATCH 0/4] libxl: fixes related to concurrency improvements
by Jim Fehlig 06 Feb '14

06 Feb '14

While reviving old patches to add job support to the libxl driver, testing revealed some problems that were difficult to encounter in the current, more serialized processing approach used in the driver. The first patch is a bug fix, plugging leaks of libxlDomainObjPrivate objects. The second patch removes the list of libxl timer registrations maintained in the driver - a hack I was never fond of. The third patch moves domain shutdown handling to a thread, instead of doing all the shutdown work in the event handler. The fourth patch fixes an issue wrt child process handling discussed in this thread http://lists.xen.org/archives/html/xen-devel/2014-01/msg01553.html Ian Jackson's latest patches on the libxl side are here http://lists.xen.org/archives/html/xen-devel/2014-02/msg00124.html Jim Fehlig (4): libxl: fix leaking libxlDomainObjPrivate libxl: remove list of timer registrations from libxlDomainObjPrivate libxl: handle domain shutdown events in a thread libxl: improve subprocess handling src/libxl/libxl_conf.h | 5 +- src/libxl/libxl_domain.c | 102 ++++++++--------------------------- src/libxl/libxl_domain.h | 8 +-- src/libxl/libxl_driver.c | 135 +++++++++++++++++++++++++++++++---------------- 4 files changed, 115 insertions(+), 135 deletions(-) -- 1.8.1.4

2 7

[libvirt] [PATCH V4] Sheepdog: Auto Adding volume and pool refresh.
by joel SIMOES 06 Feb '14

06 Feb '14

From: Joel SIMOES <joel.simoes(a)laposte.net> Libvirt lose sheepdogs volumes on pool refresh or restart. When restarting sheepdog pool, all volumes are missing. This patch add automatically all volume from the added pool. With Jan Tomko help --- src/storage/storage_backend_sheepdog.c | 68 ++++++++++++++++++++++++++++++++-- 1 file changed, 64 insertions(+), 4 deletions(-) diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c index a6981ce..7a139e7 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -109,22 +109,82 @@ virStorageBackendSheepdogAddHostArg(virCommandPtr cmd, virCommandAddArgFormat(cmd, "%d", port); } +static int +virStorageBackendSheepdogRefreshAllVol(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool) +{ + int ret = -1; + char *output = NULL; + char** lines = NULL; + char** cells = NULL; + size_t i; + + virCommandPtr cmd = virCommandNewArgList(COLLIE, "vdi", "list", "-r", NULL); + virStorageBackendSheepdogAddHostArg(cmd, pool); + virCommandSetOutputBuffer(cmd, &output); + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; + + lines = virStringSplit(output, "\n", 0); + if (lines == NULL) + goto cleanup; + + for (i = 0; lines[i]; i++) { + char *line = lines[i]; + if (line == NULL) + break; + cells = virStringSplit(line, " ", 0); + if (cells == NULL || virStringListLength(cells) <= 2) + continue; + char *cell = cells[1]; + virStorageVolDefPtr vol = NULL; + if (VIR_ALLOC(vol) < 0) + goto cleanup; + + if (VIR_STRDUP(vol->name, cell) < 0) + goto cleanup; + + vol->type = VIR_STORAGE_VOL_BLOCK; + + if (VIR_EXPAND_N(pool->volumes.objs, pool->volumes.count, 1) < 0) + goto cleanup; + pool->volumes.objs[pool->volumes.count - 1] = vol; + + if (virStorageBackendSheepdogRefreshVol(conn, pool, vol) < 0) + goto cleanup; + vol = NULL; + virStringFreeList(cells); + } + + ret = 0; + +cleanup: + virCommandFree(cmd); + virStringFreeList(lines); + virStringFreeList(cells); + return ret; +} + static int virStorageBackendSheepdogRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool) { - int ret; + int ret = -1; char *output = NULL; virCommandPtr cmd; cmd = virCommandNewArgList(COLLIE, "node", "info", "-r", NULL); virStorageBackendSheepdogAddHostArg(cmd, pool); virCommandSetOutputBuffer(cmd, &output); - ret = virCommandRun(cmd, NULL); - if (ret == 0) - ret = virStorageBackendSheepdogParseNodeInfo(pool->def, output); + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; + + if (virStorageBackendSheepdogParseNodeInfo(pool->def, output) < 0) + goto cleanup; + ret = virStorageBackendSheepdogRefreshAllVol(conn, pool); +cleanup: virCommandFree(cmd); VIR_FREE(output); return ret; -- 1.8.3.2

2 1

[libvirt] [PATCH 1/2] qemuxml2argvmock: Mock time() on non-linux platforms too
by Michal Privoznik 06 Feb '14

06 Feb '14

The qemuxml2argvtest is run on more platforms than linux. For instance FreeBSD. On these platforms we are, however, not mocking time() which results in current time being fetched from system and hence tests number 32 and 33 failing. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- tests/qemuxml2argvmock.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c index ed9fb13..bff3b0f 100644 --- a/tests/qemuxml2argvmock.c +++ b/tests/qemuxml2argvmock.c @@ -20,9 +20,8 @@ #include <config.h> -#ifdef __linux__ -# include "internal.h" -# include <time.h> +#include "internal.h" +#include <time.h> time_t time(time_t *t) { @@ -31,7 +30,3 @@ time_t time(time_t *t) *t = ret; return ret; } - -#else -/* Nothing to override on non-__linux__ platforms */ -#endif -- 1.8.5.3

4 6

[libvirt] Enabling x2apic by default in KvM (was Re: [Qemu-devel] [PATCH] target-i386: enable x2apic by default on more recent CPU models)
by Eduardo Habkost 06 Feb '14

06 Feb '14

On Tue, Feb 04, 2014 at 03:12:43PM +0100, Andreas Färber wrote: > Am 03.02.2014 20:01, schrieb Eduardo Habkost: > > On Tue, Jan 21, 2014 at 05:13:50PM +0100, Paolo Bonzini wrote: > >> Il 21/01/2014 16:51, Andreas Färber ha scritto: > >>>>> We already do that for other bits (e.g. XSAVE/OSXSAVE), > >>> Please point me to the commit, a search for xsave did not come up with a > >>> commit changing such a thing - either it did not go through my queue or > >>> it slipped me through: Bugs are no excuse to produce more bugs. > >> > >> I meant that "-cpu SandyBridge" with TCG produces a CPU that doesn't > >> have XSAVE. > >> > >>>>> and in fact it > >>>>> is the same that we do for KVM: the KVM_GET_SUPPORTED_CPUID result is > >>>>> used to trim the generic feature bits. > >>> Our model definitions are the place to put stuff that real CPUs have. > >>> Either the CPU has it or it doesn't. If it does, then this patch is > >>> fully correct and it's TCG's job to mask things out. If we're adding > >>> artificial flags to the generic model definitions just to make KVM > >>> faster, then it is wrong - we have a choice of post_initialize and > >>> realize hooks for that. > >> > >> It would make TCG faster as well, and there would be no reason > >> really to avoid the "artificial" x2apic on TCG, if TCG implemented > >> x2apic at all. > > > > So, the discussion seem to have stalled. > > > > Andreas, are you still against the patch, after the arguments from Paolo > > and me? > > Yes, I am. I had proposed to discuss solutions at FOSDEM but Paolo was > not there, so no solution yet. > > My main concern still is that if a CPU does not have a certain feature > we should not list it as one of its features but add it to its features > where sensible. Just because TCG filters it out today is not keeping > anyone from implementing it tomorrow, in which case the emulated CPUs > would suddenly gain the feature. So my question still is, what rule can > we apply for enabling x2apic? (something like greater or equal this > family, etc. - then we can put it in your post_initialize hook so that > users can still override it) The rule needs be simple and predictable, so libvirt can know in advance if the results are going to match what was requested by the user. That's the problem with enabling it automatically based on some rule more complex than the existing "it's enabled if it's on the CPU model table or on the command-line" rule. Considering the whole stack, there's a strong reason to enable x2apic by default on all cases when using KVM. But if keeping the CPU model table match real CPUs bit-by-bit is more important for QEMU (which I don't understand why), I believe we will need to ask libvirt to do it (enable x2apic explicitly in the command-line by default). Or we could make the CPU models look different in KVM and TCG mode, but I am trying to avoid that. But I still believe that enabling x2apic by default in TCG _and_ KVM mode is easier and harmless. I don't see the value in this academic exercise of trying to make the CPU model table match real CPUs bit-by-bit. I mean: I simply don't expect any user will come to us complaining because they didn't want x2apic to be enabled by default. And if some user really cares about that, they can simply use "-x2apic" in the command-line. In other words, if it is not going to affect users negatively, why are we spending energy on it? -- Eduardo

1 0

[libvirt] [PATCH v3 1/2] BSD: implement nodeGetCPUStats
by Roman Bogorodskiy 06 Feb '14

06 Feb '14

Implementation obtains CPU usage information using kern.cp_time and kern.cp_times sysctl(8)s and reports CPU utilization. --- include/libvirt/libvirt.h.in | 8 ++++ src/nodeinfo.c | 104 +++++++++++++++++++++++++++++++++++++++++++ tools/virsh-host.c | 11 ++++- 3 files changed, 121 insertions(+), 2 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index b3ce000..295d551 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -692,6 +692,14 @@ typedef enum { #define VIR_NODE_CPU_STATS_IOWAIT "iowait" /** + * VIR_NODE_CPU_STATS_INTR: + * + * The cumulative interrupt CPU time, + * since the node booting up (in nanoseconds). + */ +#define VIR_NODE_CPU_STATS_INTR "intr" + +/** * VIR_NODE_CPU_STATS_UTILIZATION: * * The CPU utilization of a node. diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 6ebfb4b..9a1548e 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -34,8 +34,10 @@ #include "conf/domain_conf.h" #if defined(__FreeBSD__) || defined(__APPLE__) +# include <sys/time.h> # include <sys/types.h> # include <sys/sysctl.h> +# include <sys/resource.h> #endif #include "c-ctype.h" @@ -99,8 +101,108 @@ appleFreebsdNodeGetMemorySize(unsigned long *memory) #endif /* defined(__FreeBSD__) || defined(__APPLE__) */ #ifdef __FreeBSD__ +# define BSD_CPU_STATS_ALL 4 # define BSD_MEMORY_STATS_ALL 4 +# define TICK_TO_NSEC (1000ull * 1000ull * 1000ull / (stathz ? stathz : hz)) + +static int +freebsdNodeGetCPUStats(int cpuNum, + virNodeCPUStatsPtr params, + int *nparams) +{ + const char *sysctl_name; + long *cpu_times; + struct clockinfo clkinfo; + size_t i, j, cpu_times_size, clkinfo_size; + int cpu_times_num, offset, hz, stathz, ret = -1; + struct field_cpu_map { + const char *field; + int idx[CPUSTATES]; + } cpu_map[] = { + {VIR_NODE_CPU_STATS_KERNEL, {CP_SYS}}, + {VIR_NODE_CPU_STATS_USER, {CP_USER, CP_NICE}}, + {VIR_NODE_CPU_STATS_IDLE, {CP_IDLE}}, + {VIR_NODE_CPU_STATS_INTR, {CP_INTR}}, + {NULL, {0}} + }; + + if ((*nparams) == 0) { + *nparams = BSD_CPU_STATS_ALL; + return 0; + } + + if ((*nparams) != BSD_CPU_STATS_ALL) { + virReportInvalidArg(*nparams, + _("nparams in %s must be equal to %d"), + __FUNCTION__, BSD_CPU_STATS_ALL); + return -1; + } + + clkinfo_size = sizeof(clkinfo); + if (sysctlbyname("kern.clockrate", &clkinfo, &clkinfo_size, NULL, 0) < 0) { + virReportSystemError(errno, + _("sysctl failed for '%s'"), + "kern.clockrate"); + return -1; + } + + stathz = clkinfo.stathz; + hz = clkinfo.hz; + + if (cpuNum == VIR_NODE_CPU_STATS_ALL_CPUS) { + sysctl_name = "kern.cp_time"; + cpu_times_num = 1; + offset = 0; + } else { + sysctl_name = "kern.cp_times"; + cpu_times_num = appleFreebsdNodeGetCPUCount(); + + if (cpuNum >= cpu_times_num) { + virReportInvalidArg(cpuNum, + _("Invalid cpuNum in %s"), + __FUNCTION__); + return -1; + } + + offset = cpu_times_num * CPUSTATES; + } + + cpu_times_size = sizeof(long) * cpu_times_num * CPUSTATES; + + if (VIR_ALLOC_N(cpu_times, cpu_times_num * CPUSTATES) < 0) + goto cleanup; + + if (sysctlbyname(sysctl_name, cpu_times, &cpu_times_size, NULL, 0) < 0) { + virReportSystemError(errno, + _("sysctl failed for '%s'"), + sysctl_name); + goto cleanup; + } + + for (i = 0; cpu_map[i].field != NULL; i++) { + virNodeCPUStatsPtr param = &params[i]; + + if (virStrcpyStatic(param->field, cpu_map[i].field) == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Field '%s' too long for destination"), + cpu_map[i].field); + goto cleanup; + } + + param->value = 0; + for (j = 0; j < ARRAY_CARDINALITY(cpu_map[i].idx); j++) + param->value += cpu_times[offset + cpu_map[i].idx[j]] * TICK_TO_NSEC; + } + + ret = 0; + +cleanup: + VIR_FREE(cpu_times); + + return ret; +} + static int freebsdNodeGetMemoryStats(virNodeMemoryStatsPtr params, int *nparams) @@ -1045,6 +1147,8 @@ int nodeGetCPUStats(int cpuNum ATTRIBUTE_UNUSED, return ret; } +#elif defined(__FreeBSD__) + return freebsdNodeGetCPUStats(cpuNum, params, nparams); #else virReportError(VIR_ERR_NO_SUPPORT, "%s", _("node CPU stats not implemented on this platform")); diff --git a/tools/virsh-host.c b/tools/virsh-host.c index 3438ae7..f4ca7db 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -347,9 +347,10 @@ cmdNodeCpuStats(vshControl *ctl, const vshCmd *cmd) unsigned long long sys; unsigned long long idle; unsigned long long iowait; + unsigned long long intr; unsigned long long util; } cpu_stats[2]; - double user_time, sys_time, idle_time, iowait_time, total_time; + double user_time, sys_time, idle_time, iowait_time, intr_time, total_time; double usage; if (vshCommandOptInt(cmd, "cpu", &cpuNum) < 0) { @@ -390,6 +391,8 @@ cmdNodeCpuStats(vshControl *ctl, const vshCmd *cmd) cpu_stats[i].idle = value; } else if (STREQ(params[j].field, VIR_NODE_CPU_STATS_IOWAIT)) { cpu_stats[i].iowait = value; + } else if (STREQ(params[j].field, VIR_NODE_CPU_STATS_INTR)) { + cpu_stats[i].intr = value; } else if (STREQ(params[j].field, VIR_NODE_CPU_STATS_UTILIZATION)) { cpu_stats[i].util = value; flag_utilization = true; @@ -406,6 +409,7 @@ cmdNodeCpuStats(vshControl *ctl, const vshCmd *cmd) vshPrint(ctl, "%-15s %20llu\n", _("system:"), cpu_stats[0].sys); vshPrint(ctl, "%-15s %20llu\n", _("idle:"), cpu_stats[0].idle); vshPrint(ctl, "%-15s %20llu\n", _("iowait:"), cpu_stats[0].iowait); + vshPrint(ctl, "%-15s %20llu\n", _("intr:"), cpu_stats[0].intr); } } else { if (flag_utilization) { @@ -418,7 +422,8 @@ cmdNodeCpuStats(vshControl *ctl, const vshCmd *cmd) sys_time = cpu_stats[1].sys - cpu_stats[0].sys; idle_time = cpu_stats[1].idle - cpu_stats[0].idle; iowait_time = cpu_stats[1].iowait - cpu_stats[0].iowait; - total_time = user_time + sys_time + idle_time + iowait_time; + intr_time = cpu_stats[1].intr - cpu_stats[0].intr; + total_time = user_time + sys_time + idle_time + iowait_time + intr_time; usage = (user_time + sys_time) / total_time * 100; @@ -432,6 +437,8 @@ cmdNodeCpuStats(vshControl *ctl, const vshCmd *cmd) _("idle:"), idle_time / total_time * 100); vshPrint(ctl, "%-15s %5.1lf%%\n", _("iowait:"), iowait_time / total_time * 100); + vshPrint(ctl, "%-15s %5.1lf%%\n", + _("intr:"), intr_time / total_time * 100); } } -- 1.8.4.3

3 8