| Message ID | 1361875228-15769-3-git-send-email-xiawenc@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes: > Compared to bdrv_can_snapshot(), this function return whether > bs* is ready to read snapshot info from instead of write. If yes, > caller can then query snapshot information, but taking snapshot > is not always possible for that *bs may be read only. > > Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> > Reviewed-by: Eric Blake <eblake@redhat.com> > --- > block.c | 19 +++++++++++++++++++ > include/block/block.h | 1 + > 2 files changed, 20 insertions(+), 0 deletions(-) > > diff --git a/block.c b/block.c > index 50dab8e..19c2d7b 100644 > --- a/block.c > +++ b/block.c > @@ -3058,6 +3058,25 @@ bool bdrv_debug_is_suspended(BlockDriverState *bs, const char *tag) > /**************************************************************/ > /* handling of snapshots */ > > +/* return whether internal snapshot can be read on @bs */ > +bool bdrv_can_read_snapshot(BlockDriverState *bs) > +{ > + BlockDriver *drv = bs->drv; > + if (!drv || !bdrv_is_inserted(bs)) { > + return false; > + } > + > + if (!drv->bdrv_snapshot_create) { > + if (bs->file != NULL) { > + return bdrv_can_read_snapshot(bs->file); > + } > + return false; > + } > + > + return true; > +} Looks like bdrv_can_read_snapshot(bs) && !bdrv_is_read_only(bs) == bdrv_can_snapshot(bs) Correct? > + > +/* return whether internal snapshot can be write on @bs */ "be written". Or more succinctly: /* Can @bs take a snapshot? */ > int bdrv_can_snapshot(BlockDriverState *bs) > { > BlockDriver *drv = bs->drv; > diff --git a/include/block/block.h b/include/block/block.h > index 5c3b911..4c48052 100644 > --- a/include/block/block.h > +++ b/include/block/block.h > @@ -321,6 +321,7 @@ void bdrv_get_full_backing_filename(BlockDriverState *bs, > char *dest, size_t sz); > BlockInfo *bdrv_query_info(BlockDriverState *s); > BlockStats *bdrv_query_stats(const BlockDriverState *bs); > +bool bdrv_can_read_snapshot(BlockDriverState *bs); > int bdrv_can_snapshot(BlockDriverState *bs); > int bdrv_is_snapshot(BlockDriverState *bs); > BlockDriverState *bdrv_snapshots(void);
于 2013-2-27 21:58, Markus Armbruster 写道: > Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes: > >> Compared to bdrv_can_snapshot(), this function return whether >> bs* is ready to read snapshot info from instead of write. If yes, >> caller can then query snapshot information, but taking snapshot >> is not always possible for that *bs may be read only. >> >> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> >> Reviewed-by: Eric Blake <eblake@redhat.com> >> --- >> block.c | 19 +++++++++++++++++++ >> include/block/block.h | 1 + >> 2 files changed, 20 insertions(+), 0 deletions(-) >> >> diff --git a/block.c b/block.c >> index 50dab8e..19c2d7b 100644 >> --- a/block.c >> +++ b/block.c >> @@ -3058,6 +3058,25 @@ bool bdrv_debug_is_suspended(BlockDriverState *bs, const char *tag) >> /**************************************************************/ >> /* handling of snapshots */ >> >> +/* return whether internal snapshot can be read on @bs */ >> +bool bdrv_can_read_snapshot(BlockDriverState *bs) >> +{ >> + BlockDriver *drv = bs->drv; >> + if (!drv || !bdrv_is_inserted(bs)) { >> + return false; >> + } >> + >> + if (!drv->bdrv_snapshot_create) { >> + if (bs->file != NULL) { >> + return bdrv_can_read_snapshot(bs->file); >> + } >> + return false; >> + } >> + >> + return true; >> +} > > Looks like > > bdrv_can_read_snapshot(bs) && !bdrv_is_read_only(bs) == bdrv_can_snapshot(bs) > > Correct? > not equal, it is tricky for that recusively check is done inside bdrv_can_snapshot(bs), that is why I did not use a common internal static function. >> + >> +/* return whether internal snapshot can be write on @bs */ > > "be written". > > Or more succinctly: > > /* Can @bs take a snapshot? */ > OK, this seems better. >> int bdrv_can_snapshot(BlockDriverState *bs) >> { >> BlockDriver *drv = bs->drv; >> diff --git a/include/block/block.h b/include/block/block.h >> index 5c3b911..4c48052 100644 >> --- a/include/block/block.h >> +++ b/include/block/block.h >> @@ -321,6 +321,7 @@ void bdrv_get_full_backing_filename(BlockDriverState *bs, >> char *dest, size_t sz); >> BlockInfo *bdrv_query_info(BlockDriverState *s); >> BlockStats *bdrv_query_stats(const BlockDriverState *bs); >> +bool bdrv_can_read_snapshot(BlockDriverState *bs); >> int bdrv_can_snapshot(BlockDriverState *bs); >> int bdrv_is_snapshot(BlockDriverState *bs); >> BlockDriverState *bdrv_snapshots(void); >
On Tue, Feb 26, 2013 at 06:40:16PM +0800, Wenchao Xia wrote: > Compared to bdrv_can_snapshot(), this function return whether > bs* is ready to read snapshot info from instead of write. If yes, > caller can then query snapshot information, but taking snapshot > is not always possible for that *bs may be read only. > > Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> > Reviewed-by: Eric Blake <eblake@redhat.com> > --- > block.c | 19 +++++++++++++++++++ > include/block/block.h | 1 + > 2 files changed, 20 insertions(+), 0 deletions(-) > > diff --git a/block.c b/block.c > index 50dab8e..19c2d7b 100644 > --- a/block.c > +++ b/block.c > @@ -3058,6 +3058,25 @@ bool bdrv_debug_is_suspended(BlockDriverState *bs, const char *tag) > /**************************************************************/ > /* handling of snapshots */ > > +/* return whether internal snapshot can be read on @bs */ > +bool bdrv_can_read_snapshot(BlockDriverState *bs) Which operations are included in "read a snapshot"? Candidate list: * bdrv_snapshot_load_tmp() * bdrv_snapshot_list() * bdrv_snapshot_dump() * bdrv_load_vmstate() * others? Please document the meaning of this function so that others can use/modify it correctly in the future. Stefan
于 2013-2-28 23:20, Stefan Hajnoczi 写道: > On Tue, Feb 26, 2013 at 06:40:16PM +0800, Wenchao Xia wrote: >> Compared to bdrv_can_snapshot(), this function return whether >> bs* is ready to read snapshot info from instead of write. If yes, >> caller can then query snapshot information, but taking snapshot >> is not always possible for that *bs may be read only. >> >> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> >> Reviewed-by: Eric Blake <eblake@redhat.com> >> --- >> block.c | 19 +++++++++++++++++++ >> include/block/block.h | 1 + >> 2 files changed, 20 insertions(+), 0 deletions(-) >> >> diff --git a/block.c b/block.c >> index 50dab8e..19c2d7b 100644 >> --- a/block.c >> +++ b/block.c >> @@ -3058,6 +3058,25 @@ bool bdrv_debug_is_suspended(BlockDriverState *bs, const char *tag) >> /**************************************************************/ >> /* handling of snapshots */ >> >> +/* return whether internal snapshot can be read on @bs */ >> +bool bdrv_can_read_snapshot(BlockDriverState *bs) > > Which operations are included in "read a snapshot"? > > Candidate list: > * bdrv_snapshot_load_tmp() > * bdrv_snapshot_list() > * bdrv_snapshot_dump() > * bdrv_load_vmstate() > * others? > > Please document the meaning of this function so that others can > use/modify it correctly in the future. > > Stefan > OK.
diff --git a/block.c b/block.c index 50dab8e..19c2d7b 100644 --- a/block.c +++ b/block.c @@ -3058,6 +3058,25 @@ bool bdrv_debug_is_suspended(BlockDriverState *bs, const char *tag) /**************************************************************/ /* handling of snapshots */ +/* return whether internal snapshot can be read on @bs */ +bool bdrv_can_read_snapshot(BlockDriverState *bs) +{ + BlockDriver *drv = bs->drv; + if (!drv || !bdrv_is_inserted(bs)) { + return false; + } + + if (!drv->bdrv_snapshot_create) { + if (bs->file != NULL) { + return bdrv_can_read_snapshot(bs->file); + } + return false; + } + + return true; +} + +/* return whether internal snapshot can be write on @bs */ int bdrv_can_snapshot(BlockDriverState *bs) { BlockDriver *drv = bs->drv; diff --git a/include/block/block.h b/include/block/block.h index 5c3b911..4c48052 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -321,6 +321,7 @@ void bdrv_get_full_backing_filename(BlockDriverState *bs, char *dest, size_t sz); BlockInfo *bdrv_query_info(BlockDriverState *s); BlockStats *bdrv_query_stats(const BlockDriverState *bs); +bool bdrv_can_read_snapshot(BlockDriverState *bs); int bdrv_can_snapshot(BlockDriverState *bs); int bdrv_is_snapshot(BlockDriverState *bs); BlockDriverState *bdrv_snapshots(void);