* [Qemu-devel] About the snapshot
@ 2011-12-06 9:07 Zhi Hui Li
2011-12-06 9:40 ` Stefan Hajnoczi
0 siblings, 1 reply; 6+ messages in thread
From: Zhi Hui Li @ 2011-12-06 9:07 UTC (permalink / raw)
To: Stefan Hajnoczi, QEMU-devel; +Cc: zhihuili
1) :
for example:
BDRVQcowState *s = bs->opaque;
s->snapshots
s->nb_snapshots
1:use the command: qemu-img snapshot ./test.qcow2 -c aa
the memory of the s->snapshot don't free,
if the s->nb_snapshots is large, Does it have some problems.
2: use the command: qemu-system-x86_64 ./test.qcow2 -snapshot
when the program ends, Does it need to free the s->snapshots ?
2):
in the function of
qcow2_update_snapshot_refcount
it has some " goto fail ";
if the function runs some times, then something makes it goto fail,
I am not sure whether it will make the refcount incorrect.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] About the snapshot
2011-12-06 9:07 [Qemu-devel] About the snapshot Zhi Hui Li
@ 2011-12-06 9:40 ` Stefan Hajnoczi
2011-12-06 10:01 ` Zhi Hui Li
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Hajnoczi @ 2011-12-06 9:40 UTC (permalink / raw)
To: Zhi Hui Li; +Cc: Kevin Wolf, zhihuili, Stefan Hajnoczi, QEMU-devel
On Tue, Dec 6, 2011 at 9:07 AM, Zhi Hui Li <zhihuili@linux.vnet.ibm.com> wrote:
>
> 1) :
>
> for example:
>
> BDRVQcowState *s = bs->opaque;
>
> s->snapshots
> s->nb_snapshots
>
>
> 1:use the command: qemu-img snapshot ./test.qcow2 -c aa
> the memory of the s->snapshot don't free,
> if the s->nb_snapshots is large, Does it have some problems.
>
> 2: use the command: qemu-system-x86_64 ./test.qcow2 -snapshot
> when the program ends, Does it need to free the s->snapshots ?
These two commands are unrelated. QEMU uses the term "snapshot" for
several different features:
1. qemu-img snapshot refers to "internal snapshots" that are contained
within qcow2 image files. The savevm/loadvm/delvm monitor commands
operate on internal snapshots.
2. qemu -snapshot refers to a temporary qcow2 image file created to
buffer any data that the guest writes. When QEMU exits your disk
image is not modified and the temporary qcow2 file is deleted. You
can also apply the buffer to the disk image using the "commit" monitor
command.
> 2):
> in the function of
> qcow2_update_snapshot_refcount
>
> it has some " goto fail ";
> if the function runs some times, then something makes it goto fail,
> I am not sure whether it will make the refcount incorrect.
When an error occurs its possible that refcount leaks are introduced
(the refcount was increment but will never be used), but we should
never decrement a reference that is still in use.
Please be more specific about the problem so that Kevin or I can take a look.
Stefan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] About the snapshot
2011-12-06 9:40 ` Stefan Hajnoczi
@ 2011-12-06 10:01 ` Zhi Hui Li
2011-12-06 10:33 ` Stefan Hajnoczi
0 siblings, 1 reply; 6+ messages in thread
From: Zhi Hui Li @ 2011-12-06 10:01 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: zhihuili, QEMU-devel
On 2011年12月06日 17:40, Stefan Hajnoczi wrote:
> On Tue, Dec 6, 2011 at 9:07 AM, Zhi Hui Li<zhihuili@linux.vnet.ibm.com> wrote:
>>
>> 1) :
>>
>> for example:
>>
>> BDRVQcowState *s = bs->opaque;
>>
>> s->snapshots
>> s->nb_snapshots
>>
>>
>> 1:use the command: qemu-img snapshot ./test.qcow2 -c aa
>> the memory of the s->snapshot don't free,
>> if the s->nb_snapshots is large, Does it have some problems.
>>
>> 2: use the command: qemu-system-x86_64 ./test.qcow2 -snapshot
>> when the program ends, Does it need to free the s->snapshots ?
>
> These two commands are unrelated. QEMU uses the term "snapshot" for
> several different features:
>
> 1. qemu-img snapshot refers to "internal snapshots" that are contained
> within qcow2 image files. The savevm/loadvm/delvm monitor commands
> operate on internal snapshots.
>
> 2. qemu -snapshot refers to a temporary qcow2 image file created to
> buffer any data that the guest writes. When QEMU exits your disk
> image is not modified and the temporary qcow2 file is deleted. You
> can also apply the buffer to the disk image using the "commit" monitor
> command.
yes, I understand what you say, but the qemu-img and savevm both call
the function of qcow2_snapshot_create, when I use the command qemu-img
snapshot ./test.qcow2 -c aa,
but the memory of the s->snapshot don't free.
>
>> 2):
>> in the function of
>> qcow2_update_snapshot_refcount
>>
>> it has some " goto fail ";
>> if the function runs some times, then something makes it goto fail,
>> I am not sure whether it will make the refcount incorrect.
>
> When an error occurs its possible that refcount leaks are introduced
> (the refcount was increment but will never be used), but we should
> never decrement a reference that is still in use.
>
> Please be more specific about the problem so that Kevin or I can take a look.
>
> Stefan
>
Ok, I got it.
thank you very much !
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] About the snapshot
2011-12-06 10:01 ` Zhi Hui Li
@ 2011-12-06 10:33 ` Stefan Hajnoczi
2011-12-06 14:55 ` Zhi Hui Li
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Hajnoczi @ 2011-12-06 10:33 UTC (permalink / raw)
To: Zhi Hui Li; +Cc: zhihuili, QEMU-devel
On Tue, Dec 6, 2011 at 10:01 AM, Zhi Hui Li <zhihuili@linux.vnet.ibm.com> wrote:
> On 2011年12月06日 17:40, Stefan Hajnoczi wrote:
>>
>> On Tue, Dec 6, 2011 at 9:07 AM, Zhi Hui Li<zhihuili@linux.vnet.ibm.com>
>> wrote:
>>>
>>>
>>> 1) :
>>>
>>> for example:
>>>
>>> BDRVQcowState *s = bs->opaque;
>>>
>>> s->snapshots
>>> s->nb_snapshots
>>>
>>>
>>> 1:use the command: qemu-img snapshot ./test.qcow2 -c aa
>>> the memory of the s->snapshot don't free,
>>> if the s->nb_snapshots is large, Does it have some problems.
>>>
>>> 2: use the command: qemu-system-x86_64 ./test.qcow2 -snapshot
>>> when the program ends, Does it need to free the s->snapshots ?
>>
>>
>> These two commands are unrelated. QEMU uses the term "snapshot" for
>> several different features:
>>
>> 1. qemu-img snapshot refers to "internal snapshots" that are contained
>> within qcow2 image files. The savevm/loadvm/delvm monitor commands
>> operate on internal snapshots.
>>
>> 2. qemu -snapshot refers to a temporary qcow2 image file created to
>> buffer any data that the guest writes. When QEMU exits your disk
>> image is not modified and the temporary qcow2 file is deleted. You
>> can also apply the buffer to the disk image using the "commit" monitor
>> command.
>
>
> yes, I understand what you say, but the qemu-img and savevm both call the
> function of qcow2_snapshot_create, when I use the command qemu-img snapshot
> ./test.qcow2 -c aa,
> but the memory of the s->snapshot don't free.
Okay, I think you're saying that in #1 s->snapshots is leaked because
qcow2_free_snapshots() is not being called from qcow2_close().
Do you want to send a patch to fix this?
Stefan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] About the snapshot
2011-12-06 10:33 ` Stefan Hajnoczi
@ 2011-12-06 14:55 ` Zhi Hui Li
2011-12-06 15:35 ` Stefan Hajnoczi
0 siblings, 1 reply; 6+ messages in thread
From: Zhi Hui Li @ 2011-12-06 14:55 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1272 bytes --]
2011/12/6 Stefan Hajnoczi <stefanha@gmail.com>
> On Tue, Dec 6, 2011 at 10:01 AM, Zhi Hui Li <zhihuili@linux.vnet.ibm.com>
> wrote:
> > On 2011年12月06日 17:40, Stefan Hajnoczi wrote:
> >>
> >> On Tue, Dec 6, 2011 at 9:07 AM, Zhi Hui Li<zhihuili@linux.vnet.ibm.com>
> >> wrote:
> >>>
> >>>
> >>> 1) :
> >>>
> >>> for example:
> >>>
> >>> BDRVQcowState *s = bs->opaque;
> >>>
> >>> s->snapshots
> >>> s->nb_snapshots
> >>>
> >>>
> >>> 1:use the command: qemu-img snapshot ./test.qcow2 -c aa
> >>> the memory of the s->snapshot don't free,
> >>> if the s->nb_snapshots is large, Does it have some problems.
> >>>
> >>> 2: use the command: qemu-system-x86_64 ./test.qcow2 -snapshot
> >>> when the program ends, Does it need to free the s->snapshots ?
> >>
> Okay, I think you're saying that in #1 s->snapshots is leaked because
> qcow2_free_snapshots() is not being called from qcow2_close().
>
> Do you want to send a patch to fix this?
>
Ok, I will send a patch tomorrow.
But I think in the #2 it also need to call qcow2_free_snapshots() , if you
have called several times savevm,
the s->snapshots will very large, when the process end, it also need to
free.
Thank you very much for your feedback !
>
> Stefan
>
>
[-- Attachment #2: Type: text/html, Size: 2200 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] About the snapshot
2011-12-06 14:55 ` Zhi Hui Li
@ 2011-12-06 15:35 ` Stefan Hajnoczi
0 siblings, 0 replies; 6+ messages in thread
From: Stefan Hajnoczi @ 2011-12-06 15:35 UTC (permalink / raw)
To: Zhi Hui Li; +Cc: qemu-devel
2011/12/6 Zhi Hui Li <zhihuili@linux.vnet.ibm.com>:
>
>
> 2011/12/6 Stefan Hajnoczi <stefanha@gmail.com>
>>
>> On Tue, Dec 6, 2011 at 10:01 AM, Zhi Hui Li <zhihuili@linux.vnet.ibm.com>
>> wrote:
>> > On 2011年12月06日 17:40, Stefan Hajnoczi wrote:
>> >>
>> >> On Tue, Dec 6, 2011 at 9:07 AM, Zhi Hui Li<zhihuili@linux.vnet.ibm.com>
>> >> wrote:
>> >>>
>> >>>
>> >>> 1) :
>> >>>
>> >>> for example:
>> >>>
>> >>> BDRVQcowState *s = bs->opaque;
>> >>>
>> >>> s->snapshots
>> >>> s->nb_snapshots
>> >>>
>> >>>
>> >>> 1:use the command: qemu-img snapshot ./test.qcow2 -c aa
>> >>> the memory of the s->snapshot don't free,
>> >>> if the s->nb_snapshots is large, Does it have some problems.
>> >>>
>> >>> 2: use the command: qemu-system-x86_64 ./test.qcow2 -snapshot
>> >>> when the program ends, Does it need to free the s->snapshots ?
>> >>
>> Okay, I think you're saying that in #1 s->snapshots is leaked because
>> qcow2_free_snapshots() is not being called from qcow2_close().
>>
>> Do you want to send a patch to fix this?
>
>
> Ok, I will send a patch tomorrow.
> But I think in the #2 it also need to call qcow2_free_snapshots() , if you
> have called several times savevm,
> the s->snapshots will very large, when the process end, it also need to
> free.
Right, I think I understand what you're saying. I was thinking about
what #1 and #2 mean differently, but it doesn't matter.
If qcow2_close() frees s->snapshots then the problem is solved in all
possible qcow2 use cases, including #2 with savevm.
Stefan
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-12-06 15:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-06 9:07 [Qemu-devel] About the snapshot Zhi Hui Li
2011-12-06 9:40 ` Stefan Hajnoczi
2011-12-06 10:01 ` Zhi Hui Li
2011-12-06 10:33 ` Stefan Hajnoczi
2011-12-06 14:55 ` Zhi Hui Li
2011-12-06 15:35 ` Stefan Hajnoczi
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).