Stefano Stabellini <stefano.stabell...@eu.citrix.com> writes:
> On Tue, 5 Jun 2012, Markus Armbruster wrote:
>> First offender is xen_config_dev_blk()'s use of disk->bdrv->filename.
>> Get the filename from disk->opts instead. Same result, except for
>> snapshots: there, we now get the filename specified by the user
>> instead of the name of the temporary image created by bdrv_open().
>> Should be an improvement.
>>
>> Second offender is blk_init()'s use of blkdev->bs->drv->format_name.
>> Simply use the appropriate interface to get the format name.
>>
>> Signed-off-by: Markus Armbruster <arm...@redhat.com>
>> ---
>> hw/xen_devconfig.c | 6 +++---
>> hw/xen_disk.c | 5 +++--
>> 2 files changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/xen_devconfig.c b/hw/xen_devconfig.c
>> index 7b7b0a2..0928613 100644
>> --- a/hw/xen_devconfig.c
>> +++ b/hw/xen_devconfig.c
>> @@ -1,6 +1,5 @@
>> #include "xen_backend.h"
>> #include "blockdev.h"
>> -#include "block_int.h" /* XXX */
>>
>> /* ------------------------------------------------------------- */
>>
>> @@ -99,10 +98,11 @@ int xen_config_dev_blk(DriveInfo *disk)
>> int cdrom = disk->media_cd;
>> const char *devtype = cdrom ? "cdrom" : "disk";
>> const char *mode = cdrom ? "r" : "w";
>> + const char *filename = qemu_opt_get(disk->opts, "file");
>>
>> snprintf(device_name, sizeof(device_name), "xvd%c", 'a' + disk->unit);
>> xen_be_printf(NULL, 1, "config disk %d [%s]: %s\n",
>> - disk->unit, device_name, disk->bdrv->filename);
>> + disk->unit, device_name, filename);
>> xen_config_dev_dirs("vbd", "qdisk", vdev, fe, be, sizeof(fe));
>>
>> /* frontend */
>> @@ -112,7 +112,7 @@ int xen_config_dev_blk(DriveInfo *disk)
>> /* backend */
>> xenstore_write_str(be, "dev", device_name);
>> xenstore_write_str(be, "type", "file");
>> - xenstore_write_str(be, "params", disk->bdrv->filename);
>> + xenstore_write_str(be, "params", filename);
>> xenstore_write_str(be, "mode", mode);
>>
>> /* common stuff */
>> diff --git a/hw/xen_disk.c b/hw/xen_disk.c
>> index 07594bc..c73b71e 100644
>> --- a/hw/xen_disk.c
>> +++ b/hw/xen_disk.c
>> @@ -40,7 +40,6 @@
>> #include <xen/io/xenbus.h>
>>
>> #include "hw.h"
>> -#include "block_int.h"
>> #include "qemu-char.h"
>> #include "xen_blkif.h"
>> #include "xen_backend.h"
>> @@ -554,6 +553,7 @@ static int blk_init(struct XenDevice *xendev)
>> {
>> struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev,
>> xendev);
>> int index, qflags, info = 0;
>> + char fmt_name[128];
>>
>> /* read xenstore entries */
>> if (blkdev->params == NULL) {
>> @@ -634,9 +634,10 @@ static int blk_init(struct XenDevice *xendev)
>> blkdev->file_blk = BLOCK_SIZE;
>> blkdev->file_size = bdrv_getlength(blkdev->bs);
>> if (blkdev->file_size < 0) {
>> + bdrv_get_format(blkdev->bs, fmt_name, sizeof(fmt_name));
>> xen_be_printf(&blkdev->xendev, 1, "bdrv_getlength: %d (%s) | drv
>> %s\n",
>> (int)blkdev->file_size, strerror(-blkdev->file_size),
>> - blkdev->bs->drv ? blkdev->bs->drv->format_name : "-");
>> + fmt_name[0] ? fmt_name : "-");
>> blkdev->file_size = 0;
>> }
>
> You might as well move fmt_name here because it is only used if
> blkdev->file_size < 0.
Matter of taste, and you're the maintainer. Want me to respin?
> Apart from this minor nitpick, both patches are OK.
Thanks!
