On Mon, 1 Apr 2013, Alex Bligh wrote: > Stefano, > > --On 1 April 2013 16:44:05 +0100 Stefano Stabellini > <stefano.stabell...@eu.citrix.com> wrote: > > >> Note this patch is compile-tested only. > > > > I think the patch looks good, just a minor comment. > > Thanks. I guess I ought to actually test it works then :-) > > >> + /* 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. > > My concern (not knowing how xenstore works) was the possibility of leaving > them as uninitialized values. I'm taking it that if I don't initialise > them, the key is just absent - correct?
Right > I'll redo and move setting sector-size and sectors into blk_connect, and > then add a second commit which removes BDRV_O_NOCACHE. Oh, and test it ... Good :)