On 02/24/2016 08:35 PM, Alberto Garcia wrote:
On Wed 24 Feb 2016 11:11:54 AM CET, Changlong Xie wrote:
-static void quorum_report_bad(QuorumAIOCB *acb, char *node_name, int ret)
+static void quorum_report_bad(QuorumOpType type, QuorumAIOCB *acb,
+ 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);
+
+ switch (type) {
+ case QUORUM_OP_TYPE_READ:
+ case QUORUM_OP_TYPE_WRITE:
+ qapi_event_send_quorum_report_bad(false, 0, !!msg, msg, node_name,
+ acb->sector_num, acb->nb_sectors,
+ &error_abort);
+ break;
+ case QUORUM_OP_TYPE_FLUSH:
+ qapi_event_send_quorum_report_bad(true, type, !!msg, msg, node_name,
+ 0, 0, &error_abort);
+ break;
A few comments:
- Why don't you set the 'type' field in read and write operations? You
defined all three values but you are only using 'flush' here.
- For the case of flush you could set sectors-count to the total size
of the BlockDriverState as Eric suggested (bdrv_nb_sectors(bs)).
Setting both to 0 could confuse clients that are not ready to deal
with flush failures.
- Since the QuorumAIOCB parameter is not used in the flush case, you
could replace it from the function prototype and use sector_num and
nb_sectors instead. That way you can also omit the switch.
Surely, all your suggestions are helpful. Also i will add "Reviewed-by"
in patch 1/3, 3/3 in next version.
Thanks
-Xie
Berto
.
