drbdadm: Fix does not allocate bitmap for stacked res
PATCH 1/1 drbdadm: Fix does not allocate bitmap for stacked res
In e01b8b3e, allow peer_devices without bitmap. However, should exclude stacked resource in this case to avoid call trace caused by no bitmap, bitmap->bitmap_index = -1.
Call Trace when init syncing to the stacked resource:
drbd_bmio_set_n_write+0x39/0x70 [drbd]
drbd_bitmap_io+0x8e/0xe0 [drbd]
bitmap_mod_after_handshake+0xcb/0x1f0 [drbd]
receive_state+0xada/0x16c0 [drbd]
drbd_receiver+0x3b1/0x660 [drbd]Signed-off-by Nick Wang <[email protected]>
CC Lars Ellenberg <[email protected]>
CC Roland Kammerer <[email protected]>
--- user/v9/drbdadm_postparse.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
user/v9/drbdadm_postparse.c +2 -1
@@ -465,7 +465,8 @@ static void add_no_bitmap_opt(struct d_resource *res)continue ; STAILQ_FOREACH(peer_device, &conn->peer_devices, connection_link) { -if (peer_device->connection->peer &&peer_diskless (peer_device)) +if (peer_device->connection->peer && !peer_device->connection->peer->lower && +peer_diskless (peer_device))insert_tail (&peer_device->pd_options,new_opt ("bitmap" ,"no" )); } }
Signature · 1 line
2.12.0
Philipp Reisner
maintainerHi Nick,
regarding you proposed patch, instead of special casing the code we should rather fix the data structure.
I will give it a bit more testing before I push it to the master. Please let me know if that works for your test-cases.
best regards,
Phil
From b5eab810ded459792c24a4bc5db5ce272ea1ee15 Mon Sep 17 00:00:00 2001
From: Philipp Reisner <[email protected]>
Date: Fri, 26 Jan 2018 16:09:01 +0100
Subject: [PATCH] v9,drbdadm,stacking: Fix setting the disk in stacked host
sections
This got broken on the long way from drbd-8.4 to 9.
--- user/v9/drbdadm_postparse.c | 73 ++++++++++++++++++++++----------------------- 1 file changed, 36 insertions(+), 37 deletions(-)
user/v9/drbdadm_postparse.c +36 -37
@@ -514,9 +514,9 @@ void set_peer_in_resource(struct d_resource* res, int peer_required)add_no_bitmap_opt (res); } -void set_disk_in_res (struct d_resource *res) +void set_stacked_disk_in_res (struct d_resource *res) { -struct d_host_info *host; +struct d_host_info *host, *lower_host;struct d_volume *a, *b;if (res->ignore)
@@ -526,41 +526,40 @@ void set_disk_in_res(struct d_resource *res)if (!host->lower)continue ; -if (host->lower->ignore) -continue ; - -check_volume_sets_equal (res, host, host->lower->me); -if (!config_valid) -/* don't even bother for broken config. */ -continue ; +for_each_host (lower_host, &host->lower->all_hosts) { +check_volume_sets_equal (res, host, lower_host); +if (!config_valid) +/* don't even bother for broken config. */ +continue ; -/* volume lists are sorted on vnr */ - a =STAILQ_FIRST (&host->volumes); - b =STAILQ_FIRST (&host->lower->me->volumes); -while (a) { -while (b && a->vnr > b->vnr) { -/* Lower resource has more volumes. -* Probably unusual, but we decided -* that it should be legal. -* Skip those that do not match */ - b =STAILQ_NEXT (b, link); - } -if (a && b && a->vnr == b->vnr) { -if (b->device) -m_asprintf (&a->disk,"%s" , b->device); -else -m_asprintf (&a->disk,"/dev/drbd%u" , b->device_minor); -/* stacked implicit volumes need internal meta data, too */ -if (!a->meta_disk) -m_asprintf (&a->meta_disk,"internal" ); -if (!a->meta_index) -m_asprintf (&a->meta_index,"internal" ); - a =STAILQ_NEXT (a, link); - b =STAILQ_NEXT (b, link); - }else { -/* config_invalid should have been set -* by check_volume_sets_equal */ -assert (0 ); +/* volume lists are sorted on vnr */ + a =STAILQ_FIRST (&host->volumes); + b =STAILQ_FIRST (&lower_host->volumes); +while (a) { +while (b && a->vnr > b->vnr) { +/* Lower resource has more volumes. +* Probably unusual, but we decided +* that it should be legal. +* Skip those that do not match */ + b =STAILQ_NEXT (b, link); + } +if (a && b && a->vnr == b->vnr) { +if (b->device) +m_asprintf (&a->disk,"%s" , b->device); +else +m_asprintf (&a->disk,"/dev/drbd%u" , b->device_minor); +/* stacked implicit volumes need internal meta data, too */ +if (!a->meta_disk) +m_asprintf (&a->meta_disk,"internal" ); +if (!a->meta_index) +m_asprintf (&a->meta_index,"internal" ); + a =STAILQ_NEXT (a, link); + b =STAILQ_NEXT (b, link); + }else { +/* config_invalid should have been set +* by check_volume_sets_equal */ +assert (0 ); + } } } }
@@ -1161,7 +1160,7 @@ void post_parse(struct resources *resources, enum pp_flags flags)// Needs "me" set already for_each_resource (res, resources)if (res->stacked_on_one) -set_disk_in_res (res); +set_stacked_disk_in_res (res);for_each_resource (res, resources)fixup_peer_devices (res);
Signature · 1 line
2.7.4
Hi Philipp,
Your fix working perfectly in my test scenario.
Best regards,
Nick
Quoted 1 line
Quoted 1 line
Quoted 1 line
On 1/26/2018 at 11:26 下午, in message <1621611.JSGMFM0nhf@fat-tyre>, Philipp
Quoted 132 lines · Reisner
Hi Nick,
regarding you proposed patch, instead of special casing the code we should rather fix the data structure.
I will give it a bit more testing before I push it to the master. Please let me know if that works for your test-cases.
best regards,
PhilFrom
b5eab810ded459792c24a4bc5db5ce272ea1ee15Mon Sep 17 00:00:00 2001
From: Philipp Reisner <[email protected]>
Date: Fri, 26 Jan 2018 16:09:01 +0100
Subject: [PATCH] v9,drbdadm,stacking: Fix setting the disk in stacked host
sectionsThis got broken on the long way from drbd-8.4 to 9.
--- user/v9/drbdadm_postparse.c | 73 ++++++++++++++++++++++----------------------- 1 file changed, 36 insertions(+), 37 deletions(-)user/v9/drbdadm_postparse.c +2
-2@@ -514,9 +514,9 @@ void set_peer_in_resource(struct d_resource* res, int peer_required)add_no_bitmap_opt (res ); } -void set_disk_in_res (struct d_resource *res) +void set_stacked_disk_in_res (struct d_resource *res) { -struct d_host_info *host; +struct d_host_info *host, *lower_host;struct d_volume *a, *b;if (res->ignore)user/v9/drbdadm_postparse.c +5
-24@@ -526,41 +526,40 @@ void set_disk_in_res(struct d_resource *res)if (!host->lower)continue ; -if (host->lower->ignore) -continue ; - -check_volume_sets_equal (res, host, host->lower->me); -if (!config_valid) -/* don't even bother for broken config. */ -continue ; +for_each_host (lower_host, &host->lower->all_hosts) { +check_volume_sets_equal (res, host, lower_host); +if (!config_valid) +/* don't even bother for broken config. */ +continue ; -/* volume lists are sorted on vnr */ - a =STAILQ_FIRST (&host->volumes); - b =STAILQ_FIRST (&host->lower->me->volumes); -while (a) { -while (b && a->vnr > b->vnr) { -/* Lower resource has more volumes. -* Probably unusual, but we decided -* that it should be legal. -* Skip those that do not match */ - b =STAILQ_NEXT (b, link); - } -if (a && b && a->vnr == b->vnr) { -if (b->device) -m_asprintf (&a->disk,"%s" , b->device); -else -m_asprintf (&a->disk,"/dev/drbd%u" , b->device_minor); -/* stacked implicit volumes need internal meta data, too */
user/v9/drbdadm_postparse.c +14
-10-if (!a->meta_disk) -m_asprintf (&a->meta_disk,"internal" ); -if (!a->meta_index) -m_asprintf (&a->meta_index,"internal" ); - a =STAILQ_NEXT (a, link); - b =STAILQ_NEXT (b, link); - }else { -/* config_invalid should have been set -* by check_volume_sets_equal */ -assert (0 ); +/* volume lists are sorted on vnr */ + a =STAILQ_FIRST (&host->volumes); + b =STAILQ_FIRST (&lower_host->volumes); +while (a) { +while (b && a->vnr > b->vnr) { +/* Lower resource has more volumes. +* Probably unusual, but we decided +* that it should be legal. +* Skip those that do not match */ + b =STAILQ_NEXT (b, link); + } +if (a && b && a->vnr == b->vnr) { +if (b->device) + m_asprintf(&a->disk, "%s",
b->device);user/v9/drbdadm_postparse.c +2
-0+else + m_asprintf(&a->disk,"/dev/drbd%u",
b->device_minor);
+ /* stacked implicit volumes need
internal meta data, too */user/v9/drbdadm_postparse.c +2
-0+if (!a->meta_disk) + m_asprintf(&a->meta_disk,"internal");
user/v9/drbdadm_postparse.c +2
-0+if (!a->meta_index) + m_asprintf(&a->meta_index,"internal");
user/v9/drbdadm_postparse.c +4
-0+ a =STAILQ_NEXT (a, link); + b =STAILQ_NEXT (b, link); + }else { +/* config_invalid should have been set
user/v9/drbdadm_postparse.c +4
-1+* by check_volume_sets_equal */ +assert (0 ); + } } } }@@ -1161,7 +1160,7 @@ void post_parse(struct resources *resources, enum pp_flags flags)// Needs "me" set already for_each_resource (res, resources)if (res->stacked_on_one) -set_disk_in_res (res); +set_stacked_disk_in_res (res);for_each_resource (res, resources)fixup_peer_devices(res);