Message ID | bce1328c87f7e5d877dead476e9e66036cc4f7d8.1690166344.git.Evanzhang@archeros.com |
---|---|
State | New |
Headers | show |
Series | [v1] block/stream:add flush l2_table_cache, ensure data integrity | expand |
On 24.07.23 10:30, Evanzhang wrote: > block_stream will not actively flush l2_table_cache,when qemu > process exception exit,causing disk data loss > > Signed-off-by: Evanzhang <Evanzhang@archeros.com> > --- > block/stream.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/block/stream.c b/block/stream.c > index e522bbd..a5e08da 100644 > --- a/block/stream.c > +++ b/block/stream.c > @@ -207,6 +207,12 @@ static int coroutine_fn stream_run(Job *job, Error **errp) > } > } > > + /* > + * Complete stream_populate,force flush l2_table_cache,to > + * avoid unexpected termination of process, l2_table loss > + */ > + qcow2_cache_flush(bs, ((BDRVQcow2State *)bs->opaque)->l2_table_cache); > + > /* Do not remove the backing file if an error was there but ignored. */ > return error; > } Hi! I think, it's more correct just call bdrv_co_flush(bs), which should do all the job. Also, stream_run() should fail if flush fails. Also, I remember I've done it for all (or at least several) blockjobs generically, so that any blockjob must succesfully flush target to report success.. But now I can find neither my patches nor the code :( Den, Kevin, Hanna, don't you remember this topic?
On 7/25/23 16:25, Vladimir Sementsov-Ogievskiy wrote: > On 24.07.23 10:30, Evanzhang wrote: >> block_stream will not actively flush l2_table_cache,when qemu >> process exception exit,causing disk data loss >> >> Signed-off-by: Evanzhang <Evanzhang@archeros.com> >> --- >> block/stream.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/block/stream.c b/block/stream.c >> index e522bbd..a5e08da 100644 >> --- a/block/stream.c >> +++ b/block/stream.c >> @@ -207,6 +207,12 @@ static int coroutine_fn stream_run(Job *job, >> Error **errp) >> } >> } >> + /* >> + * Complete stream_populate,force flush l2_table_cache,to >> + * avoid unexpected termination of process, l2_table loss >> + */ >> + qcow2_cache_flush(bs, ((BDRVQcow2State >> *)bs->opaque)->l2_table_cache); >> + >> /* Do not remove the backing file if an error was there but >> ignored. */ >> return error; >> } > > Hi! > > I think, it's more correct just call bdrv_co_flush(bs), which should > do all the job. Also, stream_run() should fail if flush fails. > > Also, I remember I've done it for all (or at least several) blockjobs > generically, so that any blockjob must succesfully flush target to > report success.. But now I can find neither my patches nor the code :( > Den, Kevin, Hanna, don't you remember this topic? > This was a part of compressed write cache series, which was postponed. https://lore.kernel.org/all/20210305173507.393137-1-vsementsov@virtuozzo.com/T/#m87315593ed5ab16e5d0e4e7a5ae6d776fbbaec77 We have it ported to 7.0 QEMU. Not a problem to port to master and resend. Will this make a sense? Den
On 25.07.23 18:13, Denis V. Lunev wrote: > On 7/25/23 16:25, Vladimir Sementsov-Ogievskiy wrote: >> On 24.07.23 10:30, Evanzhang wrote: >>> block_stream will not actively flush l2_table_cache,when qemu >>> process exception exit,causing disk data loss >>> >>> Signed-off-by: Evanzhang <Evanzhang@archeros.com> >>> --- >>> block/stream.c | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/block/stream.c b/block/stream.c >>> index e522bbd..a5e08da 100644 >>> --- a/block/stream.c >>> +++ b/block/stream.c >>> @@ -207,6 +207,12 @@ static int coroutine_fn stream_run(Job *job, Error **errp) >>> } >>> } >>> + /* >>> + * Complete stream_populate,force flush l2_table_cache,to >>> + * avoid unexpected termination of process, l2_table loss >>> + */ >>> + qcow2_cache_flush(bs, ((BDRVQcow2State *)bs->opaque)->l2_table_cache); >>> + >>> /* Do not remove the backing file if an error was there but ignored. */ >>> return error; >>> } >> >> Hi! >> >> I think, it's more correct just call bdrv_co_flush(bs), which should do all the job. Also, stream_run() should fail if flush fails. >> >> Also, I remember I've done it for all (or at least several) blockjobs generically, so that any blockjob must succesfully flush target to report success.. But now I can find neither my patches nor the code :( Den, Kevin, Hanna, don't you remember this topic? >> > This was a part of compressed write cache series, which was postponed. > > https://lore.kernel.org/all/20210305173507.393137-1-vsementsov@virtuozzo.com/T/#m87315593ed5ab16e5d0e4e7a5ae6d776fbbaec77 > > We have it ported to 7.0 QEMU. > > Not a problem to port to master and resend. > Will this make a sense? > O, thanks! Patch 01 applies with a little conflict to master, so I'll just resend it myself.
>On 25.07.23 18:13, Denis V. Lunev wrote: >>On 7/25/23 16:25, Vladimir Sementsov-Ogievskiy wrote: >>>On 24.07.23 10:30, Evanzhang wrote: >>>> On 7/26/23 01:41, Vladimir Sementsov-Ogievskiy wrote: >>>>block_stream will not actively flush l2_table_cache,when qemu >>>> process exception exit,causing disk data loss >>>> >>>>Signed-off-by: Evanzhang <Evanzhang@archeros.com> >>>>--- >>>> block/stream.c | 6 ++++++ >>>> 1 file changed, 6 insertions(+) >>>> >>> >diff --git a/block/stream.c b/block/stream.c index e522bbd..a5e08da >>>>100644 >>>> --- a/block/stream.c >>>> +++ b/block/stream.c >>>>@@ -207,6 +207,12 @@ static int coroutine_fn stream_run(Job *job, >>>> Error **errp) >>>> } >>>> } >>>> + /* >>>> + * Complete stream_populate,force flush l2_table_cache,to >>>> + * avoid unexpected termination of process, l2_table loss >>>> + */ >>>> + qcow2_cache_flush(bs, ((BDRVQcow2State >>>> +*)bs->opaque)->l2_table_cache); >>>> + >>>> /* Do not remove the backing file if an error was there but >>>> ignored. */ >>>> return error; >>>> } >>> >>> Hi! >>> >>> I think, it's more correct just call bdrv_co_flush(bs), which should do all the job. Also, stream_run() should fail if flush fails. >>> >>> Also, I remember I've done it for all (or at least several) blockjobs generically, so that any blockjob must succesfully flush target to report success.. But now I can find neither my patches nor the code :( Den, Kevin, Hanna, don't you remember this topic? >>> >> This was a part of compressed write cache series, which was postponed. >> >> https://lore.kernel.org/all/20210305173507.393137-1-vsementsov@virtuoz >> zo.com/T/#m87315593ed5ab16e5d0e4e7a5ae6d776fbbaec77 >> >> We have it ported to 7.0 QEMU. >> >> Not a problem to port to master and resend. >> Will this make a sense? >> >O, thanks! Patch 01 applies with a little conflict to master, so I'll just resend it myself. > Thanks all ! With best regards !
diff --git a/block/stream.c b/block/stream.c index e522bbd..a5e08da 100644 --- a/block/stream.c +++ b/block/stream.c @@ -207,6 +207,12 @@ static int coroutine_fn stream_run(Job *job, Error **errp) } } + /* + * Complete stream_populate,force flush l2_table_cache,to + * avoid unexpected termination of process, l2_table loss + */ + qcow2_cache_flush(bs, ((BDRVQcow2State *)bs->opaque)->l2_table_cache); + /* Do not remove the backing file if an error was there but ignored. */ return error; }
block_stream will not actively flush l2_table_cache,when qemu process exception exit,causing disk data loss Signed-off-by: Evanzhang <Evanzhang@archeros.com> --- block/stream.c | 6 ++++++ 1 file changed, 6 insertions(+)