| Message ID | 1423399192-8569-1-git-send-email-gongxiaodong1@huawei.com |
|---|---|
| State | New |
| Headers | show |
On Sun, Feb 08, 2015 at 08:39:52PM +0800, Xiaodong Gong wrote: > When open the vpc snapshot based FIXED format, its backing file, > this FIXED vpc image, could be probed as a raw image, because that > the find_image_format just checkout the first sector. > > Add a re-probe for the last sector to FIXED base vpc image,when get > a NULL or raw driver in first sector probe. > > Signed-off-by: Xiaodong Gong <gongxiaodong1@huawei.com> > --- > block.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 48 insertions(+), 7 deletions(-) This approach is likely to cause issues because it makes no distiction between headers and footers. Most image formats only have a header and would now be misdetected if a raw image had a header in its last sector. When a vpc image has a backing file, does it make sense to set the bs->backing_format to "vpc"? That way we'll try to open the backing file as a vpc image without probing. Stefan
On Mon, Feb 9, 2015 at 10:54 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote: > On Sun, Feb 08, 2015 at 08:39:52PM +0800, Xiaodong Gong wrote: > > When open the vpc snapshot based FIXED format, its backing file, > > this FIXED vpc image, could be probed as a raw image, because that > > the find_image_format just checkout the first sector. > > > > Add a re-probe for the last sector to FIXED base vpc image,when get > > a NULL or raw driver in first sector probe. > > > > Signed-off-by: Xiaodong Gong <gongxiaodong1@huawei.com> > > --- > > block.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++------- > > 1 file changed, 48 insertions(+), 7 deletions(-) > > This approach is likely to cause issues because it makes no distiction > between headers and footers. Most image formats only have a header and > would now be misdetected if a raw image had a header in its last sector. > > When a vpc image has a backing file, does it make sense to set the > bs->backing_format to "vpc"? That way we'll try to open the backing > file as a vpc image without probing. > > Stefan > It works, but this probing of format sounds good . Giving a argument of bs to bdrv_probe and each of funtion of probe get it's own checking method is better, but the foot-print of patch is longer.
On Tue, Feb 10, 2015 at 11:04:31PM +0800, Xiaodong Gong wrote: > On Mon, Feb 9, 2015 at 10:54 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote: > > > On Sun, Feb 08, 2015 at 08:39:52PM +0800, Xiaodong Gong wrote: > > > When open the vpc snapshot based FIXED format, its backing file, > > > this FIXED vpc image, could be probed as a raw image, because that > > > the find_image_format just checkout the first sector. > > > > > > Add a re-probe for the last sector to FIXED base vpc image,when get > > > a NULL or raw driver in first sector probe. > > > > > > Signed-off-by: Xiaodong Gong <gongxiaodong1@huawei.com> > > > --- > > > block.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++------- > > > 1 file changed, 48 insertions(+), 7 deletions(-) > > > > This approach is likely to cause issues because it makes no distiction > > between headers and footers. Most image formats only have a header and > > would now be misdetected if a raw image had a header in its last sector. > > > > When a vpc image has a backing file, does it make sense to set the > > bs->backing_format to "vpc"? That way we'll try to open the backing > > file as a vpc image without probing. > > > > Stefan > > > > > It works, but this probing of format sounds good . Giving a argument of bs > to bdrv_probe and each of funtion of probe get it's own checking method is > better, but the foot-print of patch is longer. I'm not suggesting a longer patch. Can the vpc .bdrv_open() code that loads the backing filename also set bs->backing_format to "vpc" so QEMU will always use vpc for the backing file? Stefan
diff --git a/block.c b/block.c index d45e4dd..e183515 100644 --- a/block.c +++ b/block.c @@ -702,20 +702,24 @@ BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size, return drv; } -static int find_image_format(BlockDriverState *bs, const char *filename, - BlockDriver **pdrv, Error **errp) +static int probe_image_format(BlockDriverState *bs, const char *filename, + int64_t offset, BlockDriver **pdrv, Error **errp, + bool isfoot) { BlockDriver *drv; uint8_t buf[BLOCK_PROBE_BUF_SIZE]; int ret = 0; - /* Return the raw BlockDriver * to scsi-generic devices or empty drives */ - if (bs->sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0) { - *pdrv = &bdrv_raw; + if (!bs || !filename) { + *pdrv = NULL; return ret; } - ret = bdrv_pread(bs, 0, buf, sizeof(buf)); + if (offset + sizeof(buf) > bs->total_sectors << BDRV_SECTOR_BITS) { + return ret; + } + + ret = bdrv_pread(bs, offset, buf, sizeof(buf)); if (ret < 0) { error_setg_errno(errp, -ret, "Could not read image for determining its " "format"); @@ -724,12 +728,49 @@ static int find_image_format(BlockDriverState *bs, const char *filename, } drv = bdrv_probe_all(buf, ret, filename); - if (!drv) { + if (!drv && isfoot) { error_setg(errp, "Could not determine image format: No compatible " "driver found"); ret = -ENOENT; } *pdrv = drv; + + return ret; +} + +static int find_image_format(BlockDriverState *bs, const char *filename, + BlockDriver **pdrv, Error **errp) +{ + int last_sector_offset; + int ret = 0; + + *pdrv = NULL; + + /* Return the raw BlockDriver * to scsi-generic devices or empty drives */ + if (bs->sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0) { + *pdrv = &bdrv_raw; + return ret; + } + + /* probe the header to guess image format */ + ret = probe_image_format(bs, filename, 0, pdrv, errp, false); + if (ret < 0) { + return ret; + } + + /* The FIXED base img of VHD only has the footer in tail. The drv could + * be NULL or raw */ + if (*pdrv && *pdrv != &bdrv_raw) { + return ret; + } + + last_sector_offset = (bs->total_sectors - 1) << BDRV_SECTOR_BITS; + ret = probe_image_format(bs, filename, last_sector_offset, pdrv, + errp, true); + if (ret < 0) { + return ret; + } + return ret; }
When open the vpc snapshot based FIXED format, its backing file, this FIXED vpc image, could be probed as a raw image, because that the find_image_format just checkout the first sector. Add a re-probe for the last sector to FIXED base vpc image,when get a NULL or raw driver in first sector probe. Signed-off-by: Xiaodong Gong <gongxiaodong1@huawei.com> --- block.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 48 insertions(+), 7 deletions(-)