From: "Pavel Dovgalyuk" <dovgaluk@ispras.ru>
To: 'Eric Blake' <eblake@redhat.com>,
'Markus Armbruster' <armbru@redhat.com>
Cc: qemu-devel@nongnu.org, 'Pavel Dovgalyuk' <pavel.dovgaluk@ispras.ru>
Subject: Re: [Qemu-devel] replay help [was: [PATCH v5 3/4] shutdown: Add source information to SHUTDOWN and RESET]
Date: Tue, 2 May 2017 13:54:25 +0300 [thread overview]
Message-ID: <001701d2c332$76ee1020$64ca3060$@ru> (raw)
In-Reply-To: <f0824e24-ea2b-4037-939b-2327435b4f24@redhat.com>
> From: Eric Blake [mailto:eblake@redhat.com]
> On 04/28/2017 10:01 AM, Markus Armbruster wrote:
> > Eric Blake <eblake@redhat.com> writes:
> >
> >> Libvirt would like to be able to distinguish between a SHUTDOWN
> >> event triggered solely by guest request and one triggered by a
> >> SIGTERM or other action on the host. While qemu_kill_report() is
> >> already able to tell whether a shutdown was triggered by a host
> >> signal (but NOT by a host UI event, such as clicking the X on
> >> the window), that information was then lost after being printed
> >> to stderr. The previous patch prepped things to use an enum
> >> internally; now it's time to wire it up through all callers, and
> >> to extend the SHUTDOWN and RESET events to report the details.
> >>
>
> >> The replay driver needs a followup patch if we want to be able to
> >> faithfully replay the difference between a host- and guest-initiated
> >> shutdown (for now, the replayed event is always attributed to host).
> >
> > I'd prefer to get this right from the start, but that requires input
> > from replay guys.
> >
> > Scandalously, replay/ is not covered by MAINTAINERS.
> > scripts/get_maintainers.pl blames it on Pavel Dovgalyuk (cc'ed), and git
> > shows recent activity. Pavel, please post a suitable patch to
> > MAINTAINERS, and please help us figure out what to do about replaying
> > reset here.
>
> In particular, my questions include:
>
> How sensitive is the enum ReplayEvents in replay-internal.h to
> additions? Do we have to worry about cross-version compatibility (where
> we can only add new events at the end), or can we reorder things at
> will? I suspect that depends on whether you ever plan to replay a
> script recorded by old qemu while running new qemu.
There is no cross-version compatibility at all.
Changing any virtual device behavior makes whole previously recorded
replay log unusable.
Therefore we can add any events and update REPLAY_VERSION to prevent
cross-version launches.
> One idea would be to carve out a range of new enums in ReplayEvents,
> similar to how you have a range of CHECKPOINT_COUNT enums carved out;
> the range would be large enough to encode each cause of shutdown, and
> then you just map the range back into a reset request with the right
> cause. Another would be to modify the replay script file format: right
> now, it records a single byte for a shutdown request, but a single new
> enum triggers that we now read a second byte for the cause (where the
> second byte is directly the cause). If replaying old streams matters,
> we still have to burn a new enum (so that you can tell the difference
> between old stream with no cause occupying one byte, vs. new stream with
> second byte giving the cause); if cross-version qemu doesn't matter (ie.
> upgrading qemu can invalidate all previously captured replay streams),
> then this version would better be served by repurposing the existing
> EVENT_SHUTDOWN enum in-place.
Replaying old stream does not matter.
I prefer new enum, because it will simplify comparisons and switches.
Pavel Dovgalyuk
next prev parent reply other threads:[~2017-05-02 10:54 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-28 2:13 [Qemu-devel] [PATCH v5 0/4] event: Add source information to SHUTDOWN Eric Blake
2017-04-28 2:13 ` [Qemu-devel] [PATCH v5 1/4] shutdown: Simplify shutdown_signal Eric Blake
2017-04-28 14:42 ` Markus Armbruster
2017-04-28 2:13 ` [Qemu-devel] [PATCH v5 2/4] shutdown: Prepare for use of an enum in reset/shutdown_request Eric Blake
2017-04-28 8:08 ` Dr. David Alan Gilbert
2017-04-28 14:35 ` Eric Blake
2017-04-28 15:27 ` Dr. David Alan Gilbert
2017-04-28 15:57 ` Eric Blake
2017-04-28 16:09 ` Dr. David Alan Gilbert
2017-04-28 18:05 ` Eric Blake
2017-04-28 18:13 ` Dr. David Alan Gilbert
2017-04-28 14:45 ` Markus Armbruster
2017-04-28 14:42 ` Markus Armbruster
2017-04-28 22:34 ` Eric Blake
2017-05-02 11:47 ` Markus Armbruster
2017-04-28 2:13 ` [Qemu-devel] [PATCH v5 3/4] shutdown: Add source information to SHUTDOWN and RESET Eric Blake
2017-04-28 15:01 ` Markus Armbruster
2017-04-28 22:44 ` [Qemu-devel] replay help [was: [PATCH v5 3/4] shutdown: Add source information to SHUTDOWN and RESET] Eric Blake
2017-05-02 10:54 ` Pavel Dovgalyuk [this message]
2017-05-02 8:13 ` [Qemu-devel] [PATCH v5 3/4] shutdown: Add source information to SHUTDOWN and RESET Pavel Dovgalyuk
2017-05-01 3:58 ` David Gibson
2017-05-05 8:36 ` Mark Cave-Ayland
2017-04-28 2:13 ` [Qemu-devel] [PATCH v5 4/4] RFC: shutdown: Expose full ShutdownCause across QMP Eric Blake
2017-04-28 22:48 ` Eric Blake
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='001701d2c332$76ee1020$64ca3060$@ru' \
--to=dovgaluk@ispras.ru \
--cc=armbru@redhat.com \
--cc=eblake@redhat.com \
--cc=pavel.dovgaluk@ispras.ru \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).