[Qemu-devel] [PATCH] xen-block: handle resize callback

[Qemu-devel] [PATCH] xen-block: handle resize callback

[Paul Durrant]

Some frontend drivers will handle dynamic resizing of PV disks, so set up
the BlockDevOps resize_cb() method during xen_block_realize() to allow
this to be done.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>
---
 hw/block/dataplane/xen-block.c |  4 +---
 hw/block/trace-events          |  1 +
 hw/block/xen-block.c           | 26 ++++++++++++++++++++++----
 3 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/hw/block/dataplane/xen-block.c b/hw/block/dataplane/xen-block.c
index d0d8905a33..c6a15da024 100644
--- a/hw/block/dataplane/xen-block.c
+++ b/hw/block/dataplane/xen-block.c
@@ -50,7 +50,6 @@ struct XenBlockDataPlane {
     unsigned int nr_ring_ref;
     void *sring;
     int64_t file_blk;
-    int64_t file_size;
     int protocol;
     blkif_back_rings_t rings;
     int more_work;
@@ -189,7 +188,7 @@ static int xen_block_parse_request(XenBlockRequest *request)
                request->req.seg[i].first_sect + 1) * dataplane->file_blk;
         request->size += len;
     }
-    if (request->start + request->size > dataplane->file_size) {
+    if (request->start + request->size > blk_getlength(dataplane->blk)) {
         error_report("error: access beyond end of file");
         goto err;
     }
@@ -638,7 +637,6 @@ XenBlockDataPlane *xen_block_dataplane_create(XenDevice *xendev,
     dataplane->xendev = xendev;
     dataplane->file_blk = conf->logical_block_size;
     dataplane->blk = conf->blk;
-    dataplane->file_size = blk_getlength(dataplane->blk);
 
     QLIST_INIT(&dataplane->inflight);
     QLIST_INIT(&dataplane->freelist);
diff --git a/hw/block/trace-events b/hw/block/trace-events
index d0851953c5..8020f9226a 100644
--- a/hw/block/trace-events
+++ b/hw/block/trace-events
@@ -126,6 +126,7 @@ xen_block_realize(const char *type, uint32_t disk, uint32_t partition) "%s d%up%
 xen_block_connect(const char *type, uint32_t disk, uint32_t partition) "%s d%up%u"
 xen_block_disconnect(const char *type, uint32_t disk, uint32_t partition) "%s d%up%u"
 xen_block_unrealize(const char *type, uint32_t disk, uint32_t partition) "%s d%up%u"
+xen_block_size(const char *type, uint32_t disk, uint32_t partition, int64_t sectors) "%s d%up%u %"PRIi64
 xen_disk_realize(void) ""
 xen_disk_unrealize(void) ""
 xen_cdrom_realize(void) ""
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index a636487b3e..8ea28381ea 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -144,6 +144,24 @@ static void xen_block_unrealize(XenDevice *xendev, Error **errp)
     }
 }
 
+static void xen_block_resize_cb(void *opaque)
+{
+    XenBlockDevice *blockdev = opaque;
+    const char *type = object_get_typename(OBJECT(blockdev));
+    XenBlockVdev *vdev = &blockdev->props.vdev;
+    BlockConf *conf = &blockdev->props.conf;
+    XenDevice *xendev = XEN_DEVICE(blockdev);
+    int64_t sectors = blk_getlength(conf->blk) / conf->logical_block_size;
+
+    trace_xen_block_size(type, vdev->disk, vdev->partition, sectors);
+
+    xen_device_backend_printf(xendev, "sectors", "%"PRIi64, sectors);
+}
+
+static const BlockDevOps xen_block_dev_ops = {
+    .resize_cb = xen_block_resize_cb,
+};
+
 static void xen_block_realize(XenDevice *xendev, Error **errp)
 {
     XenBlockDevice *blockdev = XEN_BLOCK_DEVICE(xendev);
@@ -180,7 +198,7 @@ static void xen_block_realize(XenDevice *xendev, Error **errp)
     }
 
     if (!blkconf_apply_backend_options(conf, blockdev->info & VDISK_READONLY,
-                                       false, errp)) {
+                                       true, errp)) {
         return;
     }
 
@@ -197,6 +215,7 @@ static void xen_block_realize(XenDevice *xendev, Error **errp)
         return;
     }
 
+    blk_set_dev_ops(conf->blk, &xen_block_dev_ops, blockdev);
     blk_set_guest_block_size(conf->blk, conf->logical_block_size);
 
     if (conf->discard_granularity > 0) {
@@ -215,9 +234,8 @@ static void xen_block_realize(XenDevice *xendev, Error **errp)
 
     xen_device_backend_printf(xendev, "sector-size", "%u",
                               conf->logical_block_size);
-    xen_device_backend_printf(xendev, "sectors", "%"PRIi64,
-                              blk_getlength(conf->blk) /
-                              conf->logical_block_size);
+
+    xen_block_dev_ops.resize_cb(blockdev);
 
     blockdev->dataplane =
         xen_block_dataplane_create(xendev, conf, blockdev->props.iothread);
-- 
2.20.1.2.gb21ebb6

Re: [Qemu-devel] [PATCH] xen-block: handle resize callback

[Anthony PERARD]

On Wed, Jan 23, 2019 at 09:08:49AM +0000, Paul Durrant wrote:
> Some frontend drivers will handle dynamic resizing of PV disks, so set up
> the BlockDevOps resize_cb() method during xen_block_realize() to allow
> this to be done.

"will": which drivers are you thinking about? The Linux one seems to be
able to handle resize already.

About the Linux one, it check the new size only when the backend set
its "state" to "connected" again.
It's frontend seems to implement resize with
1fa73be6be65028a7543bba8f14474b42e064a1b.
There is this is the source code:
    static void blkfront_connect(struct blkfront_info *info)
    {
        // ...
        switch (info->connected) {
        case BLKIF_STATE_CONNECTED:
                /*
                 * Potentially, the back-end may be signalling
                 * a capacity change; update the capacity.
                 */

In the backend, Linux does this:
    xenbus_printf(xbt, dev->nodename, "sectors", "%llu", ...
    /*
     * Write the current state; we will use this to synchronize
     * the front-end. If the current state is "connected" the
     * front-end will get the new size information online.
     */
     xenbus_printf(xbt, dev->nodename, "state", "%d", dev->state);

Maybe the QEMU backend needs do to the same thing, and write its current
state again?

FreeBSD doesn't seems to care about resize.

And there is nothing in blkif.h about resizing :(.

Thanks,

-- 
Anthony PERARD

Re: [Qemu-devel] [PATCH] xen-block: handle resize callback

[Paul Durrant]

> -----Original Message-----
> From: Anthony PERARD [mailto:anthony.perard@citrix.com]
> Sent: 29 January 2019 12:25
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: xen-devel@lists.xenproject.org; qemu-block@nongnu.org; qemu-
> devel@nongnu.org; Stefan Hajnoczi <stefanha@redhat.com>; Stefano
> Stabellini <sstabellini@kernel.org>; Kevin Wolf <kwolf@redhat.com>; Max
> Reitz <mreitz@redhat.com>
> Subject: Re: [PATCH] xen-block: handle resize callback
> 
> On Wed, Jan 23, 2019 at 09:08:49AM +0000, Paul Durrant wrote:
> > Some frontend drivers will handle dynamic resizing of PV disks, so set
> up
> > the BlockDevOps resize_cb() method during xen_block_realize() to allow
> > this to be done.
> 
> "will": which drivers are you thinking about? The Linux one seems to be
> able to handle resize already.

Yes, that's what I meant by 'will'... it wasn't supposed to imply future tense. English can be confusing :-/

> 
> About the Linux one, it check the new size only when the backend set
> its "state" to "connected" again.
> It's frontend seems to implement resize with
> 1fa73be6be65028a7543bba8f14474b42e064a1b.
> There is this is the source code:
>     static void blkfront_connect(struct blkfront_info *info)
>     {
>         // ...
>         switch (info->connected) {
>         case BLKIF_STATE_CONNECTED:
>                 /*
>                  * Potentially, the back-end may be signalling
>                  * a capacity change; update the capacity.
>                  */
> 
> In the backend, Linux does this:
>     xenbus_printf(xbt, dev->nodename, "sectors", "%llu", ...
>     /*
>      * Write the current state; we will use this to synchronize
>      * the front-end. If the current state is "connected" the
>      * front-end will get the new size information online.
>      */
>      xenbus_printf(xbt, dev->nodename, "state", "%d", dev->state);
> 
> Maybe the QEMU backend needs do to the same thing, and write its current
> state again?

Yes, that can easily be done. The Windows frontend simply re-reads 'sectors' whenever it sees any change in the backend area (it watches the top level key rather than just the 'state' key).

> 
> FreeBSD doesn't seems to care about resize.
> 
> And there is nothing in blkif.h about resizing :(.

Nope, hence the discrepancy between the frontend implementations. I can send a patch to xen-devel to note the existing state of affairs and perhaps standardize on the re-writing of 'state' being the official way to inform the frontend.

  Paul

> 
> Thanks,
> 
> --
> Anthony PERARD