[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: |
Stefano Stabellini |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] xen: Don't peek behind the BlockDriverState abstraction |
Date: |
Wed, 6 Jun 2012 13:23:04 +0100 |
User-agent: |
Alpine 2.02 (DEB 1266 2009-07-14) |
On Wed, 6 Jun 2012, Markus Armbruster wrote:
> 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?
Yes, please :-)
- [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