diff mbox

block: allow write-threshold on device name

Message ID 55784F7C.1060702@redhat.com
State New
Headers show

Commit Message

Eric Blake June 10, 2015, 2:53 p.m. UTC 
On 06/10/2015 07:43 AM, Kevin Wolf wrote:

> 
>>> Let's stay away from such magic, as much as we can. libvirt can just
>>> specify a node-name for the protocol layer and use that.
>>
>> Okay, I'll probably abandon this patch, then, but still work on
>> something to make node names easier for libvirt to integrate with.
> 
> Hm, okay. I would find it nice to accept device names everywhere where a
> node name is expected (just for aesthetic reasons), but I see that it
> would probably remain unused, so abandoning the patch is okay with me.

For the record, here's the state of the patch that I reached before this
email, which includes documentation and proper device name output - but
the way I implemented it was is a semantic change; so all the more
reason to require node names only for this interface (and avoid the
semantic change).  The commit message body is not quite accurate (now
that I've learned more, the active BDS of the device is NOT the node
that libvirt wants, so adding device semantics did not help libvirt), so
much as capturing the state of the patch before I abandon it.
diff mbox

Patch

From 4f2f9838fbe7b06e8703b57f4b2559295b435964 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Sat, 6 Jun 2015 16:00:26 -0600
Subject: [PATCH v2] block: allow write-threshold on device name

Commit e2462113 allowed the ability to fire an event if a BDS
node exceeds a threshold during a write, but limited the option
to only work on node names.  For convenience, expand this to
allow a device name as a way to set the threshold on the BDS
at the active layer of the device.

Previously, it was not possible to set a threshold without a
node name, so the event was only ever reported on a BDS with a
node name, even when that node was the active layer of a device.
Now, it is possible that an event can occur on a device whose
active BDS does not have a node name; but a name must still be
reported.  I found it easier to always report a device name,
when one is available, which causes a slight change in behavior
when node names are in use (the event now reports the device
name rather than the node name of the active BDS); but as the
two namespaces cannot overlap, the result is still unambiguous.
Besides, the new semantics makes libvirt's life easier for as
long as libvirt does not yet use node names, so I don't think
it will hurt.

Signed-off-by: Eric Blake <eblake@redhat.com>

---

v2: Update docs, and report device name when available.
---
 block/write-threshold.c | 7 +++----
 qapi/block-core.json    | 6 +++---
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/block/write-threshold.c b/block/write-threshold.c
index a53c1f5..0082086 100644
--- a/block/write-threshold.c
+++ b/block/write-threshold.c
@@ -1,7 +1,7 @@ 
 /*
  * QEMU System Emulator block write threshold notification
  *
- * Copyright Red Hat, Inc. 2014
+ * Copyright Red Hat, Inc. 2014, 2015
  *
  * Authors:
  *  Francesco Romani <fromani@redhat.com>
@@ -60,7 +60,7 @@  static int coroutine_fn before_write_notify(NotifierWithReturn *notifier,
     amount = bdrv_write_threshold_exceeded(bs, req);
     if (amount > 0) {
         qapi_event_send_block_write_threshold(
-            bs->node_name,
+            bdrv_get_device_or_node_name(bs),
             amount,
             bs->write_threshold_offset,
             &error_abort);
@@ -110,9 +110,8 @@  void qmp_block_set_write_threshold(const char *node_name,
     BlockDriverState *bs;
     AioContext *aio_context;

-    bs = bdrv_find_node(node_name);
+    bs = bdrv_lookup_bs(node_name, node_name, errp);
     if (!bs) {
-        error_setg(errp, "Device '%s' not found", node_name);
         return;
     }

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 8411d4f..e1b48fe 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2042,9 +2042,9 @@ 
 # means the device should be extended to avoid pausing for
 # disk exhaustion.
 # The event is one shot. Once triggered, it needs to be
-# re-registered with another block-set-threshold command.
+# re-registered with another block-set-write-threshold command.
 #
-# @node-name: graph node name on which the threshold was exceeded.
+# @node-name: device or graph node name on which the threshold was exceeded.
 #
 # @amount-exceeded: amount of data which exceeded the threshold, in bytes.
 #
@@ -2065,7 +2065,7 @@ 
 # This is useful to transparently resize thin-provisioned drives without
 # the guest OS noticing.
 #
-# @node-name: graph node name on which the threshold must be set.
+# @node-name: device or graph node name on which the threshold must be set.
 #
 # @write-threshold: configured threshold for the block device, bytes.
 #                   Use 0 to disable the threshold.
-- 
2.4.2