[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2] xen: Don't peek behind the BlockDriverState
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] xen: Don't peek behind the BlockDriverState abstraction |
Date: |
Wed, 06 Jun 2012 13:39:51 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.3 (gnu/linux) |
Stefano Stabellini <address@hidden> 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 <address@hidden>
>> ---
>> 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!
- [Qemu-devel] [PATCH 0/2] xen: Clean up BlockDriverState use, Markus Armbruster, 2012/06/05
- [Qemu-devel] [PATCH 2/2] xen: Don't peek behind the BlockDriverState abstraction, Markus Armbruster, 2012/06/05
- Re: [Qemu-devel] [PATCH 2/2] xen: Don't peek behind the BlockDriverState abstraction, Peter Maydell, 2012/06/06
- Re: [Qemu-devel] [PATCH 2/2] xen: Don't peek behind the BlockDriverState abstraction, Markus Armbruster, 2012/06/06
- Re: [Qemu-devel] [PATCH 2/2] xen: Don't peek behind the BlockDriverState abstraction, Peter Maydell, 2012/06/06
- Re: [Qemu-devel] [PATCH 2/2] xen: Don't peek behind the BlockDriverState abstraction, Markus Armbruster, 2012/06/07
- Re: [Qemu-devel] [PATCH 2/2] xen: Don't peek behind the BlockDriverState abstraction, Peter Maydell, 2012/06/07
- Re: [Qemu-devel] [PATCH 2/2] xen: Don't peek behind the BlockDriverState abstraction, Markus Armbruster, 2012/06/13
[Qemu-devel] [PATCH 1/2] xen: Don't change -drive if=xen device name during machine init, Markus Armbruster, 2012/06/05