| Message ID | 1380119840-12672-3-git-send-email-mreitz@redhat.com |
|---|---|
| State | New |
| Headers | show |
Le Wednesday 25 Sep 2013 à 16:37:19 (+0200), Max Reitz a écrit : > If an error occurs in l2_allocate, the allocated (but unused) L2 cluster > should be freed. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > block/qcow2-cluster.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c > index f6d47c9..1c3d3fc 100644 > --- a/block/qcow2-cluster.c > +++ b/block/qcow2-cluster.c > @@ -269,6 +269,10 @@ fail: > qcow2_cache_put(bs, s->l2_table_cache, (void**) table); > } > s->l1_table[l1_index] = old_l2_offset; > + if (l2_offset > 0) { > + qcow2_free_clusters(bs, l2_offset, s->l2_size * sizeof(uint64_t), > + QCOW2_DISCARD_ALWAYS); > + } > return ret; > } > > -- > 1.8.3.1 > > Reviewed-by: Benoit Canet <benoit@irqsave.net>
Am 25.09.2013 um 16:37 hat Max Reitz geschrieben: > If an error occurs in l2_allocate, the allocated (but unused) L2 cluster > should be freed. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > block/qcow2-cluster.c | 4 ++++ > 1 file changed, 4 insertions(+) This needs an update of the reference output for test case 026 (both for -nocache and writethrough). Most of the changes look expected and good, like cluster leaks disappearing. With -nocache, however, there are a few cases that failed previously and result in successful writes now. It would be interesting to see the explanation for these before we merge the patch. Kevin
On 2013-09-27 16:54, Kevin Wolf wrote: > Am 25.09.2013 um 16:37 hat Max Reitz geschrieben: >> If an error occurs in l2_allocate, the allocated (but unused) L2 cluster >> should be freed. >> >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> --- >> block/qcow2-cluster.c | 4 ++++ >> 1 file changed, 4 insertions(+) > This needs an update of the reference output for test case 026 (both for > -nocache and writethrough). Yes, right. > Most of the changes look expected and good, like cluster leaks > disappearing. With -nocache, however, there are a few cases that failed > previously and result in successful writes now. It would be interesting > to see the explanation for these before we merge the patch. I personally don't see this cases. Could you give an example? Max
Am 30.09.2013 um 11:48 hat Max Reitz geschrieben: > On 2013-09-27 16:54, Kevin Wolf wrote: > >Am 25.09.2013 um 16:37 hat Max Reitz geschrieben: > >>If an error occurs in l2_allocate, the allocated (but unused) L2 cluster > >>should be freed. > >> > >>Signed-off-by: Max Reitz <mreitz@redhat.com> > >>--- > >> block/qcow2-cluster.c | 4 ++++ > >> 1 file changed, 4 insertions(+) > >This needs an update of the reference output for test case 026 (both for > >-nocache and writethrough). > Yes, right. > > >Most of the changes look expected and good, like cluster leaks > >disappearing. With -nocache, however, there are a few cases that failed > >previously and result in successful writes now. It would be interesting > >to see the explanation for these before we merge the patch. > I personally don't see this cases. Could you give an example? Weird, I can't reproduce it any more. I guess it was a problem on my side then. Kevin
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index f6d47c9..1c3d3fc 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -269,6 +269,10 @@ fail: qcow2_cache_put(bs, s->l2_table_cache, (void**) table); } s->l1_table[l1_index] = old_l2_offset; + if (l2_offset > 0) { + qcow2_free_clusters(bs, l2_offset, s->l2_size * sizeof(uint64_t), + QCOW2_DISCARD_ALWAYS); + } return ret; }
If an error occurs in l2_allocate, the allocated (but unused) L2 cluster should be freed. Signed-off-by: Max Reitz <mreitz@redhat.com> --- block/qcow2-cluster.c | 4 ++++ 1 file changed, 4 insertions(+)