| Message ID | 1300902055-25850-3-git-send-email-stefanha@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> wrote: > The block layer caches the device size to avoid doing lseek(fd, 0, > SEEK_END) every time this value is needed. For removable media the > device size becomes stale if a new medium is inserted. This patch > simply prevents device size caching for removable media. > > A smarter solution is to update the cached device size when a new medium > is inserted. Given that there are currently bugs with CD-ROM media > change I do not want to implement that approach until we've gotten > things correct first. > > Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> > --- > block.c | 12 +++++------- > 1 files changed, 5 insertions(+), 7 deletions(-) > > diff --git a/block.c b/block.c > index 8f224b4..89f6ded 100644 > --- a/block.c > +++ b/block.c > @@ -1153,14 +1153,12 @@ int64_t bdrv_getlength(BlockDriverState *bs) > if (!drv) > return -ENOMEDIUM; > > - /* Fixed size devices use the total_sectors value for speed instead of > - issuing a length query (like lseek) on each call. Also, legacy block > - drivers don't provide a bdrv_getlength function and must use > - total_sectors. */ > - if (!bs->growable || !drv->bdrv_getlength) { if (!bs->growable || !bs->removable|| !drv->bdrv_getlength) { changing just the test don't give exactly the same result? > - return bs->total_sectors * BDRV_SECTOR_SIZE; > - } > - return drv->bdrv_getlength(bs); > + if (bs->growable || bs->removable) { > + if (drv->bdrv_getlength) { > + return drv->bdrv_getlength(bs); > + } > + } > + return bs->total_sectors * BDRV_SECTOR_SIZE; > } > > /* return 0 as number of sectors if no device present or error */
On Wed, Mar 23, 2011 at 8:18 PM, Juan Quintela <quintela@redhat.com> wrote: > Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> wrote: >> diff --git a/block.c b/block.c >> index 8f224b4..89f6ded 100644 >> --- a/block.c >> +++ b/block.c >> @@ -1153,14 +1153,12 @@ int64_t bdrv_getlength(BlockDriverState *bs) >> if (!drv) >> return -ENOMEDIUM; >> >> - /* Fixed size devices use the total_sectors value for speed instead of >> - issuing a length query (like lseek) on each call. Also, legacy block >> - drivers don't provide a bdrv_getlength function and must use >> - total_sectors. */ >> - if (!bs->growable || !drv->bdrv_getlength) { > > if (!bs->growable || !bs->removable|| !drv->bdrv_getlength) { > > changing just the test don't give exactly the same result? I didn't like the inverted logic. I think it's clearer this way. Stefan
diff --git a/block.c b/block.c index 8f224b4..89f6ded 100644 --- a/block.c +++ b/block.c @@ -1153,14 +1153,12 @@ int64_t bdrv_getlength(BlockDriverState *bs) if (!drv) return -ENOMEDIUM; - /* Fixed size devices use the total_sectors value for speed instead of - issuing a length query (like lseek) on each call. Also, legacy block - drivers don't provide a bdrv_getlength function and must use - total_sectors. */ - if (!bs->growable || !drv->bdrv_getlength) { - return bs->total_sectors * BDRV_SECTOR_SIZE; - } - return drv->bdrv_getlength(bs); + if (bs->growable || bs->removable) { + if (drv->bdrv_getlength) { + return drv->bdrv_getlength(bs); + } + } + return bs->total_sectors * BDRV_SECTOR_SIZE; } /* return 0 as number of sectors if no device present or error */
The block layer caches the device size to avoid doing lseek(fd, 0, SEEK_END) every time this value is needed. For removable media the device size becomes stale if a new medium is inserted. This patch simply prevents device size caching for removable media. A smarter solution is to update the cached device size when a new medium is inserted. Given that there are currently bugs with CD-ROM media change I do not want to implement that approach until we've gotten things correct first. Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> --- block.c | 12 +++++------- 1 files changed, 5 insertions(+), 7 deletions(-)