[libvirt] [PATCHv2 0/4] Time setting and getting in qemu guests
If we'd come up with a struct to interpret the time, it's written in stone and there's nothing we can do about it. Even if we break up the struct into function arguments. However, we can use typed parameters and have the API extensible. For example, I'm implementing seconds granularity only for now. If later somebody feels like he/she wants to use even finer granularity, we can do that by inventing a new typed param. Same goes for timezone, etc. Michal Privoznik (4): Introduce virDomain{Get,Set}Time APIs remote: Implement remote{Get,Set}Time virsh: Expose virDomain{Get,Set}Time qemu: Implement virDomain{Get,Set}Time daemon/remote.c | 50 +++++++++++++++ include/libvirt/libvirt.h.in | 24 ++++++++ src/access/viraccessperm.c | 2 +- src/access/viraccessperm.h | 7 ++- src/driver.h | 14 +++++ src/libvirt.c | 96 +++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 ++ src/qemu/qemu_agent.c | 95 ++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 6 ++ src/qemu/qemu_driver.c | 139 +++++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 47 ++++++++++++++ src/remote/remote_protocol.x | 32 +++++++++- src/remote_protocol-structs | 20 ++++++ tools/virsh-domain-monitor.c | 143 +++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 16 +++++ 15 files changed, 694 insertions(+), 3 deletions(-) -- 1.9.0
These APIs allow users to get or set time in a domain, which may come handy if the domain has been resumed just recently and NTP is not configured or hasn't kicked in yet and the guest is running something time critical. In addition, NTP may refuse to re-set the clock if the skew is too big. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- include/libvirt/libvirt.h.in | 24 +++++++++++ src/driver.h | 14 +++++++ src/libvirt.c | 96 ++++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 +++ 4 files changed, 140 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 930b7e8..93393fd 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -5278,6 +5278,30 @@ int virDomainFSTrim(virDomainPtr dom, unsigned int flags); /** + * VIR_DOMAIN_TIME_SECONDS: + * + * Macro for the virDomainGetTime and virDomainSetTime. It + * represents the time in guest in form of the number of seconds + * since the UNIX Epoch of 1970-01-01 UTC. The corresponding type + * is long long. + */ +#define VIR_DOMAIN_TIME_SECONDS "time_seconds" + +int virDomainGetTime(virDomainPtr dom, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags); + +typedef enum { + VIR_DOMAIN_TIME_SYNC = (1 << 0), /* Re-sync domain time from domain's RTC */ +} virDomainSetTimeFlags; + +int virDomainSetTime(virDomainPtr dom, + virTypedParameterPtr params, + int nparams, + unsigned int flags); + +/** * virSchedParameterType: * * A scheduler parameter field type. Provided for backwards diff --git a/src/driver.h b/src/driver.h index e66fc7a..bde98c9 100644 --- a/src/driver.h +++ b/src/driver.h @@ -1084,6 +1084,18 @@ typedef int unsigned int flags); typedef int +(*virDrvDomainGetTime)(virDomainPtr dom, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags); + +typedef int +(*virDrvDomainSetTime)(virDomainPtr dom, + virTypedParameterPtr params, + int nparams, + unsigned int flags); + +typedef int (*virDrvDomainLxcOpenNamespace)(virDomainPtr dom, int **fdlist, unsigned int flags); @@ -1354,6 +1366,8 @@ struct _virDriver { virDrvNodeSetMemoryParameters nodeSetMemoryParameters; virDrvNodeGetCPUMap nodeGetCPUMap; virDrvDomainFSTrim domainFSTrim; + virDrvDomainGetTime domainGetTime; + virDrvDomainSetTime domainSetTime; virDrvDomainSendProcessSignal domainSendProcessSignal; virDrvDomainLxcOpenNamespace domainLxcOpenNamespace; virDrvDomainMigrateBegin3Params domainMigrateBegin3Params; diff --git a/src/libvirt.c b/src/libvirt.c index 4454829..3befd2d 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -20658,3 +20658,99 @@ virDomainFSTrim(virDomainPtr dom, virDispatchError(dom->conn); return -1; } + +/** + * virDomainGetTime: + * @dom: a domain object + * @params: where to store time + * @nparams: numnber of items in @params + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Extract information about guest time and store it into + * @params. The return value is a superset of virDomainTimeType + * items. + * + * Please note that some hypervisors may require guest agent to + * be configured and running in order to run this API. + * + * Returns 0 on success, -1 otherwise. + */ +int +virDomainGetTime(virDomainPtr dom, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags) +{ + VIR_DOMAIN_DEBUG(dom, "params=%p, nparams=%d, flags=%x", + params, nparams ? *nparams : -1, flags); + + virResetLastError(); + + virCheckDomainReturn(dom, -1); + virCheckNonNullArgGoto(params, error); + virCheckNonNullArgGoto(nparams, error); + + if (dom->conn->driver->domainGetTime) { + int ret = dom->conn->driver->domainGetTime(dom, params, + nparams, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(dom->conn); + return -1; +} + +/** + * virDomainSetTime: + * @dom: a domain object, + * @params: pointer to array of virDomainTimeType items + * @nparams: the size of @params + * @flags: bitwise-OR of virDomainSetTimeFlags + * + * When a domain is suspended or restored from a file the + * domain's OS has no idea that there was a big gap in the time. + * Depending on how long the gap was, NTP might not be able to + * resynchronize the guest. + * + * This API tries to set guest time to the given value. + * + * Please note that some hypervisors may require guest agent to + * be configured and running in order to be able to run this API. + * + * Returns 0 on success, -1 otherwise. + */ +int +virDomainSetTime(virDomainPtr dom, + virTypedParameterPtr params, + int nparams, + unsigned int flags) +{ + VIR_DOMAIN_DEBUG(dom, "params=%p, nparams=%d, flags=%x", + params, nparams, flags); + + virResetLastError(); + + virCheckDomainReturn(dom, -1); + + if (virTypedParameterValidateSet(dom->conn, params, nparams) < 0) + goto error; + + if (dom->conn->driver->domainSetTime) { + int ret = dom->conn->driver->domainSetTime(dom, params, + nparams, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(dom->conn); + return -1; +} diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 9ab0c92..bccdd47 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -650,5 +650,11 @@ LIBVIRT_1.2.3 { virDomainCoreDumpWithFormat; } LIBVIRT_1.2.1; +LIBVIRT_1.2.4 { + global: + virDomainGetTime; + virDomainSetTime; +} LIBVIRT_1.2.3; + # .... define new API here using predicted next version number .... -- 1.9.0
This is also adding new ACL permission to check 'set_time'. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- daemon/remote.c | 50 ++++++++++++++++++++++++++++++++++++++++++++ src/access/viraccessperm.c | 2 +- src/access/viraccessperm.h | 7 ++++++- src/remote/remote_driver.c | 47 +++++++++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 32 +++++++++++++++++++++++++++- src/remote_protocol-structs | 20 ++++++++++++++++++ 6 files changed, 155 insertions(+), 3 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 8476961..be4d125 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -6078,6 +6078,56 @@ qemuDispatchConnectDomainMonitorEventDeregister(virNetServerPtr server ATTRIBUTE } +static int +remoteDispatchDomainGetTime(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_get_time_args *args, + remote_domain_get_time_ret *ret) +{ + virDomainPtr dom = NULL; + virTypedParameterPtr params = NULL; + int nparams = 0; + int rv = -1; + struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(dom = get_nonnull_domain(priv->conn, args->dom))) + goto cleanup; + + if (virDomainGetTime(dom, ¶ms, &nparams, args->flags) < 0) + goto cleanup; + + if (nparams > REMOTE_DOMAIN_TIME_MAX) { + virReportError(VIR_ERR_RPC, + _("Too many time fields '%d' for limit '%d'"), + nparams, REMOTE_DOMAIN_TIME_MAX); + goto cleanup; + } + + if (remoteSerializeTypedParameters(params, nparams, + &ret->params.params_val, + &ret->params.params_len, + 0) < 0) + goto cleanup; + + rv = 0; + + cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + virTypedParamsFree(params, nparams); + if (dom) + virDomainFree(dom); + return rv; +} + + /*----- Helpers. -----*/ /* get_nonnull_domain and get_nonnull_network turn an on-wire diff --git a/src/access/viraccessperm.c b/src/access/viraccessperm.c index d517c66..bbcb6c1 100644 --- a/src/access/viraccessperm.c +++ b/src/access/viraccessperm.c @@ -42,7 +42,7 @@ VIR_ENUM_IMPL(virAccessPermDomain, "init_control", "inject_nmi", "send_input", "send_signal", "fs_trim", "block_read", "block_write", "mem_read", "open_graphics", "open_device", "screenshot", - "open_namespace"); + "open_namespace", "set_time"); VIR_ENUM_IMPL(virAccessPermInterface, VIR_ACCESS_PERM_INTERFACE_LAST, diff --git a/src/access/viraccessperm.h b/src/access/viraccessperm.h index 6d14f05..6fa0f01 100644 --- a/src/access/viraccessperm.h +++ b/src/access/viraccessperm.h @@ -282,13 +282,18 @@ typedef enum { */ VIR_ACCESS_PERM_DOMAIN_SCREENSHOT, /* Trigger a screen shot */ - /** * @desc: Open domain namespace * @message: Opening domain namespaces requires authorization */ VIR_ACCESS_PERM_DOMAIN_OPEN_NAMESPACE, + /** + * @desc: Write domain time + * @message: Setting the domain time requires authorization + */ + VIR_ACCESS_PERM_DOMAIN_SET_TIME, + VIR_ACCESS_PERM_DOMAIN_LAST, } virAccessPermDomain; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index ed7dde6..ae84bd1 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -7468,6 +7468,51 @@ remoteDomainCreateWithFiles(virDomainPtr dom, } +static int +remoteDomainGetTime(virDomainPtr dom, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags) +{ + int rv = -1; + remote_domain_get_time_args args; + remote_domain_get_time_ret ret; + struct private_data *priv = dom->conn->privateData; + + remoteDriverLock(priv); + + make_nonnull_domain(&args.dom, dom); + args.flags = flags; + + memset(&ret, 0, sizeof(ret)); + if (call(dom->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_TIME, + (xdrproc_t) xdr_remote_domain_get_time_args, (char *) &args, + (xdrproc_t) xdr_remote_domain_get_time_ret, (char *) &ret) == -1) + goto done; + + if (ret.params.params_len > REMOTE_DOMAIN_TIME_MAX) { + virReportError(VIR_ERR_RPC, + _("Too many time fields '%d' for limit '%d'"), + ret.params.params_len, + REMOTE_DOMAIN_TIME_MAX); + goto cleanup; + } + + if (remoteDeserializeTypedParameters(ret.params.params_val, + ret.params.params_len, + 0, params, nparams) < 0) + goto cleanup; + + rv = 0; + cleanup: + xdr_free((xdrproc_t) xdr_remote_domain_get_time_ret, (char *) &ret); + + done: + remoteDriverUnlock(priv); + return rv; +} + + /* get_nonnull_domain and get_nonnull_network turn an on-wire * (name, uuid) pair into virDomainPtr or virNetworkPtr object. * These can return NULL if underlying memory allocations fail, @@ -7800,6 +7845,8 @@ static virDriver remote_driver = { .domainMigrateFinish3Params = remoteDomainMigrateFinish3Params, /* 1.1.0 */ .domainMigrateConfirm3Params = remoteDomainMigrateConfirm3Params, /* 1.1.0 */ .connectGetCPUModelNames = remoteConnectGetCPUModelNames, /* 1.1.3 */ + .domainGetTime = remoteDomainGetTime, /* 1.2.4 */ + .domainSetTime = remoteDomainSetTime, /* 1.2.4 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 6c445cc..9551c6b 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -235,6 +235,9 @@ const REMOTE_DOMAIN_JOB_STATS_MAX = 64; /* Upper limit on number of CPU models */ const REMOTE_CONNECT_CPU_MODELS_MAX = 8192; +/* Upper limit on number of get/set time parameters */ +const REMOTE_DOMAIN_TIME_MAX = 64; + /* UUID. VIR_UUID_BUFLEN definition comes from libvirt.h */ typedef opaque remote_uuid[VIR_UUID_BUFLEN]; @@ -2852,6 +2855,21 @@ struct remote_domain_fstrim_args { unsigned int flags; }; +struct remote_domain_get_time_args { + remote_nonnull_domain dom; + unsigned int flags; +}; + +struct remote_domain_get_time_ret { + remote_typed_param params<REMOTE_DOMAIN_TIME_MAX>; +}; + +struct remote_domain_set_time_args { + remote_nonnull_domain dom; + remote_typed_param params<REMOTE_DOMAIN_TIME_MAX>; + unsigned int flags; +}; + struct remote_domain_migrate_begin3_params_args { remote_nonnull_domain dom; remote_typed_param params<REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX>; @@ -5275,5 +5293,17 @@ enum remote_procedure { * @generate: both * @acl: domain:core_dump */ - REMOTE_PROC_DOMAIN_CORE_DUMP_WITH_FORMAT = 334 + REMOTE_PROC_DOMAIN_CORE_DUMP_WITH_FORMAT = 334, + + /** + * @generate: none + * @acl: domain:read + */ + REMOTE_PROC_DOMAIN_GET_TIME = 335, + + /** + * @generate: both + * @acl: domain:set_time + */ + REMOTE_PROC_DOMAIN_SET_TIME = 336 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 456d0da..fcad3d2 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2290,6 +2290,24 @@ struct remote_domain_fstrim_args { uint64_t minimum; u_int flags; }; +struct remote_domain_get_time_args { + remote_nonnull_domain dom; + u_int flags; +}; +struct remote_domain_get_time_ret { + struct { + u_int params_len; + remote_typed_param * params_val; + } params; +}; +struct remote_domain_set_time_args { + remote_nonnull_domain dom; + struct { + u_int params_len; + remote_typed_param * params_val; + } params; + u_int flags; +}; struct remote_domain_migrate_begin3_params_args { remote_nonnull_domain dom; struct { @@ -2762,4 +2780,6 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_EVENT_CALLBACK_PMSUSPEND_DISK = 332, REMOTE_PROC_DOMAIN_EVENT_CALLBACK_DEVICE_REMOVED = 333, REMOTE_PROC_DOMAIN_CORE_DUMP_WITH_FORMAT = 334, + REMOTE_PROC_DOMAIN_GET_TIME = 335, + REMOTE_PROC_DOMAIN_SET_TIME = 336, }; -- 1.9.0
These APIs are exposed under new virsh command 'domtime' which both gets and sets (not at the same time of course :)). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-domain-monitor.c | 143 +++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 16 +++++ 2 files changed, 159 insertions(+) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 18d551a..a3d6b6e 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -1356,6 +1356,143 @@ cmdDomstate(vshControl *ctl, const vshCmd *cmd) } /* + * "domtime" command + */ +static const vshCmdInfo info_domtime[] = { + {.name = "help", + .data = N_("domain time") + }, + {.name = "desc", + .data = N_("Gets or sets a domain time") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_domtime[] = { + {.name = "domain", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("domain name, id or uuid") + }, + {.name = "now", + .type = VSH_OT_BOOL, + .help = N_("set current host time") + }, + {.name = "pretty", + .type = VSH_OT_BOOL, + .help = N_("print domain's time in human readable form") + }, + {.name = "sync", + .type = VSH_OT_BOOL, + .help = N_("instead of setting given time, synchronize from domain's RTC"), + }, + {.name = "time", + .type = VSH_OT_INT, + .help = N_("time to set") + }, + {.name = NULL} +}; + +static bool +cmdDomTime(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + bool ret = false; + bool now = vshCommandOptBool(cmd, "now"); + bool pretty = vshCommandOptBool(cmd, "pretty"); + bool sync = vshCommandOptBool(cmd, "sync"); + long long guest_time; + virTypedParameterPtr params = NULL; + int maxparams = 0, nparams = 0; + unsigned int flags = 0; + bool doSet = false; + int rv; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + rv = vshCommandOptLongLong(cmd, "time", &guest_time); + + if (rv < 0) { + /* invalid integer format */ + vshError(ctl, "%s", + _("Unable to parse integer parameter to --time.")); + goto cleanup; + } else if (rv > 0) { + /* --time is used, so set time instead of get time. + * However, --time and --now are mutually exclusive. */ + if (now) { + vshError(ctl, _("--time and --now are mutually exclusive")); + goto cleanup; + } + + /* Neither is --time and --sync */ + if (sync) { + vshError(ctl, _("--time and --sync are mutually exclusive")); + goto cleanup; + + } + doSet = true; + } + + if (sync && now) { + vshError(ctl, _("--sync and --now are mutually exclusive")); + goto cleanup; + } + + /* --now or --sync means setting */ + doSet |= now | sync; + + if (doSet) { + if (now && ((guest_time = time(NULL)) == (time_t) -1)) { + vshError(ctl, _("Unable to get current time")); + goto cleanup; + } + + if (sync) { + flags |= VIR_DOMAIN_TIME_SYNC; + } else { + if (virTypedParamsAddLLong(¶ms, &nparams, &maxparams, + VIR_DOMAIN_TIME_SECONDS, guest_time) < 0) + goto cleanup; + } + + if (virDomainSetTime(dom, params, nparams, flags) < 0) + goto cleanup; + + } else { + if (virDomainGetTime(dom, ¶ms, &nparams, flags) < 0) + goto cleanup; + + if (virTypedParamsGetLLong(params, nparams, + VIR_DOMAIN_TIME_SECONDS, &guest_time) < 0) + goto cleanup; + + if (pretty) { + char timestr[100]; + time_t cur_time = guest_time; + struct tm time_info; + + if (!gmtime_r(&cur_time, &time_info)) { + vshError(ctl, _("Unable to format time")); + goto cleanup; + } + strftime(timestr, sizeof(timestr), "%Y-%m-%d %H:%M:%S", &time_info); + + vshPrint(ctl, _("Time: %s"), timestr); + } else { + vshPrint(ctl, _("Time: %llu"), guest_time); + } + } + + ret = true; + cleanup: + virTypedParamsFree(params, nparams); + virDomainFree(dom); + return ret; +} + +/* * "list" command */ static const vshCmdInfo info_list[] = { @@ -1911,6 +2048,12 @@ const vshCmdDef domMonitoringCmds[] = { .info = info_domstate, .flags = 0 }, + {.name = "domtime", + .handler = cmdDomTime, + .opts = opts_domtime, + .info = info_domtime, + .flags = 0 + }, {.name = "list", .handler = cmdList, .opts = opts_list, diff --git a/tools/virsh.pod b/tools/virsh.pod index ba2da20..dd0ea57 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -994,6 +994,22 @@ Returns state of an interface to VMM used to control a domain. For states other than "ok" or "error" the command also prints number of seconds elapsed since the control interface entered its current state. +=item B<domtime> I<domain> { [I<--now>] [I<--pretty>] [I<--sync>] +[I<--time> B<time>] } + +Gets or sets the domain's system time. When run without any arguments +(but I<domain>), the current domain's system time is printed out. The +I<--pretty> modifier can be used to print the time in more human +readable form. When I<--time> B<time> is specified, the domain's time is +not get but set instead. The I<--now> modifier acts like if it was an +alias for I<--time> B<$now>, which means it sets the time that is +currently on the host virsh is running at. In both cases (setting and +getting), time is in seconds relative to Epoch of 1970-01-01 in UTC. +The I<--sync> modifies the set behavior a bit: The time passed is +ignored, but the time to set is read from domain's RTC instead. Please +note, that some hypervisors may require a guest agent to be configured +in order to get or set the guest time. + =item B<domxml-from-native> I<format> I<config> Convert the file I<config> in the native guest configuration format -- 1.9.0
One caveat though, qemu-ga is expecting time and returning time in nanoseconds. With all the buffering and propagation delay, the time is already wrong once it gets to the qemu-ga, but there's nothing we can do about it. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_agent.c | 95 +++++++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 6 +++ src/qemu/qemu_driver.c | 139 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 240 insertions(+) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 4c83d7d..cde67cc 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1647,3 +1647,98 @@ qemuAgentUpdateCPUInfo(unsigned int nvcpus, return 0; } + + +int +qemuAgentGetTime(qemuAgentPtr mon, + long long *getTime) +{ + int ret = -1; + unsigned long long json_time; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + cmd = qemuAgentMakeCommand("guest-get-time", + NULL); + if (!cmd) + return ret; + + if (qemuAgentCommand(mon, cmd, &reply, + VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) + goto cleanup; + + if (!reply || qemuAgentCheckError(cmd, reply) < 0) + goto cleanup; + + if (virJSONValueObjectGetNumberUlong(reply, "return", &json_time) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed return value")); + goto cleanup; + } + + /* guest agent returns time in nanoseconds, + * we need it in seconds here */ + *getTime = json_time / 1000000000LL; + ret = 0; + + cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + + +/** + * qemuAgentSetTime: + * @setTime: time to set + * @sync: let guest agent to read domain's RTC (@setTime is ignored) + */ +int +qemuAgentSetTime(qemuAgentPtr mon, + long long setTime, + bool sync) +{ + int ret = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + if (sync) { + cmd = qemuAgentMakeCommand("guest-set-time", NULL); + } else { + /* guest agent expect time with nanosecond granularity. + * Impressing. */ + long long json_time; + + /* Check if we overflow. For some reason qemu doesn't handle unsigned + * long long on the monitor well as it silently truncates numbers to + * signed long long. Therefore we must check overflow against LLONG_MAX + * not ULLONG_MAX. */ + if (setTime > LLONG_MAX / 1000000000LL) { + virReportError(VIR_ERR_INVALID_ARG, + _("Time '%lld' is too big for guest agent"), + setTime); + return ret; + } + + json_time = setTime * 1000000000LL; + cmd = qemuAgentMakeCommand("guest-set-time", + "I:time", json_time, + NULL); + } + + if (!cmd) + return ret; + + if (qemuAgentCommand(mon, cmd, &reply, + VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) + goto cleanup; + + if (!reply || qemuAgentCheckError(cmd, reply) < 0) + goto cleanup; + + ret = 0; + cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index 5fbacdb..d711acf 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -97,4 +97,10 @@ int qemuAgentSetVCPUs(qemuAgentPtr mon, qemuAgentCPUInfoPtr cpus, size_t ncpus); int qemuAgentUpdateCPUInfo(unsigned int nvcpus, qemuAgentCPUInfoPtr cpuinfo, int ncpuinfo); + +int qemuAgentGetTime(qemuAgentPtr mon, + long long *getTime); +int qemuAgentSetTime(qemuAgentPtr mon, + long long setTime, + bool sync); #endif /* __QEMU_AGENT_H__ */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9a2de12..98b7220 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16662,6 +16662,143 @@ qemuConnectGetCPUModelNames(virConnectPtr conn, return cpuGetModels(arch, models); } +static int +qemuDomainGetTime(virDomainPtr dom, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm = NULL; + qemuDomainObjPrivatePtr priv; + virTypedParameterPtr par = NULL; + int maxpar = 0; + int npar = 0; + int ret = -1; + int rv; + long long guest_time; + + virCheckFlags(0, ret); + + if (!(vm = qemuDomObjFromDomain(dom))) + return ret; + + if (virDomainGetTimeEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + priv = vm->privateData; + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto endjob; + } + + if (!qemuDomainAgentAvailable(priv, true)) + goto endjob; + + qemuDomainObjEnterAgent(vm); + rv = qemuAgentGetTime(priv->agent, &guest_time); + qemuDomainObjExitAgent(vm); + + if (rv < 0) + goto endjob; + + if (virTypedParamsAddLLong(&par, &npar, &maxpar, + VIR_DOMAIN_TIME_SECONDS, guest_time) < 0) + goto endjob; + + *params = par; + *nparams = npar; + ret = 0; + + endjob: + if (!qemuDomainObjEndJob(driver, vm)) + vm = NULL; + + cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +} + +static int +qemuDomainSetTime(virDomainPtr dom, + virTypedParameterPtr params, + int nparams, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + qemuDomainObjPrivatePtr priv; + virDomainObjPtr vm; + long long guest_time; + bool sync = flags & VIR_DOMAIN_TIME_SYNC; + int ret = -1; + int rv; + + virCheckFlags(VIR_DOMAIN_TIME_SYNC, ret); + + if (virTypedParamsValidate(params, nparams, + VIR_DOMAIN_TIME_SECONDS, + VIR_TYPED_PARAM_ULLONG, + NULL) < 0) + return ret; + + if (!(vm = qemuDomObjFromDomain(dom))) + return ret; + + if (virDomainSetTimeEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + priv = vm->privateData; + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto endjob; + } + + if (!qemuDomainAgentAvailable(priv, true)) + goto endjob; + + if (!sync) { + rv = virTypedParamsGetLLong(params, nparams, + VIR_DOMAIN_TIME_SECONDS, &guest_time); + if (rv < 0) + goto endjob; + + if (rv == 0) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("no time specified")); + goto endjob; + } + } + + qemuDomainObjEnterAgent(vm); + rv = qemuAgentSetTime(priv->agent, guest_time, sync); + qemuDomainObjExitAgent(vm); + + if (rv < 0) + goto endjob; + + ret = 0; + + endjob: + if (!qemuDomainObjEndJob(driver, vm)) + vm = NULL; + + cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +} + static virDriver qemuDriver = { .no = VIR_DRV_QEMU, @@ -16853,6 +16990,8 @@ static virDriver qemuDriver = { .domainMigrateFinish3Params = qemuDomainMigrateFinish3Params, /* 1.1.0 */ .domainMigrateConfirm3Params = qemuDomainMigrateConfirm3Params, /* 1.1.0 */ .connectGetCPUModelNames = qemuConnectGetCPUModelNames, /* 1.1.3 */ + .domainGetTime = qemuDomainGetTime, /* 1.2.4 */ + .domainSetTime = qemuDomainSetTime, /* 1.2.4 */ }; -- 1.9.0
On 04/02/2014 03:34 AM, Michal Privoznik wrote:
If we'd come up with a struct to interpret the time, it's written in stone and there's nothing we can do about it. Even if we break up the struct into function arguments. However, we can use typed parameters and have the API extensible. For example, I'm implementing seconds granularity only for now. If later somebody feels like he/she wants to use even finer granularity, we can do that by inventing a new typed param. Same goes for timezone, etc.
But what more is there to add besides seconds and nanoseconds (struct timespec)? We don't have to support full resolution of nanoseconds, and could either document that we round (possibly to as course as seconds) or reject input with non-zero nanoseconds if we don't want to support sub-seconds right away. It just feels awkward to me to have to bundle something into a virTypedParam when the most we could ever extend this to is struct timespec, so representing struct timespec via split parameters from the get-go requires less thought when using the interface. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Wed, Apr 02, 2014 at 05:44:16AM -0600, Eric Blake wrote:
On 04/02/2014 03:34 AM, Michal Privoznik wrote:
If we'd come up with a struct to interpret the time, it's written in stone and there's nothing we can do about it. Even if we break up the struct into function arguments. However, we can use typed parameters and have the API extensible. For example, I'm implementing seconds granularity only for now. If later somebody feels like he/she wants to use even finer granularity, we can do that by inventing a new typed param. Same goes for timezone, etc.
But what more is there to add besides seconds and nanoseconds (struct timespec)? We don't have to support full resolution of nanoseconds, and could either document that we round (possibly to as course as seconds) or reject input with non-zero nanoseconds if we don't want to support sub-seconds right away. It just feels awkward to me to have to bundle something into a virTypedParam when the most we could ever extend this to is struct timespec, so representing struct timespec via split parameters from the get-go requires less thought when using the interface.
I think I'm inclined to agree that we shouuld just use an long long timesecs and possibly a second long long timenanosecs. People won't normally update the timezone at the same time as updating the BIOS time and vica verca. So in the event we ever wanted to update the timezone I could see that being a separate virDomainSetGuestTimezone API. 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 :|
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Michal Privoznik