Re: [Xen-devel] [PATCH] [RFC] Xen PV backend: Move call to bdrv_new from blk_init to blk_connect

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Mon, 1 Apr 2013, Alex Bligh wrote:
> This commit delays the point at which bdrv_new (and hence blk_open
> on the underlying device) is called from blk_init to blk_connect.
> This ensures that in an inbound live migrate, the block device is
> not opened until it has been closed at the other end. This is in
> preparation for supporting devices with open/close consistency
> without using O_DIRECT. This commit does NOT itself change O_DIRECT
> semantics.
> 
> Note this patch is compile-tested only.

I think the patch looks good, just a minor comment.


> Signed-off-by: Alex Bligh <alex@xxxxxxxxxxx>
> ---
>  hw/xen_disk.c |   73 
> +++++++++++++++++++++++++++++++++------------------------
>  1 file changed, 42 insertions(+), 31 deletions(-)
> 
> diff --git a/hw/xen_disk.c b/hw/xen_disk.c
> index 69e1d9d..3cccea8 100644
> --- a/hw/xen_disk.c
> +++ b/hw/xen_disk.c
> @@ -701,7 +701,7 @@ static void blk_alloc(struct XenDevice *xendev)
>  static int blk_init(struct XenDevice *xendev)
>  {
>      struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, 
> xendev);
> -    int index, qflags, info = 0;
> +    int info = 0;
>  
>      /* read xenstore entries */
>      if (blkdev->params == NULL) {
> @@ -744,10 +744,7 @@ static int blk_init(struct XenDevice *xendev)
>      }
>  
>      /* read-only ? */
> -    qflags = BDRV_O_NOCACHE | BDRV_O_CACHE_WB | BDRV_O_NATIVE_AIO;
> -    if (strcmp(blkdev->mode, "w") == 0) {
> -        qflags |= BDRV_O_RDWR;
> -    } else {
> +    if (strcmp(blkdev->mode, "w")) {
>          info  |= VDISK_READONLY;
>      }
>  
> @@ -756,6 +753,44 @@ static int blk_init(struct XenDevice *xendev)
>          info  |= VDISK_CDROM;
>      }
>  
> +    blkdev->file_blk  = BLOCK_SIZE;
> +
> +    /* fill info
> +     * Temporarily write zero sectors as we won't know file size until
> +     * bdrv_new has been called. blk_connect corrects this.
> +     */
> +    xenstore_write_be_int(&blkdev->xendev, "feature-flush-cache", 1);
> +    xenstore_write_be_int(&blkdev->xendev, "feature-persistent", 1);
> +    xenstore_write_be_int(&blkdev->xendev, "info", info);
> +    xenstore_write_be_int(&blkdev->xendev, "sector-size", BLOCK_SIZE);
> +    xenstore_write_be_int(&blkdev->xendev, "sectors", 0);
> +    return 0;

There is no need to fill the sector-size and sectors info here, you can
do it later in blk_connect.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel