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.
Berto
