* [Qemu-devel] [PATCH v7 0/3] modify vote rules for flush operation
@ 2016-02-26 1:38 Changlong Xie
2016-02-26 1:39 ` [Qemu-devel] [PATCH v7 1/3] docs: fix invalid node name in qmp event Changlong Xie
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Changlong Xie @ 2016-02-26 1:38 UTC (permalink / raw)
To: qemu devel, Alberto Garcia, Eric Blake, Kevin Wolf, Max Reitz,
Markus Armbruster
Cc: Dr. David Alan Gilbert
ChangLog:
v7:
1. Fix coding style in doc/qmp-event.txt
v6:
1. Make "type" mandatory for [PATCH 2/3]
v5:
1. Fix invalid node name in docs/qmp-events.txt
2. Address comments from Eric, drop QUORUM_FLUSH_ERROR. Instead of reworking
QUORUM_REPORT_BAD to make it compatible with quorum read/write/flush operations
v4:
1. Introduce QUORUM_FLUSH_ERROR event to notify flush failure.
v3:
1. *Note* that, the codes logic is different from what we talked in v2.
I just keep flush interface the same logic as quorum read/write, and think
it's reasoned.
[Ref] http://lists.nongnu.org/archive/html/qemu-devel/2016-01/msg05342.html
Changlong Xie (3):
docs: fix invalid node name in qmp event
qmp event: Refactor QUORUM_REPORT_BAD
quorum: modify vote rules for flush operation
block/quorum.c | 38 ++++++++++++++++++++++++++++----------
docs/qmp-events.txt | 11 ++++++++++-
qapi/block.json | 16 ++++++++++++++++
qapi/event.json | 4 +++-
4 files changed, 57 insertions(+), 12 deletions(-)
--
1.9.3
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH v7 1/3] docs: fix invalid node name in qmp event
2016-02-26 1:38 [Qemu-devel] [PATCH v7 0/3] modify vote rules for flush operation Changlong Xie
@ 2016-02-26 1:39 ` Changlong Xie
2016-02-26 1:39 ` [Qemu-devel] [PATCH v7 2/3] qmp event: Refactor QUORUM_REPORT_BAD Changlong Xie
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Changlong Xie @ 2016-02-26 1:39 UTC (permalink / raw)
To: qemu devel, Alberto Garcia, Eric Blake, Kevin Wolf, Max Reitz,
Markus Armbruster
Cc: Dr. David Alan Gilbert
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: Wen Congyang <wency@cn.fujitsu.com>
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
Signed-off-by: Changlong Xie <xiecl.fnst@cn.fujitsu.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
docs/qmp-events.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/docs/qmp-events.txt b/docs/qmp-events.txt
index 4e3eb9e..d6b9aea 100644
--- a/docs/qmp-events.txt
+++ b/docs/qmp-events.txt
@@ -337,7 +337,7 @@ Data:
Example:
{ "event": "QUORUM_REPORT_BAD",
- "data": { "node-name": "1.raw", "sector-num": 345435, "sectors-count": 5 },
+ "data": { "node-name": "node0", "sector-num": 345435, "sectors-count": 5 },
"timestamp": { "seconds": 1344522075, "microseconds": 745528 } }
Note: this event is rate-limited.
--
1.9.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH v7 2/3] qmp event: Refactor QUORUM_REPORT_BAD
2016-02-26 1:38 [Qemu-devel] [PATCH v7 0/3] modify vote rules for flush operation Changlong Xie
2016-02-26 1:39 ` [Qemu-devel] [PATCH v7 1/3] docs: fix invalid node name in qmp event Changlong Xie
@ 2016-02-26 1:39 ` Changlong Xie
2016-02-26 7:33 ` Alberto Garcia
2016-03-01 15:57 ` Eric Blake
2016-02-26 1:39 ` [Qemu-devel] [PATCH v7 3/3] quorum: modify vote rules for flush operation Changlong Xie
2016-03-01 15:39 ` [Qemu-devel] [PATCH v7 0/3] " Kevin Wolf
3 siblings, 2 replies; 8+ messages in thread
From: Changlong Xie @ 2016-02-26 1:39 UTC (permalink / raw)
To: qemu devel, Alberto Garcia, Eric Blake, Kevin Wolf, Max Reitz,
Markus Armbruster
Cc: Dr. David Alan Gilbert
Introduce QuorumOpType, and make QUORUM_REPORT_BAD compatible
with it.
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: Wen Congyang <wency@cn.fujitsu.com>
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
Signed-off-by: Changlong Xie <xiecl.fnst@cn.fujitsu.com>
---
block/quorum.c | 17 ++++++++++++-----
docs/qmp-events.txt | 11 ++++++++++-
qapi/block.json | 16 ++++++++++++++++
qapi/event.json | 4 +++-
4 files changed, 41 insertions(+), 7 deletions(-)
diff --git a/block/quorum.c b/block/quorum.c
index 11cc60b..8f83574 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -215,14 +215,16 @@ static QuorumAIOCB *quorum_aio_get(BDRVQuorumState *s,
return acb;
}
-static void quorum_report_bad(QuorumAIOCB *acb, char *node_name, int ret)
+static void quorum_report_bad(QuorumOpType type, uint64_t sector_num,
+ int nb_sectors, char *node_name, int ret)
{
const char *msg = NULL;
if (ret < 0) {
msg = strerror(-ret);
}
- qapi_event_send_quorum_report_bad(!!msg, msg, node_name,
- acb->sector_num, acb->nb_sectors, &error_abort);
+
+ qapi_event_send_quorum_report_bad(type, !!msg, msg, node_name,
+ sector_num, nb_sectors, &error_abort);
}
static void quorum_report_failure(QuorumAIOCB *acb)
@@ -282,6 +284,7 @@ static void quorum_aio_cb(void *opaque, int ret)
QuorumChildRequest *sacb = opaque;
QuorumAIOCB *acb = sacb->parent;
BDRVQuorumState *s = acb->common.bs->opaque;
+ QuorumOpType type;
bool rewrite = false;
if (acb->is_read && s->read_pattern == QUORUM_READ_PATTERN_FIFO) {
@@ -300,12 +303,14 @@ static void quorum_aio_cb(void *opaque, int ret)
return;
}
+ type = acb->is_read ? QUORUM_OP_TYPE_READ : QUORUM_OP_TYPE_WRITE;
sacb->ret = ret;
acb->count++;
if (ret == 0) {
acb->success_count++;
} else {
- quorum_report_bad(acb, sacb->aiocb->bs->node_name, ret);
+ quorum_report_bad(type, acb->sector_num, acb->nb_sectors,
+ sacb->aiocb->bs->node_name, ret);
}
assert(acb->count <= s->num_children);
assert(acb->success_count <= s->num_children);
@@ -338,7 +343,9 @@ static void quorum_report_bad_versions(BDRVQuorumState *s,
continue;
}
QLIST_FOREACH(item, &version->items, next) {
- quorum_report_bad(acb, s->children[item->index]->bs->node_name, 0);
+ quorum_report_bad(QUORUM_OP_TYPE_READ, acb->sector_num,
+ acb->nb_sectors,
+ s->children[item->index]->bs->node_name, 0);
}
}
}
diff --git a/docs/qmp-events.txt b/docs/qmp-events.txt
index d6b9aea..fa7574d 100644
--- a/docs/qmp-events.txt
+++ b/docs/qmp-events.txt
@@ -325,6 +325,7 @@ Emitted to report a corruption of a Quorum file.
Data:
+- "type": Quorum operation type
- "error": Error message (json-string, optional)
Only present on failure. This field contains a human-readable
error message. There are no semantics other than that the
@@ -336,10 +337,18 @@ Data:
Example:
+Read operation:
{ "event": "QUORUM_REPORT_BAD",
- "data": { "node-name": "node0", "sector-num": 345435, "sectors-count": 5 },
+ "data": { "node-name": "node0", "sector-num": 345435, "sectors-count": 5,
+ "type": "read" },
"timestamp": { "seconds": 1344522075, "microseconds": 745528 } }
+Flush operation:
+{ "event": "QUORUM_REPORT_BAD",
+ "data": { "node-name": "node0", "sector-num": 0, "sectors-count": 2097120,
+ "type": "flush", "error": "Broken pipe" },
+ "timestamp": { "seconds": 1456406829, "microseconds": 291763 } }
+
Note: this event is rate-limited.
RESET
diff --git a/qapi/block.json b/qapi/block.json
index 58e6b30..937337d 100644
--- a/qapi/block.json
+++ b/qapi/block.json
@@ -196,3 +196,19 @@
##
{ 'event': 'DEVICE_TRAY_MOVED',
'data': { 'device': 'str', 'tray-open': 'bool' } }
+
+##
+# @QuorumOpType
+#
+# An enumeration of the quorum operation types
+#
+# @read: read operation
+#
+# @write: write operation
+#
+# @flush: flush operation
+#
+# Since: 2.6
+##
+{ 'enum': 'QuorumOpType',
+ 'data': [ 'read', 'write', 'flush' ] }
diff --git a/qapi/event.json b/qapi/event.json
index 1a45a6c..8642052 100644
--- a/qapi/event.json
+++ b/qapi/event.json
@@ -325,6 +325,8 @@
#
# Emitted to report a corruption of a Quorum file
#
+# @type: quorum operation type (Since 2.6)
+#
# @error: #optional, error message. Only present on failure. This field
# contains a human-readable error message. There are no semantics other
# than that the block layer reported an error and clients should not
@@ -339,7 +341,7 @@
# Since: 2.0
##
{ 'event': 'QUORUM_REPORT_BAD',
- 'data': { '*error': 'str', 'node-name': 'str',
+ 'data': { 'type': 'QuorumOpType', '*error': 'str', 'node-name': 'str',
'sector-num': 'int', 'sectors-count': 'int' } }
##
--
1.9.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH v7 3/3] quorum: modify vote rules for flush operation
2016-02-26 1:38 [Qemu-devel] [PATCH v7 0/3] modify vote rules for flush operation Changlong Xie
2016-02-26 1:39 ` [Qemu-devel] [PATCH v7 1/3] docs: fix invalid node name in qmp event Changlong Xie
2016-02-26 1:39 ` [Qemu-devel] [PATCH v7 2/3] qmp event: Refactor QUORUM_REPORT_BAD Changlong Xie
@ 2016-02-26 1:39 ` Changlong Xie
2016-03-01 15:39 ` [Qemu-devel] [PATCH v7 0/3] " Kevin Wolf
3 siblings, 0 replies; 8+ messages in thread
From: Changlong Xie @ 2016-02-26 1:39 UTC (permalink / raw)
To: qemu devel, Alberto Garcia, Eric Blake, Kevin Wolf, Max Reitz,
Markus Armbruster
Cc: Dr. David Alan Gilbert
Keep flush interface the same logic as quorum read/write, Otherwise in
following scenario, we'll encounter unexpected errors.
Quorum has two children(A, B). A do flush sucessfully, but B flush failed.
This cause the filesystem of guest become read-only with following errors:
end_request: I/O error, dev vda, sector 11159960
Aborting journal on device vda3-8
EXT4-fs error (device vda3): ext4_journal_start_sb:327: Detected abort journal
EXT4-fs (vda3): Remounting filesystem read-only
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: Wen Congyang <wency@cn.fujitsu.com>
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
Signed-off-by: Changlong Xie <xiecl.fnst@cn.fujitsu.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
block/quorum.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/block/quorum.c b/block/quorum.c
index 8f83574..b16171b 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -767,19 +767,30 @@ static coroutine_fn int quorum_co_flush(BlockDriverState *bs)
QuorumVoteValue result_value;
int i;
int result = 0;
+ int success_count = 0;
QLIST_INIT(&error_votes.vote_list);
error_votes.compare = quorum_64bits_compare;
for (i = 0; i < s->num_children; i++) {
result = bdrv_co_flush(s->children[i]->bs);
- result_value.l = result;
- quorum_count_vote(&error_votes, &result_value, i);
+ if (result) {
+ quorum_report_bad(QUORUM_OP_TYPE_FLUSH, 0,
+ bdrv_nb_sectors(s->children[i]->bs),
+ s->children[i]->bs->node_name, result);
+ result_value.l = result;
+ quorum_count_vote(&error_votes, &result_value, i);
+ } else {
+ success_count++;
+ }
}
- winner = quorum_get_vote_winner(&error_votes);
- result = winner->value.l;
-
+ if (success_count >= s->threshold) {
+ result = 0;
+ } else {
+ winner = quorum_get_vote_winner(&error_votes);
+ result = winner->value.l;
+ }
quorum_free_vote_list(&error_votes);
return result;
--
1.9.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v7 2/3] qmp event: Refactor QUORUM_REPORT_BAD
2016-02-26 1:39 ` [Qemu-devel] [PATCH v7 2/3] qmp event: Refactor QUORUM_REPORT_BAD Changlong Xie
@ 2016-02-26 7:33 ` Alberto Garcia
2016-03-01 15:57 ` Eric Blake
1 sibling, 0 replies; 8+ messages in thread
From: Alberto Garcia @ 2016-02-26 7:33 UTC (permalink / raw)
To: Changlong Xie, qemu devel, Eric Blake, Kevin Wolf, Max Reitz,
Markus Armbruster
Cc: Dr. David Alan Gilbert
On Fri 26 Feb 2016 02:39:01 AM CET, Changlong Xie wrote:
> Introduce QuorumOpType, and make QUORUM_REPORT_BAD compatible
> with it.
>
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Cc: Wen Congyang <wency@cn.fujitsu.com>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> Signed-off-by: Changlong Xie <xiecl.fnst@cn.fujitsu.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Berto
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v7 0/3] modify vote rules for flush operation
2016-02-26 1:38 [Qemu-devel] [PATCH v7 0/3] modify vote rules for flush operation Changlong Xie
` (2 preceding siblings ...)
2016-02-26 1:39 ` [Qemu-devel] [PATCH v7 3/3] quorum: modify vote rules for flush operation Changlong Xie
@ 2016-03-01 15:39 ` Kevin Wolf
3 siblings, 0 replies; 8+ messages in thread
From: Kevin Wolf @ 2016-03-01 15:39 UTC (permalink / raw)
To: Changlong Xie
Cc: Alberto Garcia, Markus Armbruster, Dr. David Alan Gilbert,
qemu devel, Max Reitz
Am 26.02.2016 um 02:38 hat Changlong Xie geschrieben:
> ChangLog:
> v7:
> 1. Fix coding style in doc/qmp-event.txt
> v6:
> 1. Make "type" mandatory for [PATCH 2/3]
> v5:
> 1. Fix invalid node name in docs/qmp-events.txt
> 2. Address comments from Eric, drop QUORUM_FLUSH_ERROR. Instead of reworking
> QUORUM_REPORT_BAD to make it compatible with quorum read/write/flush operations
> v4:
> 1. Introduce QUORUM_FLUSH_ERROR event to notify flush failure.
> v3:
> 1. *Note* that, the codes logic is different from what we talked in v2.
> I just keep flush interface the same logic as quorum read/write, and think
> it's reasoned.
>
> [Ref] http://lists.nongnu.org/archive/html/qemu-devel/2016-01/msg05342.html
Thanks, applied to the block branch.
Kevin
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v7 2/3] qmp event: Refactor QUORUM_REPORT_BAD
2016-02-26 1:39 ` [Qemu-devel] [PATCH v7 2/3] qmp event: Refactor QUORUM_REPORT_BAD Changlong Xie
2016-02-26 7:33 ` Alberto Garcia
@ 2016-03-01 15:57 ` Eric Blake
2016-03-03 14:48 ` Alberto Garcia
1 sibling, 1 reply; 8+ messages in thread
From: Eric Blake @ 2016-03-01 15:57 UTC (permalink / raw)
To: Changlong Xie, qemu devel, Alberto Garcia, Kevin Wolf, Max Reitz,
Markus Armbruster
Cc: Dr. David Alan Gilbert
[-- Attachment #1: Type: text/plain, Size: 1640 bytes --]
On 02/25/2016 06:39 PM, Changlong Xie wrote:
> Introduce QuorumOpType, and make QUORUM_REPORT_BAD compatible
> with it.
>
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Cc: Wen Congyang <wency@cn.fujitsu.com>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> Signed-off-by: Changlong Xie <xiecl.fnst@cn.fujitsu.com>
> ---
> +Read operation:
> { "event": "QUORUM_REPORT_BAD",
> - "data": { "node-name": "node0", "sector-num": 345435, "sectors-count": 5 },
> + "data": { "node-name": "node0", "sector-num": 345435, "sectors-count": 5,
> + "type": "read" },
> "timestamp": { "seconds": 1344522075, "microseconds": 745528 } }
>
> +Flush operation:
> +{ "event": "QUORUM_REPORT_BAD",
> + "data": { "node-name": "node0", "sector-num": 0, "sectors-count": 2097120,
> + "type": "flush", "error": "Broken pipe" },
> + "timestamp": { "seconds": 1456406829, "microseconds": 291763 } }
> +
> Note: this event is rate-limited.
Question - do we care if rate limiting masks one type of failure due to
another? Or put another way, are we okay with a single rate-limiting
queue for all three types, or do we want three queues? Also, shouldn't
this have a queue per child node (I don't want to be flooded with
multiple notifications in one second that child1 has failed, but I _do_
want notifications if both child1 and child2 fail in the same second).
But that's for future patches to change; it does not need to hold up the
current series.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v7 2/3] qmp event: Refactor QUORUM_REPORT_BAD
2016-03-01 15:57 ` Eric Blake
@ 2016-03-03 14:48 ` Alberto Garcia
0 siblings, 0 replies; 8+ messages in thread
From: Alberto Garcia @ 2016-03-03 14:48 UTC (permalink / raw)
To: Eric Blake, Changlong Xie, qemu devel, Kevin Wolf, Max Reitz,
Markus Armbruster
Cc: Dr. David Alan Gilbert
>> +Read operation:
>> { "event": "QUORUM_REPORT_BAD",
>> - "data": { "node-name": "node0", "sector-num": 345435, "sectors-count": 5 },
>> + "data": { "node-name": "node0", "sector-num": 345435, "sectors-count": 5,
>> + "type": "read" },
>> "timestamp": { "seconds": 1344522075, "microseconds": 745528 } }
>>
>> +Flush operation:
>> +{ "event": "QUORUM_REPORT_BAD",
>> + "data": { "node-name": "node0", "sector-num": 0, "sectors-count": 2097120,
>> + "type": "flush", "error": "Broken pipe" },
>> + "timestamp": { "seconds": 1456406829, "microseconds": 291763 } }
>> +
>> Note: this event is rate-limited.
>
> Question - do we care if rate limiting masks one type of failure due
> to another? Or put another way, are we okay with a single
> rate-limiting queue for all three types, or do we want three queues?
> Also, shouldn't this have a queue per child node (I don't want to be
> flooded with multiple notifications in one second that child1 has
> failed, but I _do_ want notifications if both child1 and child2 fail
> in the same second).
I think you are right, thanks for pointing it out.
Berto
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-03-03 14:48 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-26 1:38 [Qemu-devel] [PATCH v7 0/3] modify vote rules for flush operation Changlong Xie
2016-02-26 1:39 ` [Qemu-devel] [PATCH v7 1/3] docs: fix invalid node name in qmp event Changlong Xie
2016-02-26 1:39 ` [Qemu-devel] [PATCH v7 2/3] qmp event: Refactor QUORUM_REPORT_BAD Changlong Xie
2016-02-26 7:33 ` Alberto Garcia
2016-03-01 15:57 ` Eric Blake
2016-03-03 14:48 ` Alberto Garcia
2016-02-26 1:39 ` [Qemu-devel] [PATCH v7 3/3] quorum: modify vote rules for flush operation Changlong Xie
2016-03-01 15:39 ` [Qemu-devel] [PATCH v7 0/3] " Kevin Wolf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.