From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=36836 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PTCjH-0001MB-TX for qemu-devel@nongnu.org; Thu, 16 Dec 2010 07:14:41 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PTCjE-0007u5-GE for qemu-devel@nongnu.org; Thu, 16 Dec 2010 07:14:37 -0500 Received: from mx1.redhat.com ([209.132.183.28]:50272) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PTCjE-0007tt-5X for qemu-devel@nongnu.org; Thu, 16 Dec 2010 07:14:36 -0500 Message-ID: <4D0A02A8.600@redhat.com> Date: Thu, 16 Dec 2010 13:14:32 +0100 From: Jes Sorensen MIME-Version: 1.0 References: <1292497454-32573-1-git-send-email-Jes.Sorensen@redhat.com> <1292497454-32573-3-git-send-email-Jes.Sorensen@redhat.com> <4D09FBEB.6020608@redhat.com> In-Reply-To: <4D09FBEB.6020608@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH 2/3] Introduce do_snapshot_blkdev() and monitor command to handle it. List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-devel@nongnu.org, armbru@redhat.com, stefanha@linux.vnet.ibm.com On 12/16/10 12:45, Kevin Wolf wrote: > Am 16.12.2010 12:04, schrieb Jes.Sorensen@redhat.com: >> From: Jes Sorensen >> >> The monitor command is: >> snapshot_blkdev [snapshot-file] [format] >> >> Default format is qcow2. For now snapshots without a snapshot-file, eg >> internal snapshots, are not supported. >> >> Signed-off-by: Jes Sorensen >> --- >> blockdev.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> blockdev.h | 1 + >> hmp-commands.hx | 19 +++++++++++++++++ >> 3 files changed, 81 insertions(+), 0 deletions(-) >> >> diff --git a/blockdev.c b/blockdev.c >> index 3b3b82d..9d6f72c 100644 >> --- a/blockdev.c >> +++ b/blockdev.c >> @@ -516,6 +516,67 @@ void do_commit(Monitor *mon, const QDict *qdict) >> } >> } >> >> +int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data) >> +{ >> + const char *device = qdict_get_str(qdict, "device"); >> + const char *filename = qdict_get_try_str(qdict, "snapshot_file"); >> + const char *format = qdict_get_try_str(qdict, "format"); >> + const char format_qcow2[] = "qcow2"; >> + BlockDriverState *bs; >> + BlockDriver *drv, *proto_drv; >> + int ret = 0; >> + int flags; >> + >> + bs = bdrv_find(device); >> + if (!bs) { >> + qerror_report(QERR_DEVICE_NOT_FOUND, device); >> + ret = -1; >> + goto out; >> + } >> + >> + if (!format) { >> + format = format_qcow2; > > Why introducing format_qcow2 instead of directly using the string > literal here? It should generate the same code - I kinda liked my style better, but I'll change it. >> + flags = bs->open_flags; >> + bdrv_close(bs); >> + ret = bdrv_open(bs, filename, flags, drv); >> + /* >> + * If reopening the image file we just created fails, we really >> + * are in trouble :( >> + */ >> + assert(ret == 0); > > bdrv_commit handles this case by setting bs->drv = NULL. After this the > device will fail all requests with -ENOMEDIUM, but at least you don't > lose your VM immediately. Well if we hit this situation something catastrophic happened. I don't see how we can continue beyond this point really. The old image was dropped in the swap and the new one is not accessible ... we're dead :( Cheers, Jes