[libvirt] [PATCH 0/5] Allow time sync on domain resume
*** BLURB HERE *** Michal Privoznik (5): Introduce virDomainResumeFlags API Implement virDomainResumeFlags in all drivers qemu: Split qemuDomainSetTime into two functions qemuDomainResumeFlags: Introduce VIR_DOMAIN_RESUME_SYNC_TIME virsh: Implement virDomainResumeFlags include/libvirt/libvirt-domain.h | 6 +++ src/driver-hypervisor.h | 5 ++ src/esx/esx_driver.c | 14 +++++- src/hyperv/hyperv_driver.c | 14 +++++- src/libvirt-domain.c | 44 ++++++++++++++++ src/libvirt_public.syms | 5 ++ src/libxl/libxl_driver.c | 14 +++++- src/lxc/lxc_driver.c | 15 +++++- src/openvz/openvz_driver.c | 13 ++++- src/parallels/parallels_driver.c | 11 +++- src/phyp/phyp_driver.c | 12 ++++- src/qemu/qemu_driver.c | 106 ++++++++++++++++++++++++++------------- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 13 ++++- src/remote_protocol-structs | 5 ++ src/test/test_driver.c | 13 ++++- src/vbox/vbox_common.c | 11 +++- src/vmware/vmware_driver.c | 12 ++++- src/xen/xen_driver.c | 14 +++++- src/xenapi/xenapi_driver.c | 21 +++++++- tools/virsh-domain.c | 13 ++++- tools/virsh.pod | 10 ++-- 22 files changed, 316 insertions(+), 56 deletions(-) -- 2.0.4
The old DomainResume API lacks flags argument. This is unfortunate, because there may exist some use cases where an additional work could be done on domain resume. However, without flags it's not possible. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- include/libvirt/libvirt-domain.h | 2 ++ src/driver-hypervisor.h | 5 +++++ src/libvirt-domain.c | 44 ++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 +++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 13 +++++++++++- src/remote_protocol-structs | 5 +++++ 7 files changed, 74 insertions(+), 1 deletion(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index b28d37d..1795dd3 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -869,6 +869,8 @@ int virDomainFree (virDomainPtr domain); */ int virDomainSuspend (virDomainPtr domain); int virDomainResume (virDomainPtr domain); +int virDomainResumeFlags (virDomainPtr domain, + unsigned int flags); int virDomainPMSuspendForDuration (virDomainPtr domain, unsigned int target, unsigned long long duration, diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index ad66629..e781475 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -116,6 +116,10 @@ typedef int (*virDrvDomainResume)(virDomainPtr domain); typedef int +(*virDrvDomainResumeFlags)(virDomainPtr domain, + unsigned int flags); + +typedef int (*virDrvDomainPMSuspendForDuration)(virDomainPtr, unsigned int target, unsigned long long duration, @@ -1205,6 +1209,7 @@ struct _virHypervisorDriver { virDrvDomainLookupByName domainLookupByName; virDrvDomainSuspend domainSuspend; virDrvDomainResume domainResume; + virDrvDomainResumeFlags domainResumeFlags; virDrvDomainPMSuspendForDuration domainPMSuspendForDuration; virDrvDomainPMWakeup domainPMWakeup; virDrvDomainShutdown domainShutdown; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 7dc3146..6dcb9ef 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -693,6 +693,50 @@ virDomainResume(virDomainPtr domain) /** + * virDomainResumeFlags: + * @domain: a domain object + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Resume a suspended domain, the process is restarted from the state where + * it was frozen by calling virDomainSuspend(). + * This function may require privileged access + * Moreover, resume may not be supported if domain is in some + * special state like VIR_DOMAIN_PMSUSPENDED. + * + * Returns 0 in case of success and -1 in case of failure. + */ +int +virDomainResumeFlags(virDomainPtr domain, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + conn = domain->conn; + + virCheckReadOnlyGoto(conn->flags, error); + + if (conn->driver->domainResumeFlags) { + int ret; + ret = conn->driver->domainResumeFlags(domain, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); + return -1; +} + + +/** * virDomainPMSuspendForDuration: * @dom: a domain object * @target: a value from virNodeSuspendTarget diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 5f95802..2daff56 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -684,4 +684,9 @@ LIBVIRT_1.2.9 { virNodeAllocPages; } LIBVIRT_1.2.8; +LIBVIRT_1.2.10 { + global: + virDomainResumeFlags; +} LIBVIRT_1.2.9; + # .... define new API here using predicted next version number .... diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 65c04d9..3dc1c8f 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8240,6 +8240,7 @@ static virHypervisorDriver hypervisor_driver = { .connectGetDomainCapabilities = remoteConnectGetDomainCapabilities, /* 1.2.7 */ .connectGetAllDomainStats = remoteConnectGetAllDomainStats, /* 1.2.8 */ .nodeAllocPages = remoteNodeAllocPages, /* 1.2.9 */ + .domainResumeFlags = remoteDomainResumeFlags, /* 1.2.10 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index db12cda..dfd816e 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -803,6 +803,11 @@ struct remote_domain_resume_args { remote_nonnull_domain dom; }; +struct remote_domain_resume_flags_args { + remote_nonnull_domain dom; + unsigned int flags; +}; + struct remote_domain_pm_suspend_for_duration_args { remote_nonnull_domain dom; unsigned int target; @@ -5505,5 +5510,11 @@ enum remote_procedure { * @generate: none * @acl: connect:write */ - REMOTE_PROC_NODE_ALLOC_PAGES = 347 + REMOTE_PROC_NODE_ALLOC_PAGES = 347, + + /** + * @generate: both + * @acl: domain:suspend + */ + REMOTE_PROC_DOMAIN_RESUME_FLAGS = 348 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 362baf9..ce9e35d 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -464,6 +464,10 @@ struct remote_domain_suspend_args { struct remote_domain_resume_args { remote_nonnull_domain dom; }; +struct remote_domain_resume_flags_args { + remote_nonnull_domain dom; + u_int flags; +}; struct remote_domain_pm_suspend_for_duration_args { remote_nonnull_domain dom; u_int target; @@ -2927,4 +2931,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_BLOCK_COPY = 345, REMOTE_PROC_DOMAIN_EVENT_CALLBACK_TUNABLE = 346, REMOTE_PROC_NODE_ALLOC_PAGES = 347, + REMOTE_PROC_DOMAIN_RESUME_FLAGS = 348, }; -- 2.0.4
On 11/05/14 13:35, Michal Privoznik wrote:
The old DomainResume API lacks flags argument. This is unfortunate, because there may exist some use cases where an additional work could be done on domain resume. However, without flags it's not possible.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- include/libvirt/libvirt-domain.h | 2 ++ src/driver-hypervisor.h | 5 +++++ src/libvirt-domain.c | 44 ++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 +++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 13 +++++++++++- src/remote_protocol-structs | 5 +++++ 7 files changed, 74 insertions(+), 1 deletion(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 7dc3146..6dcb9ef 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -693,6 +693,50 @@ virDomainResume(virDomainPtr domain)
/** + * virDomainResumeFlags: + * @domain: a domain object + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Resume a suspended domain, the process is restarted from the state where + * it was frozen by calling virDomainSuspend(). + * This function may require privileged access + * Moreover, resume may not be supported if domain is in some + * special state like VIR_DOMAIN_PMSUSPENDED. + * + * Returns 0 in case of success and -1 in case of failure. + */ +int +virDomainResumeFlags(virDomainPtr domain, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + conn = domain->conn; + + virCheckReadOnlyGoto(conn->flags, error); + + if (conn->driver->domainResumeFlags) { + int ret; + ret = conn->driver->domainResumeFlags(domain, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); + return -1; +} + + +/** * virDomainPMSuspendForDuration: * @dom: a domain object * @target: a value from virNodeSuspendTarget diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 5f95802..2daff56 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -684,4 +684,9 @@ LIBVIRT_1.2.9 { virNodeAllocPages; } LIBVIRT_1.2.8;
+LIBVIRT_1.2.10 {
It's 1.2.11 already.
+ global: + virDomainResumeFlags; +} LIBVIRT_1.2.9; + # .... define new API here using predicted next version number .... diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 65c04d9..3dc1c8f 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8240,6 +8240,7 @@ static virHypervisorDriver hypervisor_driver = { .connectGetDomainCapabilities = remoteConnectGetDomainCapabilities, /* 1.2.7 */ .connectGetAllDomainStats = remoteConnectGetAllDomainStats, /* 1.2.8 */ .nodeAllocPages = remoteNodeAllocPages, /* 1.2.9 */ + .domainResumeFlags = remoteDomainResumeFlags, /* 1.2.10 */
Same here.
};
static virNetworkDriver network_driver = {
Looks reasonable. ACK, but please hold off the push until we agree on the need of this (see Re: 4/5, 5/5 of this series) Peter
This practically boils down to: 1) rename DomainResume implementation to DomainResumeFlags 2) make DomainResume call DomainResumeFlags(dom, 0); Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/esx/esx_driver.c | 14 +++++++++++++- src/hyperv/hyperv_driver.c | 14 +++++++++++++- src/libxl/libxl_driver.c | 14 ++++++++++++-- src/lxc/lxc_driver.c | 15 +++++++++++++-- src/openvz/openvz_driver.c | 13 ++++++++++++- src/parallels/parallels_driver.c | 11 ++++++++++- src/phyp/phyp_driver.c | 12 +++++++++++- src/qemu/qemu_driver.c | 15 +++++++++++++-- src/test/test_driver.c | 13 ++++++++++++- src/vbox/vbox_common.c | 11 ++++++++++- src/vmware/vmware_driver.c | 12 +++++++++++- src/xen/xen_driver.c | 14 ++++++++++++-- src/xenapi/xenapi_driver.c | 21 +++++++++++++++++++-- 13 files changed, 161 insertions(+), 18 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 0770e89..aaca532 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -1780,7 +1780,8 @@ esxDomainSuspend(virDomainPtr domain) static int -esxDomainResume(virDomainPtr domain) +esxDomainResumeFlags(virDomainPtr domain, + unsigned int flags) { int result = -1; esxPrivate *priv = domain->conn->privateData; @@ -1791,6 +1792,8 @@ esxDomainResume(virDomainPtr domain) esxVI_TaskInfoState taskInfoState; char *taskInfoErrorMessage = NULL; + virCheckFlags(0, -1); + if (esxVI_EnsureSession(priv->primary) < 0) { return -1; } @@ -1838,6 +1841,14 @@ esxDomainResume(virDomainPtr domain) static int +esxDomainResume(virDomainPtr domain) +{ + return esxDomainResumeFlags(domain, 0); +} + + + +static int esxDomainShutdownFlags(virDomainPtr domain, unsigned int flags) { int result = -1; @@ -5310,6 +5321,7 @@ static virHypervisorDriver esxDriver = { .domainLookupByName = esxDomainLookupByName, /* 0.7.0 */ .domainSuspend = esxDomainSuspend, /* 0.7.0 */ .domainResume = esxDomainResume, /* 0.7.0 */ + .domainResumeFlags = esxDomainResumeFlags, /* 1.2.10 */ .domainShutdown = esxDomainShutdown, /* 0.7.0 */ .domainShutdownFlags = esxDomainShutdownFlags, /* 0.9.10 */ .domainReboot = esxDomainReboot, /* 0.7.0 */ diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 5ffcb85..ac8f745 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -569,12 +569,15 @@ hypervDomainSuspend(virDomainPtr domain) static int -hypervDomainResume(virDomainPtr domain) +hypervDomainResumeFlags(virDomainPtr domain, + unsigned int flags) { int result = -1; hypervPrivate *priv = domain->conn->privateData; Msvm_ComputerSystem *computerSystem = NULL; + virCheckFlags(0, -1); + if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0) { goto cleanup; } @@ -598,6 +601,14 @@ hypervDomainResume(virDomainPtr domain) static int +hypervDomainResume(virDomainPtr domain) +{ + return hypervDomainResumeFlags(domain, 0); +} + + + +static int hypervDomainDestroyFlags(virDomainPtr domain, unsigned int flags) { int result = -1; @@ -1370,6 +1381,7 @@ static virHypervisorDriver hypervDriver = { .domainLookupByName = hypervDomainLookupByName, /* 0.9.5 */ .domainSuspend = hypervDomainSuspend, /* 0.9.5 */ .domainResume = hypervDomainResume, /* 0.9.5 */ + .domainResumeFlags = hypervDomainResumeFlags, /* 1.2.10 */ .domainDestroy = hypervDomainDestroy, /* 0.9.5 */ .domainDestroyFlags = hypervDomainDestroyFlags, /* 0.9.5 */ .domainGetOSType = hypervDomainGetOSType, /* 0.9.5 */ diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index d2c077c..342d0a5 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -814,7 +814,8 @@ libxlDomainSuspend(virDomainPtr dom) static int -libxlDomainResume(virDomainPtr dom) +libxlDomainResumeFlags(virDomainPtr dom, + unsigned int flags) { libxlDriverPrivatePtr driver = dom->conn->privateData; libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); @@ -823,10 +824,12 @@ libxlDomainResume(virDomainPtr dom) virObjectEventPtr event = NULL; int ret = -1; + virCheckFlags(0, -1); + if (!(vm = libxlDomObjFromDomain(dom))) goto cleanup; - if (virDomainResumeEnsureACL(dom->conn, vm->def) < 0) + if (virDomainResumeFlagsEnsureACL(dom->conn, vm->def) < 0) goto cleanup; if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) @@ -873,6 +876,12 @@ libxlDomainResume(virDomainPtr dom) } static int +libxlDomainResume(virDomainPtr dom) +{ + return libxlDomainResumeFlags(dom, 0); +} + +static int libxlDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { virDomainObjPtr vm; @@ -4750,6 +4759,7 @@ static virHypervisorDriver libxlDriver = { .domainLookupByName = libxlDomainLookupByName, /* 0.9.0 */ .domainSuspend = libxlDomainSuspend, /* 0.9.0 */ .domainResume = libxlDomainResume, /* 0.9.0 */ + .domainResumeFlags = libxlDomainResumeFlags, /* 1.2.10 */ .domainShutdown = libxlDomainShutdown, /* 0.9.0 */ .domainShutdownFlags = libxlDomainShutdownFlags, /* 0.9.10 */ .domainReboot = libxlDomainReboot, /* 0.9.0 */ diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 6a58d50..93fd739 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -3367,7 +3367,9 @@ static int lxcDomainSuspend(virDomainPtr dom) return ret; } -static int lxcDomainResume(virDomainPtr dom) +static int +lxcDomainResumeFlags(virDomainPtr dom, + unsigned int flags) { virLXCDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; @@ -3376,12 +3378,14 @@ static int lxcDomainResume(virDomainPtr dom) virLXCDomainObjPrivatePtr priv; virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver); + virCheckFlags(0, -1); + if (!(vm = lxcDomObjFromDomain(dom))) goto cleanup; priv = vm->privateData; - if (virDomainResumeEnsureACL(dom->conn, vm->def) < 0) + if (virDomainResumeFlagsEnsureACL(dom->conn, vm->def) < 0) goto cleanup; if (!virDomainObjIsActive(vm)) { @@ -3418,6 +3422,12 @@ static int lxcDomainResume(virDomainPtr dom) } static int +lxcDomainResume(virDomainPtr dom) +{ + return lxcDomainResumeFlags(dom, 0); +} + +static int lxcDomainOpenConsole(virDomainPtr dom, const char *dev_name, virStreamPtr st, @@ -5737,6 +5747,7 @@ static virHypervisorDriver lxcDriver = { .domainLookupByName = lxcDomainLookupByName, /* 0.4.2 */ .domainSuspend = lxcDomainSuspend, /* 0.7.2 */ .domainResume = lxcDomainResume, /* 0.7.2 */ + .domainResumeFlags = lxcDomainResumeFlags, /* 1.2.10 */ .domainDestroy = lxcDomainDestroy, /* 0.4.4 */ .domainDestroyFlags = lxcDomainDestroyFlags, /* 0.9.4 */ .domainGetOSType = lxcDomainGetOSType, /* 0.4.2 */ diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index a0346b4..d74e64b 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -633,13 +633,17 @@ static int openvzDomainSuspend(virDomainPtr dom) return ret; } -static int openvzDomainResume(virDomainPtr dom) +static int +openvzDomainResumeFlags(virDomainPtr dom, + unsigned int flags) { struct openvz_driver *driver = dom->conn->privateData; virDomainObjPtr vm; const char *prog[] = {VZCTL, "--quiet", "chkpnt", PROGRAM_SENTINEL, "--resume", NULL}; int ret = -1; + virCheckFlags(0, -1); + openvzDriverLock(driver); vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); openvzDriverUnlock(driver); @@ -673,6 +677,12 @@ static int openvzDomainResume(virDomainPtr dom) } static int +openvzDomainResume(virDomainPtr dom) +{ + return openvzDomainResumeFlags(dom, 0); +} + +static int openvzDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { @@ -2577,6 +2587,7 @@ static virHypervisorDriver openvzDriver = { .domainLookupByName = openvzDomainLookupByName, /* 0.3.1 */ .domainSuspend = openvzDomainSuspend, /* 0.8.3 */ .domainResume = openvzDomainResume, /* 0.8.3 */ + .domainResumeFlags = openvzDomainResumeFlags, /* 1.2.10 */ .domainShutdown = openvzDomainShutdown, /* 0.3.1 */ .domainShutdownFlags = openvzDomainShutdownFlags, /* 0.9.10 */ .domainReboot = openvzDomainReboot, /* 0.3.1 */ diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 808dc4a..f5c1849 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -1474,14 +1474,22 @@ static int parallelsResume(virDomainObjPtr privdom) } static int -parallelsDomainResume(virDomainPtr domain) +parallelsDomainResumeFlags(virDomainPtr domain, + unsigned int flags) { + virCheckFlags(0, -1); return parallelsDomainChangeState(domain, VIR_DOMAIN_PAUSED, "paused", parallelsResume, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_UNPAUSED); } +static int +parallelsDomainResume(virDomainPtr domain) +{ + return parallelsDomainResumeFlags(domain, 0); +} + static int parallelsStart(virDomainObjPtr privdom) { return parallelsCmdRun(PRLCTL, "start", PARALLELS_UUID(privdom), NULL); @@ -2490,6 +2498,7 @@ static virHypervisorDriver parallelsDriver = { .domainGetVcpus = parallelsDomainGetVcpus, /* 1.2.6 */ .domainSuspend = parallelsDomainSuspend, /* 0.10.0 */ .domainResume = parallelsDomainResume, /* 0.10.0 */ + .domainResumeFlags = parallelsDomainResumeFlags, /* 1.2.10 */ .domainDestroy = parallelsDomainDestroy, /* 0.10.0 */ .domainShutdown = parallelsDomainShutdown, /* 0.10.0 */ .domainCreate = parallelsDomainCreate, /* 0.10.0 */ diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 09617c8..8b4cc40 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -3308,7 +3308,8 @@ phypDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) } static int -phypDomainResume(virDomainPtr dom) +phypDomainResumeFlags(virDomainPtr dom, + unsigned int flags) { int result = -1; ConnectionData *connection_data = dom->conn->networkPrivateData; @@ -3320,6 +3321,8 @@ phypDomainResume(virDomainPtr dom) char *ret = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; + virCheckFlags(0, -1); + virBufferAddLit(&buf, "chsysstate"); if (system_type == HMC) virBufferAsprintf(&buf, " -m %s", managed_system); @@ -3339,6 +3342,12 @@ phypDomainResume(virDomainPtr dom) } static int +phypDomainResume(virDomainPtr dom) +{ + return phypDomainResumeFlags(dom, 0); +} + +static int phypDomainReboot(virDomainPtr dom, unsigned int flags) { int result = -1; @@ -3731,6 +3740,7 @@ static virHypervisorDriver phypDriver = { .domainLookupByID = phypDomainLookupByID, /* 0.7.0 */ .domainLookupByName = phypDomainLookupByName, /* 0.7.0 */ .domainResume = phypDomainResume, /* 0.7.0 */ + .domainResumeFlags = phypDomainResumeFlags, /* 1.2.10 */ .domainShutdown = phypDomainShutdown, /* 0.7.0 */ .domainReboot = phypDomainReboot, /* 0.9.1 */ .domainDestroy = phypDomainDestroy, /* 0.7.3 */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6acaea8..b866ae8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1868,7 +1868,9 @@ static int qemuDomainSuspend(virDomainPtr dom) } -static int qemuDomainResume(virDomainPtr dom) +static int +qemuDomainResumeFlags(virDomainPtr dom, + unsigned int flags) { virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; @@ -1878,12 +1880,14 @@ static int qemuDomainResume(virDomainPtr dom) virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; + virCheckFlags(0, -1); + if (!(vm = qemuDomObjFromDomain(dom))) return -1; cfg = virQEMUDriverGetConfig(driver); - if (virDomainResumeEnsureACL(dom->conn, vm->def) < 0) + if (virDomainResumeFlagsEnsureACL(dom->conn, vm->def) < 0) goto cleanup; if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) @@ -1933,6 +1937,12 @@ static int qemuDomainResume(virDomainPtr dom) return ret; } +static int +qemuDomainResume(virDomainPtr dom) +{ + return qemuDomainResumeFlags(dom, 0); +} + static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { virQEMUDriverPtr driver = dom->conn->privateData; @@ -18618,6 +18628,7 @@ static virHypervisorDriver qemuDriver = { .domainLookupByName = qemuDomainLookupByName, /* 0.2.0 */ .domainSuspend = qemuDomainSuspend, /* 0.2.0 */ .domainResume = qemuDomainResume, /* 0.2.0 */ + .domainResumeFlags = qemuDomainResumeFlags, /* 1.2.10 */ .domainShutdown = qemuDomainShutdown, /* 0.2.0 */ .domainShutdownFlags = qemuDomainShutdownFlags, /* 0.9.10 */ .domainReboot = qemuDomainReboot, /* 0.9.3 */ diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 2afd6fe..1054121 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1921,13 +1921,17 @@ static int testDomainDestroy(virDomainPtr domain) return ret; } -static int testDomainResume(virDomainPtr domain) +static int +testDomainResumeFlags(virDomainPtr domain, + unsigned int flags) { testConnPtr privconn = domain->conn->privateData; virDomainObjPtr privdom; virObjectEventPtr event = NULL; int ret = -1; + virCheckFlags(0, -1); + testDriverLock(privconn); privdom = virDomainObjListFindByName(privconn->domains, domain->name); @@ -1962,6 +1966,12 @@ static int testDomainResume(virDomainPtr domain) return ret; } +static int +testDomainResume(virDomainPtr domain) +{ + return testDomainResumeFlags(domain, 0); +} + static int testDomainSuspend(virDomainPtr domain) { testConnPtr privconn = domain->conn->privateData; @@ -7375,6 +7385,7 @@ static virHypervisorDriver testDriver = { .domainLookupByName = testDomainLookupByName, /* 0.1.1 */ .domainSuspend = testDomainSuspend, /* 0.1.1 */ .domainResume = testDomainResume, /* 0.1.1 */ + .domainResumeFlags = testDomainResumeFlags, /* 1.2.10 */ .domainShutdown = testDomainShutdown, /* 0.1.1 */ .domainShutdownFlags = testDomainShutdownFlags, /* 0.9.10 */ .domainReboot = testDomainReboot, /* 0.1.1 */ diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index bc80338..7d83e04 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -2474,7 +2474,8 @@ static int vboxDomainSuspend(virDomainPtr dom) return ret; } -static int vboxDomainResume(virDomainPtr dom) +static int vboxDomainResumeFlags(virDomainPtr dom, + unsigned int flags) { vboxGlobalData *data = dom->conn->privateData; IMachine *machine = NULL; @@ -2484,6 +2485,8 @@ static int vboxDomainResume(virDomainPtr dom) PRBool isAccessible = PR_FALSE; int ret = -1; + virCheckFlags(0, -1); + if (!data->vboxObj) return ret; @@ -2525,6 +2528,11 @@ static int vboxDomainResume(virDomainPtr dom) return ret; } +static int vboxDomainResume(virDomainPtr dom) +{ + return vboxDomainResumeFlags(dom, 0); +} + static int vboxDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { vboxGlobalData *data = dom->conn->privateData; @@ -7617,6 +7625,7 @@ virHypervisorDriver vboxCommonDriver = { .domainLookupByName = vboxDomainLookupByName, /* 0.6.3 */ .domainSuspend = vboxDomainSuspend, /* 0.6.3 */ .domainResume = vboxDomainResume, /* 0.6.3 */ + .domainResumeFlags = vboxDomainResumeFlags, /* 1.2.10 */ .domainShutdown = vboxDomainShutdown, /* 0.6.3 */ .domainShutdownFlags = vboxDomainShutdownFlags, /* 0.9.10 */ .domainReboot = vboxDomainReboot, /* 0.6.3 */ diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index c3fa2dc..e16222c 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -543,7 +543,8 @@ vmwareDomainSuspend(virDomainPtr dom) } static int -vmwareDomainResume(virDomainPtr dom) +vmwareDomainResumeFlags(virDomainPtr dom, + unsigned int flags) { struct vmware_driver *driver = dom->conn->privateData; @@ -554,6 +555,8 @@ vmwareDomainResume(virDomainPtr dom) }; int ret = -1; + virCheckFlags(0, -1); + if (driver->type == VMWARE_DRIVER_PLAYER) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("vmplayer does not support libvirt suspend/resume " @@ -592,6 +595,12 @@ vmwareDomainResume(virDomainPtr dom) } static int +vmwareDomainResume(virDomainPtr dom) +{ + return vmwareDomainResumeFlags(dom, 0); +} + +static int vmwareDomainReboot(virDomainPtr dom, unsigned int flags) { struct vmware_driver *driver = dom->conn->privateData; @@ -1198,6 +1207,7 @@ static virHypervisorDriver vmwareDriver = { .domainLookupByName = vmwareDomainLookupByName, /* 0.8.7 */ .domainSuspend = vmwareDomainSuspend, /* 0.8.7 */ .domainResume = vmwareDomainResume, /* 0.8.7 */ + .domainResumeFlags = vmwareDomainResumeFlags, /* 1.2.10 */ .domainShutdown = vmwareDomainShutdown, /* 0.8.7 */ .domainShutdownFlags = vmwareDomainShutdownFlags, /* 0.9.10 */ .domainReboot = vmwareDomainReboot, /* 0.8.7 */ diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 5f7c98f..86f8556 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -949,15 +949,18 @@ xenUnifiedDomainSuspend(virDomainPtr dom) } static int -xenUnifiedDomainResume(virDomainPtr dom) +xenUnifiedDomainResumeFlags(virDomainPtr dom, + unsigned int flags) { int ret = -1; virDomainDefPtr def; + virCheckFlags(0, -1); + if (!(def = xenGetDomainDefForDom(dom))) goto cleanup; - if (virDomainResumeEnsureACL(dom->conn, def) < 0) + if (virDomainResumeFlagsEnsureACL(dom->conn, def) < 0) goto cleanup; ret = xenDaemonDomainResume(dom->conn, def); @@ -968,6 +971,12 @@ xenUnifiedDomainResume(virDomainPtr dom) } static int +xenUnifiedDomainResume(virDomainPtr dom) +{ + return xenUnifiedDomainResumeFlags(dom, 0); +} + +static int xenUnifiedDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { @@ -2765,6 +2774,7 @@ static virHypervisorDriver xenUnifiedDriver = { .domainLookupByName = xenUnifiedDomainLookupByName, /* 0.0.3 */ .domainSuspend = xenUnifiedDomainSuspend, /* 0.0.3 */ .domainResume = xenUnifiedDomainResume, /* 0.0.3 */ + .domainResumeFlags = xenUnifiedDomainResumeFlags, /* 1.2.10 */ .domainShutdown = xenUnifiedDomainShutdown, /* 0.0.3 */ .domainShutdownFlags = xenUnifiedDomainShutdownFlags, /* 0.9.10 */ .domainReboot = xenUnifiedDomainReboot, /* 0.1.0 */ diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index ed4f7e8..a78b82b 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -766,18 +766,22 @@ xenapiDomainSuspend(virDomainPtr dom) } /* - * xenapiDomainResume + * xenapiDomainResumeFlags * * Resumes a VM * Returns 0 on success or -1 in case of error */ static int -xenapiDomainResume(virDomainPtr dom) +xenapiDomainResumeFlags(virDomainPtr dom, + unsigned int flags) { /* vm.unpause() */ xen_vm vm; xen_vm_set *vms; xen_session *session = ((struct _xenapiPrivate *)(dom->conn->privateData))->session; + + virCheckFlags(0, -1); + if (xen_vm_get_by_name_label(session, &vms, dom->name) && vms->size > 0) { if (vms->size != 1) { xenapiSessionErrorHandler(dom->conn, VIR_ERR_INTERNAL_ERROR, @@ -802,6 +806,18 @@ xenapiDomainResume(virDomainPtr dom) } /* + * xenapiDomainResume + * + * Resumes a VM + * Returns 0 on success or -1 in case of error + */ +static int +xenapiDomainResume(virDomainPtr dom) +{ + return xenapiDomainResumeFlags(dom, 0); +} + +/* * xenapiDomainShutdown * * shutsdown a VM @@ -1983,6 +1999,7 @@ static virHypervisorDriver xenapiDriver = { .domainLookupByName = xenapiDomainLookupByName, /* 0.8.0 */ .domainSuspend = xenapiDomainSuspend, /* 0.8.0 */ .domainResume = xenapiDomainResume, /* 0.8.0 */ + .domainResumeFlags = xenapiDomainResumeFlags, /* 1.2.10 */ .domainShutdown = xenapiDomainShutdown, /* 0.8.0 */ .domainShutdownFlags = xenapiDomainShutdownFlags, /* 0.9.10 */ .domainReboot = xenapiDomainReboot, /* 0.8.0 */ -- 2.0.4
On 11/05/14 13:35, Michal Privoznik wrote:
This practically boils down to: 1) rename DomainResume implementation to DomainResumeFlags 2) make DomainResume call DomainResumeFlags(dom, 0);
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/esx/esx_driver.c | 14 +++++++++++++- src/hyperv/hyperv_driver.c | 14 +++++++++++++- src/libxl/libxl_driver.c | 14 ++++++++++++-- src/lxc/lxc_driver.c | 15 +++++++++++++-- src/openvz/openvz_driver.c | 13 ++++++++++++- src/parallels/parallels_driver.c | 11 ++++++++++- src/phyp/phyp_driver.c | 12 +++++++++++- src/qemu/qemu_driver.c | 15 +++++++++++++-- src/test/test_driver.c | 13 ++++++++++++- src/vbox/vbox_common.c | 11 ++++++++++- src/vmware/vmware_driver.c | 12 +++++++++++- src/xen/xen_driver.c | 14 ++++++++++++-- src/xenapi/xenapi_driver.c | 21 +++++++++++++++++++-- 13 files changed, 161 insertions(+), 18 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 0770e89..aaca532 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c
@@ -1838,6 +1841,14 @@ esxDomainResume(virDomainPtr domain)
static int +esxDomainResume(virDomainPtr domain) +{ + return esxDomainResumeFlags(domain, 0); +} + + +
3 empty lines.
+static int esxDomainShutdownFlags(virDomainPtr domain, unsigned int flags) { int result = -1; @@ -5310,6 +5321,7 @@ static virHypervisorDriver esxDriver = { .domainLookupByName = esxDomainLookupByName, /* 0.7.0 */ .domainSuspend = esxDomainSuspend, /* 0.7.0 */ .domainResume = esxDomainResume, /* 0.7.0 */ + .domainResumeFlags = esxDomainResumeFlags, /* 1.2.10 */
1.2.11
.domainShutdown = esxDomainShutdown, /* 0.7.0 */ .domainShutdownFlags = esxDomainShutdownFlags, /* 0.9.10 */ .domainReboot = esxDomainReboot, /* 0.7.0 */ diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 5ffcb85..ac8f745 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c
@@ -598,6 +601,14 @@ hypervDomainResume(virDomainPtr domain)
static int +hypervDomainResume(virDomainPtr domain) +{ + return hypervDomainResumeFlags(domain, 0); +} + +
3 empty lines.
+
+ .domainResumeFlags = hypervDomainResumeFlags, /* 1.2.10 */ + .domainResumeFlags = libxlDomainResumeFlags, /* 1.2.10 */ + .domainResumeFlags = lxcDomainResumeFlags, /* 1.2.10 */ + .domainResumeFlags = openvzDomainResumeFlags, /* 1.2.10 */ + .domainResumeFlags = parallelsDomainResumeFlags, /* 1.2.10 */ + .domainResumeFlags = phypDomainResumeFlags, /* 1.2.10 */ + .domainResumeFlags = qemuDomainResumeFlags, /* 1.2.10 */ + .domainResumeFlags = testDomainResumeFlags, /* 1.2.10 */ + .domainResumeFlags = vboxDomainResumeFlags, /* 1.2.10 */ + .domainResumeFlags = vmwareDomainResumeFlags, /* 1.2.10 */ + .domainResumeFlags = xenUnifiedDomainResumeFlags, /* 1.2.10 */ + .domainResumeFlags = xenapiDomainResumeFlags, /* 1.2.10 */
s/1.2.10/1.2.11/ Looks reasonable, I ACK it with the changes, but we should reach agreement on 4/5 before pushing. Peter
On 06.11.2014 09:40, Peter Krempa wrote:
On 11/05/14 13:35, Michal Privoznik wrote:
This practically boils down to: 1) rename DomainResume implementation to DomainResumeFlags 2) make DomainResume call DomainResumeFlags(dom, 0);
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/esx/esx_driver.c | 14 +++++++++++++- src/hyperv/hyperv_driver.c | 14 +++++++++++++- src/libxl/libxl_driver.c | 14 ++++++++++++-- src/lxc/lxc_driver.c | 15 +++++++++++++-- src/openvz/openvz_driver.c | 13 ++++++++++++- src/parallels/parallels_driver.c | 11 ++++++++++- src/phyp/phyp_driver.c | 12 +++++++++++- src/qemu/qemu_driver.c | 15 +++++++++++++-- src/test/test_driver.c | 13 ++++++++++++- src/vbox/vbox_common.c | 11 ++++++++++- src/vmware/vmware_driver.c | 12 +++++++++++- src/xen/xen_driver.c | 14 ++++++++++++-- src/xenapi/xenapi_driver.c | 21 +++++++++++++++++++-- 13 files changed, 161 insertions(+), 18 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 0770e89..aaca532 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c
@@ -1838,6 +1841,14 @@ esxDomainResume(virDomainPtr domain)
static int +esxDomainResume(virDomainPtr domain) +{ + return esxDomainResumeFlags(domain, 0); +} + + +
3 empty lines.
I know, but I want to match the style used in the rest of the file. And since we have no rule on this yet, I'm okay with keeping the lines. Michal
This is pure code movement to dig out the function internals into a separate functions as the code is to be reused later. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 86 +++++++++++++++++++++++++++++++------------------- 1 file changed, 54 insertions(+), 32 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b866ae8..903ca5d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1868,6 +1868,59 @@ static int qemuDomainSuspend(virDomainPtr dom) } +/** + * qemuDomainSetTimeImpl: + * + * Domain must have a job set as the monitor is entered here. + * + * Returns 0 on success, -1 otherwise. + */ +static int +qemuDomainSetTimeImpl(virQEMUDriverPtr driver, + virDomainObjPtr vm, + long long seconds, + unsigned int nseconds, + bool rtcSync) +{ + int rv, ret = -1; + qemuDomainObjPrivatePtr priv = vm->privateData; + + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_RTC_RESET_REINJECTION)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("cannot set time: qemu doesn't support " + "rtc-reset-reinjection command")); + goto cleanup; + } + + if (!qemuDomainAgentAvailable(priv, true)) + goto cleanup; + + qemuDomainObjEnterAgent(vm); + rv = qemuAgentSetTime(priv->agent, seconds, nseconds, rtcSync); + qemuDomainObjExitAgent(vm); + + if (rv < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + + qemuDomainObjEnterMonitor(driver, vm); + rv = qemuMonitorRTCResetReinjection(priv->mon); + qemuDomainObjExitMonitor(driver, vm); + + if (rv < 0) + goto cleanup; + + ret = 0; + cleanup: + return ret; +} + + static int qemuDomainResumeFlags(virDomainPtr dom, unsigned int flags) @@ -17711,11 +17764,9 @@ qemuDomainSetTime(virDomainPtr dom, unsigned int flags) { virQEMUDriverPtr driver = dom->conn->privateData; - qemuDomainObjPrivatePtr priv; virDomainObjPtr vm; bool rtcSync = flags & VIR_DOMAIN_TIME_SYNC; int ret = -1; - int rv; virCheckFlags(VIR_DOMAIN_TIME_SYNC, ret); @@ -17725,8 +17776,6 @@ qemuDomainSetTime(virDomainPtr dom, if (virDomainSetTimeEnsureACL(dom->conn, vm->def) < 0) goto cleanup; - priv = vm->privateData; - if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; @@ -17736,34 +17785,7 @@ qemuDomainSetTime(virDomainPtr dom, goto endjob; } - if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_RTC_RESET_REINJECTION)) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("cannot set time: qemu doesn't support " - "rtc-reset-reinjection command")); - goto endjob; - } - - if (!qemuDomainAgentAvailable(priv, true)) - goto endjob; - - qemuDomainObjEnterAgent(vm); - rv = qemuAgentSetTime(priv->agent, seconds, nseconds, rtcSync); - qemuDomainObjExitAgent(vm); - - if (rv < 0) - goto endjob; - - if (!virDomainObjIsActive(vm)) { - virReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is not running")); - goto endjob; - } - - qemuDomainObjEnterMonitor(driver, vm); - rv = qemuMonitorRTCResetReinjection(priv->mon); - qemuDomainObjExitMonitor(driver, vm); - - if (rv < 0) + if (qemuDomainSetTimeImpl(driver, vm, seconds, nseconds, rtcSync) < 0) goto endjob; ret = 0; -- 2.0.4
On 11/05/14 13:35, Michal Privoznik wrote:
This is pure code movement to dig out the function internals into a separate functions as the code is to be reused later.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 86 +++++++++++++++++++++++++++++++------------------- 1 file changed, 54 insertions(+), 32 deletions(-)
ACK Peter
This flag can be used to sync the domain's time right after domain CPUs are started. It's basically backing call of two subsequent APIs into one: 1) virDomainResume(dom) 2) virDomainSetTime(dom, 0, 0, VIR_DOMAIN_TIME_SYNC) Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- include/libvirt/libvirt-domain.h | 4 ++++ src/qemu/qemu_driver.c | 7 ++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 1795dd3..8d763c7 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -867,6 +867,10 @@ int virDomainFree (virDomainPtr domain); /* * Domain suspend/resume */ +typedef enum { + VIR_DOMAIN_RESUME_SYNC_TIME = 1 << 0, /* on resume sync domain time from its RTC */ +} virDomainResumeFlagsValues; + int virDomainSuspend (virDomainPtr domain); int virDomainResume (virDomainPtr domain); int virDomainResumeFlags (virDomainPtr domain, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 903ca5d..38926c4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1933,7 +1933,7 @@ qemuDomainResumeFlags(virDomainPtr dom, virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; - virCheckFlags(0, -1); + virCheckFlags(VIR_DOMAIN_RESUME_SYNC_TIME, -1); if (!(vm = qemuDomObjFromDomain(dom))) return -1; @@ -1974,6 +1974,11 @@ qemuDomainResumeFlags(virDomainPtr dom, goto endjob; if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) goto endjob; + + if (flags & VIR_DOMAIN_RESUME_SYNC_TIME && + qemuDomainSetTimeImpl(driver, vm, 0, 0, true) < 0) + goto endjob; + ret = 0; endjob: -- 2.0.4
On 11/05/14 13:35, Michal Privoznik wrote:
This flag can be used to sync the domain's time right after domain CPUs are started. It's basically backing call of two subsequent APIs into one:
1) virDomainResume(dom) 2) virDomainSetTime(dom, 0, 0, VIR_DOMAIN_TIME_SYNC)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- include/libvirt/libvirt-domain.h | 4 ++++ src/qemu/qemu_driver.c | 7 ++++++- 2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 1795dd3..8d763c7 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -867,6 +867,10 @@ int virDomainFree (virDomainPtr domain); /* * Domain suspend/resume */ +typedef enum { + VIR_DOMAIN_RESUME_SYNC_TIME = 1 << 0, /* on resume sync domain time from its RTC */ +} virDomainResumeFlagsValues; + int virDomainSuspend (virDomainPtr domain); int virDomainResume (virDomainPtr domain); int virDomainResumeFlags (virDomainPtr domain,
The new flag also needs docs in the header of virDomainResumeFlags where you should mention that it also needs the guest agent in various cases.
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 903ca5d..38926c4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c
@@ -1974,6 +1974,11 @@ qemuDomainResumeFlags(virDomainPtr dom, goto endjob; if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) goto endjob; + + if (flags & VIR_DOMAIN_RESUME_SYNC_TIME && + qemuDomainSetTimeImpl(driver, vm, 0, 0, true) < 0) + goto endjob; +
This is a bit problematic. As you are compounding two operations into one, if syncing of the time fails, you've already resumed the guest. Now with this code you would return failure of the entire API call, which would have the caller figure out what happened. Said this, I'm not against this idea but: 1) You need to make it painfully obvious in the error message that the guest was resumed at this point. 2) You also need to make it obvious in the function docs that this might happen and also mention that to have better control the user is better off with calling the time sync function separately. 3) You should also document that the time sync will not happen exactly at the time the guest was resumed. (Some instructions were executed with the old time setting)
ret = 0;
endjob:
Peter
On 11/06/2014 09:12 AM, Peter Krempa wrote:
On 11/05/14 13:35, Michal Privoznik wrote:
This flag can be used to sync the domain's time right after domain CPUs are started. It's basically backing call of two subsequent APIs into one:
1) virDomainResume(dom) 2) virDomainSetTime(dom, 0, 0, VIR_DOMAIN_TIME_SYNC)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- include/libvirt/libvirt-domain.h | 4 ++++ src/qemu/qemu_driver.c | 7 ++++++- 2 files changed, 10 insertions(+), 1 deletion(-)
+ + if (flags & VIR_DOMAIN_RESUME_SYNC_TIME && + qemuDomainSetTimeImpl(driver, vm, 0, 0, true) < 0) + goto endjob; +
This is a bit problematic. As you are compounding two operations into one, if syncing of the time fails, you've already resumed the guest.
Now with this code you would return failure of the entire API call, which would have the caller figure out what happened.
Said this, I'm not against this idea but: 1) You need to make it painfully obvious in the error message that the guest was resumed at this point. 2) You also need to make it obvious in the function docs that this might happen and also mention that to have better control the user is better off with calling the time sync function separately. 3) You should also document that the time sync will not happen exactly at the time the guest was resumed. (Some instructions were executed with the old time setting)
4) Modify src/remote/remote_protocol.x to add @acl: domain:set_time:VIR_DOMAIN_RESUME_SYNC_TIME as the set_time privilege may be distinct from the weaker domain:suspend privilege. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
This is done by introducing '--sync-time' to already existing 'resume' command. If the option is used, the newer Flags() API is called. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-domain.c | 13 ++++++++++++- tools/virsh.pod | 10 ++++++---- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index dfc3a8c..c02f0a2 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -5135,6 +5135,10 @@ static const vshCmdOptDef opts_resume[] = { .flags = VSH_OFLAG_REQ, .help = N_("domain name, id or uuid") }, + {.name = "sync-time", + .type = VSH_OT_BOOL, + .help = N_("sync domain's time from its RTC") + }, {.name = NULL} }; @@ -5144,11 +5148,18 @@ cmdResume(vshControl *ctl, const vshCmd *cmd) virDomainPtr dom; bool ret = true; const char *name; + unsigned int flags = 0; if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) return false; - if (virDomainResume(dom) == 0) { + if (vshCommandOptBool(cmd, "sync-time")) + flags |= VIR_DOMAIN_RESUME_SYNC_TIME; + + if ((flags && + virDomainResumeFlags(dom, flags) == 0) || + (!flags && + virDomainResume(dom) == 0)) { vshPrint(ctl, _("Domain %s resumed\n"), name); } else { vshError(ctl, _("Failed to resume domain %s"), name); diff --git a/tools/virsh.pod b/tools/virsh.pod index daddb48..001f61a 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2101,11 +2101,13 @@ is only supported with container based virtualization. Suspend a running domain. It is kept in memory but won't be scheduled anymore. -=item B<resume> I<domain> +=item B<resume> I<domain> [I<--sync-time>] -Moves a domain out of the suspended state. This will allow a previously -suspended domain to now be eligible for scheduling by the underlying -hypervisor. +Moves a domain out of the suspended state. This will allow a +previously suspended domain to now be eligible for scheduling by the +underlying hypervisor. Moreover, if I<--sync-time> is specified the +domain's time is synced right after the domain CPUs are started. Note, +that this may require guest agent on some hypervisors. =item B<dompmsuspend> I<domain> I<target> [I<--duration>] -- 2.0.4
On 11/05/14 13:35, Michal Privoznik wrote:
This is done by introducing '--sync-time' to already existing 'resume' command. If the option is used, the newer Flags() API is called.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-domain.c | 13 ++++++++++++- tools/virsh.pod | 10 ++++++---- 2 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index dfc3a8c..c02f0a2 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c
@@ -5144,11 +5148,18 @@ cmdResume(vshControl *ctl, const vshCmd *cmd) virDomainPtr dom; bool ret = true; const char *name; + unsigned int flags = 0;
if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) return false;
- if (virDomainResume(dom) == 0) { + if (vshCommandOptBool(cmd, "sync-time")) + flags |= VIR_DOMAIN_RESUME_SYNC_TIME; + + if ((flags && + virDomainResumeFlags(dom, flags) == 0) || + (!flags && + virDomainResume(dom) == 0)) { vshPrint(ctl, _("Domain %s resumed\n"), name); } else { vshError(ctl, _("Failed to resume domain %s"), name);
Here you exactly hit the problem I've described in 4/5: If time sync fails then virsh will output "Failed to resume domain blah", but the domain was actually resumed. You either need to check the domain state or switch to the two-call approach even in virsh so that you can report a proper error. In case when switching to the two call approach is better for most clients, having the flag doesn't make much sense though.
diff --git a/tools/virsh.pod b/tools/virsh.pod index daddb48..001f61a 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2101,11 +2101,13 @@ is only supported with container based virtualization. Suspend a running domain. It is kept in memory but won't be scheduled anymore.
-=item B<resume> I<domain> +=item B<resume> I<domain> [I<--sync-time>]
-Moves a domain out of the suspended state. This will allow a previously -suspended domain to now be eligible for scheduling by the underlying -hypervisor. +Moves a domain out of the suspended state. This will allow a +previously suspended domain to now be eligible for scheduling by the +underlying hypervisor. Moreover, if I<--sync-time> is specified the +domain's time is synced right after the domain CPUs are started. Note,
"right away" may suggest that nothing happening after the resume will use the old time stamps.
+that this may require guest agent on some hypervisors.
=item B<dompmsuspend> I<domain> I<target> [I<--duration>]
My stance on this patch depends on what happens in 4/5 Peter
participants (3)
-
Eric Blake -
Michal Privoznik -
Peter Krempa