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