Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CA-363700: update xenopsd platformdata if rtc-timeoffset changes #4665

Merged
merged 1 commit into from
Apr 1, 2022

Conversation

robhoes
Copy link
Member

@robhoes robhoes commented Mar 31, 2022

The timeoffset parameter of a VM, which controls its timezone settings,
is present in the VM metadata in two places: in the build spec as well
as in the platformdata. This also drives the fact that the timeoffset is
written to two places in xenstore:

/vm/<uuid>/rtc/timeoffset
/local/domain/<domid>/platform/timeoffset

It turns out that the PV tools and/or QEMU in some way rely on both of
these paths.

When a VM starts, its timeoffset platform key is used to set the value
of both bits of metadata that are sent to xenopsd. If the timezone
inside of the VM is changed, then an event is sent back to xapi, which
updates the platform key in its xapi.

If a VM resumes, it is similar to start, but additionally the VM's
runtime metadata is sent to xenopsd as well, so the VM is resumed with
exactly the same state as before it suspended.

Since a recent change, the platformdata has been made part of xenopsd's
runtime VM metadata, and is therefore persisted across a suspend/resume
cycle or live migration. This means that the timeoffset in the
platformdata now comes from this xenopsd-level metadata rather than from
xapi's metadata.

This all would have been fine, if xenopsd were to update the timeoffset
in its persistent platformdata whenever it changes in the VM (besides
sending the event to xapi). Unfortunately, this was not the case, which
meant that, after a suspend/resume cycle, the VM's timezone reverted
back to whatever it was when the VM started, and any changes after that
are forgotten. The situation for live migration is similar.

This commit adds the missing metadata update in xenopsd.

Signed-off-by: Rob Hoes rob.hoes@citrix.com

The timeoffset parameter of a VM, which controls its timezone settings,
is present in the VM metadata in two places: in the build spec as well
as in the platformdata. This also drives the fact that the timeoffset is
written to two places in xenstore:

    /vm/<uuid>/rtc/timeoffset
    /local/domain/<domid>/platform/timeoffset

It turns out that the PV tools and/or QEMU in some way rely on both of
these paths.

When a VM starts, its timeoffset platform key is used to set the value
of both bits of metadata that are sent to xenopsd. If the timezone
inside of the VM is changed, then an event is sent back to xapi, which
updates the platform key in its xapi.

If a VM resumes, it is similar to start, but additionally the VM's
runtime metadata is sent to xenopsd as well, so the VM is resumed with
exactly the same state as before it suspended.

Since a recent change, the platformdata has been made part of xenopsd's
runtime VM metadata, and is therefore persisted across a suspend/resume
cycle or live migration. This means that the timeoffset in the
platformdata now comes from this xenopsd-level metadata rather than from
xapi's metadata.

This all would have been fine, if xenopsd were to update the timeoffset
in its persistent platformdata whenever it changes in the VM (besides
sending the event to xapi). Unfortunately, this was not the case, which
meant that, after a suspend/resume cycle, the VM's timezone reverted
back to whatever it was when the VM started, and any changes after that
are forgotten. The situation for live migration is similar.

This commit adds the missing metadata update in xenopsd.

Signed-off-by: Rob Hoes <rob.hoes@citrix.com>
Comment on lines +4791 to +4794
let platformdata =
("timeoffset", timeoffset)
:: List.remove_assoc "timeoffset" persistent.platformdata
in
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Listext has a replace_assoc function, not sure if it's of any advantage here:

Suggested change
let platformdata =
("timeoffset", timeoffset)
:: List.remove_assoc "timeoffset" persistent.platformdata
in
let platformdata =
Xapi_stdext_std.Listext.List.replace_assoc "timeoffset"
timeoffset persistent.platformdata
in

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I usually try to use the standard library unless it becomes too cumbersome.

@robhoes robhoes merged commit 83c6852 into xapi-project:master Apr 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants