xen-block: only advertize discard to the frontend when it is enabled...

...and properly enable it when synthesizing a drive.

The Xen toolstack sets 'discard-enable' to '1' in xenstore when it wants
to enable discard on a specified image. The code in
xen_block_drive_create() correctly parses this and uses it to set
'discard' to 'unmap' for the file_layer, but fails to do the same for the
driver_layer (which effectively disables it). Meanwhile the code in
xen_block_realize() advertizes discard support to the frontend in the
default case (because conf->discard_granularity defaults to -1), even when
the underlying image may not handle it.

This patch adds the missing option to the driver_layer in
xen_block_driver_create() and checks whether BDRV_O_UNMAP is actually
set on the block device before advertizing discard to the frontend.
In the case that discard is supported it also makes sure that the
granularity is set to the physical block size.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Message-Id: <20190320142825.24565-1-paul.durrant@citrix.com>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

1 files changed, 9 insertions, 3 deletions

diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index 9c722b9b95..475a67845d 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -232,8 +232,14 @@ static void xen_block_realize(XenDevice *xendev, Error **errp)
     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) {
+    if (conf->discard_granularity == -1) {
+        conf->discard_granularity = conf->physical_block_size;
+    }
+
+    if (blk_get_flags(conf->blk) & BDRV_O_UNMAP) {
         xen_device_backend_printf(xendev, "feature-discard", "%u", 1);
+        xen_device_backend_printf(xendev, "discard-granularity", "%u",
+                                  conf->discard_granularity);
     }
 
     xen_device_backend_printf(xendev, "feature-flush-cache", "%u", 1);
@@ -755,6 +761,7 @@ static XenBlockDrive *xen_block_drive_create(const char *id,
     drive->id = g_strdup(id);
 
     file_layer = qdict_new();
+    driver_layer = qdict_new();
 
     qdict_put_str(file_layer, "driver", "file");
     qdict_put_str(file_layer, "filename", filename);
@@ -782,6 +789,7 @@ static XenBlockDrive *xen_block_drive_create(const char *id,
 
         if (!qemu_strtoul(discard_enable, NULL, 2, &value) && !!value) {
             qdict_put_str(file_layer, "discard", "unmap");
+            qdict_put_str(driver_layer, "discard", "unmap");
         }
     }
 
@@ -791,8 +799,6 @@ static XenBlockDrive *xen_block_drive_create(const char *id,
      */
     qdict_put_str(file_layer, "locking", "off");
 
-    driver_layer = qdict_new();
-
     qdict_put_str(driver_layer, "driver", driver);
     g_free(driver);