vinyl-cache/vinyl-cache

Fork 8

Broken umem storage #2147

New issue

Closed

opened 2016-11-26 15:29:48 +01:00 by dridi · 9 comments

dridi commented

2016-11-26 15:29:48 +01:00

(Migrated from github.com)

I broke the build on SunOS platforms after fiddling [1] with the build system. Even after fixing [2] my mistake it stayed broken:

storage/storage_umem.c:120:13: error: 'struct VSC_C_main' has no member named 'sma_nbytes'
storage/storage_umem.c:121:13: error: 'struct VSC_C_main' has no member named 'sma_bfree'
storage/storage_umem.c:123:3: error: passing argument 1 of 'Lck__Unlock' from incompatible pointer type [-Werror]
In file included from storage/storage_umem.c:43:0:
./cache/cache.h:840:6: note: expected 'struct lock *' but argument is of type 'int *'
storage/storage_umem.c: In function 'smu_init':
storage/storage_umem.c:144:2: error: implicit declaration of function 'VNUM_2bytes' [-Werror=implicit-function-declaration]
storage/storage_umem.c:144:2: error: nested extern declaration of 'VNUM_2bytes' [-Werror=nested-externs]
storage/storage_umem.c:144:4: error: assignment makes pointer from integer without a cast [-Werror]
storage/storage_umem.c: In function 'smu_open':
storage/storage_umem.c:156:2: error: passing argument 1 of 'pthread_mutex_init' from incompatible pointer type [-Werror]
In file included from ./cache/cache.h:48:0,
from storage/storage_umem.c:43:
/usr/include/pthread.h:256:12: note: expected 'struct pthread_mutex_t * restrict' but argument is of type 'int *'
storage/storage_umem.c: At top level:
storage/storage_umem.c:163:2: error: initialization from incompatible pointer type [-Werror]
storage/storage_umem.c:163:2: error: (near initialization for 'smu_stevedore.open') [-Werror]
storage/storage_umem.c:164:2: error: unknown field 'alloc' specified in initializer
storage/storage_umem.c:164:2: error: initialization from incompatible pointer type [-Werror]
storage/storage_umem.c:164:2: error: (near initialization for 'smu_stevedore.close') [-Werror]
storage/storage_umem.c:165:2: error: unknown field 'free' specified in initializer
storage/storage_umem.c:165:2: error: initialization from incompatible pointer type [-Werror]
storage/storage_umem.c:165:2: error: (near initialization for 'smu_stevedore.allocobj') [-Werror]
storage/storage_umem.c:166:2: error: unknown field 'trim' specified in initializer
storage/storage_umem.c:166:2: error: initialization from incompatible pointer type [-Werror]
storage/storage_umem.c:166:2: error: (near initialization for 'smu_stevedore.baninfo') [-Werror]
storage/storage_umem.c:167:2: error: initialized field overwritten [-Werror=override-init]
storage/storage_umem.c:167:2: error: (near initialization for 'smu_stevedore.allocobj') [-Werror=override-init]
cc1: all warnings being treated as errors
Makefile:2338: recipe for target 'varnishd-storage_umem.o' failed
make[5]: *** [varnishd-storage_umem.o] Error 1
make[5]: *** Waiting for unfinished jobs....
make[5]: Leaving directory '/home/uplex/vtest/varnish-cache/bin/varnishd'
Makefile:824: recipe for target 'all' failed
make[4]: *** [all] Error 2
make[4]: Leaving directory '/home/uplex/vtest/varnish-cache/bin/varnishd'
Makefile:392: recipe for target 'all-recursive' failed
make[3]: *** [all-recursive] Error 1
make[3]: Leaving directory '/home/uplex/vtest/varnish-cache/bin'
Makefile:534: recipe for target 'all-recursive' failed

The code is still using an MTX macro that was removed in 2008 [3] so I'm assuming that it hasn't been compiled for a most of the project's lifetime. I think it's [1] that fixed libumem detection.

[1] d6216cc
[2] 6550d4b
[3] 6952977

