| Message ID | 1381787553-12497-4-git-send-email-xiawenc@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On 14.10.2013 23:52, Wenchao Xia wrote: > Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> > --- > block/qcow2-snapshot.c | 7 +++++++ > 1 files changed, 7 insertions(+), 0 deletions(-) > > diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c > index 4bd494b..c933b7f 100644 > --- a/block/qcow2-snapshot.c > +++ b/block/qcow2-snapshot.c > @@ -304,6 +304,13 @@ static int qcow2_write_snapshots(BlockDriverState *bs, Error **errp) > PRIi64 " with size %" PRIu64 ":%d (%s)", > offsetof(QCowHeader, nb_snapshots), sizeof(header_data), > ret, strerror(-ret)); > + /* > + * If the snapshot data part have been updated on disk, Then the s/have/has/; s/Then/then/ > + * clusters at snapshot_offset may be used in next snapshot operation. > + * If we free those clusters in fail path, they may be allocated and > + * made dirty causing damage, so skip cluster free for safe. s/for/to be/ > + */ > + snapshots_offset = 0; > goto fail; > } Other then that: Reviewed-by: Max Reitz <mreitz@redhat.com>
On 02.11.2013 14:04, Max Reitz wrote: > On 14.10.2013 23:52, Wenchao Xia wrote: >> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> >> --- >> block/qcow2-snapshot.c | 7 +++++++ >> 1 files changed, 7 insertions(+), 0 deletions(-) >> >> diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c >> index 4bd494b..c933b7f 100644 >> --- a/block/qcow2-snapshot.c >> +++ b/block/qcow2-snapshot.c >> @@ -304,6 +304,13 @@ static int qcow2_write_snapshots(BlockDriverState *bs, Error **errp) >> PRIi64 " with size %" PRIu64 ":%d (%s)", >> offsetof(QCowHeader, nb_snapshots), sizeof(header_data), >> ret, strerror(-ret)); >> + /* >> + * If the snapshot data part have been updated on disk, Then the > s/have/has/; s/Then/then/ > >> + * clusters at snapshot_offset may be used in next snapshot operation. >> + * If we free those clusters in fail path, they may be allocated and >> + * made dirty causing damage, so skip cluster free for safe. > s/for/to be/ > >> + */ >> + snapshots_offset = 0; >> goto fail; >> } > Other then that: Reviewed-by: Max Reitz <mreitz@redhat.com> *than, of course; if I am already correcting other people's orthography, I have to do the same for mine. ;-) Max
δΊ 2013/11/2 21:56, Max Reitz ει: > On 02.11.2013 14:04, Max Reitz wrote: >> On 14.10.2013 23:52, Wenchao Xia wrote: >>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> >>> --- >>> block/qcow2-snapshot.c | 7 +++++++ >>> 1 files changed, 7 insertions(+), 0 deletions(-) >>> >>> diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c >>> index 4bd494b..c933b7f 100644 >>> --- a/block/qcow2-snapshot.c >>> +++ b/block/qcow2-snapshot.c >>> @@ -304,6 +304,13 @@ static int qcow2_write_snapshots(BlockDriverState *bs, Error **errp) >>> PRIi64 " with size %" PRIu64 ":%d (%s)", >>> offsetof(QCowHeader, nb_snapshots), sizeof(header_data), >>> ret, strerror(-ret)); >>> + /* >>> + * If the snapshot data part have been updated on disk, Then the >> s/have/has/; s/Then/then/ >> >>> + * clusters at snapshot_offset may be used in next snapshot operation. >>> + * If we free those clusters in fail path, they may be allocated and >>> + * made dirty causing damage, so skip cluster free for safe. >> s/for/to be/ >> >>> + */ >>> + snapshots_offset = 0; >>> goto fail; >>> } >> Other then that: Reviewed-by: Max Reitz <mreitz@redhat.com> > > *than, of course; if I am already correcting other people's orthography, > I have to do the same for mine. ;-) > > Max > Thanks for reviewing, I'll fix the issues you pointed out and respin.
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c index 4bd494b..c933b7f 100644 --- a/block/qcow2-snapshot.c +++ b/block/qcow2-snapshot.c @@ -304,6 +304,13 @@ static int qcow2_write_snapshots(BlockDriverState *bs, Error **errp) PRIi64 " with size %" PRIu64 ":%d (%s)", offsetof(QCowHeader, nb_snapshots), sizeof(header_data), ret, strerror(-ret)); + /* + * If the snapshot data part have been updated on disk, Then the + * clusters at snapshot_offset may be used in next snapshot operation. + * If we free those clusters in fail path, they may be allocated and + * made dirty causing damage, so skip cluster free for safe. + */ + snapshots_offset = 0; goto fail; }
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> --- block/qcow2-snapshot.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-)