From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:55417) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T1x8c-0004wY-PX for qemu-devel@nongnu.org; Thu, 16 Aug 2012 06:17:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T1x8Y-0006cI-C2 for qemu-devel@nongnu.org; Thu, 16 Aug 2012 06:17:14 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55033) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T1x8Y-0006bv-3F for qemu-devel@nongnu.org; Thu, 16 Aug 2012 06:17:10 -0400 Message-ID: <502CC8A2.4090304@redhat.com> Date: Thu, 16 Aug 2012 12:17:06 +0200 From: Pavel Hrdina MIME-Version: 1.0 References: <699332045f54bbea299eefd50f7198826e638170.1345016001.git.phrdina@redhat.com> <502C78C0.8020508@redhat.com> In-Reply-To: <502C78C0.8020508@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 16/18] qapi: Convert info snapshots List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: =?UTF-8?B?QmVub8OudCBDYW5ldA==?= , qemu-devel@nongnu.org On 08/16/2012 06:36 AM, Eric Blake wrote: > On 08/15/2012 01:41 AM, Pavel Hrdina wrote: >> Signed-off-by: Pavel Hrdina >> --- >> hmp.c | 33 +++++++++++++++++++++++++++++++++ >> hmp.h | 1 + >> monitor.c | 2 +- >> qapi-schema.json | 34 ++++++++++++++++++++++++++++++++++ >> qmp-commands.hx | 30 ++++++++++++++++++++++++++++++ >> savevm.c | 51 ++++++++++++++++++++++++--------------------------- >> sysemu.h | 2 -- >> 7 files changed, 123 insertions(+), 30 deletions(-) >> >> @@ -1053,6 +1053,40 @@ >> { 'command': 'query-block-jobs', 'returns': ['BlockJobInfo'] } >> >> ## >> +# @SnapshotInfo: >> +# > We've got competition here: > https://lists.gnu.org/archive/html/qemu-devel/2012-08/msg02961.html > > These two patches need to converge into a single design. I can use his design. >> +# Snapshot list. This structure contains list of snapshots for virtual machine. >> +# >> +# @id: id of the snapshot. >> +# >> +# @tag: human readable tag of the snapshot. >> +# >> +# @vm_size: size of the snapshot in Bytes. > As this is a new QMP command, you should use '-', not '_'. Benoit's > patch named this @vm-state-size. > >> +# >> +# @date: date and time of the snapshot as unix timestamp. >> +# >> +# @vm_clock: time in the guest in nsecs. > For online internal snapshots (those created by savevm), the vm clock > offset makes sense. But for offline snapshots (those created by > 'qemu-img snapshot', with no VM state), there is no vm clock offset. > 'qemu-img info' lists 00:00:00.000 as a result, but I wonder if it would > be better to leave this field (and vm-state-size) as #optional and omit > them from the JSON for offline snapshots. > > And since qemu refuses to load offline snapshots, it might be worth an > additional field in the JSON that says whether a snapshot is online or > offline, to make it easier for the parsing application to determine > whether it can be loaded or must go through qemu-img while the guest is > offline (then again, keeping vm-state-size as non-optional, and checking > for 0 size, somewhat covers this). As you said, this is covered and I think that checking for 0 size is good enough. >> +# >> +# Since: 1.2 > 1.3 (entire series...) >