I broke the build on SunOS platforms after fiddling [1] with the build system. Even after fixing [2] my mistake it stayed broken: ``` storage/storage_umem.c:120:13: error: 'struct VSC_C_main' has no member named 'sma_nbytes' storage/storage_umem.c:121:13: error: 'struct VSC_C_main' has no member named 'sma_bfree' storage/storage_umem.c:123:3: error: passing argument 1 of 'Lck__Unlock' from incompatible pointer type [-Werror] In file included from storage/storage_umem.c:43:0: ./cache/cache.h:840:6: note: expected 'struct lock *' but argument is of type 'int *' storage/storage_umem.c: In function 'smu_init': storage/storage_umem.c:144:2: error: implicit declaration of function 'VNUM_2bytes' [-Werror=implicit-function-declaration] storage/storage_umem.c:144:2: error: nested extern declaration of 'VNUM_2bytes' [-Werror=nested-externs] storage/storage_umem.c:144:4: error: assignment makes pointer from integer without a cast [-Werror] storage/storage_umem.c: In function 'smu_open': storage/storage_umem.c:156:2: error: passing argument 1 of 'pthread_mutex_init' from incompatible pointer type [-Werror] In file included from ./cache/cache.h:48:0, from storage/storage_umem.c:43: /usr/include/pthread.h:256:12: note: expected 'struct pthread_mutex_t * restrict' but argument is of type 'int *' storage/storage_umem.c: At top level: storage/storage_umem.c:163:2: error: initialization from incompatible pointer type [-Werror] storage/storage_umem.c:163:2: error: (near initialization for 'smu_stevedore.open') [-Werror] storage/storage_umem.c:164:2: error: unknown field 'alloc' specified in initializer storage/storage_umem.c:164:2: error: initialization from incompatible pointer type [-Werror] storage/storage_umem.c:164:2: error: (near initialization for 'smu_stevedore.close') [-Werror] storage/storage_umem.c:165:2: error: unknown field 'free' specified in initializer storage/storage_umem.c:165:2: error: initialization from incompatible pointer type [-Werror] storage/storage_umem.c:165:2: error: (near initialization for 'smu_stevedore.allocobj') [-Werror] storage/storage_umem.c:166:2: error: unknown field 'trim' specified in initializer storage/storage_umem.c:166:2: error: initialization from incompatible pointer type [-Werror] storage/storage_umem.c:166:2: error: (near initialization for 'smu_stevedore.baninfo') [-Werror] storage/storage_umem.c:167:2: error: initialized field overwritten [-Werror=override-init] storage/storage_umem.c:167:2: error: (near initialization for 'smu_stevedore.allocobj') [-Werror=override-init] cc1: all warnings being treated as errors Makefile:2338: recipe for target 'varnishd-storage_umem.o' failed make[5]: *** [varnishd-storage_umem.o] Error 1 make[5]: *** Waiting for unfinished jobs.... make[5]: Leaving directory '/home/uplex/vtest/varnish-cache/bin/varnishd' Makefile:824: recipe for target 'all' failed make[4]: *** [all] Error 2 make[4]: Leaving directory '/home/uplex/vtest/varnish-cache/bin/varnishd' Makefile:392: recipe for target 'all-recursive' failed make[3]: *** [all-recursive] Error 1 make[3]: Leaving directory '/home/uplex/vtest/varnish-cache/bin' Makefile:534: recipe for target 'all-recursive' failed ``` The code is still using an `MTX` macro that was removed in 2008 [3] so I'm assuming that it hasn't been compiled for a most of the project's lifetime. I think it's [1] that fixed `libumem` detection. [1] d6216cc [2] 6550d4b [3] 6952977

dridi commented

2016-11-28 08:33:32 +01:00

(Migrated from github.com)

FWIW, we could also have umem on other systems: https://github.com/omniti-labs/portableumem

Although, I'm not sure how relevant this is, considering it hasn't been used in Varnish for at least 8 years.

FWIW, we could also have `umem` on other systems: https://github.com/omniti-labs/portableumem Although, I'm not sure how relevant this is, considering it hasn't been used in Varnish for at least 8 years.

nigoroll commented

2016-11-28 13:09:05 +01:00

(Migrated from github.com)

@Dridi as discussed on IRC; thank you for exposing this issue which you didn't cause but rather made visible

fgsch commented

2017-01-14 14:43:37 +01:00

(Migrated from github.com)

Considering this has been broken and unnoticed for ETOOLONG, shouldn't we just kill the code?
If anyone wants to fix it the code is still available.

Considering this has been broken and unnoticed for ETOOLONG, shouldn't we just kill the code? If anyone wants to fix it the code is still available.

nigoroll commented

2017-01-14 14:47:27 +01:00

(Migrated from github.com)

@fgsch yes, open tickets which are not being actively worked on are something we should avoid.
I'm OK with closing it, but I actually DO want to work on it when I can.

@fgsch yes, open tickets which are not being actively worked on are something we should avoid. I'm OK with closing it, but I actually DO want to work on it when I can.

fgsch commented

2017-01-14 14:58:06 +01:00

(Migrated from github.com)

I was talking about the code, not the ticket.
Having broken code that nobody is working on (and has not been used for a very long time) does not make any sense to me. If someone feels like fixing it we can add it back once it's working.

I was talking about the code, not the ticket. Having broken code that nobody is working on (and has not been used for a very long time) does not make any sense to me. If someone feels like fixing it we can add it back once it's working.

nigoroll commented

2017-01-14 14:59:10 +01:00

(Migrated from github.com)

yes you are right, but my response would be the same re the code.

dridi commented

2017-01-16 07:25:30 +01:00

(Migrated from github.com)

The fact that the code has been dead for the last 8 years should be enough to consider a rewrite from scratch when umem is reintroduced. The stevedore API changed a couple times since then, collateral APIs have probably changed too (at least locks did) and there's also this idea of having storage modules.

So I say we prune it, and it's either reintroduced as a simple storage or as the first in-tree module.

bsdphk commented

2017-01-16 13:57:06 +01:00

(Migrated from github.com)

I vote kill

fgsch commented

2017-01-16 13:59:16 +01:00

(Migrated from github.com)

Kill it is and should arrive at GH and your friendly mirrors shortly.

This discussion has been locked. Commenting is limited to contributors.

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

vinyl-cache/vinyl-cache#2147

No description provided.

Rows
Columns