Re: [libvirt] [PATCH] rbd: use unsigned long long instead of uint64_t
On Mon, Mar 28, 2016 at 15:19:36 +0800, Shanzhi Yu wrote: > Fix below error: > storage/storage_backend_rbd.c: In function > 'virStorageBackendRBDSetAllocation': > storage/storage_backend_rbd.c:337:5: error: format '%llu' expects argument of > type 'long long unsigned int', but argument 8 has type 'uint64_t' > [-Werror=format=] > VIR_DEBUG("Found %llu bytes allocated for RBD image %s", > > Signed-off-by: Shanzhi Yu > --- > src/storage/storage_backend_rbd.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) NACK, the correct approach is to use "size_t" as virStorageBackendRBDRefreshVolInfoCb is getting that as the size info. The callback has to be fixed too. I've pushed the correct version. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 10/10] virt-admin: Introduce srv-workertune command
Wire up the server threadpool tunable APIs to virt-admin client. --- tools/virt-admin.c | 132 + 1 file changed, 132 insertions(+) diff --git a/tools/virt-admin.c b/tools/virt-admin.c index edb8690..dec1096 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -353,6 +353,127 @@ cmdSrvList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) return ret; } + +/* + * Command srv-workertune + * + */ + +static const vshCmdInfo info_srv_workertune[] = { +{.name = "help", + .data = N_("Get or set server workerpool parameters") +}, +{.name = "desc", + .data = N_("Get or set server workerpool parameters. Some parameters are " +"read-only, thus only \n" +"a subset of all supported parameters can actually be " +"set via OPTIONS.\n" +"To retrieve workerpool parameters, use the command " +"without any options: \n\n" +"virt-admin # srv-workertune ") +}, +{.name = NULL} +}; + +static const vshCmdOptDef opts_srv_workertune[] = { +{.name = "server", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("Server to get threadpool parameters for."), +}, +{.name = "min-workers", + .type = VSH_OT_INT, + .help = N_("Change the value of bottom limit to number of workers."), +}, +{.name = "max-workers", + .type = VSH_OT_INT, + .help = N_("Change the value of top limit to number of workers."), +}, +{.name = "priority-workers", + .type = VSH_OT_INT, + .help = N_("Change the current number of priority workers"), +}, +{.name = NULL} +}; + +static bool +cmdSrvWorkertune(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) +{ +virTypedParameterPtr params = NULL; +int nparams = 0; +int maxparams = 0; +unsigned int val; +unsigned int min, max; +int rv = 0; +size_t i; +bool ret = false; +const char *srvname = NULL; +virAdmServerPtr srv = NULL; +vshAdmControlPtr priv = ctl->privData; + +if (vshCommandOptStringReq(ctl, cmd, "server", &srvname) < 0) +return false; + +#define PARSE_WORKERTUNE_PARAM(NAME, FIELD) \ +if ((rv = vshCommandOptUInt(ctl, cmd, NAME, &val)) < 0) {\ +vshError(ctl, _("Unable to parse integer parameter '%s'"), NAME);\ +goto cleanup;\ +} else if (rv > 0) { \ +if (virTypedParamsAddUInt(¶ms, &nparams, &maxparams, \ + FIELD, val) < 0) \ +goto save_error; \ +} + +PARSE_WORKERTUNE_PARAM("max-workers", VIR_THREADPOOL_WORKERS_MAX); +PARSE_WORKERTUNE_PARAM("min-workers", VIR_THREADPOOL_WORKERS_MIN); +PARSE_WORKERTUNE_PARAM("priority-workers", VIR_THREADPOOL_WORKERS_PRIORITY); + +#undef PARSE_WORKERTUNE_PARAM + +if (virTypedParamsGetUInt(params, nparams, + VIR_THREADPOOL_WORKERS_MAX, &max) && +virTypedParamsGetUInt(params, nparams, + VIR_THREADPOOL_WORKERS_MIN, &min) && min > max) { +vshError(ctl, "%s", _("--min-workers must be less than --max-workers")); +goto cleanup; +} + +if (!(srv = virAdmConnectLookupServer(priv->conn, srvname, 0))) +goto cleanup; + +/* set server threadpool parameters */ +if (nparams) { +if (virAdmServerSetThreadPoolParameters(srv, params, +nparams, 0) < 0) +goto error; +} else { +if (virAdmServerGetThreadPoolParameters(srv, ¶ms, +&nparams, 0) < 0) { +vshError(ctl, "%s", + _("Unable to get server workerpool parameters")); +goto cleanup; +} + +for (i = 0; i < nparams; i++) +vshPrint(ctl, "%-15s: %d\n", params[i].field, params[i].value.ui); +} + +ret = true; + + cleanup: +virTypedParamsFree(params, nparams); +if (srv) +virAdmServerFree(srv); +return ret; + + save_error: +vshSaveLibvirtError(); + + error: +vshError(ctl, "%s", _("Unable to change server workerpool parameters")); +goto cleanup; +} + static void * vshAdmConnectionHandler(vshControl *ctl) { @@ -655,9 +776,20 @@ static const vshCmdDef monitoringCmds[] = { {.name = NULL} }; +static const vshCmdDef managementCmds[] = { +{.name = "srv-workertune", + .handler = cmdSrvWorkertune, + .opts = opts_srv_workertune, + .info = info_srv_workertune, + .flags = 0 +}, +{.name = NULL} +}; + static const vshCmdGrp cmdGroups[] = { {"Virt-admin itself", "virt-admin", vsh
[libvirt] [PATCH v2 03/10] admin: Enable usage of typed parameters
Make all relevant changes to admin protocol, in order to achieve $(subj) --- cfg.mk | 2 +- src/admin/admin_protocol.x | 24 src/admin_protocol-structs | 25 + 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/cfg.mk b/cfg.mk index f5573db..3ae8433 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1225,7 +1225,7 @@ exclude_file_name_regexp--sc_prohibit_include_public_headers_brackets = \ ^(tools/|examples/|include/libvirt/(virterror|libvirt(-(admin|qemu|lxc))?)\.h$$) exclude_file_name_regexp--sc_prohibit_int_ijk = \ - ^(src/remote_protocol-structs|src/remote/remote_protocol.x|cfg.mk|include/)$ + ^(src/remote_protocol-structs|src/remote/remote_protocol.x|cfg.mk|include/|src/admin_protocol-structs|src/admin/admin_protocol.x)$ exclude_file_name_regexp--sc_prohibit_getenv = \ ^tests/.*\.[ch]$$ diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x index 6590980..57dbb6b 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -22,6 +22,7 @@ * Author: Martin Kletzander */ +%#include %#include "virxdrdefs.h" /*- Data types. -*/ @@ -41,12 +42,35 @@ typedef string admin_nonnull_string; /* A long string, which may be NULL. */ typedef admin_nonnull_string *admin_string; +union admin_typed_param_value switch (int type) { + case VIR_TYPED_PARAM_INT: + int i; + case VIR_TYPED_PARAM_UINT: + unsigned int ui; + case VIR_TYPED_PARAM_LLONG: + hyper l; + case VIR_TYPED_PARAM_ULLONG: + unsigned hyper ul; + case VIR_TYPED_PARAM_DOUBLE: + double d; + case VIR_TYPED_PARAM_BOOLEAN: + int b; + case VIR_TYPED_PARAM_STRING: + admin_nonnull_string s; +}; + +struct admin_typed_param { +admin_nonnull_string field; +admin_typed_param_value value; +}; + /* A server which may NOT be NULL */ struct admin_nonnull_server { admin_nonnull_string name; }; /*- Protocol. -*/ + struct admin_connect_open_args { unsigned int flags; }; diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs index d8aca06..26c8443 100644 --- a/src/admin_protocol-structs +++ b/src/admin_protocol-structs @@ -1,4 +1,29 @@ /* -*- c -*- */ +enum { +VIR_TYPED_PARAM_INT = 1, +VIR_TYPED_PARAM_UINT = 2, +VIR_TYPED_PARAM_LLONG = 3, +VIR_TYPED_PARAM_ULLONG = 4, +VIR_TYPED_PARAM_DOUBLE = 5, +VIR_TYPED_PARAM_BOOLEAN = 6, +VIR_TYPED_PARAM_STRING = 7, +}; +struct admin_typed_param_value { +inttype; +union { +inti; +u_int ui; +int64_tl; +uint64_t ul; +double d; +intb; +admin_nonnull_string s; +} admin_typed_param_value_u; +}; +struct admin_typed_param { +admin_nonnull_string field; +admin_typed_param_valuevalue; +}; struct admin_nonnull_server { admin_nonnull_string name; }; -- 2.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 06/10] util: Add more getters to threadpool parameters
In order for the client to see all thread counts and limits, current total and free worker count getters need to be introduced. Client might also be interested in the job queue length, so provide a getter for that too. --- src/libvirt_private.syms | 3 +++ src/util/virthreadpool.c | 15 +++ src/util/virthreadpool.h | 3 +++ 3 files changed, 21 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index af133c5..c2bff17 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2333,6 +2333,9 @@ virThreadJobSetWorker; # util/virthreadpool.h virThreadPoolFree; +virThreadPoolGetCurrentWorkers; +virThreadPoolGetFreeWorkers; +virThreadPoolGetJobQueueDepth; virThreadPoolGetMaxWorkers; virThreadPoolGetMinWorkers; virThreadPoolGetPriorityWorkers; diff --git a/src/util/virthreadpool.c b/src/util/virthreadpool.c index e2e9fe4..518b880 100644 --- a/src/util/virthreadpool.c +++ b/src/util/virthreadpool.c @@ -299,6 +299,21 @@ size_t virThreadPoolGetPriorityWorkers(virThreadPoolPtr pool) return pool->nPrioWorkers; } +size_t virThreadPoolGetCurrentWorkers(virThreadPoolPtr pool) +{ +return pool->nWorkers; +} + +size_t virThreadPoolGetFreeWorkers(virThreadPoolPtr pool) +{ +return pool->freeWorkers; +} + +size_t virThreadPoolGetJobQueueDepth(virThreadPoolPtr pool) +{ +return pool->jobQueueDepth; +} + /* * @priority - job priority * Return: 0 on success, -1 otherwise diff --git a/src/util/virthreadpool.h b/src/util/virthreadpool.h index 538b62f..bc0c907 100644 --- a/src/util/virthreadpool.h +++ b/src/util/virthreadpool.h @@ -46,6 +46,9 @@ virThreadPoolPtr virThreadPoolNewFull(size_t minWorkers, size_t virThreadPoolGetMinWorkers(virThreadPoolPtr pool); size_t virThreadPoolGetMaxWorkers(virThreadPoolPtr pool); size_t virThreadPoolGetPriorityWorkers(virThreadPoolPtr pool); +size_t virThreadPoolGetCurrentWorkers(virThreadPoolPtr pool); +size_t virThreadPoolGetFreeWorkers(virThreadPoolPtr pool); +size_t virThreadPoolGetJobQueueDepth(virThreadPoolPtr pool); void virThreadPoolFree(virThreadPoolPtr pool); -- 2.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 07/10] admin: Prepare admin protocol for future worker related procedures
Before any getter or setter methods can be introduced, first specify a set of public attributes/flags that these methods will be compatible with. --- include/libvirt/libvirt-admin.h | 60 + src/admin/admin_protocol.x | 3 +++ 2 files changed, 63 insertions(+) diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h index 25bcbf4..35849b1 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -110,6 +110,66 @@ virAdmServerPtr virAdmConnectLookupServer(virAdmConnectPtr conn, const char *name, unsigned int flags); +/* Manage threadpool attributes */ + +/** + * VIR_THREADPOOL_WORKERS_MIN: + * Macro for the threadpool minWorkers limit: represents the bottom limit to + * number of active workers in threadpool, as uint. + */ + +# define VIR_THREADPOOL_WORKERS_MIN "minWorkers" + +/** + * VIR_THREADPOOL_WORKERS_MAX: + * Macro for the threadpool maxWorkers limit: represents the upper limit to + * number of active workers in threadpool, as uint. The value of this limit has + * to be greater than VIR_THREADPOOL_WORKERS_MIN at all times. + */ + +# define VIR_THREADPOOL_WORKERS_MAX "maxWorkers" + +/** + * VIR_THREADPOOL_WORKERS_PRIORITY: + * Macro for the threadpool nPrioWorkers attribute: represents the current number + * of active priority workers in threadpool, as uint. + */ + +# define VIR_THREADPOOL_WORKERS_PRIORITY "prioWorkers" + +/** + * VIR_THREADPOOL_WORKERS_FREE: + * Macro for the threadpool freeWorkers attribute: represents the current number + * of free workers available to accomplish a job, as uint. + * + * NOTE: This attribute is read-only and any attempt to set it will be denied + * by daemon + */ + +# define VIR_THREADPOOL_WORKERS_FREE "freeWorkers" + +/** + * VIR_THREADPOOL_WORKERS_CURRENT: + * Macro for the threadpool nWorkers attribute: represents the current number + * of active ordinary workers in threadpool, as uint. + * + * NOTE: This attribute is read-only and any attempt to set it will be denied + * by daemon + */ + +# define VIR_THREADPOOL_WORKERS_CURRENT "nWorkers" + +/** + * VIR_THREADPOOL_JOB_QUEUE_DEPTH: + * Macro for the threadpool jobQueueDepth attribute: represents the current + * number of jobs waiting in a queue to be processed, as uint. + * + * NOTE: This attribute is read-only and any attempt to set it will be denied + * by daemon + */ + +# define VIR_THREADPOOL_JOB_QUEUE_DEPTH "jobQueueDepth" + # ifdef __cplusplus } # endif diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x index 57dbb6b..4ab2323 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -36,6 +36,9 @@ const ADMIN_STRING_MAX = 4194304; /* Upper limit on list of servers */ const ADMIN_SERVER_LIST_MAX = 16384; +/* Upper limit on number of threadpool stats */ +const ADMIN_SERVER_THREADPOOL_STATS_MAX = 32; + /* A long string, which may NOT be NULL. */ typedef string admin_nonnull_string; -- 2.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 08/10] admin: Introduce virAdmServerGethreadPoolParameters
New API to retrieve current server workerpool specs. Since it uses typed parameters, more specs to retrieve can be further included in the pool of supported ones. --- daemon/admin.c | 59 +++ daemon/admin_server.c | 69 + daemon/admin_server.h | 6 include/libvirt/libvirt-admin.h | 6 po/POTFILES.in | 1 + src/admin/admin_protocol.x | 20 ++-- src/admin/admin_remote.c| 43 + src/admin_protocol-structs | 11 +++ src/libvirt-admin.c | 46 +++ src/libvirt_admin_private.syms | 2 ++ src/libvirt_admin_public.syms | 1 + src/rpc/virnetserver.c | 22 + src/rpc/virnetserver.h | 13 13 files changed, 296 insertions(+), 3 deletions(-) diff --git a/daemon/admin.c b/daemon/admin.c index 3169cdd..5335cce 100644 --- a/daemon/admin.c +++ b/daemon/admin.c @@ -37,6 +37,7 @@ #include "virnetserver.h" #include "virstring.h" #include "virthreadjob.h" +#include "virtypedparam.h" #define VIR_FROM_THIS VIR_FROM_ADMIN @@ -133,4 +134,62 @@ adminConnectGetLibVersion(virNetDaemonPtr dmn ATTRIBUTE_UNUSED, return 0; } +static int +adminDispatchServerGetThreadpoolParameters(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + struct admin_server_get_threadpool_parameters_args *args, + struct admin_server_get_threadpool_parameters_ret *ret) +{ +int rv = -1; +virNetServerPtr srv = NULL; +virTypedParameterPtr params = NULL; +int nparams = 0; +struct daemonAdmClientPrivate *priv = +virNetServerClientGetPrivateData(client); + +if (!(srv = virNetDaemonGetServer(priv->dmn, args->server.name))) { +virReportError(VIR_ERR_NO_SERVER, + _("no server with matching name '%s' found"), + args->server.name); +goto cleanup; +} + +if (adminDaemonGetThreadPoolParameters(srv, ¶ms, &nparams, + args->flags) < 0) +goto cleanup; + +if (nparams > ADMIN_SERVER_THREADPOOL_PARAMETERS_MAX) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("Number of threadpool parameters %d exceeds max " + "allowed limit: %d"), nparams, + ADMIN_SERVER_THREADPOOL_PARAMETERS_MAX); +goto cleanup; +} + +if (nparams) { +if (VIR_ALLOC_N(ret->params.params_val, nparams) < 0) +goto cleanup; + +ret->params.params_len = nparams; + +if (virTypedParamsSerialize(params, nparams, +(virTypedParameterRemotePtr *) &ret->params.params_val, +&ret->params.params_len, 0) < 0) +goto cleanup; +} else { +ret->params.params_len = 0; +ret->params.params_val = NULL; +} + +rv = 0; + cleanup: +if (rv < 0) +virNetMessageSaveError(rerr); + +virTypedParamsFree(params, nparams); +virObjectUnref(srv); +return rv; +} #include "admin_dispatch.h" diff --git a/daemon/admin_server.c b/daemon/admin_server.c index 1d30ea5..00a310b 100644 --- a/daemon/admin_server.c +++ b/daemon/admin_server.c @@ -31,6 +31,7 @@ #include "virnetdaemon.h" #include "virnetserver.h" #include "virstring.h" +#include "virthreadpool.h" #define VIR_FROM_THIS VIR_FROM_ADMIN @@ -68,3 +69,71 @@ adminConnectLookupServer(virNetDaemonPtr dmn, return virNetDaemonGetServer(dmn, name); } + +int +adminDaemonGetThreadPoolParameters(virNetServerPtr srv, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags) +{ +int ret = -1; +int maxparams = 0; +size_t minWorkers; +size_t maxWorkers; +size_t nWorkers; +size_t freeWorkers; +size_t nPrioWorkers; +size_t jobQueueDepth; +virTypedParameterPtr tmpparams = NULL; + +virCheckFlags(0, -1); + +if (virNetServerGetThreadPoolParameters(srv, &minWorkers, &maxWorkers, +&nWorkers, &freeWorkers, +&nPrioWorkers, +&jobQueueDepth) < 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to retrieve threadpool parameters")); +goto cleanup; +} + +if (virTypedParamsAddUInt(&tmpparams, nparams, + &maxparams, VIR_THREADPOOL_WORKERS_MIN, +
[libvirt] [PATCH v2 09/10] admin: Introduce virAdmServerSetThreadPoolParameters
Since threadpool increments the current number of threads according to current load, i.e. how many jobs are waiting in the queue. The count however, is constrained by max and min limits of workers. The logic of this new API works like this: 1) setting the minimum a) When the limit is increased, depending on the current number of threads, new threads are possibly spawned if the current number of threads is less than the new minimum limit b) Decreasing the minimum limit has no possible effect on the current number of threads 2) setting the maximum a) Icreasing the maximum limit has no immediate effect on the current number of threads, it only allows the threadpool to spawn more threads when new jobs, that would otherwise end up queued, arrive. b) Decreasing the maximum limit may affect the current number of threads, if the current number of threads is less than the new maximum limit. Since there may be some ongoing time-consuming jobs that would effectively block this API from killing any threads. Therefore, this API is asynchronous with best-effort execution, i.e. the necessary number of workers will be terminated once they finish their previous job, unless other workers had already terminated, decreasing the limit to the requested value. 3) setting priority workers - both increase and decrease in count of these workers have an immediate impact on the current number of workers, new ones will be spawned or some of them get terminated respectively. --- daemon/admin.c | 43 +++ daemon/admin_server.c | 43 +++ daemon/admin_server.h | 5 +++ include/libvirt/libvirt-admin.h | 5 +++ src/admin/admin_protocol.x | 13 ++- src/admin/admin_remote.c| 34 ++ src/admin_protocol-structs | 9 + src/libvirt-admin.c | 37 src/libvirt_admin_private.syms | 1 + src/libvirt_admin_public.syms | 1 + src/libvirt_private.syms| 1 + src/rpc/virnetserver.c | 15 src/util/virthreadpool.c| 77 +++-- src/util/virthreadpool.h| 5 +++ 14 files changed, 285 insertions(+), 4 deletions(-) diff --git a/daemon/admin.c b/daemon/admin.c index 5335cce..8d21fd7 100644 --- a/daemon/admin.c +++ b/daemon/admin.c @@ -192,4 +192,47 @@ adminDispatchServerGetThreadpoolParameters(virNetServerPtr server ATTRIBUTE_UNUS virObjectUnref(srv); return rv; } + +static int +adminDispatchServerSetThreadpoolParameters(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + struct admin_server_set_threadpool_parameters_args *args) +{ +int rv = -1; +virNetServerPtr srv = NULL; +virTypedParameterPtr params = NULL; +int nparams = 0; +struct daemonAdmClientPrivate *priv = +virNetServerClientGetPrivateData(client); + +if (!(srv = virNetDaemonGetServer(priv->dmn, args->server.name))) { +virReportError(VIR_ERR_NO_SERVER, + _("no server with matching name '%s' found"), + args->server.name); +goto cleanup; +} + +if (virTypedParamsDeserialize((virTypedParameterRemotePtr) args->params.params_val, + args->params.params_len, + ADMIN_SERVER_THREADPOOL_PARAMETERS_MAX, + ¶ms, + &nparams) < 0) +goto cleanup; + + +if (adminDaemonSetThreadPoolParameters(srv, params, + nparams, args->flags) < 0) +goto cleanup; + +rv = 0; + cleanup: +if (rv < 0) +virNetMessageSaveError(rerr); + +virTypedParamsFree(params, nparams); +virObjectUnref(srv); +return rv; +} #include "admin_dispatch.h" diff --git a/daemon/admin_server.c b/daemon/admin_server.c index 00a310b..37feff4 100644 --- a/daemon/admin_server.c +++ b/daemon/admin_server.c @@ -32,6 +32,7 @@ #include "virnetserver.h" #include "virstring.h" #include "virthreadpool.h" +#include "virtypedparam.h" #define VIR_FROM_THIS VIR_FROM_ADMIN @@ -137,3 +138,45 @@ adminDaemonGetThreadPoolParameters(virNetServerPtr srv, return ret; } + +int +adminDaemonSetThreadPoolParameters(virNetServerPtr srv, + virTypedParameterPtr params, + int nparams, + unsigned int flags) +{ +long long int minWorkers = -1; +long long
[libvirt] [PATCH v2 00/10] Introduce worker tuning APIs
NOTE: patch 2/10 moves typed params definition from libvirt-host.h to libvirt-common.h.in to enable it for admin as well -> therefore libvirt-common.h must be regenerated with config.status - also I'd like to open a discussion about virt-admin commands naming, since I was sort of of out ideas when I copied numatune,memtune,etc. design into "workertune"; I thought about srv-threadpool-info and srv-threadpool-set to be a little consistent with srv-list, but for some reason I found it quite long-ish so I dropped that... v2: - repost of v1 due to rebase conflicts - due to rebase conflicts and overall changes since the original v1 po/POTFILES.in (patch 1/10) had to be updated Erik Skultety (10): po: Fix record ordering in POTFILES.in libvirt-host: Move virTypedParam* to libvirt-common admin: Enable usage of typed parameters util: Refactor thread creation by introducing virThreadPoolExpand util: Report system error when virThreadCreateFull fails util: Add more getters to threadpool parameters admin: Prepare admin protocol for future worker related procedures admin: Introduce virAdmServerGethreadPoolParameters admin: Introduce virAdmServerSetThreadPoolParameters virt-admin: Introduce srv-workertune command cfg.mk | 2 +- daemon/admin.c | 102 +++ daemon/admin_server.c | 112 + daemon/admin_server.h | 11 ++ include/libvirt/libvirt-admin.h | 71 + include/libvirt/libvirt-common.h.in | 185 ++ include/libvirt/libvirt-host.h | 186 -- po/POTFILES.in | 4 +- src/admin/admin_protocol.x | 54 +- src/admin/admin_remote.c| 77 ++ src/admin_protocol-structs | 45 + src/libvirt-admin.c | 83 +++ src/libvirt_admin_private.syms | 3 + src/libvirt_admin_public.syms | 2 + src/libvirt_private.syms| 4 + src/rpc/virnetserver.c | 37 +++ src/rpc/virnetserver.h | 13 +++ src/util/virthreadpool.c| 196 src/util/virthreadpool.h| 8 ++ tools/virt-admin.c | 132 20 files changed, 1073 insertions(+), 254 deletions(-) -- 2.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 02/10] libvirt-host: Move virTypedParam* to libvirt-common
Commits 0472cef6, 9afc115f, 8cd1d546 exported typed params handlers internally, but a commit which would move the public definition from libvirt-host to libvirt-common was missing. --- include/libvirt/libvirt-common.h.in | 185 +++ include/libvirt/libvirt-host.h | 186 2 files changed, 185 insertions(+), 186 deletions(-) diff --git a/include/libvirt/libvirt-common.h.in b/include/libvirt/libvirt-common.h.in index efbb91b..772e40f 100644 --- a/include/libvirt/libvirt-common.h.in +++ b/include/libvirt/libvirt-common.h.in @@ -120,6 +120,191 @@ typedef enum { # endif } virConnectCloseReason; +/** + * virTypedParameterType: + * + * Express the type of a virTypedParameter + */ +typedef enum { +VIR_TYPED_PARAM_INT = 1, /* integer case */ +VIR_TYPED_PARAM_UINT= 2, /* unsigned integer case */ +VIR_TYPED_PARAM_LLONG = 3, /* long long case */ +VIR_TYPED_PARAM_ULLONG = 4, /* unsigned long long case */ +VIR_TYPED_PARAM_DOUBLE = 5, /* double case */ +VIR_TYPED_PARAM_BOOLEAN = 6, /* boolean(character) case */ +VIR_TYPED_PARAM_STRING = 7, /* string case */ + +# ifdef VIR_ENUM_SENTINELS +VIR_TYPED_PARAM_LAST +# endif +} virTypedParameterType; + +/** + * virTypedParameterFlags: + * + * Flags related to libvirt APIs that use virTypedParameter. + * + * These enums should not conflict with those of virDomainModificationImpact. + */ +typedef enum { +/* 1 << 0 is reserved for virDomainModificationImpact */ +/* 1 << 1 is reserved for virDomainModificationImpact */ + +/* Older servers lacked the ability to handle string typed + * parameters. Attempts to set a string parameter with an older + * server will fail at the client, but attempts to retrieve + * parameters must not return strings from a new server to an + * older client, so this flag exists to identify newer clients to + * newer servers. This flag is automatically set when needed, so + * the user does not have to worry about it; however, manually + * setting the flag can be used to reject servers that cannot + * return typed strings, even if no strings would be returned. + */ +VIR_TYPED_PARAM_STRING_OKAY = 1 << 2, + +} virTypedParameterFlags; + +/** + * VIR_TYPED_PARAM_FIELD_LENGTH: + * + * Macro providing the field length of virTypedParameter name + */ +# define VIR_TYPED_PARAM_FIELD_LENGTH 80 + +/** + * virTypedParameter: + * + * A named parameter, including a type and value. + * + * The types virSchedParameter, virBlkioParameter, and + * virMemoryParameter are aliases of this type, for use when + * targeting libvirt earlier than 0.9.2. + */ +typedef struct _virTypedParameter virTypedParameter; + +struct _virTypedParameter { +char field[VIR_TYPED_PARAM_FIELD_LENGTH]; /* parameter name */ +int type; /* parameter type, virTypedParameterType */ +union { +int i; /* type is INT */ +unsigned int ui;/* type is UINT */ +long long int l;/* type is LLONG */ +unsigned long long int ul; /* type is ULLONG */ +double d; /* type is DOUBLE */ +char b; /* type is BOOLEAN */ +char *s;/* type is STRING, may not be NULL */ +} value; /* parameter value */ +}; + +/** + * virTypedParameterPtr: + * + * a pointer to a virTypedParameter structure. + */ +typedef virTypedParameter *virTypedParameterPtr; + + +virTypedParameterPtr +virTypedParamsGet (virTypedParameterPtr params, + int nparams, + const char *name); +int +virTypedParamsGetInt(virTypedParameterPtr params, + int nparams, + const char *name, + int *value); +int +virTypedParamsGetUInt (virTypedParameterPtr params, + int nparams, + const char *name, + unsigned int *value); +int +virTypedParamsGetLLong (virTypedParameterPtr params, + int nparams, + const char *name, + long long *value); +int +virTypedParamsGetULLong (virTypedParameterPtr params, + int nparams, + const char *name, + unsigned long long *value); +int +virTypedParamsGetDouble (virTypedParameterPtr params, + int nparams, + const char *name, + double *value); +int +virTypedParamsGetBoolean(virTypedParameterPtr params, + int nparams, + const char *name, + int *value); +int +virTypedParamsGetString (virTypedParameterPtr params, + int nparams, + const char *name, + const char **
[libvirt] [PATCH v2 01/10] po: Fix record ordering in POTFILES.in
Commit a474371f broke ordering of the records. Now if a file required for translation is missing, syntax-check (po-check) will fail and suggest a patch to add the missing file. However, if the ordering is broken, the patch will be understandably inapplicable (cleanly), since the diff isn't against the actual POTFILES.in as is, but against POTFILES.in as it should look like with the required file missing instead. --- po/POTFILES.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/po/POTFILES.in b/po/POTFILES.in index fccb5ec..3ffd524 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -1,5 +1,5 @@ -daemon/admin_dispatch.h daemon/admin.c +daemon/admin_dispatch.h daemon/libvirtd-config.c daemon/libvirtd.c daemon/qemu_dispatch.h -- 2.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 04/10] util: Refactor thread creation by introducing virThreadPoolExpand
When either creating a threadpool, or creating a new thread to accomplish a job that had been placed into the jobqueue, every time thread-specific data need to be allocated, threadpool needs to be (re)-allocated and thread count indicators updated. Make the code clearer to read by compressing these operations into a more complex one. --- src/util/virthreadpool.c | 109 +++ 1 file changed, 44 insertions(+), 65 deletions(-) diff --git a/src/util/virthreadpool.c b/src/util/virthreadpool.c index f640448..8af4ec0 100644 --- a/src/util/virthreadpool.c +++ b/src/util/virthreadpool.c @@ -157,6 +157,43 @@ static void virThreadPoolWorker(void *opaque) virMutexUnlock(&pool->mutex); } +static int +virThreadPoolExpand(virThreadPoolPtr pool, size_t gain, bool priority) +{ +virThreadPtr workers = priority ? pool->prioWorkers : pool->workers; +size_t *curWorkers = priority ? &pool->nPrioWorkers : &pool->nWorkers; +size_t i = 0; +struct virThreadPoolWorkerData *data = NULL; + +if (VIR_EXPAND_N(workers, *curWorkers, gain) < 0) +return -1; + +for (i = 0; i < gain; i++) { +if (VIR_ALLOC(data) < 0) +goto error; + +data->pool = pool; +data->cond = priority ? &pool->prioCond : &pool->cond; +data->priority = priority; + +if (virThreadCreateFull(&workers[i], +false, +virThreadPoolWorker, +pool->jobFuncName, +true, +data) < 0) { +VIR_FREE(data); +goto error; +} +} + +return 0; + + error: +*curWorkers -= gain - i; +return -1; +} + virThreadPoolPtr virThreadPoolNewFull(size_t minWorkers, size_t maxWorkers, @@ -166,8 +203,6 @@ virThreadPoolNewFull(size_t minWorkers, void *opaque) { virThreadPoolPtr pool; -size_t i; -struct virThreadPoolWorkerData *data = NULL; if (minWorkers > maxWorkers) minWorkers = maxWorkers; @@ -188,58 +223,23 @@ virThreadPoolNewFull(size_t minWorkers, if (virCondInit(&pool->quit_cond) < 0) goto error; -if (VIR_ALLOC_N(pool->workers, minWorkers) < 0) -goto error; - pool->minWorkers = minWorkers; pool->maxWorkers = maxWorkers; -for (i = 0; i < minWorkers; i++) { -if (VIR_ALLOC(data) < 0) -goto error; -data->pool = pool; -data->cond = &pool->cond; - -if (virThreadCreateFull(&pool->workers[i], -false, -virThreadPoolWorker, -pool->jobFuncName, -true, -data) < 0) { -goto error; -} -pool->nWorkers++; -} +if (virThreadPoolExpand(pool, minWorkers, false) < 0) +goto error; if (prioWorkers) { if (virCondInit(&pool->prioCond) < 0) goto error; -if (VIR_ALLOC_N(pool->prioWorkers, prioWorkers) < 0) -goto error; -for (i = 0; i < prioWorkers; i++) { -if (VIR_ALLOC(data) < 0) -goto error; -data->pool = pool; -data->cond = &pool->prioCond; -data->priority = true; - -if (virThreadCreateFull(&pool->prioWorkers[i], -false, -virThreadPoolWorker, -pool->jobFuncName, -true, -data) < 0) { -goto error; -} -pool->nPrioWorkers++; -} +if (virThreadPoolExpand(pool, prioWorkers, true) < 0) +goto error; } return pool; error: -VIR_FREE(data); virThreadPoolFree(pool); return NULL; @@ -307,36 +307,15 @@ int virThreadPoolSendJob(virThreadPoolPtr pool, void *jobData) { virThreadPoolJobPtr job; -struct virThreadPoolWorkerData *data = NULL; virMutexLock(&pool->mutex); if (pool->quit) goto error; if (pool->freeWorkers - pool->jobQueueDepth <= 0 && -pool->nWorkers < pool->maxWorkers) { -if (VIR_EXPAND_N(pool->workers, pool->nWorkers, 1) < 0) -goto error; - -if (VIR_ALLOC(data) < 0) { -pool->nWorkers--; -goto error; -} - -data->pool = pool; -data->cond = &pool->cond; - -if (virThreadCreateFull(&pool->workers[pool->nWorkers - 1], -false, -virThreadPoolWorker, -pool->jobFuncName, -true, -data) < 0) { -VIR_FREE(data); -
[libvirt] [PATCH v2 05/10] util: Report system error when virThreadCreateFull fails
Otherwise 'Unknown' error will be returned to client. --- po/POTFILES.in | 1 + src/util/virthreadpool.c | 1 + 2 files changed, 2 insertions(+) diff --git a/po/POTFILES.in b/po/POTFILES.in index 3ffd524..dabc581 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -237,6 +237,7 @@ src/util/virstoragefile.c src/util/virstring.c src/util/virsysinfo.c src/util/virthreadjob.c +src/util/virthreadpool.c src/util/virtime.c src/util/virtpm.c src/util/virtypedparam.c diff --git a/src/util/virthreadpool.c b/src/util/virthreadpool.c index 8af4ec0..e2e9fe4 100644 --- a/src/util/virthreadpool.c +++ b/src/util/virthreadpool.c @@ -183,6 +183,7 @@ virThreadPoolExpand(virThreadPoolPtr pool, size_t gain, bool priority) true, data) < 0) { VIR_FREE(data); +virReportSystemError(errno, "%s", _("Failed to create thread")); goto error; } } -- 2.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/10] Introduce worker tuning APIs
> I wanted to review this, but got a lot of rebase conflicts. Can you > rebase and resend please? > > Michal > Rebased and resent as v2, since I had to drop one patch and add a new one. Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Request for testing: migration of libvirt wiki site
On Thu, Mar 24, 2016 at 11:49:50AM +0100, Michal Privoznik wrote: > On 22.03.2016 17:11, Daniel P. Berrange wrote: > > I'm preparing for a migration of the libvirt wiki off the host it is > > currently running on, to OpenShift hosting. This isn't entirely > > straightforward, as I'm also upgrading the version of mediawiki and > > as a result had to re-write the CSS customization from scratch. > > > > I may well have messed something up, so would appreciate if people > > could have a quick look at this testing instance of the wiki and > > confirm that it looks / works reasonably for them. > > > > http://wiki-libvirt.rhcloud.com > > > > NB, any edits you make in this test instance will be discarded when > > I load a fresh database dump from the current live wiki. > > > > If all looks good, I'll look at getting DNS updated to switch over > > later this week. > > > > I know you've already done this, but post mortem: what's the reasoning > behind? The host it was previously running on was RHEL-4 based which is no longer receiving any security updates and stuck on php4 and the hardware was 11 years old with no backups ! Not a particularly great mix :-) Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] nss: FreeBSD support
On 27.03.2016 20:07, Roman Bogorodskiy wrote: > * tools/nss/libvirt_nss.[ch]: add BSD-comptabile wrappers and >register via the nss_module_register() interface > * m4/virt-nss.m4: add checks if we're building NSS for FreeBSD > * tools/Makefile.am: handle target library name differences, as >Linux needs libnss_libvirt.so.2 and FreeBSD needs >nss_libvirt.so.1. Also, different syms files have to be used >as Linux needs to export all the methods while FreeBSD >only needs to have nss_module_register() > * tests/nsstest.c, tests/nssmock.c: s/__linux__/NSS/ > * libvirt_nss_bsd.syms: FreeBSD syms file > --- > m4/virt-nss.m4 | 18 +- > tests/nssmock.c| 6 +- > tests/nsstest.c| 2 +- > tools/Makefile.am | 16 - > tools/nss/libvirt_nss.c| 139 > +++-- > tools/nss/libvirt_nss.h| 9 +++ > tools/nss/libvirt_nss_bsd.syms | 9 +++ > 7 files changed, 189 insertions(+), 10 deletions(-) > create mode 100644 tools/nss/libvirt_nss_bsd.syms > > diff --git a/m4/virt-nss.m4 b/m4/virt-nss.m4 > index 3fa4ad3..3d6e8f4 100644 > --- a/m4/virt-nss.m4 > +++ b/m4/virt-nss.m4 > @@ -23,6 +23,7 @@ AC_DEFUN([LIBVIRT_CHECK_NSS],[ >[enable Name Servie Switch plugin for resolving guest IP addresses])], >[], [with_nss_plugin=check]) > > + bsd_nss=no >fail=0 >if test "x$with_nss_plugin" != "xno" ; then > AC_CHECK_HEADERS([nss.h], [ > @@ -39,11 +40,26 @@ AC_DEFUN([LIBVIRT_CHECK_NSS],[ > > if test "x$with_nss_plugin" = "xyes" ; then >AC_DEFINE_UNQUOTED([NSS], 1, [whether nss plugin is enabled]) > + > + AC_CHECK_TYPE([struct gaih_addrtuple], > +[AC_DEFINE([HAVE_STRUCT_GAIH_ADDRTUPLE], [1], > + [Defined if struct gaih_addrtuple exists in nss.h])], > +[], [[#include > +]]) > + > + AC_CHECK_TYPES([ns_mtab, nss_module_unregister_fn], > + [AC_DEFINE([HAVE_BSD_NSS], > +[1], > +[whether using BSD style NSS]) > + bsd_nss=yes > + ], > + [], > + [#include ]) > fi >fi > >AM_CONDITIONAL(WITH_NSS, [test "x$with_nss_plugin" = "xyes"]) > - > + AM_CONDITIONAL(WITH_BSD_NSS, [test "x$bsd_nss" = "xyes"]) > ]) > > AC_DEFUN([LIBVIRT_RESULT_NSS],[ > diff --git a/tests/nssmock.c b/tests/nssmock.c > index b4a4260..31b1177 100644 > --- a/tests/nssmock.c > +++ b/tests/nssmock.c > @@ -20,7 +20,7 @@ > > #include > > -#ifdef __linux__ > +#ifdef NSS > # include > # include > # include > @@ -107,7 +107,7 @@ open(const char *path, int flags, ...) > va_list ap; > mode_t mode; > va_start(ap, flags); > -mode = va_arg(ap, mode_t); > +mode = va_arg(ap, int); Why this change? I mean, even in the context it can be seen that @mode is type of mode_t. Otherwise looking good. ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] nss: FreeBSD support
Michal Privoznik wrote: > On 27.03.2016 20:07, Roman Bogorodskiy wrote: > > * tools/nss/libvirt_nss.[ch]: add BSD-comptabile wrappers and > >register via the nss_module_register() interface > > * m4/virt-nss.m4: add checks if we're building NSS for FreeBSD > > * tools/Makefile.am: handle target library name differences, as > >Linux needs libnss_libvirt.so.2 and FreeBSD needs > >nss_libvirt.so.1. Also, different syms files have to be used > >as Linux needs to export all the methods while FreeBSD > >only needs to have nss_module_register() > > * tests/nsstest.c, tests/nssmock.c: s/__linux__/NSS/ > > * libvirt_nss_bsd.syms: FreeBSD syms file > > --- > > m4/virt-nss.m4 | 18 +- > > tests/nssmock.c| 6 +- > > tests/nsstest.c| 2 +- > > tools/Makefile.am | 16 - > > tools/nss/libvirt_nss.c| 139 > > +++-- > > tools/nss/libvirt_nss.h| 9 +++ > > tools/nss/libvirt_nss_bsd.syms | 9 +++ > > 7 files changed, 189 insertions(+), 10 deletions(-) > > create mode 100644 tools/nss/libvirt_nss_bsd.syms > > > > diff --git a/m4/virt-nss.m4 b/m4/virt-nss.m4 > > index 3fa4ad3..3d6e8f4 100644 > > --- a/m4/virt-nss.m4 > > +++ b/m4/virt-nss.m4 > > @@ -23,6 +23,7 @@ AC_DEFUN([LIBVIRT_CHECK_NSS],[ > >[enable Name Servie Switch plugin for resolving guest IP > > addresses])], > >[], [with_nss_plugin=check]) > > > > + bsd_nss=no > >fail=0 > >if test "x$with_nss_plugin" != "xno" ; then > > AC_CHECK_HEADERS([nss.h], [ > > @@ -39,11 +40,26 @@ AC_DEFUN([LIBVIRT_CHECK_NSS],[ > > > > if test "x$with_nss_plugin" = "xyes" ; then > >AC_DEFINE_UNQUOTED([NSS], 1, [whether nss plugin is enabled]) > > + > > + AC_CHECK_TYPE([struct gaih_addrtuple], > > +[AC_DEFINE([HAVE_STRUCT_GAIH_ADDRTUPLE], [1], > > + [Defined if struct gaih_addrtuple exists in nss.h])], > > +[], [[#include > > +]]) > > + > > + AC_CHECK_TYPES([ns_mtab, nss_module_unregister_fn], > > + [AC_DEFINE([HAVE_BSD_NSS], > > +[1], > > +[whether using BSD style NSS]) > > + bsd_nss=yes > > + ], > > + [], > > + [#include ]) > > fi > >fi > > > >AM_CONDITIONAL(WITH_NSS, [test "x$with_nss_plugin" = "xyes"]) > > - > > + AM_CONDITIONAL(WITH_BSD_NSS, [test "x$bsd_nss" = "xyes"]) > > ]) > > > > AC_DEFUN([LIBVIRT_RESULT_NSS],[ > > diff --git a/tests/nssmock.c b/tests/nssmock.c > > index b4a4260..31b1177 100644 > > --- a/tests/nssmock.c > > +++ b/tests/nssmock.c > > @@ -20,7 +20,7 @@ > > > > #include > > > > -#ifdef __linux__ > > +#ifdef NSS > > # include > > # include > > # include > > @@ -107,7 +107,7 @@ open(const char *path, int flags, ...) > > va_list ap; > > mode_t mode; > > va_start(ap, flags); > > -mode = va_arg(ap, mode_t); > > +mode = va_arg(ap, int); > > Why this change? I mean, even in the context it can be seen that @mode > is type of mode_t. Yeah, I should have mentioned that in the commit log. I have an error like this: In file included from ../gnulib/lib/stdio.h:51:0, from nssmock.c:24: nssmock.c: In function 'open': nssmock.c:110:27: error: 'mode_t' is promoted to 'int' when passed through '...' [-Werror] mode = va_arg(ap, mode_t); ^ nssmock.c:110:27: note: (so you should pass 'int' not 'mode_t' to 'va_arg') nssmock.c:110:27: note: if this code is reached, the program will abort cc1: all warnings being treated as errors Makefile:4854: recipe for target 'nssmock_la-nssmock.lo' failed This is with: gcc version 4.8.5 (FreeBSD Ports Collection) > Otherwise looking good. ACK > > Michal Roman Bogorodskiy -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] nss: FreeBSD support
On 29.03.2016 12:08, Roman Bogorodskiy wrote: > Michal Privoznik wrote: > >> On 27.03.2016 20:07, Roman Bogorodskiy wrote: >>> * tools/nss/libvirt_nss.[ch]: add BSD-comptabile wrappers and >>>register via the nss_module_register() interface >>> * m4/virt-nss.m4: add checks if we're building NSS for FreeBSD >>> * tools/Makefile.am: handle target library name differences, as >>>Linux needs libnss_libvirt.so.2 and FreeBSD needs >>>nss_libvirt.so.1. Also, different syms files have to be used >>>as Linux needs to export all the methods while FreeBSD >>>only needs to have nss_module_register() >>> * tests/nsstest.c, tests/nssmock.c: s/__linux__/NSS/ >>> * libvirt_nss_bsd.syms: FreeBSD syms file >>> --- >>> m4/virt-nss.m4 | 18 +- >>> tests/nssmock.c| 6 +- >>> tests/nsstest.c| 2 +- >>> tools/Makefile.am | 16 - >>> tools/nss/libvirt_nss.c| 139 >>> +++-- >>> tools/nss/libvirt_nss.h| 9 +++ >>> tools/nss/libvirt_nss_bsd.syms | 9 +++ >>> 7 files changed, 189 insertions(+), 10 deletions(-) >>> create mode 100644 tools/nss/libvirt_nss_bsd.syms >>> >>> diff --git a/m4/virt-nss.m4 b/m4/virt-nss.m4 >>> index 3fa4ad3..3d6e8f4 100644 >>> --- a/m4/virt-nss.m4 >>> +++ b/m4/virt-nss.m4 >>> @@ -23,6 +23,7 @@ AC_DEFUN([LIBVIRT_CHECK_NSS],[ >>>[enable Name Servie Switch plugin for resolving guest IP >>> addresses])], >>>[], [with_nss_plugin=check]) >>> >>> + bsd_nss=no >>>fail=0 >>>if test "x$with_nss_plugin" != "xno" ; then >>> AC_CHECK_HEADERS([nss.h], [ >>> @@ -39,11 +40,26 @@ AC_DEFUN([LIBVIRT_CHECK_NSS],[ >>> >>> if test "x$with_nss_plugin" = "xyes" ; then >>>AC_DEFINE_UNQUOTED([NSS], 1, [whether nss plugin is enabled]) >>> + >>> + AC_CHECK_TYPE([struct gaih_addrtuple], >>> +[AC_DEFINE([HAVE_STRUCT_GAIH_ADDRTUPLE], [1], >>> + [Defined if struct gaih_addrtuple exists in nss.h])], >>> +[], [[#include >>> +]]) >>> + >>> + AC_CHECK_TYPES([ns_mtab, nss_module_unregister_fn], >>> + [AC_DEFINE([HAVE_BSD_NSS], >>> +[1], >>> +[whether using BSD style NSS]) >>> + bsd_nss=yes >>> + ], >>> + [], >>> + [#include ]) >>> fi >>>fi >>> >>>AM_CONDITIONAL(WITH_NSS, [test "x$with_nss_plugin" = "xyes"]) >>> - >>> + AM_CONDITIONAL(WITH_BSD_NSS, [test "x$bsd_nss" = "xyes"]) >>> ]) >>> >>> AC_DEFUN([LIBVIRT_RESULT_NSS],[ >>> diff --git a/tests/nssmock.c b/tests/nssmock.c >>> index b4a4260..31b1177 100644 >>> --- a/tests/nssmock.c >>> +++ b/tests/nssmock.c >>> @@ -20,7 +20,7 @@ >>> >>> #include >>> >>> -#ifdef __linux__ >>> +#ifdef NSS >>> # include >>> # include >>> # include >>> @@ -107,7 +107,7 @@ open(const char *path, int flags, ...) >>> va_list ap; >>> mode_t mode; >>> va_start(ap, flags); >>> -mode = va_arg(ap, mode_t); >>> +mode = va_arg(ap, int); >> >> Why this change? I mean, even in the context it can be seen that @mode >> is type of mode_t. > > Yeah, I should have mentioned that in the commit log. > > I have an error like this: > > In file included from ../gnulib/lib/stdio.h:51:0, > from nssmock.c:24: > nssmock.c: In function 'open': > nssmock.c:110:27: error: 'mode_t' is promoted to 'int' when passed through > '...' [-Werror] > mode = va_arg(ap, mode_t); >^ > nssmock.c:110:27: note: (so you should pass 'int' not 'mode_t' to 'va_arg') > nssmock.c:110:27: note: if this code is reached, the program will abort > cc1: all warnings being treated as errors > Makefile:4854: recipe for target 'nssmock_la-nssmock.lo' failed > > This is with: > > gcc version 4.8.5 (FreeBSD Ports Collection) > Ah, okay then. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Publishing libvirt 'patches' JSON database?
On Thu, Mar 24, 2016 at 09:31:44AM +0100, Kashyap Chamarthy wrote: > On Wed, Mar 23, 2016 at 06:46:53PM +, Daniel P. Berrange wrote: > > On Wed, Mar 23, 2016 at 04:34:26PM +0100, Kashyap Chamarthy wrote: > > [snip] > > > > So things turn out to be a bit more complicated for libvirt than for > > QEMU, because we have one mailing list accepting patches for 10+ > > different git repositories. > > Ah-ha, good point. > > > Patches has no built-in support for this > > model, so you have to create a separate configuration file for each > > repo, so that it stores the patches json file separately for each > > and then pass that config file when fetching > > > > git clone git://github.com/stefanha/patches.git > > > > REPOS="libvirt libvirt-designer libvirt-glib libvirt-java > > libvirt-perl libvirt-php libvirt-python libvirt-sandbox > > libvirt-snmp libvirt-tck" > > for repo in $REPOS > > do > > cat > $HOME/.patchesrc-$repo < > [options] > > patches_dir=$HOME/.patches/$repo > > > > [fetch] > > url=http://libvirt.org/patches/$repo/patches.json > > EOF > > Yeah, this nicely solves it currently, thanks for clearly spelling it > out. > > How often are the JSON databases updated? Once daily at least, I > presume. Once an hour Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 2/2] qemu: Add support to QXL's max_outputs parameter
On Fri, Mar 25, 2016 at 10:58:13AM +0100, Christophe Fergeau wrote: Hey, On Thu, Mar 24, 2016 at 05:01:52PM +0100, Martin Kletzander wrote: Historically, we added heads=1 to videos, but for example for qxl, we did not reflect that on the command line. Implementing that now could mean that if user were to migrate from older to newer libvirt, the command-line for qemu would differ. In order for that not to happen a migration cookie flag is introduced. Signed-off-by: Martin Kletzander --- src/qemu/qemu_command.c| 8 +++ src/qemu/qemu_migration.c | 64 -- .../qemuxml2argv-video-qxl-heads.args | 28 ++ .../qemuxml2argv-video-qxl-heads.xml | 47 tests/qemuxml2argvtest.c | 8 +++ .../qemuxml2xmlout-video-qxl-heads.xml | 47 tests/qemuxml2xmltest.c| 2 + 7 files changed, 198 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-heads.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-heads.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-video-qxl-heads.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0331789b6b59..f6b102e96bb2 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3985,6 +3985,14 @@ qemuBuildDeviceVideoStr(const virDomainDef *def, /* QEMU accepts mebibytes for vgamem_mb. */ virBufferAsprintf(&buf, ",vgamem_mb=%u", video->vgamem / 1024); } + +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_MAX_OUTPUTS) && +virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_MAX_OUTPUTS)) { This test could be more similar to the one for vgamem/vram64 above for consistency, and be if ((video->primary && virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_MAX_OUTPUTS)) || (!video->primary && virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_MAX_OUTPUTS))) { } Since both caps will always be set/not set, this is not going to make a difference, and your version is easier to read imo. well, yes, just in a corner case with "special" qemu. I'm in favor of using the parameter for all cards or none, so that's why I went with this approach. +if (video->heads) +virBufferAsprintf(&buf, ",max_outputs=%u", video->heads); +} else { +video->heads = 0; +} Is it typical to modify the domain def content from within qemu_command.c ? Not much, we're in the middle of refactoring some of that. I rebased this version and it's now in a bit different place. The trick here is that we're modifying the live XML, not the one on the disk, so the definition is not changed on the disk, but if someone calls dumpxml on the running domain, there will be no heads= parameter to reflect what we told qemu. } else if (video->vram && ((video->type == VIR_DOMAIN_VIDEO_TYPE_VGA && virQEMUCapsGet(qemuCaps, QEMU_CAPS_VGA_VGAMEM)) || diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 8bc76bf1671d..f83b6362d228 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -87,6 +87,7 @@ enum qemuMigrationCookieFlags { QEMU_MIGRATION_COOKIE_FLAG_NBD, QEMU_MIGRATION_COOKIE_FLAG_STATS, QEMU_MIGRATION_COOKIE_FLAG_MEMORY_HOTPLUG, +QEMU_MIGRATION_COOKIE_FLAG_VIDEO_HEADS, QEMU_MIGRATION_COOKIE_FLAG_LAST }; @@ -100,7 +101,8 @@ VIR_ENUM_IMPL(qemuMigrationCookieFlag, "network", "nbd", "statistics", - "memory-hotplug"); + "memory-hotplug", + "video-heads"); enum qemuMigrationCookieFeatures { QEMU_MIGRATION_COOKIE_GRAPHICS = (1 << QEMU_MIGRATION_COOKIE_FLAG_GRAPHICS), @@ -110,6 +112,7 @@ enum qemuMigrationCookieFeatures { QEMU_MIGRATION_COOKIE_NBD = (1 << QEMU_MIGRATION_COOKIE_FLAG_NBD), QEMU_MIGRATION_COOKIE_STATS = (1 << QEMU_MIGRATION_COOKIE_FLAG_STATS), QEMU_MIGRATION_COOKIE_MEMORY_HOTPLUG = (1 << QEMU_MIGRATION_COOKIE_FLAG_MEMORY_HOTPLUG), +QEMU_MIGRATION_COOKIE_VIDEO_HEADS = (1 << QEMU_MIGRATION_COOKIE_FLAG_VIDEO_HEADS), }; typedef struct _qemuMigrationCookieGraphics qemuMigrationCookieGraphics; @@ -1386,6 +1389,9 @@ qemuMigrationBakeCookie(qemuMigrationCookiePtr mig, if (flags & QEMU_MIGRATION_COOKIE_MEMORY_HOTPLUG) mig->flagsMandatory |= QEMU_MIGRATION_COOKIE_MEMORY_HOTPLUG; +if (flags & QEMU_MIGRATION_COOKIE_VIDEO_HEADS) +mig->flagsMandatory |= QEMU_MIGRATION_COOKIE_VIDEO_HEADS; + if (!(*cookieout = qemuMigrationCookieXMLFormatStr(driver, mig))) return -1; @@ -3091,6 +3097,7 @@ qemuMigrationBeginPhase(virQEMUDriverPtr driver, qemuDomainObjPrivatePtr priv = vm->privateData; virCapsPtr caps = NULL; unsigned int cookieFlags = QEMU_MIGRATION_COOKIE_LOCKSTATE; +
[libvirt] [PATCH] docs: Document NSS module
While we have a wiki page describing the feature [1] since the feature is distributed in our .tar.gz we ought to document it. So I went ahead, copied the wiki page and reformatted so it fits our docs coding style. 1: http://wiki.libvirt.org/page/NSS_module Signed-off-by: Michal Privoznik --- docs/nss.html.in | 141 +++ docs/sitemap.html.in | 4 ++ 2 files changed, 145 insertions(+) create mode 100644 docs/nss.html.in diff --git a/docs/nss.html.in b/docs/nss.html.in new file mode 100644 index 000..84ea8df --- /dev/null +++ b/docs/nss.html.in @@ -0,0 +1,141 @@ + +http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd";> +http://www.w3.org/1999/xhtml";> + +Libvirt NSS module + + + + +When it comes to managing guests and executing commands inside them, logging +into guest operating system and doing the job is convenient. Users are used +to ssh in this case. Ideally: + + +ssh user@virtualMachine + + +would be nice. But depending on virtual network configuration it might not +be always possible. For instance, when using libvirt NATed network it's +dnsmasq (spawned by libvirt) who assigns IP addresses to domains. But by +default, the dnsmasq process is then not consulted when it comes to host +name translation. Users work around this problem by configuring their +libvirt network to assign static IP addresses and maintaining +/etc/hosts file in sync. But this puts needless burden onto +users. This is where NSS module comes handy. + + +Installation + + +Installing the module is really easy: + + + +# yum install libvirt-nss + + +Configuration + + +Enabling the module is really easy. Just add libvirt into +/etc/nsswitch.conf file. For instance: + + + +$ cat /etc/nsswitch.conf +# /etc/nsswitch.conf: +passwd: compat +shadow: compat +group: compat +hosts: files libvirt dns +# ... + + + +So, in this specific case, whenever ssh program is looking up the host user +is trying to connect to, files module is consulted first (which +boils down to looking up the host name in /etc/hosts file), if +not found libvirt module is consulted then. The DNS is the last +effort then, if none of the previous modules matched the host in question. +Therefore users should consider the order in which they want the modules to +lookup given host name. + + +How does it work? + + +Whenever an Unix process wants to do a host name translation +http://linux.die.net/man/3/gethostbyname";>gethostbyname() +or some variant of it is called. This is a glibc function that takes a +string containing the host name, crunch it and produces a list of IP +addresses assigned to that host. Now, glibc developers made a really good +decision when implementing the internals of the function when they decided +to make the function pluggable. Since there can be several sources for the +records (e.g. /etc/hosts file, DNS, LDAP, etc.) it would not +make much sense to create one big implementation containing all possible +cases. What they have done instead is this pluggable mechanism. Small +plugins implementing nothing but specific technology for lookup process are +provided and the function then calls those plugins. There is just one +configuration file that instructs the lookup function in which order should +the plugins be called and which plugins should be loaded. For more info +reading https://en.wikipedia.org/wiki/Name_Service_Switch";>wiki +page is recommended. + + + +And this is point where libvirt comes in. Libvirt provides plugin for the +NSS ecosystem. For some time now libvirt keeps a list of assigned IP +addresses for libvirt networks. The NSS plugin does no more than search the +list trying to find matching record for given host name. When found, +matching IP address is returned to the caller. If not found, translation +process continues with the next plugin configured. At this point it is +important to stress the order in which plugins are called. Users should be +aware that a hostname might match in multiple plugins and right after first +match, translation process is terminated and no other plugin is consulted. +Therefore, if there are two different records for the same host name users +should carefully chose the lookup order. + + +Limitations + + + The libvirt NSS module matches only hostnames provided by guest. If +the libvirt name and one advertised by guest differs, the latter is +matched. + The module works only in that cases where IP addresses are assigned by +dnsmasq spawned by libvirt. Libvirt NATed networks are typical +example. + + + +These limitation are result of libvirt's internal implementation. While +libvirt can report IP ad
Re: [libvirt] [PATCH] docs: Document NSS module
On Tue, Mar 29, 2016 at 01:21:52PM +0200, Michal Privoznik wrote: > While we have a wiki page describing the feature [1] since the > feature is distributed in our .tar.gz we ought to document it. So > I went ahead, copied the wiki page and reformatted so it fits our > docs coding style. > > 1: http://wiki.libvirt.org/page/NSS_module > > Signed-off-by: Michal Privoznik > --- > docs/nss.html.in | 141 > +++ > docs/sitemap.html.in | 4 ++ > 2 files changed, 145 insertions(+) > create mode 100644 docs/nss.html.in ACK Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] SCSI storage: Initialise the pool size parameters to zero while refreshing
On 03/22/2016 04:34 AM, Nitesh Konkar wrote: > If the pool creation thread happens to detect the luns > in the scsi target,the size parameters will be populated > as part of the refresh called from storagePoolCreate(). > The commit <4a85bf3e2f> added additional refresh for > SCSI which should also have these values initialised > to zero, otherwise the values would appear exactly double. > A separate refresh would correct the values, but without > an explicit pool-refresh, the allocation/capacity would > appear exactly double after pool-create. > --- > src/storage/storage_backend_scsi.c | 3 +++ > 1 file changed, 3 insertions(+) > Your commit history is a bit off - commit id '4a85bf3e2f' was for the virStorageVolPoolRefreshThread not the virStoragePoolFCRefreshThread. Two very different threads and problems! In any case, not clearing the pool allocation, capacity, and available values in the FCRefreshThread was an oversight, but then again - the refreshPool ran and would clear them. However, it seems "timing" has changed since the original commit (there are storage state driver and FindLUs processing changes) and perhaps there is "enough time" for the luns to appear. I adjusted the commit to use commit id '512b874' and adjusted the commit description a bit and pushed. Thanks for catching this! John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Host device assignment driver name vfio/ kvm
Hi, I was testing Host device assignment in OpenStack environment where the driver name is vfio or kvm. My setup is as follow: 1. Fedora 21 2. Libvirt 1.3.0 which I compiled 3. OpenStack master I have also other setups with older Libvirt version and the same OpenStack environment. I notice that on my fedora environment the driver name is vfio were in my old environment the driver name is kvm. According to Libvirt documentation default is "vfio" on systems where the VFIO driver is available and loaded, see [1] I remove the vfio modules by removing vfio, vfio_iommu_type1, vfio_pci but when I boot a vm the drive name is vfio How can change the driver name to be kvm? Another thing that I encounter is an error when suspending VM (in OpenStack environment) when the driver name is vfio. In such case I am getting the following error from Libvirt: 2016-03-28 11:42:59.527 1966 ERROR oslo_messaging.rpc.dispatcher File "/usr/lib64/python2.7/site-packages/libvirt.py", line 560, in attachDeviceFlags 2016-03-28 11:42:59.527 1966 ERROR oslo_messaging.rpc.dispatcher if ret == -1: raise libvirtError ('virDomainAttachDeviceFlags() failed', dom=self) 2016-03-28 11:42:59.527 1966 ERROR oslo_messaging.rpc.dispatcher libvirtError: internal error: unable to execute QEMU command 'device_add': Device initialization failed. I would appreciate for some pointers on what can cause this issue. [1] https://libvirt.org/formatdomain.html#elementsHostDev -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 3/8] perf: implement a set of util functions for perf event
On Mon, Mar 28, 2016 at 09:30:28PM +0800, Qiaowei Ren wrote: > This patch implement a set of interfaces for perf event. Based on > these interfaces, we can implement internal driver API for perf, > and get the results of perf conuter you care about. > > Signed-off-by: Qiaowei Ren > --- > include/libvirt/virterror.h | 2 + > src/Makefile.am | 1 + > src/libvirt_private.syms| 12 ++ > src/util/virerror.c | 2 + > src/util/virperf.c | 307 > > src/util/virperf.h | 63 + src/util/virperf.c needs to be added to po/POTFILES.in to get 'make syntax-check' to pass Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 5/8] perf: add new xml element
On Mon, Mar 28, 2016 at 09:30:30PM +0800, Qiaowei Ren wrote: > This patch adds new xml element, and so we can have the option of > also having perf events enabled immediately at startup. > > Signed-off-by: Qiaowei Ren > --- > docs/schemas/domaincommon.rng | 27 +++ > src/conf/domain_conf.c| 111 > ++ > src/conf/domain_conf.h| 10 +++ > src/qemu/qemu_driver.c| 26 ++ > src/qemu/qemu_process.c | 8 +- > tests/domainschemadata/domain-perf-simple.xml | 20 + > 6 files changed, 200 insertions(+), 2 deletions(-) > create mode 100644 tests/domainschemadata/domain-perf-simple.xml > +static void > +virDomainPerfDefFormat(virBufferPtr buf, virDomainPerfDefPtr perf) > +{ > +size_t i; > +virBufferAddLit(buf, "\n"); We must skip ading of when no perf events are enabled otherwise we break many existing test cases. eg add in bool wantPerf = false; for (i = 0; i < VIR_PERF_EVENT_LAST; i++) { if (perf->events[i]) wantPerf = true; } if (!wantPerf) return; before this line. > +virBufferAdjustIndent(buf, 2); > + > +for (i = 0; i < VIR_PERF_EVENT_LAST; i++) { > +if (perf->events[i]) > +virBufferAsprintf(buf, "\n", > + virPerfEventTypeToString(i), > + virTristateBoolTypeToString(perf->events[i])); > +} > + > +virBufferAdjustIndent(buf, -2); > +virBufferAddLit(buf, "\n"); > +} Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 7/8] virsh: implement new command to support perf
On Mon, Mar 28, 2016 at 09:30:32PM +0800, Qiaowei Ren wrote: > This patch add new perf command to enable/disable perf event > for a guest domain. > > Signed-off-by: Qiaowei Ren > --- > tools/virsh-domain.c | 128 > +++ > tools/virsh.pod | 20 > 2 files changed, 148 insertions(+) > > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c > index 17be5b4..0d020a7 100644 > --- a/tools/virsh-domain.c > +++ b/tools/virsh-domain.c > @@ -8506,6 +8506,128 @@ cmdMemtune(vshControl *ctl, const vshCmd *cmd) > } > > /* > + * "perf" command > + */ > +static const vshCmdInfo info_perf[] = { > +{.name = "help", > +.data = N_("Get or set perf event") > +}, > +{.name = "desc", > +.data = N_("Get or set the current perf events for a guest" > + " domain.\n" > + "To get the perf events list use following command: > \n\n" > + "virsh # perf ") > +}, > +{.name = NULL} > +}; > + > +static const vshCmdOptDef opts_perf[] = { > +{.name = "domain", > + .type = VSH_OT_DATA, > + .flags = VSH_OFLAG_REQ, > + .help = N_("domain name, id or uuid") > +}, > +{.name = "enable", > + .type = VSH_OT_STRING, > + .help = N_("perf events which will be enabled") > +}, > +{.name = "disable", > + .type = VSH_OT_STRING, > + .help = N_("perf events which will be disabled") > +}, > +{.name = NULL} > +}; > + > +static int > +virshParseEventStr(vshControl *ctl, > + const char *event, > + bool state, > + virTypedParameterPtr *params, > + int *nparams, > + int *maxparams) > +{ > +char **tok = NULL; > +size_t i, ntok; > +int ret = -1; > + > +if (!(tok = virStringSplitCount(event, "|", 0, &ntok))) > +return -1; > + > +if (ntok > VIR_PERF_EVENT_LAST) { > +vshError(ctl, _("event string '%s' has too many fields"), event); > +goto cleanup; > +} > + > +for(i = 0; i < ntok; i++) { Needs space after the 'for' > +if ((*tok[i] != '\0') && > +virTypedParamsAddBoolean(params, nparams, > + maxparams, tok[i], state) < 0) > +goto cleanup; > +} > + > +ret = 0; > + cleanup: > +virStringFreeList(tok); > +return ret; > +} > + > +static bool > +cmdPerf(vshControl *ctl, const vshCmd *cmd) > +{ > +virDomainPtr dom; > +int nparams = 0; > +int maxparams = 0; > +size_t i; > +virTypedParameterPtr params = NULL; > +bool ret = false; > +const char *enable = NULL, *disable = NULL; > + > +if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) > +return false; > + > +if (vshCommandOptStringReq(ctl, cmd, "enable", &enable) < 0 || > +vshCommandOptStringReq(ctl, cmd, "disable", &disable) < 0) > +return false; > + > +if (enable && virshParseEventStr(ctl, enable, true, > + ¶ms, &nparams, &maxparams) < 0) > +goto cleanup; > + > +if (disable && virshParseEventStr(ctl, disable, false, > + ¶ms, &nparams, &maxparams) < 0) > + goto cleanup; Using tab instead of space Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 5/8] perf: add new xml element
On Mon, Mar 28, 2016 at 09:30:30PM +0800, Qiaowei Ren wrote: > This patch adds new xml element, and so we can have the option of > also having perf events enabled immediately at startup. > > Signed-off-by: Qiaowei Ren > --- > docs/schemas/domaincommon.rng | 27 +++ > src/conf/domain_conf.c| 111 > ++ > src/conf/domain_conf.h| 10 +++ > src/qemu/qemu_driver.c| 26 ++ > src/qemu/qemu_process.c | 8 +- > tests/domainschemadata/domain-perf-simple.xml | 20 + > 6 files changed, 200 insertions(+), 2 deletions(-) > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c > index f7d30e9..cf7df1a 100644 > --- a/src/qemu/qemu_process.c > +++ b/src/qemu/qemu_process.c > @@ -5254,8 +5254,12 @@ qemuProcessLaunch(virConnectPtr conn, > goto cleanup; > > priv->perf = virPerfNew(); > -if (!priv->perf) > -goto cleanup; > +if (priv->perf) { > +for (size_t i = 0; i < VIR_PERF_EVENT_LAST; i++) { We don't declare variables in the for loop > +if (vm->def->perf->events[i] == VIR_TRISTATE_BOOL_YES) > +virPerfEventEnable(priv->perf, i, vm->pid); > +} > +} Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 0/8] Add perf and Intel CMT feature support
On Mon, Mar 28, 2016 at 09:30:25PM +0800, Qiaowei Ren wrote: > The series mainly adds Intel CMT feature support into libvirt. CMT is > new introduced PQos (Platform Qos) feature to monitor the usage of > cache by applications running on the platform. > > Currently CMT patches has been merged into Linux kernel mainline. > The CMT implementation in Linux kernel is based on perf mechanism and > there is no support for perf in libvirt, and so this series firstly > add perf support into libvirt, including two public API and a set of > util interfaces. And based on these APIs and interfaces, thie series > implements CMT perf event support. > > Current state: > > * 1/8 - perf: add new public APIs for perf event > - ACKed by Daniel > > * 2/8 - perf: implement the remote protocol for perf event > - ACKed by Daniel > > * 3/8 - perf: implement a set of util functions for perf event > - ACKed by Daniel > > * 4/8 - qemu_driver: add support to perf event > - restart perf in qemuProcessReconnect and qemuProcessAttach > in patch 6/8 > > * 5/8 - perf: add new xml element > - ACKed by Daniel > > * 6/8 - perf: reenable perf events when libvirtd restart > - add qemuDomainPerfRestart() into qemuProcessReconnect() and > qemuProcessAttach() > > * 7/8 - virsh: implement new command to support perf > - ACKed by Daniel > > * 8/8 - virsh: extend domstats command > - ACKed by Daniel ACK to all patches. I had some comments inline - in future just make sure you run 'make check && make syntax-check' against each patch before submitting. Since the fixes were easy I've made them myself and pushed to GIT master. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] bhyve: tiny cleanup and fix
Maxim Nestratov (2): bhyve: cleanup unnecessary variables bhyve: fix invalid hostsysinfo freeing src/bhyve/bhyve_driver.c | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) -- 2.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] bhyve: fix invalid hostsysinfo freeing
--- src/bhyve/bhyve_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 4d341b5..5526bb0 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -1172,7 +1172,7 @@ bhyveStateCleanup(void) virObjectUnref(bhyve_driver->domains); virObjectUnref(bhyve_driver->caps); virObjectUnref(bhyve_driver->xmlopt); -virObjectUnref(bhyve_driver->hostsysinfo); +virSysinfoDefFree(bhyve_driver->hostsysinfo); virObjectUnref(bhyve_driver->closeCallbacks); virObjectEventStateFree(bhyve_driver->domainEventState); -- 2.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] bhyve: implement domainShutdown
On Mon, Mar 28, 2016 at 10:27:18AM +0300, Roman Bogorodskiy wrote: > Bhyve supports ACPI shutdown by issuing SIGTERM signal to the bhyve > process. Add the bhyveDomainShutdown() function and > virBhyveProcessShutdown() helper function that just sends SIGTERM to > VM's bhyve process. If a guest supports ACPI shutdown then process > will be terminated and this event will be noticed by the bhyve monitor > code that will handle setting proper status and clean up VM's resources. > > Also, remove a warning in domainDestroy in case if > virProcessKillPainfully() returns 1, meaning that it killed process > using SIGKILL. This behavior should be expected when using 'destroy'. Hmm, so destroy is supposed to be equivalent to physically removing the power plug. The existing code is calling virProcessShutdownPainfully which starts by sending SIGTERM and then switches to SIGKILL. So this means that your virDomainDestroy implementation is mistakenly trying todo a graceful shutdown initially and then switching to hard shutdown after a bit of a delay. Has bhyve always used SIGTERM to trigger ACPI shutdown, or is this a recent addition ? I would tend to suggest we need to go straight to SIGKILL for virDomainDestroy to avoid doing ACPI shutdown when we don't want it. It is kind of a shame they used SIGTERM for triggering ACPI imho but oh well. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] bhyve: cleanup: remove unnecessary variable
--- src/bhyve/bhyve_driver.c | 5 - 1 file changed, 5 deletions(-) diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 9219890..4d341b5 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -1187,8 +1187,6 @@ bhyveStateInitialize(bool privileged, virStateInhibitCallback callback ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { -virConnectPtr conn = NULL; - if (!privileged) { VIR_INFO("Not running privileged, disabling driver"); return 0; @@ -1259,12 +1257,9 @@ bhyveStateInitialize(bool privileged, virBhyveProcessReconnectAll(bhyve_driver); -virObjectUnref(conn); - return 0; cleanup: -virObjectUnref(conn); bhyveStateCleanup(); return -1; } -- 2.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] bhyve: cleanup: remove unnecessary variable
On Tue, Mar 29, 2016 at 03:20:55PM +0300, Maxim Nestratov wrote: > --- > src/bhyve/bhyve_driver.c | 5 - > 1 file changed, 5 deletions(-) > > diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c > index 9219890..4d341b5 100644 > --- a/src/bhyve/bhyve_driver.c > +++ b/src/bhyve/bhyve_driver.c > @@ -1187,8 +1187,6 @@ bhyveStateInitialize(bool privileged, > virStateInhibitCallback callback ATTRIBUTE_UNUSED, > void *opaque ATTRIBUTE_UNUSED) > { > -virConnectPtr conn = NULL; > - > if (!privileged) { > VIR_INFO("Not running privileged, disabling driver"); > return 0; > @@ -1259,12 +1257,9 @@ bhyveStateInitialize(bool privileged, > > virBhyveProcessReconnectAll(bhyve_driver); > > -virObjectUnref(conn); > - > return 0; > > cleanup: > -virObjectUnref(conn); > bhyveStateCleanup(); > return -1; > } ACK Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] bhyve: fix invalid hostsysinfo freeing
On Tue, Mar 29, 2016 at 03:20:56PM +0300, Maxim Nestratov wrote: > --- > src/bhyve/bhyve_driver.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c > index 4d341b5..5526bb0 100644 > --- a/src/bhyve/bhyve_driver.c > +++ b/src/bhyve/bhyve_driver.c > @@ -1172,7 +1172,7 @@ bhyveStateCleanup(void) > virObjectUnref(bhyve_driver->domains); > virObjectUnref(bhyve_driver->caps); > virObjectUnref(bhyve_driver->xmlopt); > -virObjectUnref(bhyve_driver->hostsysinfo); > +virSysinfoDefFree(bhyve_driver->hostsysinfo); > virObjectUnref(bhyve_driver->closeCallbacks); > virObjectEventStateFree(bhyve_driver->domainEventState); ACK Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Plan for next release
I'm a bit behind, I forgot ! I think we should freeze tomorrow (Wed 30), then make an rc2 on Friday and shoot for a release next Monday (Apr 4), works for everybody ? Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] qemu: Add capability bit for qemu secret object
On Thu, Mar 24, 2016 at 01:53:19PM -0400, John Ferlan wrote: > Add a capability bit for the qemu secret object. > > Adjust the 2.6.0-1 caps/replies to add the secret object. For the > .replies it's take from the '{"execute":"qom-list-types"}' output. > > Signed-off-by: John Ferlan > --- > src/qemu/qemu_capabilities.c| 2 ++ > src/qemu/qemu_capabilities.h| 1 + > tests/qemucapabilitiesdata/caps_2.6.0-1.caps| 1 + > tests/qemucapabilitiesdata/caps_2.6.0-1.replies | 3 +++ > 4 files changed, 7 insertions(+) ACK Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 00/11] vz: change vz driver to be stateful driver and other enhancements
There is no benefit in providing two ways of connecting to vz driver: by connecting via daemon and directly from client. Both ways finally come to a host where vz daemon sits. Always connecting via daemon allows us to have a single list of domains and share it among all connections. Maxim Nestratov (11): virsh: report when vz driver is compiled vz: change the order of capabilities reported vz: remove drivername field from vzConn structure vz: build driver as module and don't register it on client's side vz: pass vzConnPtr to prlsdkXxx functions instead of virConnectPtr vz: make vzConn structure be a new lockable object and use it vz: implement connectGetSysinfo hypervisor callback vz: remove close callback implementations vz: remove vzDriverLock/Unlock function vz: minor cleanup vz: change vzConnectIsAlive behavior daemon/Makefile.am | 4 + daemon/libvirtd.c | 9 + src/Makefile.am| 21 ++- src/libvirt.c | 7 - src/vz/vz_driver.c | 479 ++--- src/vz/vz_sdk.c| 36 ++-- src/vz/vz_sdk.h| 6 +- src/vz/vz_utils.c | 18 +- src/vz/vz_utils.h | 15 +- tools/virsh.c | 3 + 10 files changed, 341 insertions(+), 257 deletions(-) -- 2.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 03/11] vz: remove drivername field from vzConn structure
No need to remember connection name and have corresponding domain type to keep backward compatibility with former 'parallels' driver. It is enough to be able to accept 'parallels' uri and domain types. Signed-off-by: Maxim Nestratov --- src/vz/vz_driver.c | 2 -- src/vz/vz_utils.c | 5 + src/vz/vz_utils.h | 1 - 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 47a5060..9de88cd 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -228,8 +228,6 @@ vzOpenDefault(virConnectPtr conn) goto err_free; } -privconn->drivername = conn->driver->name; - if (prlsdkInit()) { VIR_DEBUG("%s", _("Can't initialize Parallels SDK")); goto err_free; diff --git a/src/vz/vz_utils.c b/src/vz/vz_utils.c index fed48a5..64e469c 100644 --- a/src/vz/vz_utils.c +++ b/src/vz/vz_utils.c @@ -178,10 +178,7 @@ vzNewDomain(vzConnPtr privconn, char *name, const unsigned char *uuid) pdom->cache.stats = PRL_INVALID_HANDLE; pdom->cache.count = -1; -if (STREQ(privconn->drivername, "vz")) -def->virtType = VIR_DOMAIN_VIRT_VZ; -else -def->virtType = VIR_DOMAIN_VIRT_PARALLELS; +def->virtType = VIR_DOMAIN_VIRT_VZ; if (!(dom = virDomainObjListAdd(privconn->domains, def, privconn->xmlopt, diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h index f373850..b415b0f 100644 --- a/src/vz/vz_utils.h +++ b/src/vz/vz_utils.h @@ -70,7 +70,6 @@ struct _vzConn { virCapsPtr caps; virDomainXMLOptionPtr xmlopt; virObjectEventStatePtr domainEventState; -const char *drivername; /* Immutable pointer, self-locking APIs */ virConnectCloseCallbackDataPtr closeCallback; unsigned long vzVersion; -- 2.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 07/11] vz: implement connectGetSysinfo hypervisor callback
Signed-off-by: Maxim Nestratov --- src/vz/vz_driver.c | 26 ++ src/vz/vz_utils.h | 1 + 2 files changed, 27 insertions(+) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 36030a7..4c30305 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -176,6 +176,7 @@ static void vzConnDispose(void * obj) virObjectUnref(conn->domains); virObjectUnref(conn->caps); virObjectUnref(conn->xmlopt); +virSysinfoDefFree(conn->hostsysinfo); virObjectEventStateFree(conn->domainEventState); } @@ -299,6 +300,7 @@ vzConnObjNew(void) return NULL; } +conn->hostsysinfo = virSysinfoRead(); return conn; } @@ -375,6 +377,29 @@ static char *vzConnectGetHostname(virConnectPtr conn ATTRIBUTE_UNUSED) return virGetHostname(); } +static char * +vzConnectGetSysinfo(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned int flags) +{ +vzConnPtr privconn = vzGetConnection(); +if (!privconn) +return NULL; +virBuffer buf = VIR_BUFFER_INITIALIZER; + +virCheckFlags(0, NULL); + +if (!privconn->hostsysinfo) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Host SMBIOS information is not available")); +return NULL; +} + +if (virSysinfoFormat(&buf, privconn->hostsysinfo) < 0) +return NULL; +if (virBufferCheckError(&buf) < 0) +return NULL; + +return virBufferContentAndReset(&buf); +} static int vzConnectListDomains(virConnectPtr conn ATTRIBUTE_UNUSED, int *ids, int maxids) @@ -1568,6 +1593,7 @@ static virHypervisorDriver vzDriver = { .connectClose = vzConnectClose, /* 0.10.0 */ .connectGetVersion = vzConnectGetVersion, /* 0.10.0 */ .connectGetHostname = vzConnectGetHostname, /* 0.10.0 */ +.connectGetSysinfo = vzConnectGetSysinfo, /* 1.3.3 */ .connectGetMaxVcpus = vzConnectGetMaxVcpus, /* 1.2.21 */ .nodeGetInfo = vzNodeGetInfo, /* 0.10.0 */ .nodeGetCPUStats = vzNodeGetCPUStats, /* 1.2.21 */ diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h index 00e795f..a2b86c7 100644 --- a/src/vz/vz_utils.h +++ b/src/vz/vz_utils.h @@ -72,6 +72,7 @@ struct _vzConn { virObjectEventStatePtr domainEventState; /* Immutable pointer, self-locking APIs */ virConnectCloseCallbackDataPtr closeCallback; +virSysinfoDefPtr hostsysinfo; unsigned long vzVersion; vzCapabilities vzCaps; }; -- 2.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 02/11] vz: change the order of capabilities reported
'vz' goes first now to make clients like virt-manager choose 'vz' instead of 'parallels' Signed-off-by: Maxim Nestratov --- src/vz/vz_driver.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index e12a95a..47a5060 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -109,10 +109,10 @@ vzBuildCapabilities(void) VIR_DOMAIN_OSTYPE_EXE }; virArch archs[] = { VIR_ARCH_I686, VIR_ARCH_X86_64 }; -const char *const emulators[] = { "parallels", "vz" }; +const char *const emulators[] = { "vz", "parallels"}; virDomainVirtType virt_types[] = { -VIR_DOMAIN_VIRT_PARALLELS, -VIR_DOMAIN_VIRT_VZ +VIR_DOMAIN_VIRT_VZ, +VIR_DOMAIN_VIRT_PARALLELS }; size_t i, j, k; -- 2.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 01/11] virsh: report when vz driver is compiled
Signed-off-by: Maxim Nestratov --- tools/virsh.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/virsh.c b/tools/virsh.c index 57b4ff3..89a2113 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -580,6 +580,9 @@ virshShowVersion(vshControl *ctl ATTRIBUTE_UNUSED) #ifdef WITH_OPENVZ vshPrint(ctl, " OpenVZ"); #endif +#ifdef WITH_VZ +vshPrint(ctl, " Virtuozzo"); +#endif #ifdef WITH_VMWARE vshPrint(ctl, " VMware"); #endif -- 2.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 09/11] vz: remove vzDriverLock/Unlock function
We don't need them anymore as all pointers within vzConn structure are not changed during the time it exists. Signed-off-by: Maxim Nestratov --- src/vz/vz_driver.c | 31 --- src/vz/vz_utils.h | 2 -- 2 files changed, 33 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 42b0c02..b069671 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -65,17 +65,6 @@ VIR_LOG_INIT("parallels.parallels_driver"); static int vzConnectClose(virConnectPtr conn); static virClassPtr vzConnClass; -void -vzDriverLock(vzConnPtr driver) -{ -virObjectLock(driver); -} - -void -vzDriverUnlock(vzConnPtr driver) -{ -virObjectUnlock(driver); -} static virMutex vz_driver_lock; static vzConnPtr vz_driver; @@ -222,9 +211,7 @@ vzConnectGetCapabilities(virConnectPtr conn ATTRIBUTE_UNUSED) return NULL; char *xml; -vzDriverLock(privconn); xml = virCapabilitiesFormatXML(privconn->caps); -vzDriverUnlock(privconn); virObjectUnref(privconn); return xml; } @@ -409,10 +396,8 @@ vzConnectListDomains(virConnectPtr conn ATTRIBUTE_UNUSED, int *ids, int maxids) if (!privconn) return -1; -vzDriverLock(privconn); n = virDomainObjListGetActiveIDs(privconn->domains, ids, maxids, NULL, NULL); -vzDriverUnlock(privconn); virObjectUnref(privconn); return n; @@ -426,10 +411,8 @@ vzConnectNumOfDomains(virConnectPtr conn ATTRIBUTE_UNUSED) if (!privconn) return -1; -vzDriverLock(privconn); count = virDomainObjListNumOfDomains(privconn->domains, true, NULL, NULL); -vzDriverUnlock(privconn); virObjectUnref(privconn); return count; @@ -445,11 +428,9 @@ vzConnectListDefinedDomains(virConnectPtr conn ATTRIBUTE_UNUSED, if (!privconn) return -1; -vzDriverLock(privconn); memset(names, 0, sizeof(*names) * maxnames); n = virDomainObjListGetInactiveNames(privconn->domains, names, maxnames, NULL, NULL); -vzDriverUnlock(privconn); virObjectUnref(privconn); return n; @@ -463,10 +444,8 @@ vzConnectNumOfDefinedDomains(virConnectPtr conn ATTRIBUTE_UNUSED) if (!privconn) return -1; -vzDriverLock(privconn); count = virDomainObjListNumOfDomains(privconn->domains, false, NULL, NULL); -vzDriverUnlock(privconn); virObjectUnref(privconn); return count; } @@ -482,10 +461,8 @@ vzConnectListAllDomains(virConnectPtr conn, return -1; virCheckFlags(VIR_CONNECT_LIST_DOMAINS_FILTERS_ALL, -1); -vzDriverLock(privconn); ret = virDomainObjListExport(privconn->domains, conn, domains, NULL, flags); -vzDriverUnlock(privconn); virObjectUnref(privconn); return ret; @@ -497,13 +474,11 @@ vzDomainLookupByID(virConnectPtr conn, int id) virDomainPtr ret = NULL; virDomainObjPtr dom; -vzDriverLock(privconn); vzConnPtr privconn = vzGetConnection(); if (!privconn) return NULL; dom = virDomainObjListFindByID(privconn->domains, id); -vzDriverUnlock(privconn); if (dom == NULL) { virReportError(VIR_ERR_NO_DOMAIN, NULL); @@ -527,13 +502,11 @@ vzDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) virDomainPtr ret = NULL; virDomainObjPtr dom; -vzDriverLock(privconn); vzConnPtr privconn = vzGetConnection(); if (!privconn) return NULL; dom = virDomainObjListFindByUUID(privconn->domains, uuid); -vzDriverUnlock(privconn); if (dom == NULL) { char uuidstr[VIR_UUID_STRING_BUFLEN]; @@ -563,9 +536,7 @@ vzDomainLookupByName(virConnectPtr conn, const char *name) virDomainPtr ret = NULL; virDomainObjPtr dom; -vzDriverLock(privconn); dom = virDomainObjListFindByName(privconn->domains, name); -vzDriverUnlock(privconn); if (dom == NULL) { virReportError(VIR_ERR_NO_DOMAIN, @@ -736,7 +707,6 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) if (flags & VIR_DOMAIN_DEFINE_VALIDATE) parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; -vzDriverLock(privconn); if ((def = virDomainDefParseString(xml, privconn->caps, privconn->xmlopt, parse_flags)) == NULL) goto cleanup; @@ -808,7 +778,6 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) virObjectUnlock(newdom); } virDomainDefFree(def); -vzDriverUnlock(privconn); virObjectUnref(privconn); return retdom; } diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h index 406ddd9..73b493f 100644 --- a/src/vz/vz_utils.h +++ b/src/vz/vz_utils.h @@ -102,8 +102,6 @@ virDomainObjPtr vzDomObjFromDomainRef(virDomainPtr domain); char * vzGetOutp
[libvirt] [PATCH 04/11] vz: build driver as module and don't register it on client's side
Make it possible to build vz driver as a module and don't link it with libvirt.so statically. Remove registering it on client's side as far as we start relying on daemon Signed-off-by: Maxim Nestratov --- daemon/Makefile.am | 4 daemon/libvirtd.c | 9 + src/Makefile.am| 21 - src/libvirt.c | 7 --- src/vz/vz_driver.c | 27 +-- 5 files changed, 54 insertions(+), 14 deletions(-) diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 2dbe81b..78d7d21 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -233,6 +233,10 @@ if WITH_VBOX libvirtd_LDADD += ../src/libvirt_driver_vbox.la endif WITH_VBOX +if WITH_VZ +libvirtd_LDADD += ../src/libvirt_driver_vz.la +endif WITH_VZ + if WITH_STORAGE libvirtd_LDADD += ../src/libvirt_driver_storage.la endif WITH_STORAGE diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 3d38a46..92b4080 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -102,6 +102,9 @@ # include "nwfilter/nwfilter_driver.h" # endif #endif +#ifdef WITH_VZ +# include "vz/vz_driver.h" +#endif #include "configmake.h" @@ -390,6 +393,9 @@ static void daemonInitialize(void) # ifdef WITH_BHYVE virDriverLoadModule("bhyve"); # endif +# ifdef WITH_VZ +virDriverLoadModule("vz"); +# endif #else # ifdef WITH_NETWORK networkRegister(); @@ -430,6 +436,9 @@ static void daemonInitialize(void) # ifdef WITH_BHYVE bhyveRegister(); # endif +# ifdef WITH_VZ +vzRegister(); +# endif #endif } diff --git a/src/Makefile.am b/src/Makefile.am index dad7bab..b26be1b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -629,6 +629,7 @@ DRIVER_SOURCE_FILES = \ $(NULL) STATEFUL_DRIVER_SOURCE_FILES = \ + $(VZ_DRIVER_SOURCES) \ $(BHYVE_DRIVER_SOURCES) \ $(INTERFACE_DRIVER_SOURCES) \ $(LIBXL_DRIVER_SOURCES) \ @@ -885,7 +886,9 @@ HYPERV_DRIVER_EXTRA_DIST = \ hyperv/hyperv_wmi_generator.py \ $(HYPERV_DRIVER_GENERATED) -VZ_DRIVER_SOURCES =\ +VZ_DRIVER_SOURCES =\ + datatypes.c \ + util/vircommand.c \ vz/vz_driver.h \ vz/vz_driver.c \ vz/vz_utils.c \ @@ -1478,13 +1481,21 @@ libvirt_driver_hyperv_la_SOURCES = $(HYPERV_DRIVER_SOURCES) endif WITH_HYPERV if WITH_VZ +noinst_LTLIBRARIES += libvirt_driver_vz_impl.la +libvirt_driver_vz_la_SOURCES = +libvirt_driver_vz_la_LIBADD = libvirt_driver_vz_impl.la +if WITH_DRIVER_MODULES +mod_LTLIBRARIES += libvirt_driver_vz.la +libvirt_driver_vz_la_LIBADD += ../gnulib/lib/libgnu.la +libvirt_driver_vz_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) +else ! WITH_DRIVER_MODULES noinst_LTLIBRARIES += libvirt_driver_vz.la -libvirt_la_BUILT_LIBADD += libvirt_driver_vz.la -libvirt_driver_vz_la_CFLAGS = \ +endif ! WITH_DRIVER_MODULES +libvirt_driver_vz_impl_la_CFLAGS = \ -I$(srcdir)/conf $(AM_CFLAGS) \ $(PARALLELS_SDK_CFLAGS) $(LIBNL_CFLAGS) -libvirt_driver_vz_la_LIBADD = $(PARALLELS_SDK_LIBS) $(LIBNL_LIBS) -libvirt_driver_vz_la_SOURCES = $(VZ_DRIVER_SOURCES) +libvirt_driver_vz_impl_la_SOURCES = $(VZ_DRIVER_SOURCES) +libvirt_driver_vz_impl_la_LIBADD = $(PARALLELS_SDK_LIBS) $(LIBNL_LIBS) endif WITH_VZ if WITH_BHYVE diff --git a/src/libvirt.c b/src/libvirt.c index dd58e9c..a21d00e 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -92,9 +92,6 @@ #ifdef WITH_XENAPI # include "xenapi/xenapi_driver.h" #endif -#ifdef WITH_VZ -# include "vz/vz_driver.h" -#endif #ifdef WITH_BHYVE # include "bhyve/bhyve_driver.h" #endif @@ -433,10 +430,6 @@ virGlobalInit(void) if (xenapiRegister() == -1) goto error; # endif -# ifdef WITH_VZ -if (vzRegister() == -1) -goto error; -# endif #endif #ifdef WITH_REMOTE if (remoteRegister() == -1) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 9de88cd..d3dcf3d 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -1561,6 +1561,26 @@ static virConnectDriver vzConnectDriver = { .hypervisorDriver = &vzDriver, }; +static int +vzStateCleanup(void) +{ +return 0; +} + +static int +vzStateInitialize(bool privileged ATTRIBUTE_UNUSED, + virStateInhibitCallback callback ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ +return 0; +} + +static virStateDriver vzStateDriver = { +.name = "vz", +.stateInitialize = vzStateInitialize, +.stateCleanup = vzStateCleanup, +}; + /* Parallels domain type backward compatibility*/ static virHypervisorDriver parallelsDriver; static virConnectDriver parallelsConnectDriver; @@ -1588,10 +1608,13 @@ vzRegister(void) parallelsDriver.name = "Parallels"; parallelsConnectDriv
[libvirt] [PATCH 06/11] vz: make vzConn structure be a new lockable object and use it
This patch introduces a new 'vzConn' lockable object and provides helper functions to allocate/destroy it. Now we store domain related objects such as domain list, capabitilies etc. within a single vz_driver vzConn structure, which is shared by all driver connections. It is allocated in a lazy manner in any function that is trying to acces it. When a connection to vz daemon drops, vzDestroyConnection is called, which prevents further usage of vz_driver until a new connection to it is established. Signed-off-by: Maxim Nestratov --- src/vz/vz_driver.c | 329 ++--- src/vz/vz_sdk.c| 9 +- src/vz/vz_utils.c | 13 ++- src/vz/vz_utils.h | 9 +- 4 files changed, 234 insertions(+), 126 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 7de21d8..36030a7 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -63,18 +63,24 @@ VIR_LOG_INIT("parallels.parallels_driver"); #define PRLCTL "prlctl" static int vzConnectClose(virConnectPtr conn); +static virClassPtr vzConnClass; void vzDriverLock(vzConnPtr driver) { -virMutexLock(&driver->lock); +virObjectLock(driver); } void vzDriverUnlock(vzConnPtr driver) { -virMutexUnlock(&driver->lock); +virObjectUnlock(driver); } +static virMutex vz_driver_lock; +static vzConnPtr vz_driver; + +static vzConnPtr +vzConnObjNew(void); static int vzCapsAddGuestDomain(virCapsPtr caps, @@ -158,15 +164,67 @@ vzBuildCapabilities(void) goto cleanup; } +static void vzConnDispose(void * obj) +{ +vzConnPtr conn = obj; + +if (conn->server) { +prlsdkUnsubscribeFromPCSEvents(conn); +prlsdkDisconnect(conn); +} + +virObjectUnref(conn->domains); +virObjectUnref(conn->caps); +virObjectUnref(conn->xmlopt); +virObjectEventStateFree(conn->domainEventState); +} + +static int vzConnOnceInit(void) +{ +if (!(vzConnClass = virClassNew(virClassForObjectLockable(), +"vzConn", +sizeof(vzConn), +vzConnDispose))) +return -1; + +return 0; +} +VIR_ONCE_GLOBAL_INIT(vzConn) + +vzConnPtr +vzGetConnection(void) +{ +virMutexLock(&vz_driver_lock); +if (!vz_driver) +vz_driver = vzConnObjNew(); +virObjectRef(vz_driver); +virMutexUnlock(&vz_driver_lock); +return vz_driver; +} + +void +vzDestroyConnection(void) +{ +vzConnPtr privconn; +virMutexLock(&vz_driver_lock); +privconn = vz_driver; +vz_driver = NULL; +virMutexUnlock(&vz_driver_lock); +virObjectUnref(privconn); +} + static char * -vzConnectGetCapabilities(virConnectPtr conn) +vzConnectGetCapabilities(virConnectPtr conn ATTRIBUTE_UNUSED) { -vzConnPtr privconn = conn->privateData; +vzConnPtr privconn = vzGetConnection(); +if (!privconn) +return NULL; char *xml; vzDriverLock(privconn); xml = virCapabilitiesFormatXML(privconn->caps); vzDriverUnlock(privconn); +virObjectUnref(privconn); return xml; } @@ -214,78 +272,42 @@ virDomainDefParserConfig vzDomainDefParserConfig = { .domainPostParseCallback = vzDomainDefPostParse, }; - -static int -vzOpenDefault(virConnectPtr conn) +static vzConnPtr +vzConnObjNew(void) { -vzConnPtr privconn; - -if (VIR_ALLOC(privconn) < 0) -return VIR_DRV_OPEN_ERROR; -if (virMutexInit(&privconn->lock) < 0) { -virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot initialize mutex")); -goto err_free; -} - -if (prlsdkInit()) { -VIR_DEBUG("%s", _("Can't initialize Parallels SDK")); -goto err_free; -} - -if (prlsdkConnect(privconn) < 0) -goto err_free; - -if (vzInitVersion(privconn) < 0) -goto error; - -if (!(privconn->caps = vzBuildCapabilities())) -goto error; +vzConnPtr conn; -vzDomainDefParserConfig.priv = &privconn->vzCaps; -if (!(privconn->xmlopt = virDomainXMLOptionNew(&vzDomainDefParserConfig, - NULL, NULL))) -goto error; - -if (!(privconn->domains = virDomainObjListNew())) -goto error; - -if (!(privconn->domainEventState = virObjectEventStateNew())) -goto error; - -if (prlsdkSubscribeToPCSEvents(privconn)) -goto error; - -if (!(privconn->closeCallback = virNewConnectCloseCallbackData())) -goto error; - -conn->privateData = privconn; +if (vzConnInitialize() < 0) +return NULL; -if (prlsdkLoadDomains(privconn)) -goto error; +if (!(conn = virObjectLockableNew(vzConnClass))) +return NULL; -return VIR_DRV_OPEN_SUCCESS; +vzDomainDefParserConfig.priv = &conn->vzCaps; + +if (!(conn->caps = vzBuildCapabilities()) || +!(conn->xmlopt = virDomainXMLOptionNew(&vzDomainDefParserConfig, +
[libvirt] [PATCH 10/11] vz: minor cleanup
remove unnecessary vzConnectClose prototype and make local structure vzDomainDefParserConfig be static Signed-off-by: Maxim Nestratov --- src/vz/vz_driver.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index b069671..83bdabe 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -62,7 +62,6 @@ VIR_LOG_INIT("parallels.parallels_driver"); #define PRLCTL "prlctl" -static int vzConnectClose(virConnectPtr conn); static virClassPtr vzConnClass; static virMutex vz_driver_lock; @@ -254,7 +253,7 @@ vzDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, } -virDomainDefParserConfig vzDomainDefParserConfig = { +static virDomainDefParserConfig vzDomainDefParserConfig = { .macPrefix = {0x42, 0x1C, 0x00}, .devicesPostParseCallback = vzDomainDeviceDefPostParse, .domainPostParseCallback = vzDomainDefPostParse, -- 2.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 05/11] vz: pass vzConnPtr to prlsdkXxx functions instead of virConnectPtr
Signed-off-by: Maxim Nestratov --- src/vz/vz_driver.c | 6 +++--- src/vz/vz_sdk.c| 25 +++-- src/vz/vz_sdk.h| 6 +++--- 3 files changed, 17 insertions(+), 20 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index d3dcf3d..7de21d8 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -679,10 +679,10 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) if (!newdom) goto cleanup; if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) { -if (prlsdkCreateVm(conn, def)) +if (prlsdkCreateVm(privconn, def)) goto cleanup; } else if (def->os.type == VIR_DOMAIN_OSTYPE_EXE) { -if (prlsdkCreateCt(conn, def)) +if (prlsdkCreateCt(privconn, def)) goto cleanup; } else { virReportError(VIR_ERR_INVALID_ARG, @@ -717,7 +717,7 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) goto cleanup; } } else { -if (prlsdkApplyConfig(conn, olddom, def)) +if (prlsdkApplyConfig(privconn, olddom, def)) goto cleanup; if (prlsdkUpdateDomain(privconn, olddom)) diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 629906e..541060a 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -3600,7 +3600,7 @@ prlsdkSetBootOrderVm(PRL_HANDLE sdkdom, virDomainDefPtr def) } static int -prlsdkDoApplyConfig(virConnectPtr conn, +prlsdkDoApplyConfig(vzConnPtr privconn, PRL_HANDLE sdkdom, virDomainDefPtr def, virDomainDefPtr olddef) @@ -3666,11 +3666,11 @@ prlsdkDoApplyConfig(virConnectPtr conn, if (olddef) { for (i = 0; i < olddef->nnets; i++) -prlsdkCleanupBridgedNet(conn->privateData, olddef->nets[i]); +prlsdkCleanupBridgedNet(privconn, olddef->nets[i]); } for (i = 0; i < def->nnets; i++) { -if (prlsdkAddNet(conn->privateData, sdkdom, def->nets[i], IS_CT(def)) < 0) +if (prlsdkAddNet(privconn, sdkdom, def->nets[i], IS_CT(def)) < 0) goto error; } @@ -3691,7 +3691,7 @@ prlsdkDoApplyConfig(virConnectPtr conn, } for (i = 0; i < def->ndisks; i++) { -if (prlsdkAddDisk(conn->privateData, sdkdom, def->disks[i]) < 0) +if (prlsdkAddDisk(privconn, sdkdom, def->disks[i]) < 0) goto error; } @@ -3709,17 +3709,16 @@ prlsdkDoApplyConfig(virConnectPtr conn, VIR_FREE(mask); for (i = 0; i < def->nnets; i++) -prlsdkCleanupBridgedNet(conn->privateData, def->nets[i]); +prlsdkCleanupBridgedNet(privconn, def->nets[i]); return -1; } int -prlsdkApplyConfig(virConnectPtr conn, +prlsdkApplyConfig(vzConnPtr privconn, virDomainObjPtr dom, virDomainDefPtr new) { -vzConnPtr privconn = conn->privateData; PRL_HANDLE sdkdom = PRL_INVALID_HANDLE; PRL_HANDLE job = PRL_INVALID_HANDLE; int ret; @@ -3732,7 +3731,7 @@ prlsdkApplyConfig(virConnectPtr conn, if (PRL_FAILED(waitJob(job))) return -1; -ret = prlsdkDoApplyConfig(conn, sdkdom, new, dom->def); +ret = prlsdkDoApplyConfig(privconn, sdkdom, new, dom->def); if (ret == 0) { job = PrlVm_CommitEx(sdkdom, PVCF_DETACH_HDD_BUNDLE); @@ -3746,9 +3745,8 @@ prlsdkApplyConfig(virConnectPtr conn, } int -prlsdkCreateVm(virConnectPtr conn, virDomainDefPtr def) +prlsdkCreateVm(vzConnPtr privconn, virDomainDefPtr def) { -vzConnPtr privconn = conn->privateData; PRL_HANDLE sdkdom = PRL_INVALID_HANDLE; PRL_HANDLE job = PRL_INVALID_HANDLE; PRL_HANDLE result = PRL_INVALID_HANDLE; @@ -3772,7 +3770,7 @@ prlsdkCreateVm(virConnectPtr conn, virDomainDefPtr def) pret = PrlVmCfg_SetOfflineManagementEnabled(sdkdom, 0); prlsdkCheckRetGoto(pret, cleanup); -ret = prlsdkDoApplyConfig(conn, sdkdom, def, NULL); +ret = prlsdkDoApplyConfig(privconn, sdkdom, def, NULL); if (ret) goto cleanup; @@ -3786,9 +3784,8 @@ prlsdkCreateVm(virConnectPtr conn, virDomainDefPtr def) } int -prlsdkCreateCt(virConnectPtr conn, virDomainDefPtr def) +prlsdkCreateCt(vzConnPtr privconn, virDomainDefPtr def) { -vzConnPtr privconn = conn->privateData; PRL_HANDLE sdkdom = PRL_INVALID_HANDLE; PRL_GET_VM_CONFIG_PARAM_DATA confParam; PRL_HANDLE job = PRL_INVALID_HANDLE; @@ -3834,7 +3831,7 @@ prlsdkCreateCt(virConnectPtr conn, virDomainDefPtr def) } -ret = prlsdkDoApplyConfig(conn, sdkdom, def, NULL); +ret = prlsdkDoApplyConfig(privconn, sdkdom, def, NULL); if (ret) goto cleanup; diff --git a/src/vz/vz_sdk.h b/src/vz/vz_sdk.h index 2f11d4f..a1b2e52 100644 --- a/src/vz/vz_sdk.h +++ b/src/vz/vz_sdk.h @@ -53,11 +53,11 @@ prlsdkDomainChangeStateLocked(vzConnPtr privconn, virDomainObjPtr dom,
[libvirt] [PATCH 11/11] vz: change vzConnectIsAlive behavior
Now it detects if a connection to vz dispatcher is up or not Signed-off-by: Maxim Nestratov --- src/vz/vz_driver.c | 5 + 1 file changed, 5 insertions(+) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 83bdabe..8abd49f 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -809,6 +809,11 @@ static int vzConnectIsSecure(virConnectPtr conn ATTRIBUTE_UNUSED) static int vzConnectIsAlive(virConnectPtr conn ATTRIBUTE_UNUSED) { +vzConnPtr privconn = vzGetConnection(); +if (!privconn) +return 0; + +virObjectUnref(privconn); return 1; } -- 2.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 08/11] vz: remove close callback implementations
These methods are not necessary because vz becomes a stateful driver and clients always work via daemon Signed-off-by: Maxim Nestratov --- src/vz/vz_driver.c | 52 src/vz/vz_sdk.c| 2 -- src/vz/vz_utils.h | 2 -- 3 files changed, 56 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 4c30305..42b0c02 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -1537,56 +1537,6 @@ vzNodeGetFreeMemory(virConnectPtr conn ATTRIBUTE_UNUSED) return freeMem; } -static int -vzConnectRegisterCloseCallback(virConnectPtr conn, - virConnectCloseFunc cb, - void *opaque, - virFreeCallback freecb) -{ -vzConnPtr privconn = conn->privateData; -int ret = -1; - -vzDriverLock(privconn); - -if (virConnectCloseCallbackDataGetCallback(privconn->closeCallback) != NULL) { -virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("A close callback is already registered")); -goto cleanup; -} - -virConnectCloseCallbackDataRegister(privconn->closeCallback, conn, cb, -opaque, freecb); -ret = 0; - - cleanup: -vzDriverUnlock(privconn); - -return ret; -} - -static int -vzConnectUnregisterCloseCallback(virConnectPtr conn, virConnectCloseFunc cb) -{ -vzConnPtr privconn = conn->privateData; -int ret = -1; - -vzDriverLock(privconn); - -if (virConnectCloseCallbackDataGetCallback(privconn->closeCallback) != cb) { -virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("A different callback was requested")); -goto cleanup; -} - -virConnectCloseCallbackDataUnregister(privconn->closeCallback, cb); -ret = 0; - - cleanup: -vzDriverUnlock(privconn); - -return ret; -} - static virHypervisorDriver vzDriver = { .name = "vz", .connectOpen = vzConnectOpen,/* 0.10.0 */ @@ -1650,8 +1600,6 @@ static virHypervisorDriver vzDriver = { .domainBlockStatsFlags = vzDomainBlockStatsFlags, /* 1.2.17 */ .domainInterfaceStats = vzDomainInterfaceStats, /* 1.2.17 */ .domainMemoryStats = vzDomainMemoryStats, /* 1.2.17 */ -.connectRegisterCloseCallback = vzConnectRegisterCloseCallback, /* 1.3.2 */ -.connectUnregisterCloseCallback = vzConnectUnregisterCloseCallback, /* 1.3.2 */ }; static virConnectDriver vzConnectDriver = { diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index e6f6129..59bd89a 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1956,8 +1956,6 @@ prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR opaque) prlEvent = PRL_INVALID_HANDLE; break; case PET_DSP_EVT_DISP_CONNECTION_CLOSED: -virConnectCloseCallbackDataCall(privconn->closeCallback, -VIR_CONNECT_CLOSE_REASON_EOF); vzDestroyConnection(); break; default: diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h index a2b86c7..406ddd9 100644 --- a/src/vz/vz_utils.h +++ b/src/vz/vz_utils.h @@ -70,8 +70,6 @@ struct _vzConn { virCapsPtr caps; virDomainXMLOptionPtr xmlopt; virObjectEventStatePtr domainEventState; -/* Immutable pointer, self-locking APIs */ -virConnectCloseCallbackDataPtr closeCallback; virSysinfoDefPtr hostsysinfo; unsigned long vzVersion; vzCapabilities vzCaps; -- 2.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] qemu: Create domain master key
On Thu, Mar 24, 2016 at 01:53:20PM -0400, John Ferlan wrote: > Add a masterKey to _qemuDomainObjPrivate to store a random domain master > key in order to support the ability to encrypt/decrypt sensitive data > shared between libvirt and qemu. The key will be base64 encoded and written > to a file to be used by the command line building code to share with qemu. > > New API's from this patch: > > qemuDomainGetMasterKeyFilePath: > Return a path to where the key is located > > qemuDomainWriteMasterKeyFile: (private) > Base64 the masterKey and write to the *KeyFilePath > > qemuDomainMasterKeyReadFile: > Using the *KeyFilePath, open/read the file, base64 decode, and > store in masterKey. Expected use only from qemuProcessReconnect > > qemuDomainGenerateRandomKey: (private) > Generate a random key using available algorithms > > The key is generated either from the gnutls_rnd function if it > exists or a less cryptographically strong mechanisum as a series of 8 > bit random numbers from virRandomBits stored as a byte string. > > qemuDomainMasterKeyRemove: > Remove traces of the master key, remove the *KeyFilePath > > qemuDomainMasterKeyCreate: > Generate the key and save a base64 encoded value of the key > in the location returned by qemuDomainGetMasterKeyFilePath. > > This API will first ensure the QEMU_CAPS_OBJECT_SECRET is set > in the capabilities. If not, then there's no need to generate > the secret or file. > > The creation of the key will be attempted from qemuProcessPrepareHost > once the libDir directory structure exists. > > The removal of the key will handled from qemuProcessStop just prior > to deleting the libDir tree. > > Since the key will not be written out to the domain object XML file, > the qemuProcessReconnect will read the saved, decode, and restore > the masterKey value after a series of checks. > > Signed-off-by: John Ferlan > --- > src/qemu/qemu_domain.c | 269 > > src/qemu/qemu_domain.h | 13 +++ > src/qemu/qemu_process.c | 11 ++ > 3 files changed, 293 insertions(+) > > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index 53cb2b6..fecf668 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -44,6 +44,12 @@ > #include "virthreadjob.h" > #include "viratomic.h" > #include "virprocess.h" > +#include "virrandom.h" > +#include "base64.h" > +#include > +#if HAVE_GNUTLS_CRYPTO_H > +# include > +#endif > #include "logging/log_manager.h" > > #include "storage/storage_driver.h" > @@ -465,6 +471,269 @@ qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo, > } > > > +/* qemuDomainGetMasterKeyFilePath: > + * @libDir: Directory path to domain lib files > + * > + * This API will generate a path of the domain's libDir (e.g. > + * (/var/lib/libvirt/qemu/domain-#-$NAME/) and a name 'master.key'. > + * > + * This API will check and fail if the libDir is NULL as that results > + * in an invalid path generated. > + * > + * This API does not check if the resulting path exists, that is left > + * up to the caller. > + * > + * Returns path to memory containing the name of the file. It is up to the > + * caller to free; otherwise, NULL on failure. > + */ > +char * > +qemuDomainGetMasterKeyFilePath(const char *libDir) > +{ > +if (!libDir) { > +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("invalid path for master key file")); > +return NULL; > +} > +return virFileBuildPath(libDir, "master.key", NULL); > +} How about calling this master-key.aes to make it explicit what this key is intended to be used with. > +/* qemuDomainWriteMasterKeyFile: > + * @priv: pointer to domain private object > + * > + * Get the desired path to the masterKey file, base64 encode the masterKey, > + * and store it in the file. The QEMU secrets code is happy to get data in either raw or base64 format. I wonder if there's a compelling reason to use base64 instead of just writing it out as raw bytes. > +static int > +qemuDomainWriteMasterKeyFile(qemuDomainObjPrivatePtr priv) > +{ > +char *path; > +char *base64Key = NULL; > +int ret = -1; > + > +if (!(path = qemuDomainGetMasterKeyFilePath(priv->libDir))) > +return -1; > + > +/* base64 encode the key to store it */ > +base64_encode_alloc(priv->masterKey, QEMU_DOMAIN_MASTER_KEY_LEN, > +&base64Key); > +if (!base64Key) { > +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("failed to encode master key")); > +goto cleanup; > +} > + > +if (virFileWriteStr(path, base64Key, 0600) < 0) { > +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("failed to write master key file for domain")); > +goto cleanup; > +} > + > +ret = 0; > + > + cleanup: > +if (base64Key) { > + /* clear heap, free memory */ >
Re: [libvirt] [PATCH 3/3] qemu: Introduce qemuBuildMasterKeyCommandLine
On Thu, Mar 24, 2016 at 01:53:21PM -0400, John Ferlan wrote: > If the -object secret capability exists, then get the path to the > base64 encoded masterKey file and provide that to qemu. Checking > for the existence of the file before passing to qemu could be done, > but causes issues in mock test environment. > > Since the qemuDomainObjPrivate is not available when building the > command line, the qemuBuildHasMasterKey API will have to suffice > as the primary arbiter for whether the capability exists in order > to find/return the path to the master key for usage. > > Created the qemuDomainGetMasterKeyAlias API which will be used by > later patches to define the 'keyid' (eg, masterKey) to be used by > other secrets to provide the id to qemu for the master key. > > Signed-off-by: John Ferlan > --- > src/qemu/qemu_alias.c | 17 ++ > src/qemu/qemu_alias.h | 3 + > src/qemu/qemu_command.c| 68 > ++ > .../qemuxml2argvdata/qemuxml2argv-master-key.args | 23 > tests/qemuxml2argvdata/qemuxml2argv-master-key.xml | 30 ++ > tests/qemuxml2argvtest.c | 2 + > 6 files changed, 143 insertions(+) > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-master-key.args > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-master-key.xml > > diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c > index efd9222..b57b967 100644 > --- a/src/qemu/qemu_alias.c > +++ b/src/qemu/qemu_alias.c > @@ -484,3 +484,20 @@ qemuAssignDeviceAliases(virDomainDefPtr def, > virQEMUCapsPtr qemuCaps) > > return 0; > } > + > + > +/* qemuDomainGetMasterKeyAlias: > + * > + * Generate and return the masterKey alias > + * > + * Returns NULL or a string containing the master key alias > + */ > +char * > +qemuDomainGetMasterKeyAlias(void) > +{ > +char *alias; > + > +ignore_value(VIR_STRDUP(alias, "masterKey0")); > + > +return alias; > +} > diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h > index a2eaa27..299a6d4 100644 > --- a/src/qemu/qemu_alias.h > +++ b/src/qemu/qemu_alias.h > @@ -61,4 +61,7 @@ int qemuAssignDeviceAliases(virDomainDefPtr def, > virQEMUCapsPtr qemuCaps); > > int qemuDomainDeviceAliasIndex(const virDomainDeviceInfo *info, > const char *prefix); > + > +char *qemuDomainGetMasterKeyAlias(void); > + > #endif /* __QEMU_ALIAS_H__*/ > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index 0331789..2b1dc93 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -151,6 +151,71 @@ VIR_ENUM_IMPL(qemuNumaPolicy, > VIR_DOMAIN_NUMATUNE_MEM_LAST, >"interleave"); > > /** > + * qemuBuildHasMasterKey: > + * @qemuCaps: QEMU binary capabilities > + * > + * Return true if this binary supports the secret -object, false otherwise. > + */ > +static bool > +qemuBuildHasMasterKey(virQEMUCapsPtr qemuCaps) > +{ > +return virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_SECRET); > +} > + > + > +/** > + * qemuBuildMasterKeyCommandLine: > + * @cmd: the command to modify > + * @qemuCaps qemu capabilities object > + * @domainLibDir: location to find the master key > + > + * Formats the command line for a master key if available > + * > + * Returns 0 on success, -1 w/ error message on failure > + */ > +static int > +qemuBuildMasterKeyCommandLine(virCommandPtr cmd, > + virQEMUCapsPtr qemuCaps, > + const char *domainLibDir) > +{ > +int ret = -1; > +char *alias = NULL; > +char *path = NULL; > + > +/* If the -object secret does not exist, then just return. This just > + * means the domain won't be able to use a secret master key and is > + * not a failure. > + */ > +if (!qemuBuildHasMasterKey(qemuCaps)) { > +VIR_INFO("secret object is not supported by this QEMU binary"); > +return 0; > +} > + > +if (!(alias = qemuDomainGetMasterKeyAlias())) > +return -1; > + > +/* Get the path. NB, the mocked test will not have the created > + * file so we cannot check for existence, which is no different > + * than other command line options which do not check for the > + * existence of socket files before using. > + */ > +if (!(path = qemuDomainGetMasterKeyFilePath(domainLibDir))) > +goto cleanup; > + > +virCommandAddArg(cmd, "-object"); > +virCommandAddArgFormat(cmd, "secret,id=%s,format=base64,file=%s", Reference my question in previous patch about whether we should just use format=raw instead of base64 > + alias, path); > + > +ret = 0; > + > + cleanup: > +VIR_FREE(alias); > +VIR_FREE(path); > +return ret; > +} ACK in general though Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://vir
Re: [libvirt] [PATCH] qemu: tolerate panic device on S390
On Wed, Mar 23, 2016 at 02:56:03PM +0100, Boris Fiuczynski wrote: > If a panic device is being defined without a model in a domain > the default value is always overwritten with model ISA. An ISA > bus does not exist on S390 and therefore specifying a panic device > results in an unsupported configuration. > Since the s390 architecture inherently provides a crash detection > capability the panic device should be tolerated in the domain xml. > > This patch achieves the toleration by setting the model to the > default value in case the architecture is S390. > > Signed-off-by: Boris Fiuczynski > Reviewed-by: Cornelia Huck > --- > src/qemu/qemu_domain.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index 9f9fae3..96561c0 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -1605,6 +1605,9 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, > if (ARCH_IS_PPC64(def->os.arch) && > STRPREFIX(def->os.machine, "pseries")) > dev->data.panic->model = VIR_DOMAIN_PANIC_MODEL_PSERIES; > +else if (ARCH_IS_S390(def->os.arch)) > +/* since S390 does not support ISA use default for toleration */ > +dev->data.panic->model = VIR_DOMAIN_PANIC_MODEL_DEFAULT; > else > dev->data.panic->model = VIR_DOMAIN_PANIC_MODEL_ISA; > } I rather think we should define an explicit model for the s390 panic hardware, as we did for ppc64 Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] qemu: Create domain master key
On Thu, Mar 24, 2016 at 13:53:20 -0400, John Ferlan wrote: > Add a masterKey to _qemuDomainObjPrivate to store a random domain master > key in order to support the ability to encrypt/decrypt sensitive data > shared between libvirt and qemu. The key will be base64 encoded and written > to a file to be used by the command line building code to share with qemu. > > New API's from this patch: > > qemuDomainGetMasterKeyFilePath: > Return a path to where the key is located > > qemuDomainWriteMasterKeyFile: (private) > Base64 the masterKey and write to the *KeyFilePath > > qemuDomainMasterKeyReadFile: > Using the *KeyFilePath, open/read the file, base64 decode, and > store in masterKey. Expected use only from qemuProcessReconnect > > qemuDomainGenerateRandomKey: (private) > Generate a random key using available algorithms > > The key is generated either from the gnutls_rnd function if it > exists or a less cryptographically strong mechanisum as a series of 8 > bit random numbers from virRandomBits stored as a byte string. > > qemuDomainMasterKeyRemove: > Remove traces of the master key, remove the *KeyFilePath > > qemuDomainMasterKeyCreate: > Generate the key and save a base64 encoded value of the key > in the location returned by qemuDomainGetMasterKeyFilePath. > > This API will first ensure the QEMU_CAPS_OBJECT_SECRET is set > in the capabilities. If not, then there's no need to generate > the secret or file. > > The creation of the key will be attempted from qemuProcessPrepareHost > once the libDir directory structure exists. > > The removal of the key will handled from qemuProcessStop just prior > to deleting the libDir tree. > > Since the key will not be written out to the domain object XML file, > the qemuProcessReconnect will read the saved, decode, and restore > the masterKey value after a series of checks. > > Signed-off-by: John Ferlan > --- > src/qemu/qemu_domain.c | 269 > > src/qemu/qemu_domain.h | 13 +++ > src/qemu/qemu_process.c | 11 ++ > 3 files changed, 293 insertions(+) > > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index 53cb2b6..fecf668 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c [...] > @@ -465,6 +471,269 @@ qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo, > } > > > +/* qemuDomainGetMasterKeyFilePath: > + * @libDir: Directory path to domain lib files > + * > + * This API will generate a path of the domain's libDir (e.g. > + * (/var/lib/libvirt/qemu/domain-#-$NAME/) and a name 'master.key'. > + * > + * This API will check and fail if the libDir is NULL as that results > + * in an invalid path generated. > + * > + * This API does not check if the resulting path exists, that is left > + * up to the caller. > + * > + * Returns path to memory containing the name of the file. It is up to the > + * caller to free; otherwise, NULL on failure. Whoah, docs longer than the code itself. > + */ > +char * > +qemuDomainGetMasterKeyFilePath(const char *libDir) > +{ > +if (!libDir) { > +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("invalid path for master key file")); > +return NULL; And the code is rather self-explanatory. > +} > +return virFileBuildPath(libDir, "master.key", NULL); > +} > + > + > +/* qemuDomainWriteMasterKeyFile: > + * @priv: pointer to domain private object > + * > + * Get the desired path to the masterKey file, base64 encode the masterKey, > + * and store it in the file. > + * > + * Returns 0 on success, -1 on failure with error message indicating failure > + */ > +static int > +qemuDomainWriteMasterKeyFile(qemuDomainObjPrivatePtr priv) > +{ > +char *path; > +char *base64Key = NULL; > +int ret = -1; > + > +if (!(path = qemuDomainGetMasterKeyFilePath(priv->libDir))) > +return -1; > + > +/* base64 encode the key to store it */ > +base64_encode_alloc(priv->masterKey, QEMU_DOMAIN_MASTER_KEY_LEN, > +&base64Key); > +if (!base64Key) { > +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("failed to encode master key")); > +goto cleanup; > +} > + > +if (virFileWriteStr(path, base64Key, 0600) < 0) { > +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("failed to write master key file for domain")); > +goto cleanup; > +} > + > +ret = 0; > + > + cleanup: > +if (base64Key) { > + /* clear heap, free memory */ > +memset(base64Key, 0, strlen(base64Key)); > +VIR_FREE(base64Key); > +} > + > +VIR_FREE(path); > + > +return ret; > +} > + > + > +/* qemuDomainMasterKeyReadFile: > + * @priv: pointer to domain private object > + * > + * Expected to be called during qemuProcessReconnect once the domain > + * libDir has been generated through qemuStateInitialize calling >
Re: [libvirt] [PATCH 2/2] bhyve: fix invalid hostsysinfo freeing
29.03.2016 15:29, Daniel P. Berrange пишет: On Tue, Mar 29, 2016 at 03:20:56PM +0300, Maxim Nestratov wrote: --- src/bhyve/bhyve_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 4d341b5..5526bb0 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -1172,7 +1172,7 @@ bhyveStateCleanup(void) virObjectUnref(bhyve_driver->domains); virObjectUnref(bhyve_driver->caps); virObjectUnref(bhyve_driver->xmlopt); -virObjectUnref(bhyve_driver->hostsysinfo); +virSysinfoDefFree(bhyve_driver->hostsysinfo); virObjectUnref(bhyve_driver->closeCallbacks); virObjectEventStateFree(bhyve_driver->domainEventState); ACK Regards, Daniel Thanks, pushed. Maxim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tools: virt-host-validate: fix KVM check on s390
On Mon, Mar 28, 2016 at 02:45:18PM -0400, John Ferlan wrote: > > > On 03/21/2016 07:55 AM, Bjoern Walk wrote: > > Since kernel version 4.5, s390 has the 'sie' flag to declare its > > hardware virtualization support. Let's make virt-host-validate aware so > > this check is passed correctly. > > > > Signed-off-by: Bjoern Walk > > Reviewed-by: Boris Fiuczynski > > --- > > tools/virt-host-validate-qemu.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > I've never seen the output of /proc/cpuinfo on an S390, but looking at > the source code I found it does seem to print with the "features" for a > cpuinfo on an s390 machine (as opposed the "flags" for a vmx/svm machine). > > The virHostValidateHasCPUFlag doesn't really distinguish "flags" or > "features" field being specified as a label (although I imagine that > could have internationalization impact if we tried to limit that more). > > Anyway, before I push this - I wanted to see if there were any "other" > thoughts regarding letting this be a bit more generic. My one concern is > some field name some day has "sie" added (e.g. "easier" or "transient" > would match). > > One change that may help with that would be to check "sie " instead of > "sie" (similarly "svm" could be "svm " and "vmx" could be "vmx "). The > source code prints the output as "..."%s ", int_hwcap_str[i]..." Yes this is a good point, but also its a pre-existing problem with the virHostValidateHasCPUFlag method, so not a reason to block this patch. It can be fixed separately as desired. > > diff --git a/tools/virt-host-validate-qemu.c > > b/tools/virt-host-validate-qemu.c > > index a9f6c1e..01fb24b 100644 > > --- a/tools/virt-host-validate-qemu.c > > +++ b/tools/virt-host-validate-qemu.c > > @@ -31,7 +31,8 @@ int virHostValidateQEMU(void) > > > > virHostMsgCheck("QEMU", "%s", ("for hardware virtualization")); > > if (virHostValidateHasCPUFlag("svm") || > > -virHostValidateHasCPUFlag("vmx")) { > > +virHostValidateHasCPUFlag("vmx") || > > +virHostValidateHasCPUFlag("sie")) { > > virHostMsgPass(); > > if (virHostValidateDeviceExists("QEMU", "/dev/kvm", > > VIR_HOST_VALIDATE_FAIL, > > What we should do here though is check the host architecture before checking CPU flags. eg you'll want to make this code do something like this bool hasHWVirt = false; switch (virArchFromHost()) { case VIR_ARCH_I686: case VIR_ARCH_X86_64: if (virHostValidateHasCPUFlag("svm") || virHostValidateHasCPUFlag("vmx")) hasHWVirt = true; break; case VIR_ARCH_S390: case VIR_ARCH_S390X: if (virHostValidateHasCPUFlag("sie")) hasHWVirt = true; } if (hasHWVirt) { do the /dev/kvm check... } Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] qemu_process: add check for hyperv features
Commit 7068b56c introduced several hyperv features. Not all hyperv features are supported by old enough kernels and we shouldn't allow to start a guest if kernel doesn't support any of the hyperv feature. There is one exception, for backward compatibility we cannot error out if one of the RELAXED, VAPIC or SPINLOCKS isn't supported, for the same reason we ignore invtsc, to not break restoring saved domains with older libvirt. Signed-off-by: Pavel Hrdina --- src/cpu/cpu_x86.c | 8 src/cpu/cpu_x86_data.h | 8 src/qemu/qemu_process.c | 31 +++ 3 files changed, 47 insertions(+) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 90949f6..b7f1690 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -75,6 +75,14 @@ static const struct x86_kvm_feature x86_kvm_features[] = {VIR_CPU_x86_KVM_PV_UNHALT,{ .function = 0x4001, .eax = 0x0080 }}, {VIR_CPU_x86_KVM_CLOCKSOURCE_STABLE_BIT, { .function = 0x4001, .eax = 0x0100 }}, +{VIR_CPU_x86_KVM_HV_RUNTIME, { .function = 0x4003, .eax = 0x0001 }}, +{VIR_CPU_x86_KVM_HV_SYNIC, { .function = 0x4003, .eax = 0x0004 }}, +{VIR_CPU_x86_KVM_HV_STIMER,{ .function = 0x4003, .eax = 0x0008 }}, +{VIR_CPU_x86_KVM_HV_RELAXED, { .function = 0x4003, .eax = 0x0020 }}, +{VIR_CPU_x86_KVM_HV_SPINLOCK, { .function = 0x4003, .eax = 0x0022 }}, +{VIR_CPU_x86_KVM_HV_VAPIC, { .function = 0x4003, .eax = 0x0030 }}, +{VIR_CPU_x86_KVM_HV_VPINDEX, { .function = 0x4003, .eax = 0x0040 }}, +{VIR_CPU_x86_KVM_HV_RESET, { .function = 0x4003, .eax = 0x0080 }}, }; struct x86_model { diff --git a/src/cpu/cpu_x86_data.h b/src/cpu/cpu_x86_data.h index 88dccf6..777cc8d 100644 --- a/src/cpu/cpu_x86_data.h +++ b/src/cpu/cpu_x86_data.h @@ -48,6 +48,14 @@ struct _virCPUx86CPUID { # define VIR_CPU_x86_KVM_PV_EOI "__kvm_pv_eoi" # define VIR_CPU_x86_KVM_PV_UNHALT"__kvm_pv_unhalt" # define VIR_CPU_x86_KVM_CLOCKSOURCE_STABLE_BIT "__kvm_clocksource_stable" +# define VIR_CPU_x86_KVM_HV_RUNTIME "__kvm_hv_runtime" +# define VIR_CPU_x86_KVM_HV_SYNIC "__kvm_hv_synic" +# define VIR_CPU_x86_KVM_HV_STIMER"__kvm_hv_stimer" +# define VIR_CPU_x86_KVM_HV_RELAXED "__kvm_hv_relaxed" +# define VIR_CPU_x86_KVM_HV_SPINLOCK "__kvm_hv_spinlock" +# define VIR_CPU_x86_KVM_HV_VAPIC "__kvm_hv_vapic" +# define VIR_CPU_x86_KVM_HV_VPINDEX "__kvm_hv_vpindex" +# define VIR_CPU_x86_KVM_HV_RESET "__kvm_hv_reset" typedef struct _virCPUx86Data virCPUx86Data; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 9334a75..07b9df2 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3928,6 +3928,37 @@ qemuProcessVerifyGuestCPU(virQEMUDriverPtr driver, } } +for (i = 0; i < VIR_DOMAIN_HYPERV_LAST; i++) { +if (def->hyperv_features[i] == VIR_TRISTATE_SWITCH_ON) { +char *cpuFeature; +if (virAsprintf(&cpuFeature, "__kvm_hv_%s", +virDomainHypervTypeToString(i)) < 0) +goto cleanup; +if (!cpuHasFeature(guestcpu, cpuFeature)) { +switch ((virDomainHyperv) i) { +case VIR_DOMAIN_HYPERV_RELAXED: +case VIR_DOMAIN_HYPERV_VAPIC: +case VIR_DOMAIN_HYPERV_SPINLOCKS: +VIR_WARN("host doesn't support hyperv '%s' feature", + virDomainHypervTypeToString(i)); +break; +case VIR_DOMAIN_HYPERV_VPINDEX: +case VIR_DOMAIN_HYPERV_RUNTIME: +case VIR_DOMAIN_HYPERV_SYNIC: +case VIR_DOMAIN_HYPERV_STIMER: +case VIR_DOMAIN_HYPERV_RESET: +case VIR_DOMAIN_HYPERV_VENDOR_ID: +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("host doesn't support hyperv '%s' feature"), + virDomainHypervTypeToString(i)); +goto cleanup; +break; +case VIR_DOMAIN_HYPERV_LAST: +break; +} +} +} +} if (def->cpu && def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) { for (i = 0; i < def->cpu->nfeatures; i++) { -- 2.7.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] qemu_process: skip only cpu features
This check is there to allow restore saved domain with older libvirt where we included invtsc by default for host-passthrough model. Don't skip the whole function, but only the part that checks for invtsc. Signed-off-by: Pavel Hrdina --- src/qemu/qemu_process.c | 26 -- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 205d9ae..9334a75 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3904,11 +3904,6 @@ qemuProcessVerifyGuestCPU(virQEMUDriverPtr driver, bool ret = false; size_t i; -/* no features are passed to QEMU with -cpu host - * so it makes no sense to verify them */ -if (def->cpu && def->cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) -return true; - switch (arch) { case VIR_ARCH_I686: case VIR_ARCH_X86_64: @@ -3933,17 +3928,20 @@ qemuProcessVerifyGuestCPU(virQEMUDriverPtr driver, } } -for (i = 0; def->cpu && i < def->cpu->nfeatures; i++) { -virCPUFeatureDefPtr feature = &def->cpu->features[i]; -if (feature->policy != VIR_CPU_FEATURE_REQUIRE) -continue; +if (def->cpu && def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) { +for (i = 0; i < def->cpu->nfeatures; i++) { +virCPUFeatureDefPtr feature = &def->cpu->features[i]; -if (STREQ(feature->name, "invtsc") && -!cpuHasFeature(guestcpu, feature->name)) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("host doesn't support invariant TSC")); -goto cleanup; +if (feature->policy != VIR_CPU_FEATURE_REQUIRE) +continue; + +if (STREQ(feature->name, "invtsc") && +!cpuHasFeature(guestcpu, feature->name)) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("host doesn't support invariant TSC")); +goto cleanup; +} } } break; -- 2.7.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] some additional hyper-v features fixes
Pavel Hrdina (3): docs: fix qemu version for hyperv features qemu_process: skip only cpu features qemu_process: add check for hyperv features docs/formatdomain.html.in | 4 ++-- src/cpu/cpu_x86.c | 8 +++ src/cpu/cpu_x86_data.h| 8 +++ src/qemu/qemu_process.c | 57 +++ 4 files changed, 61 insertions(+), 16 deletions(-) -- 2.7.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] docs: fix qemu version for hyperv features
Signed-off-by: Pavel Hrdina --- docs/formatdomain.html.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index a1d7c4a..957b839 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1558,13 +1558,13 @@ synic Enable Synthetic Interrupt Controller (SyNIC) on, off - 1.3.3 (QEMU 2.5) + 1.3.3 (QEMU 2.6) stimer Enable SyNIC timers on, off - 1.3.3 (QEMU 2.5) + 1.3.3 (QEMU 2.6) reset -- 2.7.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tools: virt-host-validate: fix KVM check on s390
On Tue, 2016-03-29 at 14:01 +0100, Daniel P. Berrange wrote: > > > virHostMsgCheck("QEMU", "%s", ("for hardware virtualization")); > > > if (virHostValidateHasCPUFlag("svm") || > > > -virHostValidateHasCPUFlag("vmx")) { > > > +virHostValidateHasCPUFlag("vmx") || > > > +virHostValidateHasCPUFlag("sie")) { > > > virHostMsgPass(); > > > if (virHostValidateDeviceExists("QEMU", "/dev/kvm", > > > VIR_HOST_VALIDATE_FAIL, > > What we should do here though is check the host architecture before > checking CPU flags. > > eg you'll want to make this code do something like this > >bool hasHWVirt = false; >switch (virArchFromHost()) { >case VIR_ARCH_I686: >case VIR_ARCH_X86_64: > if (virHostValidateHasCPUFlag("svm") || > virHostValidateHasCPUFlag("vmx")) >hasHWVirt = true; > break; >case VIR_ARCH_S390: >case VIR_ARCH_S390X: > if (virHostValidateHasCPUFlag("sie")) >hasHWVirt = true; >} >if (hasHWVirt) { >do the /dev/kvm check... >} ... And I guess I should come up with suitable checks for ppc64 and aarch64 as well :) Cheers. -- Andrea Bolognani Software Engineer - Virtualization Team -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Plan for next release
On 29.03.2016 14:29, Daniel Veillard wrote: > I'm a bit behind, I forgot ! > I think we should freeze tomorrow (Wed 30), then make an rc2 on Friday > and shoot for a release next Monday (Apr 4), > >works for everybody ? Yay, first release with NSS module. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Fix /proc/**/stat parsing
Since commit v1.3.2-119-g1e34a8f which enabled debug-threads in QEMU qemuGetProcessInfo would fail to parse stats for any thread with a space in its name. https://bugzilla.redhat.com/show_bug.cgi?id=1316803 Signed-off-by: Jiri Denemark --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7e01459..dec99ca 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1460,7 +1460,7 @@ qemuGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, long *vm_rss, if (!pidinfo || fscanf(pidinfo, /* pid -> stime */ - "%*d %*s %*c %*d %*d %*d %*d %*d %*u %*u %*u %*u %*u %llu %llu" + "%*d (%*[^)]) %*c %*d %*d %*d %*d %*d %*u %*u %*u %*u %*u %llu %llu" /* cutime -> endcode */ "%*d %*d %*d %*d %*d %*d %*u %*u %ld %*u %*u %*u" /* startstack -> processor */ -- 2.7.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/6] virsh: Add support for byte granularity and scaled integers for virsh block APIs
On Thu, Mar 24, 2016 at 15:48:57 -0400, John Ferlan wrote: > > > On 03/18/2016 04:56 AM, Peter Krempa wrote: > > Quite some time ago we've added support for byte granularity for block job > > bandwidth. Make it work in virsh and add support for scaled integers too. > > > > Peter Krempa (6): > > vsh: Tweak error message for scaled integers > > vsh: Refactor vshCommandOptScaledInt > > virsh: blockjob: Support --bytes and scaled integers as bandwidth > > virsh: blockcommit: Support --bytes and scaled integers > > virsh: blockcopy: Support --bytes and scaled integers > > virsh: blockpull: Support --bytes and scaled integers > > > > tests/virsh-optparse | 6 ++--- > > tools/virsh-domain.c | 73 > > > > tools/virsh.pod | 37 ++ > > tools/vsh.c | 66 +-- > > tools/vsh.h | 4 +++ > > 5 files changed, 137 insertions(+), 49 deletions(-) > > > > > Note specific nits from patch 3 > > I think the commit messages for patches 4-6 shouldn't be: > > "Reuse the approach and helper from the last patch." > > since "last patch" causes me to go find the "last patch"... > > cut-copy-paste what they're using vshBlockJobOptionBandwidth and > allowing the --bytes on the set. > > If we really wanted to be picky, patch 3 could introduce the new > function with patches 4-7 using it and indicating so in their commit > messages. > > ACK series with at least the typos fixed and commit messages adjusted. > Your choice if you want to extract out the function. I've split out the function addition and fixed the commit messages and comments. Thanks; pushed. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix /proc/**/stat parsing
On Tue, Mar 29, 2016 at 03:51:49PM +0200, Jiri Denemark wrote: > Since commit v1.3.2-119-g1e34a8f which enabled debug-threads in QEMU > qemuGetProcessInfo would fail to parse stats for any thread with a space > in its name. > > https://bugzilla.redhat.com/show_bug.cgi?id=1316803 > > Signed-off-by: Jiri Denemark > --- ACK Pavel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] bhyve: implement domainShutdown
Daniel P. Berrange wrote: > On Mon, Mar 28, 2016 at 10:27:18AM +0300, Roman Bogorodskiy wrote: > > Bhyve supports ACPI shutdown by issuing SIGTERM signal to the bhyve > > process. Add the bhyveDomainShutdown() function and > > virBhyveProcessShutdown() helper function that just sends SIGTERM to > > VM's bhyve process. If a guest supports ACPI shutdown then process > > will be terminated and this event will be noticed by the bhyve monitor > > code that will handle setting proper status and clean up VM's resources. > > > > Also, remove a warning in domainDestroy in case if > > virProcessKillPainfully() returns 1, meaning that it killed process > > using SIGKILL. This behavior should be expected when using 'destroy'. > > Hmm, so destroy is supposed to be equivalent to physically removing > the power plug. The existing code is calling virProcessShutdownPainfully > which starts by sending SIGTERM and then switches to SIGKILL. So this > means that your virDomainDestroy implementation is mistakenly trying todo > a graceful shutdown initially and then switching to hard shutdown after a > bit of a delay. Indeed, you're right. Probably doesn't make practical difference due to timeout being quite low, but makes no sense to wait. Will fix. > Has bhyve always used SIGTERM to trigger ACPI shutdown, or is this a > recent addition ? I would tend to suggest we need to go straight to > SIGKILL for virDomainDestroy to avoid doing ACPI shutdown when we don't > want it. It was actually added back in the end of 2013, so it was in -CURRENT for quite some time already. It's also available in 10.1, the earliest supported version [1] of the 10.x branch. It's not in 10.0 though. > It is kind of a shame they used SIGTERM for triggering ACPI imho but > oh well. Agreed, it doesn't seem very flexible. At least I don't see a way how to probe if this feature is supported by the given bhyve binary and how to check if the command was successful or not (except relying on the libvirt's bhyve/bhyve_monitor.c code to detect process termination). 1: https://www.freebsd.org/security/security.html#sup > Regards, > Daniel > -- > |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| > |: http://libvirt.org -o- http://virt-manager.org :| > |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| > |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| Roman Bogorodskiy signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 0/8] Add perf and Intel CMT feature support
> -Original Message- > From: Daniel P. Berrange [mailto:berra...@redhat.com] > Sent: Tuesday, March 29, 2016 8:14 PM > To: Ren, Qiaowei > Cc: libvir-list@redhat.com; Jiri Denemark > Subject: Re: [PATCH v4 0/8] Add perf and Intel CMT feature support > > On Mon, Mar 28, 2016 at 09:30:25PM +0800, Qiaowei Ren wrote: > > The series mainly adds Intel CMT feature support into libvirt. CMT is > > new introduced PQos (Platform Qos) feature to monitor the usage of > > cache by applications running on the platform. > > > > Currently CMT patches has been merged into Linux kernel mainline. > > The CMT implementation in Linux kernel is based on perf mechanism and > > there is no support for perf in libvirt, and so this series firstly > > add perf support into libvirt, including two public API and a set of > > util interfaces. And based on these APIs and interfaces, thie series > > implements CMT perf event support. > > > > Current state: > > > > * 1/8 - perf: add new public APIs for perf event > > - ACKed by Daniel > > > > * 2/8 - perf: implement the remote protocol for perf event > > - ACKed by Daniel > > > > * 3/8 - perf: implement a set of util functions for perf event > > - ACKed by Daniel > > > > * 4/8 - qemu_driver: add support to perf event > > - restart perf in qemuProcessReconnect and qemuProcessAttach > > in patch 6/8 > > > > * 5/8 - perf: add new xml element > > - ACKed by Daniel > > > > * 6/8 - perf: reenable perf events when libvirtd restart > > - add qemuDomainPerfRestart() into qemuProcessReconnect() and > > qemuProcessAttach() > > > > * 7/8 - virsh: implement new command to support perf > > - ACKed by Daniel > > > > * 8/8 - virsh: extend domstats command > > - ACKed by Daniel > > ACK to all patches. I had some comments inline - in future just make sure you > run > 'make check && make syntax-check' against each patch before submitting. Since > the fixes were easy I've made them myself and pushed to GIT master. > > Ok! Got it! Sorry, I just rebased it into latest code and compiled it and ignored this step. Thanks very much for your nice help. ^-^ Thanks, Qiaowei -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] qemu: Create domain master key
[...] >> >> +/* qemuDomainGetMasterKeyFilePath: >> + * @libDir: Directory path to domain lib files >> + * >> + * This API will generate a path of the domain's libDir (e.g. >> + * (/var/lib/libvirt/qemu/domain-#-$NAME/) and a name 'master.key'. >> + * >> + * This API will check and fail if the libDir is NULL as that results >> + * in an invalid path generated. >> + * >> + * This API does not check if the resulting path exists, that is left >> + * up to the caller. >> + * >> + * Returns path to memory containing the name of the file. It is up to the >> + * caller to free; otherwise, NULL on failure. >> + */ >> +char * >> +qemuDomainGetMasterKeyFilePath(const char *libDir) >> +{ >> +if (!libDir) { >> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", >> + _("invalid path for master key file")); >> +return NULL; >> +} >> +return virFileBuildPath(libDir, "master.key", NULL); >> +} > > How about calling this master-key.aes to make it explicit what this > key is intended to be used with. > > OK >> +/* qemuDomainWriteMasterKeyFile: >> + * @priv: pointer to domain private object >> + * >> + * Get the desired path to the masterKey file, base64 encode the masterKey, >> + * and store it in the file. > > The QEMU secrets code is happy to get data in either raw or base64 > format. I wonder if there's a compelling reason to use base64 instead > of just writing it out as raw bytes. > No compelling reason comes to mind - one extra level of obscurity mostly. One less place to have an error in the Write/Read functions for base64 conversions. [...] >> +static char * >> +qemuDomainGenerateRandomKey(size_t nbytes) >> +{ >> +char *key; >> +#if HAVE_GNUTLS_CRYPTO_H >> +int ret; >> +#else >> +size_t i; >> +#endif >> + >> +if (VIR_ALLOC_N(key, nbytes) < 0) >> +return NULL; >> + >> +#if HAVE_GNUTLS_CRYPTO_H >> +/* Generate a master key using gnutls if possible */ >> +if ((ret = gnutls_rnd(GNUTLS_RND_RANDOM, key, nbytes)) < 0) { >> +virReportError(VIR_ERR_INTERNAL_ERROR, >> + _("failed to generate master key, ret=%d"), ret); >> +VIR_FREE(key); >> +return NULL; >> +} >> +#else >> +/* Generate a less cryptographically strong master key */ >> +for (i = 0; i < nbytes; i++) >> +key[i] = virRandomBits(8); > > It is probably better to just read nbytes from /dev/urandom > directly, as that's much closer to what gnutls_rnd would > do as compared to virRandomBits. > OK >> +#endif >> + >> +return key; >> +} >> + >> + >> +/* qemuDomainMasterKeyRemove: >> + * @priv: Pointer to the domain private object >> + * >> + * Remove the traces of the master key, clear the heap, clear the file, >> + * delete the file. >> + */ >> +void >> +qemuDomainMasterKeyRemove(qemuDomainObjPrivatePtr priv) >> +{ >> +char *path = NULL; >> + >> +if (!priv->masterKey) >> +return; >> + >> +/* Clear the heap */ >> +memset(priv->masterKey, 0, QEMU_DOMAIN_MASTER_KEY_LEN); >> +VIR_FREE(priv->masterKey); >> + >> +/* Clear and remove the master key file */ >> +path = qemuDomainGetMasterKeyFilePath(priv->libDir); >> +if (path && virFileExists(path)) { >> +/* Write a "0" to the file even though we're about to delete it */ >> +ignore_value(virFileWriteStr(path, "0", 0600)); > > In the src/storage/storage_backend.c we have a fnuction that can use > scrub to wipe out a file. We should probably put that function into > src/util/virfile.c as virFileWipe() or something like that. Given Peter's continued objection, should we just skip this. IDC either way, but don't If it was desired, then I assume you are looking for just something that will move/rename virStorageBackendWipeLocal (adjusting params), right? > >> +unlink(path); >> +} >> +VIR_FREE(path); >> +} > > >> @@ -212,6 +213,9 @@ struct _qemuDomainObjPrivate { >> char *machineName; >> char *libDir;/* base path for per-domain files */ >> char *channelTargetDir; /* base path for per-domain channel targets */ >> + >> +char *masterKey; /* random key for encryption (not to be saved in our */ >> + /* private XML) - need to restore at process reconnect >> */ > > I'd be inclined to declare this as uint8_t * to make it clear that its > binary data, not a null terminated string, and also declare a masterKeyLen > field to track length, so we only use the constant at time of generation. > OK Tks - John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] qemu: Create domain master key
On 03/29/2016 08:56 AM, Peter Krempa wrote: [,,,] >> >> +/* qemuDomainGetMasterKeyFilePath: >> + * @libDir: Directory path to domain lib files >> + * >> + * This API will generate a path of the domain's libDir (e.g. >> + * (/var/lib/libvirt/qemu/domain-#-$NAME/) and a name 'master.key'. >> + * >> + * This API will check and fail if the libDir is NULL as that results >> + * in an invalid path generated. >> + * >> + * This API does not check if the resulting path exists, that is left >> + * up to the caller. >> + * >> + * Returns path to memory containing the name of the file. It is up to the >> + * caller to free; otherwise, NULL on failure. > > Whoah, docs longer than the code itself. > Setting future expectations. I'll shorten it though. >> + */ >> +char * >> +qemuDomainGetMasterKeyFilePath(const char *libDir) >> +{ >> +if (!libDir) { >> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", >> + _("invalid path for master key file")); >> +return NULL; > > And the code is rather self-explanatory. > >> +} >> +return virFileBuildPath(libDir, "master.key", NULL); >> +} >> + [...] >> +qemuDomainMasterKeyReadFile(qemuDomainObjPrivatePtr priv) >> +{ >> +char *path; >> +char *base64Key = NULL; >> +int base64KeySize = 0; >> +char *masterKey = NULL; >> +size_t masterKeySize = 0; >> +int ret = -1; >> + >> +/* If we don't have the capability, then do nothing. */ >> +if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_SECRET)) >> +return 0; >> + >> +if (!(path = qemuDomainGetMasterKeyFilePath(priv->libDir))) >> +return -1; >> + >> +if (!virFileExists(path)) { >> +virReportError(VIR_ERR_INTERNAL_ERROR, >> + _("domain master key file doesn't exist in %s"), >> + priv->libDir); >> +goto cleanup; >> +} >> + >> +/* Read the base64 encooded secret from the file, decode it, and >> + * store in the domain private object. >> + */ >> +if ((base64KeySize = virFileReadAll(path, 1024, &base64Key)) < 0) >> +goto cleanup; >> + >> +if (!base64_decode_alloc(base64Key, base64KeySize, >> + &masterKey, &masterKeySize)) { >> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", >> + _("invalid base64 in domain master key file")); >> +goto cleanup; >> +} >> + >> +if (!masterKey || masterKeySize != QEMU_DOMAIN_MASTER_KEY_LEN) { >> +virReportError(VIR_ERR_INTERNAL_ERROR, >> + _("invalid encoded master key read, size=%zd"), >> + masterKeySize); >> +goto cleanup; >> +} >> + >> +priv->masterKey = masterKey; >> +masterKey = NULL; > > so this is NULL now and masterKeySize > 0 > doh. Yep. and that's what happens when adding the cleanup: late >> + >> +ret = 0; >> + >> + cleanup: >> +if (masterKeySize > 0) >> +memset(masterKey, 0, masterKeySize); > > ... memset(NULL, 0, 32); > > This crashes on success path. > >> +VIR_FREE(masterKey); >> + >> +if (base64KeySize > 0) >> +memset(base64Key, 0, base64KeySize); >> +VIR_FREE(base64Key); >> + >> +VIR_FREE(path); >> + >> +return ret; >> +} >> + [...] >> +void >> +qemuDomainMasterKeyRemove(qemuDomainObjPrivatePtr priv) >> +{ >> +char *path = NULL; >> + >> +if (!priv->masterKey) >> +return; >> + >> +/* Clear the heap */ >> +memset(priv->masterKey, 0, QEMU_DOMAIN_MASTER_KEY_LEN); >> +VIR_FREE(priv->masterKey); >> + >> +/* Clear and remove the master key file */ >> +path = qemuDomainGetMasterKeyFilePath(priv->libDir); >> +if (path && virFileExists(path)) { >> +/* Write a "0" to the file even though we're about to delete it */ > > This still doesn't make any sense. The filesystem needs to guarantee > this anyways. This is just a false sense of security. > See my note in response to Dan's review. IDC either way. >> +ignore_value(virFileWriteStr(path, "0", 0600)); >> +unlink(path); >> +} >> +VIR_FREE(path); >> +} >> + >> + >> +/* qemuDomainMasterKeyCreate: >> + * @priv: Pointer to the domain private object >> + * >> + * As long as the underlying qemu has the secret capability, >> + * generate and store as a base64 encoded value a random 32-byte >> + * key to be used as a secret shared with qemu to share sensitive data. >> + * >> + * Returns: 0 on success, -1 w/ error message on failure >> + */ >> +int >> +qemuDomainMasterKeyCreate(qemuDomainObjPrivatePtr priv) >> +{ >> +char *key = NULL; >> + >> +/* If we don't have the capability, then do nothing. */ >> +if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_SECRET)) >> +return 0; >> + >> +if (!(key = qemuDomainGenerateRandomKey(QEMU_DOMAIN_MASTER_KEY_LEN))) >> +goto error; >> + >> +priv->masterKey = key; >> +key = NULL; > > key isn't touched after the previous l
Re: [libvirt] [PATCH 2/3] qemu: Create domain master key
On Tue, Mar 29, 2016 at 10:33:34AM -0400, John Ferlan wrote: > > > [...] > > >> > >> +/* qemuDomainGetMasterKeyFilePath: > >> + * @libDir: Directory path to domain lib files > >> + * > >> + * This API will generate a path of the domain's libDir (e.g. > >> + * (/var/lib/libvirt/qemu/domain-#-$NAME/) and a name 'master.key'. > >> + * > >> + * This API will check and fail if the libDir is NULL as that results > >> + * in an invalid path generated. > >> + * > >> + * This API does not check if the resulting path exists, that is left > >> + * up to the caller. > >> + * > >> + * Returns path to memory containing the name of the file. It is up to the > >> + * caller to free; otherwise, NULL on failure. > >> + */ > >> +char * > >> +qemuDomainGetMasterKeyFilePath(const char *libDir) > >> +{ > >> +if (!libDir) { > >> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > >> + _("invalid path for master key file")); > >> +return NULL; > >> +} > >> +return virFileBuildPath(libDir, "master.key", NULL); > >> +} > > > > How about calling this master-key.aes to make it explicit what this > > key is intended to be used with. > > > > > > OK > > >> +/* qemuDomainWriteMasterKeyFile: > >> + * @priv: pointer to domain private object > >> + * > >> + * Get the desired path to the masterKey file, base64 encode the > >> masterKey, > >> + * and store it in the file. > > > > The QEMU secrets code is happy to get data in either raw or base64 > > format. I wonder if there's a compelling reason to use base64 instead > > of just writing it out as raw bytes. > > > > No compelling reason comes to mind - one extra level of obscurity > mostly. One less place to have an error in the Write/Read functions for > base64 conversions. > > [...] > > >> +static char * > >> +qemuDomainGenerateRandomKey(size_t nbytes) > >> +{ > >> +char *key; > >> +#if HAVE_GNUTLS_CRYPTO_H > >> +int ret; > >> +#else > >> +size_t i; > >> +#endif > >> + > >> +if (VIR_ALLOC_N(key, nbytes) < 0) > >> +return NULL; > >> + > >> +#if HAVE_GNUTLS_CRYPTO_H > >> +/* Generate a master key using gnutls if possible */ > >> +if ((ret = gnutls_rnd(GNUTLS_RND_RANDOM, key, nbytes)) < 0) { > >> +virReportError(VIR_ERR_INTERNAL_ERROR, > >> + _("failed to generate master key, ret=%d"), ret); > >> +VIR_FREE(key); > >> +return NULL; > >> +} > >> +#else > >> +/* Generate a less cryptographically strong master key */ > >> +for (i = 0; i < nbytes; i++) > >> +key[i] = virRandomBits(8); > > > > It is probably better to just read nbytes from /dev/urandom > > directly, as that's much closer to what gnutls_rnd would > > do as compared to virRandomBits. > > > > OK > > >> +#endif > >> + > >> +return key; > >> +} > >> + > >> + > >> +/* qemuDomainMasterKeyRemove: > >> + * @priv: Pointer to the domain private object > >> + * > >> + * Remove the traces of the master key, clear the heap, clear the file, > >> + * delete the file. > >> + */ > >> +void > >> +qemuDomainMasterKeyRemove(qemuDomainObjPrivatePtr priv) > >> +{ > >> +char *path = NULL; > >> + > >> +if (!priv->masterKey) > >> +return; > >> + > >> +/* Clear the heap */ > >> +memset(priv->masterKey, 0, QEMU_DOMAIN_MASTER_KEY_LEN); > >> +VIR_FREE(priv->masterKey); > >> + > >> +/* Clear and remove the master key file */ > >> +path = qemuDomainGetMasterKeyFilePath(priv->libDir); > >> +if (path && virFileExists(path)) { > >> +/* Write a "0" to the file even though we're about to delete it */ > >> +ignore_value(virFileWriteStr(path, "0", 0600)); > > > > In the src/storage/storage_backend.c we have a fnuction that can use > > scrub to wipe out a file. We should probably put that function into > > src/util/virfile.c as virFileWipe() or something like that. > > Given Peter's continued objection, should we just skip this. IDC either > way, but don't > > If it was desired, then I assume you are looking for just something that > will move/rename virStorageBackendWipeLocal (adjusting params), right? Ok lets just ignore it - we can easily add it later if desired. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] docs: fix qemu version for hyperv features
On 03/29/2016 09:31 AM, Pavel Hrdina wrote: > Signed-off-by: Pavel Hrdina > --- > docs/formatdomain.html.in | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > ACK - Read that one wrong yesterday John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] spec: fix trigger already defined issue on systems without systemd
Le 11/03/2016 15:18, Jean-Marc LIGER a écrit : There is a trigger already defined issue when you try to rebuild libvirt >= 1.3.0 for el6 with copr and most probably koji. Regards, Jean-Marc diff -uri a/libvirt.spec.in b/libvirt.spec.in --- a/libvirt.spec.in2016-03-01 04:21:48.0 +0100 +++ b/libvirt.spec.in2016-03-11 14:57:43.347652313 +0100 @@ -1767,16 +1767,6 @@ fi %endif -%if %{with_systemd} -%else -%triggerpostun daemon -- libvirt-daemon < 1.2.1 -if [ "$1" -ge "1" ]; then -/sbin/service virtlockd reload > /dev/null 2>&1 || : -/sbin/service virtlogd reload > /dev/null 2>&1 || : -/sbin/service libvirtd condrestart > /dev/null 2>&1 -fi -%endif - # In upgrade scenario we must explicitly enable virtlockd/virtlogd # sockets, if libvirtd is already enabled and start them if # libvirtd is running, otherwise you'll get failures to start @@ -1789,6 +1779,12 @@ /bin/systemctl is-active libvirtd.service 1>/dev/null 2>&1 && /bin/systemctl start virtlogd.socket || : %else +%triggerpostun daemon -- libvirt-daemon < 1.2.1 +if [ "$1" -ge "1" ]; then +/sbin/service virtlockd reload > /dev/null 2>&1 || : +/sbin/service virtlogd reload > /dev/null 2>&1 || : +/sbin/service libvirtd condrestart > /dev/null 2>&1 +fi /sbin/chkconfig libvirtd 1>/dev/null 2>&1 && /sbin/chkconfig virtlogd on || : /sbin/service libvirtd status 1>/dev/null 2>&1 && -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list I've modified my patch after the nss update, someone could revue this patch before the next release ? Regards, Jean-Marc diff -uri a/libvirt.spec.in b/libvirt.spec.in --- a/libvirt.spec.in2016-03-29 16:40:37.985927719 +0200 +++ b/libvirt.spec.in2016-03-29 16:28:01.808864888 +0200 @@ -1783,16 +1783,6 @@ fi %endif -%if %{with_systemd} -%else -%triggerpostun daemon -- libvirt-daemon < 1.2.1 -if [ "$1" -ge "1" ]; then -/sbin/service virtlockd reload > /dev/null 2>&1 || : -/sbin/service virtlogd reload > /dev/null 2>&1 || : -/sbin/service libvirtd condrestart > /dev/null 2>&1 -fi -%endif - # In upgrade scenario we must explicitly enable virtlockd/virtlogd # sockets, if libvirtd is already enabled and start them if # libvirtd is running, otherwise you'll get failures to start @@ -1805,6 +1795,12 @@ /bin/systemctl is-active libvirtd.service 1>/dev/null 2>&1 && /bin/systemctl start virtlogd.socket || : %else +%triggerpostun daemon -- libvirt-daemon < 1.2.1 +if [ "$1" -ge "1" ]; then +/sbin/service virtlockd reload > /dev/null 2>&1 || : +/sbin/service virtlogd reload > /dev/null 2>&1 || : +/sbin/service libvirtd condrestart > /dev/null 2>&1 +fi /sbin/chkconfig libvirtd 1>/dev/null 2>&1 && /sbin/chkconfig virtlogd on || : /sbin/service libvirtd status 1>/dev/null 2>&1 && -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] qemu: hotplug: Improve error when hotpluging memory with auto-placement
The numad advice is not valid at the point of hotplug so we are not reusing it. If the user would want to hot-add memory with automatic placement we'd report: 'Advice from numad is needed in case of automatic numa placement' Change it to: 'automatic placement of memory is not possible when hotplugging' --- src/conf/numa_conf.c | 20 +--- src/conf/numa_conf.h | 1 + src/libvirt_private.syms | 1 + src/qemu/qemu_hotplug.c | 7 +++ 4 files changed, 26 insertions(+), 3 deletions(-) diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index e0d5688..e10fed8 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -419,6 +419,22 @@ virDomainNumatuneFormatNodeset(virDomainNumaPtr numatune, } +/** + * virDomainNumatuneNeedNumadAdvice: + * @numatune: domain numa definition + * + * Returns true if the numa configuration requires automatic placement data + * gathered by numad. + */ +bool +virDomainNumatuneNeedNumadAdvice(virDomainNumaPtr numatune) +{ +return numatune && + numatune->memory.specified && + numatune->memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO; +} + + int virDomainNumatuneMaybeGetNodeset(virDomainNumaPtr numatune, virBitmapPtr auto_nodeset, @@ -434,9 +450,7 @@ virDomainNumatuneMaybeGetNodeset(virDomainNumaPtr numatune, !numatune->memory.specified) return 0; -if (numatune->memory.specified && -numatune->memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO && -!auto_nodeset) { +if (virDomainNumatuneNeedNumadAdvice(numatune) && !auto_nodeset) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Advice from numad is needed in case of " "automatic numa placement")); diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h index 90deacb..82f0155 100644 --- a/src/conf/numa_conf.h +++ b/src/conf/numa_conf.h @@ -155,5 +155,6 @@ int virDomainNumaDefCPUFormat(virBufferPtr buf, virDomainNumaPtr def); unsigned int virDomainNumaGetCPUCountTotal(virDomainNumaPtr numa); +bool virDomainNumatuneNeedNumadAdvice(virDomainNumaPtr numatune); #endif /* __NUMA_CONF_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7c44047..8b88667 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -692,6 +692,7 @@ virDomainNumatuneMaybeFormatNodeset; virDomainNumatuneMaybeGetNodeset; virDomainNumatuneMemModeTypeFromString; virDomainNumatuneMemModeTypeToString; +virDomainNumatuneNeedNumadAdvice; virDomainNumatuneNodesetIsAvailable; virDomainNumatuneNodeSpecified; virDomainNumatuneParseXML; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index e82dbf5..84475b7 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1760,6 +1760,13 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, if (vm->def->mem.cur_balloon == virDomainDefGetMemoryActual(vm->def)) fix_balloon = true; +if (virDomainNumatuneNeedNumadAdvice(vm->def->numa) && !mem->sourceNodes) { +virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("automatic placement of memory is not possible when " + "hotplugging")); +goto cleanup; +} + if (!(devstr = qemuBuildMemoryDeviceStr(mem))) goto cleanup; -- 2.7.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] qemu: command: Pass numad nodeset when formatting memory devices at boot
When starting up a VM libvirtd asks numad to place the VM in case of automatic nodeset. The nodeset would not be passed to the memory device formatter and the user would get an error. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1269715 --- src/qemu/qemu_command.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 45c5398..8545533 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2945,7 +2945,8 @@ static char * qemuBuildMemoryDimmBackendStr(virDomainMemoryDefPtr mem, virDomainDefPtr def, virQEMUCapsPtr qemuCaps, - virQEMUDriverConfigPtr cfg) + virQEMUDriverConfigPtr cfg, + virBitmapPtr auto_nodeset) { virJSONValuePtr props = NULL; char *alias = NULL; @@ -2962,7 +2963,7 @@ qemuBuildMemoryDimmBackendStr(virDomainMemoryDefPtr mem, goto cleanup; if (qemuBuildMemoryBackendStr(mem->size, mem->pagesize, - mem->targetNode, mem->sourceNodes, NULL, + mem->targetNode, mem->sourceNodes, auto_nodeset, def, qemuCaps, cfg, &backendType, &props, true) < 0) goto cleanup; @@ -7200,7 +7201,7 @@ qemuBuildNumaCommandLine(virCommandPtr cmd, char *dimmStr; if (!(backStr = qemuBuildMemoryDimmBackendStr(def->mems[i], def, - qemuCaps, cfg))) + qemuCaps, cfg, nodeset))) return -1; if (!(dimmStr = qemuBuildMemoryDeviceStr(def->mems[i]))) { -- 2.7.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] qemu: command: Split up formatting of -numa and memory devices
They recently were extracted to a separate function. They don't belong together though. Since -numa formatting is pretty compact, move it to the main function and rename qemuBuildNumaCommandLine to qemuBuildMemoryDeviceCommandLine. --- src/qemu/qemu_command.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8545533..2d0ca97 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7182,18 +7182,14 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, static int -qemuBuildNumaCommandLine(virCommandPtr cmd, - virQEMUDriverConfigPtr cfg, - virDomainDefPtr def, - virQEMUCapsPtr qemuCaps, - virBitmapPtr nodeset) +qemuBuildMemoryDeviceCommandLine(virCommandPtr cmd, + virQEMUDriverConfigPtr cfg, + virDomainDefPtr def, + virQEMUCapsPtr qemuCaps, + virBitmapPtr nodeset) { size_t i; -if (virDomainNumaGetNodeCount(def->numa) && -qemuBuildNumaArgStr(cfg, def, cmd, qemuCaps, nodeset) < 0) -return -1; - /* memory hotplug requires NUMA to be enabled - we already checked * that memory devices are present only when NUMA is */ for (i = 0; i < def->nmems; i++) { @@ -9260,7 +9256,11 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildIOThreadCommandLine(cmd, def, qemuCaps) < 0) goto error; -if (qemuBuildNumaCommandLine(cmd, cfg, def, qemuCaps, nodeset) < 0) +if (virDomainNumaGetNodeCount(def->numa) && +qemuBuildNumaArgStr(cfg, def, cmd, qemuCaps, nodeset) < 0) +goto error; + +if (qemuBuildMemoryDeviceCommandLine(cmd, cfg, def, qemuCaps, nodeset) < 0) goto error; virUUIDFormat(def->uuid, uuid); -- 2.7.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] qemu: fix automatic NUMA pinning with memory hotplug
We forgot to propagate the automatic nodeset and reported non-ideal errors. Peter Krempa (3): qemu: command: Pass numad nodeset when formatting memory devices at boot qemu: command: Split up formatting of -numa and memory devices qemu: hotplug: Improve error when hotpluging memory with auto-placement src/conf/numa_conf.c | 20 +--- src/conf/numa_conf.h | 1 + src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 27 ++- src/qemu/qemu_hotplug.c | 7 +++ 5 files changed, 40 insertions(+), 16 deletions(-) -- 2.7.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] bhyve: implement domainShutdown
On 03/29/2016 08:24 AM, Daniel P. Berrange wrote: On Mon, Mar 28, 2016 at 10:27:18AM +0300, Roman Bogorodskiy wrote: Bhyve supports ACPI shutdown by issuing SIGTERM signal to the bhyve process. Add the bhyveDomainShutdown() function and virBhyveProcessShutdown() helper function that just sends SIGTERM to VM's bhyve process. If a guest supports ACPI shutdown then process will be terminated and this event will be noticed by the bhyve monitor code that will handle setting proper status and clean up VM's resources. Also, remove a warning in domainDestroy in case if virProcessKillPainfully() returns 1, meaning that it killed process using SIGKILL. This behavior should be expected when using 'destroy'. Hmm, so destroy is supposed to be equivalent to physically removing the power plug. The existing code is calling virProcessShutdownPainfully which starts by sending SIGTERM and then switches to SIGKILL. So this means that your virDomainDestroy implementation is mistakenly trying todo a graceful shutdown initially and then switching to hard shutdown after a bit of a delay. For context - the qemu driver does this for destroy as well. I think at least one of the reasons is to allow the qemu process to flush the buffers for the disk image files. I recall this was causing some amount of pain for ovirt (or maybe someone else, I forget) Has bhyve always used SIGTERM to trigger ACPI shutdown, or is this a recent addition ? I would tend to suggest we need to go straight to SIGKILL for virDomainDestroy to avoid doing ACPI shutdown when we don't want it. It is kind of a shame they used SIGTERM for triggering ACPI imho but oh well. Yes, it means there is no way to implement a "clean power off" (which would give bhyve the chance to clean up some critical things before pulling the plug). -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] bhyve: implement domainShutdown
On Tue, Mar 29, 2016 at 10:52:09AM -0400, Laine Stump wrote: > On 03/29/2016 08:24 AM, Daniel P. Berrange wrote: > >On Mon, Mar 28, 2016 at 10:27:18AM +0300, Roman Bogorodskiy wrote: > >>Bhyve supports ACPI shutdown by issuing SIGTERM signal to the bhyve > >>process. Add the bhyveDomainShutdown() function and > >>virBhyveProcessShutdown() helper function that just sends SIGTERM to > >>VM's bhyve process. If a guest supports ACPI shutdown then process > >>will be terminated and this event will be noticed by the bhyve monitor > >>code that will handle setting proper status and clean up VM's resources. > >> > >>Also, remove a warning in domainDestroy in case if > >>virProcessKillPainfully() returns 1, meaning that it killed process > >>using SIGKILL. This behavior should be expected when using 'destroy'. > >Hmm, so destroy is supposed to be equivalent to physically removing > >the power plug. The existing code is calling virProcessShutdownPainfully > >which starts by sending SIGTERM and then switches to SIGKILL. So this > >means that your virDomainDestroy implementation is mistakenly trying todo > >a graceful shutdown initially and then switching to hard shutdown after a > >bit of a delay. > > For context - the qemu driver does this for destroy as well. I think at > least one of the reasons is to allow the qemu process to flush the buffers > for the disk image files. I recall this was causing some amount of pain for > ovirt (or maybe someone else, I forget) Yep it is different in QEMU - SIGTERM doesn't trigger any guest OS ACPI event with QEMU - it merely lets QEMU gracefully close the block layer functions and flush I/O. > >Has bhyve always used SIGTERM to trigger ACPI shutdown, or is this a > >recent addition ? I would tend to suggest we need to go straight to > >SIGKILL for virDomainDestroy to avoid doing ACPI shutdown when we don't > >want it. > > > >It is kind of a shame they used SIGTERM for triggering ACPI imho but > >oh well. > > Yes, it means there is no way to implement a "clean power off" (which would > give bhyve the chance to clean up some critical things before pulling the > plug). Yeah, exactly, it is desirable to be able to do a immediate forced shutdown of guest OS while still letting the host process exit cleanly. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] Decrease execution complexity of formating iothread scheduler info
On 22.03.2016 15:00, Peter Krempa wrote: > While refactoring the old way to store iothread scheduler info I've added an > algorithm that isn't entirely optimal but allows to store the scheduler info > in > a sane way. Unfortunately when you specify an insane number of iothreads the > code takes ages to execute. > > To avoid this series being completely useless except for the one corner case > I've opted to finally add support for self expanding bitmaps, which might > become > useful in the future. The self-expanding bitmap is then used instead of one of > the loops that was necessary to determine the maximum iothread ID. > > Peter Krempa (3): > util: bitmap: Intoduce self-expanding bitmap APIs > conf: decrease iterations complexity when formatting iothreads > conf: Remove now unused virDomainIOThreadIDMap > > src/conf/domain_conf.c | 51 ++ > src/conf/domain_conf.h | 3 -- > src/libvirt_private.syms | 4 ++- > src/util/virbitmap.c | 93 > > src/util/virbitmap.h | 8 + > tests/virbitmaptest.c| 51 ++ > 6 files changed, 173 insertions(+), 37 deletions(-) > ACK series. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] some additional hyper-v features fixes
On Tue, Mar 29, 2016 at 15:31:14 +0200, Pavel Hrdina wrote: > Pavel Hrdina (3): > docs: fix qemu version for hyperv features > qemu_process: skip only cpu features > qemu_process: add check for hyperv features > > docs/formatdomain.html.in | 4 ++-- > src/cpu/cpu_x86.c | 8 +++ > src/cpu/cpu_x86_data.h| 8 +++ > src/qemu/qemu_process.c | 57 > +++ > 4 files changed, 61 insertions(+), 16 deletions(-) ACK series Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] qemu_process: add check for hyperv features
On 03/29/2016 09:31 AM, Pavel Hrdina wrote: > Commit 7068b56c introduced several hyperv features. Not all hyperv > features are supported by old enough kernels and we shouldn't allow to > start a guest if kernel doesn't support any of the hyperv feature. > > There is one exception, for backward compatibility we cannot error out > if one of the RELAXED, VAPIC or SPINLOCKS isn't supported, for the same > reason we ignore invtsc, to not break restoring saved domains with older > libvirt. >From yesterday's dialog, there's also commit id '59fc0d06' which adds "hv_crash" and commit id '600bca59' which adds "hv_time". Neither is handled via these bits, but wouldn't both fall into the same trap since both were added after commit '2e8f9080'? > > Signed-off-by: Pavel Hrdina > --- > src/cpu/cpu_x86.c | 8 > src/cpu/cpu_x86_data.h | 8 > src/qemu/qemu_process.c | 31 +++ > 3 files changed, 47 insertions(+) > Now I was going to ask about a capability bit for this, but seeing none in previous commits, I thought I was safe... I guess not. The rest looks OK to me, although I wonder if there's a way to avoid missing this for future changes John > diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c > index 90949f6..b7f1690 100644 > --- a/src/cpu/cpu_x86.c > +++ b/src/cpu/cpu_x86.c > @@ -75,6 +75,14 @@ static const struct x86_kvm_feature x86_kvm_features[] = > {VIR_CPU_x86_KVM_PV_UNHALT,{ .function = 0x4001, .eax = > 0x0080 }}, > {VIR_CPU_x86_KVM_CLOCKSOURCE_STABLE_BIT, > { .function = 0x4001, .eax = > 0x0100 }}, > +{VIR_CPU_x86_KVM_HV_RUNTIME, { .function = 0x4003, .eax = > 0x0001 }}, > +{VIR_CPU_x86_KVM_HV_SYNIC, { .function = 0x4003, .eax = > 0x0004 }}, > +{VIR_CPU_x86_KVM_HV_STIMER,{ .function = 0x4003, .eax = > 0x0008 }}, > +{VIR_CPU_x86_KVM_HV_RELAXED, { .function = 0x4003, .eax = > 0x0020 }}, > +{VIR_CPU_x86_KVM_HV_SPINLOCK, { .function = 0x4003, .eax = > 0x0022 }}, > +{VIR_CPU_x86_KVM_HV_VAPIC, { .function = 0x4003, .eax = > 0x0030 }}, > +{VIR_CPU_x86_KVM_HV_VPINDEX, { .function = 0x4003, .eax = > 0x0040 }}, > +{VIR_CPU_x86_KVM_HV_RESET, { .function = 0x4003, .eax = > 0x0080 }}, > }; > > struct x86_model { > diff --git a/src/cpu/cpu_x86_data.h b/src/cpu/cpu_x86_data.h > index 88dccf6..777cc8d 100644 > --- a/src/cpu/cpu_x86_data.h > +++ b/src/cpu/cpu_x86_data.h > @@ -48,6 +48,14 @@ struct _virCPUx86CPUID { > # define VIR_CPU_x86_KVM_PV_EOI "__kvm_pv_eoi" > # define VIR_CPU_x86_KVM_PV_UNHALT"__kvm_pv_unhalt" > # define VIR_CPU_x86_KVM_CLOCKSOURCE_STABLE_BIT "__kvm_clocksource_stable" > +# define VIR_CPU_x86_KVM_HV_RUNTIME "__kvm_hv_runtime" > +# define VIR_CPU_x86_KVM_HV_SYNIC "__kvm_hv_synic" > +# define VIR_CPU_x86_KVM_HV_STIMER"__kvm_hv_stimer" > +# define VIR_CPU_x86_KVM_HV_RELAXED "__kvm_hv_relaxed" > +# define VIR_CPU_x86_KVM_HV_SPINLOCK "__kvm_hv_spinlock" > +# define VIR_CPU_x86_KVM_HV_VAPIC "__kvm_hv_vapic" > +# define VIR_CPU_x86_KVM_HV_VPINDEX "__kvm_hv_vpindex" > +# define VIR_CPU_x86_KVM_HV_RESET "__kvm_hv_reset" > > > typedef struct _virCPUx86Data virCPUx86Data; > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c > index 9334a75..07b9df2 100644 > --- a/src/qemu/qemu_process.c > +++ b/src/qemu/qemu_process.c > @@ -3928,6 +3928,37 @@ qemuProcessVerifyGuestCPU(virQEMUDriverPtr driver, > } > } > > +for (i = 0; i < VIR_DOMAIN_HYPERV_LAST; i++) { > +if (def->hyperv_features[i] == VIR_TRISTATE_SWITCH_ON) { > +char *cpuFeature; > +if (virAsprintf(&cpuFeature, "__kvm_hv_%s", > +virDomainHypervTypeToString(i)) < 0) > +goto cleanup; > +if (!cpuHasFeature(guestcpu, cpuFeature)) { > +switch ((virDomainHyperv) i) { > +case VIR_DOMAIN_HYPERV_RELAXED: > +case VIR_DOMAIN_HYPERV_VAPIC: > +case VIR_DOMAIN_HYPERV_SPINLOCKS: > +VIR_WARN("host doesn't support hyperv '%s' feature", > + virDomainHypervTypeToString(i)); > +break; > +case VIR_DOMAIN_HYPERV_VPINDEX: > +case VIR_DOMAIN_HYPERV_RUNTIME: > +case VIR_DOMAIN_HYPERV_SYNIC: > +case VIR_DOMAIN_HYPERV_STIMER: > +case VIR_DOMAIN_HYPERV_RESET: > +case VIR_DOMAIN_HYPERV_VENDOR_ID: > +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("host doesn't support hyperv '%s' > feature"), > + virDomainHypervTypeToString(i)); > +goto c
Re: [libvirt] [PATCH 1/3] util: bitmap: Intoduce self-expanding bitmap APIs
Doh - I was looking at these while I see Michal posted an ACK series... so hopefully before you push... $SUBJ Introduce On 03/22/2016 10:00 AM, Peter Krempa wrote: > In some cases it's impractical to use the regular APIs as the bitmap > size needs to be pre-declared. These new APIs allow to use bitmaps that > self expand. > > The new code adds a property to the bitmap to track the alocation of allocation > memory so that VIR_RESIZE_N can be used. > --- > src/libvirt_private.syms | 3 ++ > src/util/virbitmap.c | 93 > > src/util/virbitmap.h | 8 + > tests/virbitmaptest.c| 51 ++ > 4 files changed, 155 insertions(+) > [...] > diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c > index 967a5c8..92f1e6e 100644 > --- a/tests/virbitmaptest.c > +++ b/tests/virbitmaptest.c > @@ -590,6 +590,54 @@ test11(const void *opaque) > return ret; > } > > +#define TEST_MAP(sz, expect) > \ > +do { > \ > +char *actual = virBitmapFormat(map); > \ > +if (virBitmapSize(map) != sz) { > \ > +fprintf(stderr, "\n expected bitmap size: '%d' actual size: " > \ > +"'%zu'\n", sz, virBitmapSize(map)); > \ According to Coverity, actual can be leaked here... > +goto cleanup; > \ > +} > \ > +if (STRNEQ_NULLABLE(expect, actual)) { > \ > +fprintf(stderr, "\n expected bitmap contents '%s' actual > contents "\ > +"'%s'\n", NULLSTR(expect), NULLSTR(actual)); > \ > +VIR_FREE(actual); > \ > +goto cleanup; > \ > +} > \ > +VIR_FREE(actual); > \ > +} while (0) > + > +/* test self-expanding bitmap APIs */ > +static int > +test12(const void *opaque ATTRIBUTE_UNUSED) > +{ > +virBitmapPtr map = NULL; > +int ret = -1; > + > +if (!(map = virBitmapNewEmpty())) > +return -1; > + > +TEST_MAP(0, ""); > + > +if (virBitmapSetBitExpand(map, 100) < 0) > +goto cleanup; > + > +TEST_MAP(101, "100"); > + > +if (virBitmapClearBitExpand(map, 150) < 0) > +goto cleanup; > + > +TEST_MAP(151, "100"); > + > +ret = 0; > + > + cleanup: > +virBitmapFree(map); > +return ret; > +} > +#undef TEST_MAP > + > + John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] qemu_process: add check for hyperv features
On Tue, Mar 29, 2016 at 11:10:42AM -0400, John Ferlan wrote: > On 03/29/2016 09:31 AM, Pavel Hrdina wrote: > > Commit 7068b56c introduced several hyperv features. Not all hyperv > > features are supported by old enough kernels and we shouldn't allow to > > start a guest if kernel doesn't support any of the hyperv feature. > > > > There is one exception, for backward compatibility we cannot error out > > if one of the RELAXED, VAPIC or SPINLOCKS isn't supported, for the same > > reason we ignore invtsc, to not break restoring saved domains with older > > libvirt. > > From yesterday's dialog, there's also commit id '59fc0d06' which adds > "hv_crash" and commit id '600bca59' which adds "hv_time". > > Neither is handled via these bits, but wouldn't both fall into the same > trap since both were added after commit '2e8f9080'? Even though libvirt passes those options to QEMU in the same -cpu argument, they are unrelated to this patch series. And for the same reason as for RELAXED, VAPIC and SPINLOCKS, we cannot error out to not break things so there is nothing to do about it. > > > > > Signed-off-by: Pavel Hrdina > > --- > > src/cpu/cpu_x86.c | 8 > > src/cpu/cpu_x86_data.h | 8 > > src/qemu/qemu_process.c | 31 +++ > > 3 files changed, 47 insertions(+) > > > > Now I was going to ask about a capability bit for this, but seeing none > in previous commits, I thought I was safe... I guess not. I don't know what you mean by this, what capability? Pavel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] qemu_process: add check for hyperv features
[...] >> >> Now I was going to ask about a capability bit for this, but seeing none >> in previous commits, I thought I was safe... I guess not. > > I don't know what you mean by this, what capability? > Sorry - context was I was thinking about the code I looked at yesterday that didn't have any capability checking for the bits in previous commits for hyperv feature bits. Not this patch. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] qemu_process: add check for hyperv features
29.03.2016 16:31, Pavel Hrdina пишет: Commit 7068b56c introduced several hyperv features. Not all hyperv features are supported by old enough kernels and we shouldn't allow to start a guest if kernel doesn't support any of the hyperv feature. There is one exception, for backward compatibility we cannot error out if one of the RELAXED, VAPIC or SPINLOCKS isn't supported, for the same reason we ignore invtsc, to not break restoring saved domains with older libvirt. Signed-off-by: Pavel Hrdina --- src/cpu/cpu_x86.c | 8 src/cpu/cpu_x86_data.h | 8 src/qemu/qemu_process.c | 31 +++ 3 files changed, 47 insertions(+) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 90949f6..b7f1690 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -75,6 +75,14 @@ static const struct x86_kvm_feature x86_kvm_features[] = {VIR_CPU_x86_KVM_PV_UNHALT,{ .function = 0x4001, .eax = 0x0080 }}, {VIR_CPU_x86_KVM_CLOCKSOURCE_STABLE_BIT, { .function = 0x4001, .eax = 0x0100 }}, +{VIR_CPU_x86_KVM_HV_RUNTIME, { .function = 0x4003, .eax = 0x0001 }}, +{VIR_CPU_x86_KVM_HV_SYNIC, { .function = 0x4003, .eax = 0x0004 }}, +{VIR_CPU_x86_KVM_HV_STIMER,{ .function = 0x4003, .eax = 0x0008 }}, +{VIR_CPU_x86_KVM_HV_RELAXED, { .function = 0x4003, .eax = 0x0020 }}, +{VIR_CPU_x86_KVM_HV_SPINLOCK, { .function = 0x4003, .eax = 0x0022 }}, +{VIR_CPU_x86_KVM_HV_VAPIC, { .function = 0x4003, .eax = 0x0030 }}, +{VIR_CPU_x86_KVM_HV_VPINDEX, { .function = 0x4003, .eax = 0x0040 }}, +{VIR_CPU_x86_KVM_HV_RESET, { .function = 0x4003, .eax = 0x0080 }}, }; struct x86_model { diff --git a/src/cpu/cpu_x86_data.h b/src/cpu/cpu_x86_data.h index 88dccf6..777cc8d 100644 --- a/src/cpu/cpu_x86_data.h +++ b/src/cpu/cpu_x86_data.h @@ -48,6 +48,14 @@ struct _virCPUx86CPUID { # define VIR_CPU_x86_KVM_PV_EOI "__kvm_pv_eoi" # define VIR_CPU_x86_KVM_PV_UNHALT"__kvm_pv_unhalt" # define VIR_CPU_x86_KVM_CLOCKSOURCE_STABLE_BIT "__kvm_clocksource_stable" +# define VIR_CPU_x86_KVM_HV_RUNTIME "__kvm_hv_runtime" +# define VIR_CPU_x86_KVM_HV_SYNIC "__kvm_hv_synic" +# define VIR_CPU_x86_KVM_HV_STIMER"__kvm_hv_stimer" +# define VIR_CPU_x86_KVM_HV_RELAXED "__kvm_hv_relaxed" +# define VIR_CPU_x86_KVM_HV_SPINLOCK "__kvm_hv_spinlock" +# define VIR_CPU_x86_KVM_HV_VAPIC "__kvm_hv_vapic" +# define VIR_CPU_x86_KVM_HV_VPINDEX "__kvm_hv_vpindex" +# define VIR_CPU_x86_KVM_HV_RESET "__kvm_hv_reset" typedef struct _virCPUx86Data virCPUx86Data; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 9334a75..07b9df2 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3928,6 +3928,37 @@ qemuProcessVerifyGuestCPU(virQEMUDriverPtr driver, } } +for (i = 0; i < VIR_DOMAIN_HYPERV_LAST; i++) { +if (def->hyperv_features[i] == VIR_TRISTATE_SWITCH_ON) { +char *cpuFeature; +if (virAsprintf(&cpuFeature, "__kvm_hv_%s", +virDomainHypervTypeToString(i)) < 0) +goto cleanup; +if (!cpuHasFeature(guestcpu, cpuFeature)) { +switch ((virDomainHyperv) i) { +case VIR_DOMAIN_HYPERV_RELAXED: +case VIR_DOMAIN_HYPERV_VAPIC: +case VIR_DOMAIN_HYPERV_SPINLOCKS: +VIR_WARN("host doesn't support hyperv '%s' feature", + virDomainHypervTypeToString(i)); +break; +case VIR_DOMAIN_HYPERV_VPINDEX: +case VIR_DOMAIN_HYPERV_RUNTIME: +case VIR_DOMAIN_HYPERV_SYNIC: +case VIR_DOMAIN_HYPERV_STIMER: +case VIR_DOMAIN_HYPERV_RESET: +case VIR_DOMAIN_HYPERV_VENDOR_ID: +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("host doesn't support hyperv '%s' feature"), + virDomainHypervTypeToString(i)); +goto cleanup; +break; +case VIR_DOMAIN_HYPERV_LAST: +break; +} +} +} +} if (def->cpu && def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) { for (i = 0; i < def->cpu->nfeatures; i++) { Hmm, qemu already checks them and simply ignores most of them and doesn't prevent guest from starting in case they are not supported and optional. In case they are reqired it fails. Why should we check them here? At least we should follow the logic qemu has. Extract from target-i386/kvm.c does the following: if (cpu->hyperv_rel
[libvirt] [PATCH] host-validate: Improve CPU flags processing
Instead of relying on substring search, tokenize the input and process each CPU flag separately. This ensures CPU flag detection will continue to work correctly even if we start looking for CPU flags whose name might appear as part of other CPU flags' names. The result of processing is stored in a virBitmap, which means we don't have to parse /proc/cpuinfo in its entirety for each single CPU flag we want to check. Moreover, use of the newly-introduced virHostValidateCPUFlag enumeration ensures we don't go looking for random CPU flags which might actually be simple typos. --- The concern was raised by John in https://www.redhat.com/archives/libvir-list/2016-March/msg01301.html when discussing a new check on the "sie" CPU flag. tools/virt-host-validate-common.c | 50 +-- tools/virt-host-validate-common.h | 13 +- tools/virt-host-validate-qemu.c | 12 -- 3 files changed, 60 insertions(+), 15 deletions(-) diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c index 8ebf73e..57fa332 100644 --- a/tools/virt-host-validate-common.c +++ b/tools/virt-host-validate-common.c @@ -31,7 +31,6 @@ #endif /* HAVE_MNTENT_H */ #include -#include "virutil.h" #include "viralloc.h" #include "virfile.h" #include "virt-host-validate-common.h" @@ -39,6 +38,10 @@ #define VIR_FROM_THIS VIR_FROM_NONE +VIR_ENUM_IMPL(virHostValidateCPUFlag, VIR_HOST_VALIDATE_CPU_FLAG_LAST, + "vmx", + "svm"); + static bool quiet; void virHostMsgSetQuiet(bool quietFlag) @@ -188,29 +191,47 @@ int virHostValidateNamespace(const char *hvname, } -bool virHostValidateHasCPUFlag(const char *name) +virBitmapPtr virHostValidateGetCPUFlags(void) { -FILE *fp = fopen("/proc/cpuinfo", "r"); -bool ret = false; +FILE *fp; +virBitmapPtr flags; -if (!fp) -return false; +if (!(fp = fopen("/proc/cpuinfo", "r"))) +return NULL; + +if (!(flags = virBitmapNewQuiet(VIR_HOST_VALIDATE_CPU_FLAG_LAST))) +return NULL; do { char line[1024]; +char *saveptr; +char *token; if (!fgets(line, sizeof(line), fp)) break; -if (strstr(line, name)) { -ret = true; -break; +if (!(token = strtok_r(line, " \t\n", &saveptr))) +continue; + +/* The line we're interested in is marked either as "flags" or + * as "Features" depending on the architecture, so check both + * prefixes. It's very unlikely that there will be no whitespace + * between the line name and the colon, but handle that as well */ +if (strcmp(token, "flags") && strcmp(token, "flags:") && +strcmp(token, "Features") && strcmp(token, "Features:")) +continue; + +while ((token = strtok_r(NULL, " \t\n", &saveptr))) { +int value; + +if ((value = virHostValidateCPUFlagTypeFromString(token)) >= 0) +ignore_value(virBitmapSetBit(flags, value)); } } while (1); VIR_FORCE_FCLOSE(fp); -return ret; +return flags; } @@ -359,15 +380,20 @@ int virHostValidateCGroupController(const char *hvname, int virHostValidateIOMMU(const char *hvname, virHostValidateLevel level) { +virBitmapPtr flags; struct stat sb; const char *bootarg = NULL; bool isAMD = false, isIntel = false; -if (virHostValidateHasCPUFlag("vmx")) +flags = virHostValidateGetCPUFlags(); + +if (flags && virBitmapIsBitSet(flags, VIR_HOST_VALIDATE_CPU_FLAG_VMX)) isIntel = true; -else if (virHostValidateHasCPUFlag("svm")) +else if (flags && virBitmapIsBitSet(flags, VIR_HOST_VALIDATE_CPU_FLAG_SVM)) isAMD = true; +virBitmapFree(flags); + virHostMsgCheck(hvname, "%s", _("for device assignment IOMMU support")); if (isIntel) { diff --git a/tools/virt-host-validate-common.h b/tools/virt-host-validate-common.h index d4c4759..26e006b 100644 --- a/tools/virt-host-validate-common.h +++ b/tools/virt-host-validate-common.h @@ -23,6 +23,8 @@ # define __VIRT_HOST_VALIDATE_COMMON_H__ # include "internal.h" +# include "virutil.h" +# include "virbitmap.h" typedef enum { VIR_HOST_VALIDATE_FAIL, @@ -32,6 +34,15 @@ typedef enum { VIR_HOST_VALIDATE_LAST, } virHostValidateLevel; +typedef enum { +VIR_HOST_VALIDATE_CPU_FLAG_VMX = 0, +VIR_HOST_VALIDATE_CPU_FLAG_SVM, + +VIR_HOST_VALIDATE_CPU_FLAG_LAST, +} virHostValidateCPUFlag; + +VIR_ENUM_DECL(virHostValidateCPUFlag); + extern void virHostMsgSetQuiet(bool quietFlag); extern void virHostMsgCheck(const char *prefix, @@ -53,7 +64,7 @@ extern int virHostValidateDeviceAccessible(const char *hvname, virHostValidateLevel level, const char *hint); -extern bool virHostValidateHasCPUFlag(const char *name); +exter
[libvirt] [PATCH] perf: fix build on non-Linux
* Sync stubbed functions with prototypes * Add missing ATTRIBUTE_UNUSED where needed Pushing under the build breaker rule. --- src/util/virperf.c | 17 + 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/util/virperf.c b/src/util/virperf.c index 42eee85..9dc4e25 100644 --- a/src/util/virperf.c +++ b/src/util/virperf.c @@ -262,6 +262,7 @@ virPerfReadEvent(virPerfPtr perf, #else int virPerfEventEnable(virPerfPtr perf ATTRIBUTE_UNUSED, + virPerfEventType type ATTRIBUTE_UNUSED, pid_t pid ATTRIBUTE_UNUSED) { virReportSystemError(ENXIO, "%s", @@ -271,7 +272,7 @@ virPerfEventEnable(virPerfPtr perf ATTRIBUTE_UNUSED, int virPerfEventDisable(virPerfPtr perf ATTRIBUTE_UNUSED, -int event ATTRIBUTE_UNUSED) +virPerfEventType type ATTRIBUTE_UNUSED) { virReportSystemError(ENXIO, "%s", _("Perf not supported on this platform")); @@ -279,15 +280,15 @@ virPerfEventDisable(virPerfPtr perf ATTRIBUTE_UNUSED, } bool -virPerfEventIsEnabled(virPerfPtr perf, - virPerfEventType type) +virPerfEventIsEnabled(virPerfPtr perf ATTRIBUTE_UNUSED, + virPerfEventType type ATTRIBUTE_UNUSED) { return false; } int -virPerfGetEventFd(virPerfPtr perf, - virPerfEventType type) +virPerfGetEventFd(virPerfPtr perf ATTRIBUTE_UNUSED, + virPerfEventType type ATTRIBUTE_UNUSED) { virReportSystemError(ENXIO, "%s", _("Perf not supported on this platform")); @@ -295,9 +296,9 @@ virPerfGetEventFd(virPerfPtr perf, } int -virPerfReadEvent(virPerfPtr perf, - virPerfEventType type - uint64_t *value) +virPerfReadEvent(virPerfPtr perf ATTRIBUTE_UNUSED, + virPerfEventType type ATTRIBUTE_UNUSED, + uint64_t *value ATTRIBUTE_UNUSED) { virReportSystemError(ENXIO, "%s", _("Perf not supported on this platform")); -- 2.7.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] host-validate: Fix suggestion for missing cpu cgroup
If the cpu cgroup is not found when validating an host for LXC support, virt-host-validate will suggest to enable the CONFIG_CGROUP_SCHED kconfig option. The appropriate option is really CONFIG_CGROUP_CPU. The QEMU checks already get that right, so no changes needed. --- tools/virt-host-validate-lxc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/virt-host-validate-lxc.c b/tools/virt-host-validate-lxc.c index 89a6388..2b906cc 100644 --- a/tools/virt-host-validate-lxc.c +++ b/tools/virt-host-validate-lxc.c @@ -70,7 +70,7 @@ int virHostValidateLXC(void) if (virHostValidateCGroupController("LXC", "cpu", VIR_HOST_VALIDATE_FAIL, -"CGROUP_SCHED") < 0) +"CGROUP_CPU") < 0) ret = -1; if (virHostValidateCGroupController("LXC", "cpuacct", -- 2.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] bhyve: implement domainShutdown
Daniel P. Berrange wrote: > On Tue, Mar 29, 2016 at 10:52:09AM -0400, Laine Stump wrote: > > On 03/29/2016 08:24 AM, Daniel P. Berrange wrote: > > >On Mon, Mar 28, 2016 at 10:27:18AM +0300, Roman Bogorodskiy wrote: > > >>Bhyve supports ACPI shutdown by issuing SIGTERM signal to the bhyve > > >>process. Add the bhyveDomainShutdown() function and > > >>virBhyveProcessShutdown() helper function that just sends SIGTERM to > > >>VM's bhyve process. If a guest supports ACPI shutdown then process > > >>will be terminated and this event will be noticed by the bhyve monitor > > >>code that will handle setting proper status and clean up VM's resources. > > >> > > >>Also, remove a warning in domainDestroy in case if > > >>virProcessKillPainfully() returns 1, meaning that it killed process > > >>using SIGKILL. This behavior should be expected when using 'destroy'. > > >Hmm, so destroy is supposed to be equivalent to physically removing > > >the power plug. The existing code is calling virProcessShutdownPainfully > > >which starts by sending SIGTERM and then switches to SIGKILL. So this > > >means that your virDomainDestroy implementation is mistakenly trying todo > > >a graceful shutdown initially and then switching to hard shutdown after a > > >bit of a delay. > > > > For context - the qemu driver does this for destroy as well. I think at > > least one of the reasons is to allow the qemu process to flush the buffers > > for the disk image files. I recall this was causing some amount of pain for > > ovirt (or maybe someone else, I forget) > > Yep it is different in QEMU - SIGTERM doesn't trigger any guest OS ACPI > event with QEMU - it merely lets QEMU gracefully close the block > layer functions and flush I/O. > > > >Has bhyve always used SIGTERM to trigger ACPI shutdown, or is this a > > >recent addition ? I would tend to suggest we need to go straight to > > >SIGKILL for virDomainDestroy to avoid doing ACPI shutdown when we don't > > >want it. > > > > > >It is kind of a shame they used SIGTERM for triggering ACPI imho but > > >oh well. > > > > Yes, it means there is no way to implement a "clean power off" (which would > > give bhyve the chance to clean up some critical things before pulling the > > plug). > > Yeah, exactly, it is desirable to be able to do a immediate forced shutdown > of guest OS while still letting the host process exit cleanly. That's an interesting point. I noticed that there's also 'bhyvectl --force-poweroff' command, if it does what it looks like it should do then I guess it would be good to use it instead of the kill -9 call. I'll check its behaviour and update the patch accordingly. > Regards, > Daniel > -- > |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| > |: http://libvirt.org -o- http://virt-manager.org :| > |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| > |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| Roman Bogorodskiy signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] util: bitmap: Intoduce self-expanding bitmap APIs
On Tue, Mar 29, 2016 at 11:12:38 -0400, John Ferlan wrote: > > Doh - I was looking at these while I see Michal posted an ACK series... > so hopefully before you push... I've fixed the things you've pointed out and pushed this. Thanks. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v7 2/4] arm: enhance kvm_arm_create_scratch_host_vcpu
On 24 March 2016 at 02:55, Peter Xu wrote: > Some more lines to make sure we allow NULL for 1st/3rd parameter. > > Signed-off-by: Peter Xu > --- > target-arm/kvm.c | 14 +- > target-arm/kvm_arm.h | 6 -- > 2 files changed, 17 insertions(+), 3 deletions(-) > > diff --git a/target-arm/kvm.c b/target-arm/kvm.c > index 969ab0b..bb6935e 100644 > --- a/target-arm/kvm.c > +++ b/target-arm/kvm.c > @@ -62,13 +62,17 @@ bool kvm_arm_create_scratch_host_vcpu(const uint32_t > *cpus_to_try, > goto err; > } > > +if (!init) { > +goto finish; > +} > + > ret = ioctl(vmfd, KVM_ARM_PREFERRED_TARGET, init); > if (ret >= 0) { > ret = ioctl(cpufd, KVM_ARM_VCPU_INIT, init); > if (ret < 0) { > goto err; > } > -} else { > +} else if (cpus_to_try) { > /* Old kernel which doesn't know about the > * PREFERRED_TARGET ioctl: we know it will only support > * creating one kind of guest CPU which is its preferred > @@ -85,8 +89,16 @@ bool kvm_arm_create_scratch_host_vcpu(const uint32_t > *cpus_to_try, > if (ret < 0) { > goto err; > } > +} else { > +/* > + * Here, we got init != NULL but cpus_to_retry == NULL, > + * meanwhile, we failed to probe one preferred target type > + * (no matter what). So we fail. > + */ > +goto err; aka "treat a NULL cpus_to_retry like an empty list". > } > > +finish: > fdarray[0] = kvmfd; > fdarray[1] = vmfd; > fdarray[2] = cpufd; > diff --git a/target-arm/kvm_arm.h b/target-arm/kvm_arm.h > index 07f0c72..4f01a99 100644 > --- a/target-arm/kvm_arm.h > +++ b/target-arm/kvm_arm.h > @@ -124,9 +124,11 @@ void kvm_arm_reset_vcpu(ARMCPU *cpu); > * kvm_arm_create_scratch_host_vcpu: > * @cpus_to_try: array of QEMU_KVM_ARM_TARGET_* values (terminated with > * QEMU_KVM_ARM_TARGET_NONE) to try as fallback if the kernel does not > - * know the PREFERRED_TARGET ioctl > + * know the PREFERRED_TARGET ioctl. If NULL is provided, will try > + * to use perferred target, and fail if no preferred found. "preferred". Better phrased as "a NULL pointer is treated like an empty array", I think. > * @fdarray: filled in with kvmfd, vmfd, cpufd file descriptors in that order > - * @init: filled in with the necessary values for creating a host vcpu > + * @init: filled in with the necessary values for creating a host > + * vcpu. If NULL is provided, will not init the vCPU. > * > * Create a scratch vcpu in its own VM of the type preferred by the host > * kernel (as would be used for '-cpu host'), for purposes of probing it > -- > 2.4.3 > (I'll fix this up in target-arm.next, it's a minor nit.) thanks -- PMM -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v7 0/4] ARM: add query-gic-capabilities QMP command
On 24 March 2016 at 02:55, Peter Xu wrote: > This patch is to add ARM-specific command "query-gic-capability". > > The new command can report which kind of GIC device the host/QEMU > support. The returned result is in the form of array. > > Sample command and output: > > {"execute": "query-gic-capability"} > {"return": [{"emulated": false, "version": 3, "kernel": false}, > {"emulated": true, "version": 2, "kernel": true}]} I really strongly want this in 2.6 but we are very very nearly out of time. Eric or Markus, could I trouble one of you to provide an ack for this series for the QMP protocol aspects sometime before end of Wednesday, please? I also don't think I've seen an indication from the libvirt folks that this is definitely the API they want; did I miss it? thanks -- PMM -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] libxl: only disable domain death events in libxlDomainCleanup
On 03/28/2016 07:36 PM, Chun Yan Liu wrote: > Reviewed-by: Chunyan Liu Thanks for taking a look! I'm going to push this series since it just splits your previous patch, which I'm happy to ACK after the split :-). It is a nice cleanup and plugs potential resource leaks, so good stuff for the 1.3.3 release. Regards, Jim > On 3/29/2016 at 08:54 AM, in message > <1459212889-5490-4-git-send-email-jfeh...@suse.com>, Jim Fehlig > wrote: >> Remove disabling domain death events from libxlDomainStart error >> path. The domain death event is already disabled in libxlDomainCleanup. >> >> Signed-off-by: Jim Fehlig >> --- >> src/libxl/libxl_domain.c | 4 >> 1 file changed, 4 deletions(-) >> >> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c >> index 068bfb6..04962a0 100644 >> --- a/src/libxl/libxl_domain.c >> +++ b/src/libxl/libxl_domain.c >> @@ -1144,10 +1144,6 @@ libxlDomainStart(libxlDriverPrivatePtr driver, >> virDomainObjPtr vm, >> >> destroy_dom: >> ret = -1; >> -if (priv->deathW) { >> -libxl_evdisable_domain_death(cfg->ctx, priv->deathW); >> -priv->deathW = NULL; >> -} >> libxlDomainDestroyInternal(driver, vm); >> vm->def->id = -1; >> virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, >> VIR_DOMAIN_SHUTOFF_FAILED); > > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/4] qemu: Add capability bit for qemu secret object
Add a capability bit for the qemu secret object. Adjust the 2.6.0-1 caps/replies to add the secret object. For the .replies it's take from the '{"execute":"qom-list-types"}' output. Signed-off-by: John Ferlan --- src/qemu/qemu_capabilities.c| 2 ++ src/qemu/qemu_capabilities.h| 1 + tests/qemucapabilitiesdata/caps_2.6.0-1.caps| 1 + tests/qemucapabilitiesdata/caps_2.6.0-1.replies | 3 +++ 4 files changed, 7 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 2823843..5d09dc8 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -321,6 +321,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "qxl-vga.vram64_size_mb", /* 215 */ "chardev-logfile", "debug-threads", + "secret", ); @@ -1575,6 +1576,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "virtio-input-host-device", QEMU_CAPS_VIRTIO_INPUT_HOST }, { "virtio-input-host-pci", QEMU_CAPS_VIRTIO_INPUT_HOST }, { "mptsas1068", QEMU_CAPS_SCSI_MPTSAS1068 }, +{ "secret", QEMU_CAPS_OBJECT_SECRET }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBalloon[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index caf3d1b..ae0d9b3 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -351,6 +351,7 @@ typedef enum { QEMU_CAPS_QXL_VGA_VRAM64, /* -device qxl-vga.vram64_size_mb */ QEMU_CAPS_CHARDEV_LOGFILE, /* -chardev logfile= */ QEMU_CAPS_NAME_DEBUG_THREADS, /* Is -name debug-threads= available */ +QEMU_CAPS_OBJECT_SECRET, /* -object secret */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-1.caps b/tests/qemucapabilitiesdata/caps_2.6.0-1.caps index 549759c..a43293d 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0-1.caps +++ b/tests/qemucapabilitiesdata/caps_2.6.0-1.caps @@ -178,4 +178,5 @@ + diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-1.replies b/tests/qemucapabilitiesdata/caps_2.6.0-1.replies index d2b58b5..7590b5b 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0-1.replies +++ b/tests/qemucapabilitiesdata/caps_2.6.0-1.replies @@ -1224,6 +1224,9 @@ "name": "kvm-accel" }, { + "name": "secret" +}, +{ "name": "i82559c" }, { -- 2.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 4/4] qemu: Introduce qemuBuildMasterKeyCommandLine
If the -object secret capability exists, then get the path to the masterKey file and provide that to qemu. Checking for the existence of the file before passing to qemu could be done, but causes issues in mock test environment. Since the qemuDomainObjPrivate is not available when building the command line, the qemuBuildHasMasterKey API will have to suffice as the primary arbiter for whether the capability exists in order to find/return the path to the master key for usage. Created the qemuDomainGetMasterKeyAlias API which will be used by later patches to define the 'keyid' (eg, masterKey) to be used by other secrets to provide the id to qemu for the master key. Signed-off-by: John Ferlan --- src/qemu/qemu_alias.c | 17 ++ src/qemu/qemu_alias.h | 3 + src/qemu/qemu_command.c| 68 ++ .../qemuxml2argvdata/qemuxml2argv-master-key.args | 23 tests/qemuxml2argvdata/qemuxml2argv-master-key.xml | 30 ++ tests/qemuxml2argvtest.c | 2 + 6 files changed, 143 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-master-key.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-master-key.xml diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index efd9222..b57b967 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -484,3 +484,20 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) return 0; } + + +/* qemuDomainGetMasterKeyAlias: + * + * Generate and return the masterKey alias + * + * Returns NULL or a string containing the master key alias + */ +char * +qemuDomainGetMasterKeyAlias(void) +{ +char *alias; + +ignore_value(VIR_STRDUP(alias, "masterKey0")); + +return alias; +} diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h index a2eaa27..299a6d4 100644 --- a/src/qemu/qemu_alias.h +++ b/src/qemu/qemu_alias.h @@ -61,4 +61,7 @@ int qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps); int qemuDomainDeviceAliasIndex(const virDomainDeviceInfo *info, const char *prefix); + +char *qemuDomainGetMasterKeyAlias(void); + #endif /* __QEMU_ALIAS_H__*/ diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 45c5398..01b6a26 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -151,6 +151,71 @@ VIR_ENUM_IMPL(qemuNumaPolicy, VIR_DOMAIN_NUMATUNE_MEM_LAST, "interleave"); /** + * qemuBuildHasMasterKey: + * @qemuCaps: QEMU binary capabilities + * + * Return true if this binary supports the secret -object, false otherwise. + */ +static bool +qemuBuildHasMasterKey(virQEMUCapsPtr qemuCaps) +{ +return virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_SECRET); +} + + +/** + * qemuBuildMasterKeyCommandLine: + * @cmd: the command to modify + * @qemuCaps qemu capabilities object + * @domainLibDir: location to find the master key + + * Formats the command line for a master key if available + * + * Returns 0 on success, -1 w/ error message on failure + */ +static int +qemuBuildMasterKeyCommandLine(virCommandPtr cmd, + virQEMUCapsPtr qemuCaps, + const char *domainLibDir) +{ +int ret = -1; +char *alias = NULL; +char *path = NULL; + +/* If the -object secret does not exist, then just return. This just + * means the domain won't be able to use a secret master key and is + * not a failure. + */ +if (!qemuBuildHasMasterKey(qemuCaps)) { +VIR_INFO("secret object is not supported by this QEMU binary"); +return 0; +} + +if (!(alias = qemuDomainGetMasterKeyAlias())) +return -1; + +/* Get the path. NB, the mocked test will not have the created + * file so we cannot check for existence, which is no different + * than other command line options which do not check for the + * existence of socket files before using. + */ +if (!(path = qemuDomainGetMasterKeyFilePath(domainLibDir))) +goto cleanup; + +virCommandAddArg(cmd, "-object"); +virCommandAddArgFormat(cmd, "secret,id=%s,format=raw,file=%s", + alias, path); + +ret = 0; + + cleanup: +VIR_FREE(alias); +VIR_FREE(path); +return ret; +} + + +/** * qemuVirCommandGetFDSet: * @cmd: the command to modify * @fd: fd to reassign to the child @@ -9235,6 +9300,9 @@ qemuBuildCommandLine(virConnectPtr conn, if (!standalone) virCommandAddArg(cmd, "-S"); /* freeze CPU */ +if (qemuBuildMasterKeyCommandLine(cmd, qemuCaps, domainLibDir) < 0) +goto error; + if (enableFips) virCommandAddArg(cmd, "-enable-fips"); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-master-key.args b/tests/qemuxml2argvdata/qemuxml2argv-master-key.args new file mode 100644 index 000..de030eb --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-master-ke
[libvirt] [PATCH v2 3/4] qemu: Create domain master key
Add a masterKey and masterKeyLen to _qemuDomainObjPrivate to store a random domain master key and its length in order to support the ability to encrypt/decrypt sensitive data shared between libvirt and qemu. The key will be base64 encoded and written to a file to be used by the command line building code to share with qemu. New API's from this patch: qemuDomainGetMasterKeyFilePath: Return a path to where the key is located qemuDomainWriteMasterKeyFile: (private) Open (create/trunc) the masterKey path and write the masterKey qemuDomainMasterKeyReadFile: Using the master key path, open/read the file, and store the masterKey and masterKeyLen. Expected use only from qemuProcessReconnect qemuDomainGenerateRandomKey: (private) Generate a random key using available algorithms The key is generated either from the gnutls_rnd function if it exists or a less cryptographically strong mechanism using virGenerateRandomBytes qemuDomainMasterKeyRemove: Remove traces of the master key, remove the *KeyFilePath qemuDomainMasterKeyCreate: Generate the domain master key and save the key in the location returned by qemuDomainGetMasterKeyFilePath. This API will first ensure the QEMU_CAPS_OBJECT_SECRET is set in the capabilities. If not, then there's no need to generate the secret or file. The creation of the key will be attempted from qemuProcessPrepareHost once the libDir directory structure exists. The removal of the key will handled from qemuProcessStop just prior to deleting the libDir tree. Since the key will not be written out to the domain object XML file, the qemuProcessReconnect will read the saved file and restore the masterKey and masterKeyLen. Signed-off-by: John Ferlan --- src/qemu/qemu_domain.c | 252 src/qemu/qemu_domain.h | 15 +++ src/qemu/qemu_process.c | 11 +++ 3 files changed, 278 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 53cb2b6..99a91d2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -44,6 +44,12 @@ #include "virthreadjob.h" #include "viratomic.h" #include "virprocess.h" +#include "virrandom.h" +#include "base64.h" +#include +#if HAVE_GNUTLS_CRYPTO_H +# include +#endif #include "logging/log_manager.h" #include "storage/storage_driver.h" @@ -465,6 +471,252 @@ qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo, } +/* qemuDomainGetMasterKeyFilePath: + * @libDir: Directory path to domain lib files + * + * Generate a path to the domain master key file for libDir. + * It's up to the caller to handle checking if path exists. + * + * Returns path to memory containing the name of the file. It is up to the + * caller to free; otherwise, NULL on failure. + */ +char * +qemuDomainGetMasterKeyFilePath(const char *libDir) +{ +if (!libDir) { +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("invalid path for master key file")); +return NULL; +} +return virFileBuildPath(libDir, "master-key.aes", NULL); +} + + +/* qemuDomainWriteMasterKeyFile: + * @priv: pointer to domain private object + * + * Get the desired path to the masterKey file and store it in the path. + * + * Returns 0 on success, -1 on failure with error message indicating failure + */ +static int +qemuDomainWriteMasterKeyFile(qemuDomainObjPrivatePtr priv) +{ +char *path; +int fd = -1; +int ret = -1; + +if (!(path = qemuDomainGetMasterKeyFilePath(priv->libDir))) +return -1; + +if ((fd = open(path, O_WRONLY|O_TRUNC|O_CREAT, 0600)) < 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to open domain master key file for write")); +goto cleanup; +} + +if (safewrite(fd, priv->masterKey, priv->masterKeyLen) < 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to write master key file for domain")); +goto cleanup; +} + +ret = 0; + + cleanup: +VIR_FORCE_CLOSE(fd); +VIR_FREE(path); + +return ret; +} + + +/* qemuDomainMasterKeyReadFile: + * @priv: pointer to domain private object + * + * Expected to be called during qemuProcessReconnect once the domain + * libDir has been generated through qemuStateInitialize calling + * virDomainObjListLoadAllConfigs which will restore the libDir path + * to the domain private object. + * + * This function will get the path to the master key file and if it + * exists, it will read the contents of the file saving it in priv->masterKey. + * + * Once the file exists, the validity checks may cause failures; however, + * if the file doesn't exist or the capability doesn't exist, we just + * return (mostly) quietly. + * + * Returns 0 on success or lack of capability + *-1 on failure with error message indicating failure + */ +int +qemuDomainMasterKeyReadFile(qemuDomainObjPrivatePtr priv) +{ +char *path; +int fd = -1
[libvirt] [PATCH v2 2/4] util: Introduce virGenerateRandomBytes
Using the existing virUUIDGenerateRandomBytes, move API to virutil.c and add it to libvirt_private.syms. This will be used as a fallback for generating a domain master key. Signed-off-by: John Ferlan --- src/libvirt_private.syms | 1 + src/util/virutil.c | 36 src/util/virutil.h | 3 +++ src/util/viruuid.c | 30 +- 4 files changed, 41 insertions(+), 29 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7c44047..3d54c39 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2422,6 +2422,7 @@ virEnumToString; virFindFCHostCapableVport; virFindSCSIHostByPCI; virFormatIntDecimal; +virGenerateRandomBytes; virGetDeviceID; virGetDeviceUnprivSGIO; virGetEnvAllowSUID; diff --git a/src/util/virutil.c b/src/util/virutil.c index b401f8d..c55f6f6 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -2669,3 +2669,39 @@ virMemoryMaxValue(bool capped) else return LLONG_MAX; } + + +/** + * virGenerateRandomBytes + * @buf: Pointer to location to store bytes + * @buflen: Number of bytes to store + * + * Generate a stream of random bytes into @buf of size @buflen + */ +int +virGenerateRandomBytes(unsigned char *buf, + size_t buflen) +{ +int fd; + +if ((fd = open("/dev/urandom", O_RDONLY)) < 0) +return errno; + +while (buflen > 0) { +ssize_t n; + +if ((n = read(fd, buf, buflen)) <= 0) { +if (errno == EINTR) +continue; +VIR_FORCE_CLOSE(fd); +return n < 0 ? errno : ENODATA; +} + +buf += n; +buflen -= n; +} + +VIR_FORCE_CLOSE(fd); + +return 0; +} diff --git a/src/util/virutil.h b/src/util/virutil.h index b121de0..a398b38 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -254,6 +254,9 @@ unsigned long long virMemoryLimitTruncate(unsigned long long value); bool virMemoryLimitIsSet(unsigned long long value); unsigned long long virMemoryMaxValue(bool ulong); +int virGenerateRandomBytes(unsigned char *buf, size_t buflen) +ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; + /** * VIR_ASSIGN_IS_OVERFLOW: * @rvalue: value that is checked (evaluated twice) diff --git a/src/util/viruuid.c b/src/util/viruuid.c index 615d419..e2d9fbf 100644 --- a/src/util/viruuid.c +++ b/src/util/viruuid.c @@ -53,34 +53,6 @@ VIR_LOG_INIT("util.uuid"); static unsigned char host_uuid[VIR_UUID_BUFLEN]; static int -virUUIDGenerateRandomBytes(unsigned char *buf, - int buflen) -{ -int fd; - -if ((fd = open("/dev/urandom", O_RDONLY)) < 0) -return errno; - -while (buflen > 0) { -int n; - -if ((n = read(fd, buf, buflen)) <= 0) { -if (errno == EINTR) -continue; -VIR_FORCE_CLOSE(fd); -return n < 0 ? errno : ENODATA; -} - -buf += n; -buflen -= n; -} - -VIR_FORCE_CLOSE(fd); - -return 0; -} - -static int virUUIDGeneratePseudoRandomBytes(unsigned char *buf, int buflen) { @@ -108,7 +80,7 @@ virUUIDGenerate(unsigned char *uuid) if (uuid == NULL) return -1; -if ((err = virUUIDGenerateRandomBytes(uuid, VIR_UUID_BUFLEN))) { +if ((err = virGenerateRandomBytes(uuid, VIR_UUID_BUFLEN))) { char ebuf[1024]; VIR_WARN("Falling back to pseudorandom UUID," " failed to generate random bytes: %s", -- 2.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/4] Add a domain masterKey secret for qemu,
v1: http://www.redhat.com/archives/libvir-list/2016-March/msg01206.html Patch 1 is already ACK'd. I assume this code won't go into 1.3.3, but would hopefully be early in 1.3.4 and I didn't want to break up the capability bits across releases... Differences to v1 - Patch 2 is new - it's taking the virUUIDGenerateRandomBytes and making it generic since we'll use it in Patch 3 (it already opens/reads from /dev/urandom, so I figured it'd be better to share than cut, copy, paste). - Patch 3 has changes from review: * Less comments in qemuDomainGetMasterKeyFilePath * Master key no longer base64 encoded to be written (or read). Instead the Write code will open, truncate, and write the secret directly. The Read code will read the secret directly * The fallback algorithm for key generation uses virGenerateRandomBytes * Changed 'masterKey' from "char *" to "uint8_t *" and added the masterKeyLen - Patch 4 changes in order to tell qemu the format of the file is 'raw'. Also affects test .args file Removed references to encode/decode, adjusted commit messages. Ran through Coverity checker... happy... Created a domain that would pass/read the file... Killed libvirtd, restarted and read the masterKey file properly. Also ensured the #else of the secret generation compiled... John Ferlan (4): qemu: Add capability bit for qemu secret object util: Introduce virGenerateRandomBytes qemu: Create domain master key qemu: Introduce qemuBuildMasterKeyCommandLine src/libvirt_private.syms | 1 + src/qemu/qemu_alias.c | 17 ++ src/qemu/qemu_alias.h | 3 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c| 68 ++ src/qemu/qemu_domain.c | 252 + src/qemu/qemu_domain.h | 15 ++ src/qemu/qemu_process.c| 11 + src/util/virutil.c | 36 +++ src/util/virutil.h | 3 + src/util/viruuid.c | 30 +-- tests/qemucapabilitiesdata/caps_2.6.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_2.6.0-1.replies| 3 + .../qemuxml2argvdata/qemuxml2argv-master-key.args | 23 ++ tests/qemuxml2argvdata/qemuxml2argv-master-key.xml | 30 +++ tests/qemuxml2argvtest.c | 2 + 17 files changed, 469 insertions(+), 29 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-master-key.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-master-key.xml -- 2.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v7 1/4] arm: qmp: add query-gic-capabilities interface
On 03/23/2016 08:55 PM, Peter Xu wrote: > This patch add "query-gic-capabilities" but does not implemnet it. The s/implemnet/implement/ > command is ARM-only. The command will return a list of GICCapability > struct that describes all GIC versions that current QEMU and system > support. > > Libvirt is possibly the first consumer of this new command. > > Before this patch, user will successfully configure all kinds of GIC > devices for ARM guests, no matter whether current QEMU/kernel support > it. If the specified GIC version/type is not supported, user will got an > ambiguous "QEMU boot failure" when trying to start the VM. This is not > user-friendly. > > With this patch, libvirt should be able to query which type (and which > version) of GIC device that we support. Use this information, libvirt > can warn the user during configuration of guests when specified GIC > device type is not supported. Or better, we can just list those versions > that we support, and filter out those not-supported ones. > > For example, if we got the query result: > > {"return": [{"emulated": false, "version": 3, "kernel": true}, > {"emulated": true, "version": 2, "kernel": false}]} > > Then it means that we support emulated GIC version 2 using: > > qemu-system-aarch64 -M virt,accel=tcg,gic-version=2 ... > > or kvm-accelerated GIC version 3 using: > > qemu-system-aarch64 -M virt,accel=kvm,gic-version=3 ... This helps - it shows how libvirt will map the query into qemu command lines. > > If we specify other explicit GIC version rather than the above, QEMU > will not be able to boot. > > Besides, the community is working on a more generic way to query these > kind of information. However, due to the eagerness of this command, we > decided to first implement this ad-hoc one, then when the generic method > is ready, we can move on to that one smoothly. > > Signed-off-by: Peter Xu > --- Interface looks fine from QMP perspective: Reviewed-by: Eric Blake Libvirt should be able to cope with the information here, and introspection will let us know if we ever expand the struct. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/1] python: add python binding for Perf API
This patch adds the python binding for virDomainSetPerfEvents and virDomainSetPerfEvents API. Signed-off-by: Qiaowei Ren --- generator.py | 2 ++ libvirt-override-api.xml | 11 ++ libvirt-override.c | 93 3 files changed, 106 insertions(+) diff --git a/generator.py b/generator.py index d9ae17e..fb6a493 100755 --- a/generator.py +++ b/generator.py @@ -433,6 +433,8 @@ skip_impl = ( 'virDomainGetMemoryParameters', 'virDomainSetNumaParameters', 'virDomainGetNumaParameters', +'virDomainSetPerfEvents', +'virDomainGetPerfEvents', 'virDomainGetVcpus', 'virDomainPinVcpu', 'virDomainPinVcpuFlags', diff --git a/libvirt-override-api.xml b/libvirt-override-api.xml index 1a0e314..54b45f7 100644 --- a/libvirt-override-api.xml +++ b/libvirt-override-api.xml @@ -344,6 +344,17 @@ + + Enable or disable the particular list of perf events + + + + + + Get all perf events setting. + + + Change the bandwidth tunables for a interface device diff --git a/libvirt-override.c b/libvirt-override.c index ce36280..11ef15e 100644 --- a/libvirt-override.c +++ b/libvirt-override.c @@ -1059,6 +1059,97 @@ libvirt_virDomainGetNumaParameters(PyObject *self ATTRIBUTE_UNUSED, } static PyObject * +libvirt_virDomainSetPerfEvents(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ +virDomainPtr domain; +PyObject *pyobj_domain, *info; +PyObject *ret = NULL; +int i_retval; +int nparams = 0; +Py_ssize_t size = 0; +virTypedParameterPtr params = NULL, new_params = NULL; + +if (!PyArg_ParseTuple(args, + (char *)"OO:virDomainSetNumaParameters", + &pyobj_domain, &info)) +return NULL; +domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + +if ((size = PyDict_Size(info)) < 0) +return NULL; + +if (size == 0) { +PyErr_Format(PyExc_LookupError, + "Need non-empty dictionary to set attributes"); +return NULL; +} + +LIBVIRT_BEGIN_ALLOW_THREADS; +i_retval = virDomainGetPerfEvents(domain, ¶ms, &nparams); +LIBVIRT_END_ALLOW_THREADS; + +if (i_retval < 0) +return VIR_PY_INT_FAIL; + +if (nparams == 0) { +PyErr_Format(PyExc_LookupError, + "Domain has no settable attributes"); +return NULL; +} + +new_params = setPyVirTypedParameter(info, params, nparams); +if (!new_params) +goto cleanup; + +LIBVIRT_BEGIN_ALLOW_THREADS; +i_retval = virDomainSetPerfEvents(domain, new_params, size); +LIBVIRT_END_ALLOW_THREADS; + +if (i_retval < 0) { +ret = VIR_PY_INT_FAIL; +goto cleanup; +} + +ret = VIR_PY_INT_SUCCESS; + + cleanup: +virTypedParamsFree(params, nparams); +virTypedParamsFree(new_params, size); +return ret; +} + +static PyObject * +libvirt_virDomainGetPerfEvents(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ +PyObject *pyobj_domain; +virDomainPtr domain; +virTypedParameterPtr params = NULL; +int nparams = 0; +PyObject *dict = NULL; +int rc; + +if (!PyArg_ParseTuple(args, (char *) "O:virDomainGetPerfEvents", + &pyobj_domain)) +return NULL; +domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + +LIBVIRT_BEGIN_ALLOW_THREADS; +rc = virDomainGetPerfEvents(domain, ¶ms, &nparams); +LIBVIRT_END_ALLOW_THREADS; +if (rc < 0) +return VIR_PY_NONE; + +if (!(dict = getPyVirTypedParameter(params, nparams))) +goto cleanup; + + cleanup: +virTypedParamsFree(params, nparams); +return dict; +} + +static PyObject * libvirt_virDomainSetInterfaceParameters(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { @@ -8686,6 +8777,8 @@ static PyMethodDef libvirtMethods[] = { {(char *) "virDomainGetMemoryParameters", libvirt_virDomainGetMemoryParameters, METH_VARARGS, NULL}, {(char *) "virDomainSetNumaParameters", libvirt_virDomainSetNumaParameters, METH_VARARGS, NULL}, {(char *) "virDomainGetNumaParameters", libvirt_virDomainGetNumaParameters, METH_VARARGS, NULL}, +{(char *) "virDomainSetPerfEvents", libvirt_virDomainSetPerfEvents, METH_VARARGS, NULL}, +{(char *) "virDomainGetPerfEvents", libvirt_virDomainGetPerfEvents, METH_VARARGS, NULL}, {(char *) "virDomainSetInterfaceParameters", libvirt_virDomainSetInterfaceParameters, METH_VARARGS, NULL}, {(char *) "virDomainGetInterfaceParameters", libvirt_virDomainGetInterfaceParameters, METH_VARARGS, NULL}, {(char *) "virDomainGetVcpus", libvirt_virDomainGetVcpus, METH_VARARGS, NULL}, -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Entering freeze for libvirt-1.3.3
As suggested yesterday, I tagged RC1 in git and pushed signed tarball and rpms to the usual place: ftp://libvirt.org/libvirt/ Seems to work for me, but please give it some testing ! I expect to have an rc2 on Friday, and do the final release on the week-end or on Monday if everything loks good, Please try it and report, thanks, Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] host-validate: Improve CPU flags processing
On Tue, Mar 29, 2016 at 18:09:44 +0200, Andrea Bolognani wrote: > Instead of relying on substring search, tokenize the input > and process each CPU flag separately. This ensures CPU flag > detection will continue to work correctly even if we start > looking for CPU flags whose name might appear as part of > other CPU flags' names. > > The result of processing is stored in a virBitmap, which > means we don't have to parse /proc/cpuinfo in its entirety > for each single CPU flag we want to check. > > Moreover, use of the newly-introduced virHostValidateCPUFlag > enumeration ensures we don't go looking for random CPU flags > which might actually be simple typos. > --- > The concern was raised by John in > > https://www.redhat.com/archives/libvir-list/2016-March/msg01301.html > > when discussing a new check on the "sie" CPU flag. > > tools/virt-host-validate-common.c | 50 > +-- > tools/virt-host-validate-common.h | 13 +- > tools/virt-host-validate-qemu.c | 12 -- > 3 files changed, 60 insertions(+), 15 deletions(-) > > diff --git a/tools/virt-host-validate-common.c > b/tools/virt-host-validate-common.c > index 8ebf73e..57fa332 100644 > --- a/tools/virt-host-validate-common.c > +++ b/tools/virt-host-validate-common.c [...] > @@ -188,29 +191,47 @@ int virHostValidateNamespace(const char *hvname, > } > > > -bool virHostValidateHasCPUFlag(const char *name) > +virBitmapPtr virHostValidateGetCPUFlags(void) > { > -FILE *fp = fopen("/proc/cpuinfo", "r"); > -bool ret = false; > +FILE *fp; > +virBitmapPtr flags; > > -if (!fp) > -return false; > +if (!(fp = fopen("/proc/cpuinfo", "r"))) > +return NULL; > + > +if (!(flags = virBitmapNewQuiet(VIR_HOST_VALIDATE_CPU_FLAG_LAST))) Since you are already using libvirt's utils ... > +return NULL; > > do { > char line[1024]; > +char *saveptr; > +char *token; > > if (!fgets(line, sizeof(line), fp)) > break; > > -if (strstr(line, name)) { > -ret = true; > -break; > +if (!(token = strtok_r(line, " \t\n", &saveptr))) Why not virStringSplit ... > +continue; > + > +/* The line we're interested in is marked either as "flags" or > + * as "Features" depending on the architecture, so check both > + * prefixes. It's very unlikely that there will be no whitespace > + * between the line name and the colon, but handle that as well */ > +if (strcmp(token, "flags") && strcmp(token, "flags:") && And STREQ? Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list