[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2] sheepdog: support 'qemu-img snapshot -a'
From: |
Liu Yuan |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] sheepdog: support 'qemu-img snapshot -a' |
Date: |
Thu, 06 Jun 2013 21:09:57 +0800 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130510 Thunderbird/17.0.6 |
On 06/06/2013 08:46 PM, Kevin Wolf wrote:
> Am 06.06.2013 um 13:57 hat Liu Yuan geschrieben:
>> Just call sd_create_branch() to rollback the image is good enough
>>
>> Cc: address@hidden
>> Cc: MORITA Kazutaka <address@hidden>
>> Cc: Kevin Wolf <address@hidden>
>> Cc: Stefan Hajnoczi <address@hidden>
>> Signed-off-by: Liu Yuan <address@hidden>
>> ---
>> block/sheepdog.c | 8 +++++---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/block/sheepdog.c b/block/sheepdog.c
>> index 94218ac..cb5ca4a 100644
>> --- a/block/sheepdog.c
>> +++ b/block/sheepdog.c
>> @@ -2072,9 +2072,11 @@ static int sd_snapshot_goto(BlockDriverState *bs,
>> const char *snapshot_id)
>> }
>>
>> if (!s->inode.vm_state_size) {
>> - error_report("Invalid snapshot");
>> - ret = -ENOENT;
>> - goto out;
>> + /* qemu-img asks us to rollback, we need to do it right now */
>> + ret = sd_create_branch(s);
>> + if (ret) {
>> + goto out;
>> + }
>> }
>
> I'm not sure how snapshots work internally for Sheepdog, but it seems
> odd to me that you need to do this only for disk-only snapshots, but not
> when the snapshot has VM state. (Also, note that 'qemu-img snapshot -a'
> works on images with a VM state, so the comment doesn't seem to be
> completely accurate)
>
> Would you mind explaining to me how this works in detail?
>
Hmm, the original code isn't written by me and this snapshot mechanism
exists since day 0. I just hacks it to work now. So I'll try to explain
on my understanding.
When we do a snapshot such as 'savedvm' or 'qemu-img snapshot', the
active vdi is snapshotted and marked as snapshot and a new vdi is
created as copy-on-write on the previous active vdi, then this new vdi
becomes active vdi. For e.g,
As1 --> As2 --> A
We take snapshot of vdi A twice, tagged s1 and s2 respectively. I guess
this is quit similar to qcow2 snapshots, only inode object with a bitmap
is created.
So when we 'loadvm' or 'qemu-img snapshot -a' to A, current logic just
handle 'loadvm', that .bdrv_snapshot_goto only reloads inode object,
that is, for e.g, we 'savevm s1', and mark it as snapshot, the chain
would like
As1 --> As2 --> A
|
v
just reload As1's inode object
Only when the write comes from VM, we do the following stuff
- delete active vdi A
- created a new inode based on the previously reloaded As1's inode
The chain will look like:
As1 --> As2
|
V
A
This is how sheepdog handles savevm/loadvm.
So for 'qemu-img snapshot -a', we should create the branch in the
.bdrv_snapshot_goto.
As you pointed out, we need to consider vm state even for 'snapshot -a',
so I need to rework the patch 2/2.
Thanks,
Yuan
- [Qemu-devel] [PATCH 0/2] fix 'qemu-img snapshot -a' operation for sheepdog, Liu Yuan, 2013/06/06
- [Qemu-devel] [PATCH 1/2] sheepdog: fix snapshot tag initialization, Liu Yuan, 2013/06/06
- [Qemu-devel] [PATCH 2/2] sheepdog: support 'qemu-img snapshot -a', Liu Yuan, 2013/06/06
- Re: [Qemu-devel] [PATCH 2/2] sheepdog: support 'qemu-img snapshot -a', Kevin Wolf, 2013/06/06
- Re: [Qemu-devel] [PATCH 2/2] sheepdog: support 'qemu-img snapshot -a',
Liu Yuan <=
- Re: [Qemu-devel] [PATCH 2/2] sheepdog: support 'qemu-img snapshot -a', Kevin Wolf, 2013/06/07
- Re: [Qemu-devel] [PATCH 2/2] sheepdog: support 'qemu-img snapshot -a', Liu Yuan, 2013/06/07
- Re: [Qemu-devel] [PATCH 2/2] sheepdog: support 'qemu-img snapshot -a', Kevin Wolf, 2013/06/07
- Re: [Qemu-devel] [PATCH 2/2] sheepdog: support 'qemu-img snapshot -a', Liu Yuan, 2013/06/07
- Re: [Qemu-devel] [PATCH 2/2] sheepdog: support 'qemu-img snapshot -a', Kevin Wolf, 2013/06/07
- Re: [Qemu-devel] [PATCH 2/2] sheepdog: support 'qemu-img snapshot -a', Liu Yuan, 2013/06/07
- Re: [Qemu-devel] [PATCH 2/2] sheepdog: support 'qemu-img snapshot -a', Liu Yuan, 2013/06/06