In debug mode, we capture the exec output from targetcli, which may
contain block volume credentials, this fix will clip off those auth
details.
Fixes: CVE-2020-10762
Reviewed-by: Xiubo Li <xiubli@redhat.com>
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Unfortunately the sensitive information is leaked in the logs,
at both gluster-blockd.log and cmd_history.log.
This patch will fix it and avoid printing the auth credentials.
Reviewed-by: Xiubo Li <xiubli@redhat.com>
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
If the version is not provided by the package itself, check if the rpm
nvr has the version string that can be used
Reviewed-by: Xiubo Li <xiubli@redhat.com>
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
The gbCreate3 is not that correct here, because it will not only for
the V3, but also for the followed versions.
Reviewed-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Signed-off-by: Xiubo Li <xiubli@redhat.com>
Set the tcmur_cmd_time_out=43s as default, which is larger than
each IO's timeout value(default is 30s) in the client/initiator side,
and at the same time it will be larger than 42s, the Gluster ping
timeout.
Reviewed-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Signed-off-by: Xiubo Li <xiubli@redhat.com>
Problem:
-------
Right now, if any block volume is failed to load as part of service bringup
or node reboot, may be because of an issue from the backend, then there is
no way to reload that single block volume, we need to reload/restart all the\
block volumes present in the node just to load one block volume, this interrupts
ongoing I/O (via this path) for all the volumes hosted within the node.
Solution:
--------
Add ability to reload a single block volume without touching other block volumes
in the node.
$ gluster-block help
usage:
gluster-block [timeout <seconds>] <command> <volname[/blockname]> [<args>] [--json*]
commands:
[...]
reload <volname/blockname>
reload a block device.
[...]
Reviewed-by: Xiubo Li <xiubli@redhat.com>
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Previously we had code which allows passing more than HA number of hosts to the
create command. But the future was incomplete and this code was left dead.
This PR removes it all, may be once we have self-healing, this sort of feature
addition becomes more easy.
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Reviewed-by: Xiubo Li <xiubli@redhat.com>
This will fix the following issues:
1, When the StorageObject creation failed the iqn will still be
created and won't be deleted in case of:
When creating BVs with the same NAME, the second time it will
fail duing to the targetcli cache db only allows one [user/$NAME]
exist, and then fails it leaving the IQN not deleted correctly.
That means there will be two different IQNs both mapped to the
same StorageObject.
2, For the case above we will also find that after the second
creation failing in the /etc/target/saveconfig.json there will be
one StorageObject with the two Targets.
In theory, there should be one StorageObject with only 1 Target.
This patch will check all the StorageObjects in saveconfig.json
before creating, if there already has one StorageObject with the
same NAME requested, it will fail directly.
Signed-off-by: Xiubo Li <xiubli@redhat.com>
Reviewed-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
This will avoid something like the following odd case:
LOG("mgmt", GB_LOG_INFO, "tmp=%s", tmp);
It will always give us:
INFO: tmp=mgmt [at block_svc_routines.c+4394 :<block_create_common>]
This is because the tmp dups to the "char *tmp" in the LOG
macro.
Signed-off-by: Xiubo Li <xiubli@redhat.com>
Reviewed-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
The old code has covered the raw errors from targetcli command, but
some of the raw errors will be very useful to get the root cause of
some bugs.
Signed-off-by: Xiubo Li <xiubli@redhat.com>
Reviewed-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
This will still need to depend the taregetcli's saveconfig support,
the rtslib PR is open-iscsi/rtslib-fb#150.
When creating the BV with the 'block-size <SIZE>' option:
In case all the ha nodes have supported the rtslib-fb#150:
1) if local node support the block-size, but if there is any of
the remote nodes is not, it will fail with the cap not match error.
2) if local node does not support the block-size, no matter whether
the remote nodes support it or not, it will always ignore the
block-size option due to the exist bug.
In case if there is any of the ha nodes does not support rtslib-fb#150:
3) if local node support the block-size, it will always fails with the
cap not match error.
4) if local mode does not support the block-size, no matter whether
the remote nodes support it or not, it will ignore the block-size
option due to the exist bug.
Signed-off-by: Xiubo Li <xiubli@redhat.com>
Reviewed-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
This also will fix the following warnings when running:
$ gluster-blockd --version
May 05 18:33:17 rhel3 gluster-blockd[18005]: Error opening log file: No such file or directory
May 05 18:33:17 rhel3 gluster-blockd[18005]: Logging to stderr.
This is because that all the logfiles are initialized as nil, so
opening the nil file will fail as above.
Signed-off-by: Xiubo Li <xiubli@redhat.com>
Reviewed-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Problem:
-------
Build was failing with:
[root@localhost x86_64]# rpmlint gluster-block-0.4-1.x86_64.rpm
gluster-block.x86_64: E: incoherent-logrotate-file /etc/logrotate.d/gluster-blockd
1 packages and 0 specfiles checked; 1 errors.
Solution:
--------
incoherent-logrotate-file ?
Solution:- The log file or directory name should be the same as the
package name in lower case.
Hence renaming the logrotate filename back to gluster-block
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Reviewed-by: Xiubo Li <xiubli@redhat.com>
Problem:
-------
We are hitting one case that when trying to change the config file and
close it, all the uncomment options will be fall back to the DEFAULT values.
Resolution:
----------
In-case if the editor (vim) follows write to a new file (.swp, .tmp ..)
and move it to actual file name later. There is a window, where we will
encounter one case that the file data is not flushed to the disk, so in
another process(here) when reading it will be empty.
Let just wait and try again.
Signed-off-by: Xiubo Li <xiubli@redhat.com>
Reviewed-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Problem:
-------
Currently when the new values are the same with the current ones,
we still can see the folloiwng logs:
"logDir/logLevel... now is ..."
We need to slient them.
Resolution:
----------
Just check and skip them when not any changes.
Signed-off-by: Xiubo Li <xiubli@redhat.com>
Reviewed-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Problems:
-------
Since we have make the cli and server routines into 2 different
processes in gluster-blockd daemon, the gbConf data will also
be have different copies. If the gbConf data is changed by the
dyn-config thread in the cli(parent) process, the new changes won't
be visible for the server(child) process.
Resolution:
----------
For the gbConf data, use the shared memory instead of the in the
data segment.
Signed-off-by: Xiubo Li <xiubli@redhat.com>
Reviewed-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Problem:
-------
There has tons of warnings when building the rpms:
../utils/utils.h:166:25: warning: comparison with string literal results in unspecified behavior [-Waddress]
if ((str) == "mgmt") \
^
gluster-block.c:463:3: warning: (near initialization for 'mobj.block_name') [-Wmissing-braces]
gluster-block.c:464:3: warning: missing braces around initializer [-Wmissing-braces]
blockModifySizeCli msobj = {0, };
^
block_svc_routines.c: In function 'block_delete_cli_1_svc_st':
block_svc_routines.c:4707:37: warning: ?: using integer constants in boolean context [-Wint-in-bool-context]
LOG("cmdlog", errCode?GB_LOG_ERROR:GB_LOG_INFO, "%s", reply->out);
../utils/utils.h:169:17: note: in definition of macro 'LOG'
if (level <= gbConf.logLevel) { \
^~~~~
Resolution:
----------
Use the strcmp() instead and add extra braces when initializing.
Signed-off-by: Xiubo Li <xiubli@redhat.com>
Reviewed-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
When the logDir is changed via the /etc/sysconfig/gluster-blockd
config file, the logrotate should be changed too.
Fixes: #199
Signed-off-by: Xiubo Li <xiubli@redhat.com>
Reviewed-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
This will add the dyn logDir support based the dyn-config without
restarting the gluster-blockd service.
Fixes: #199
Signed-off-by: Xiubo Li <xiubli@redhat.com>
Reviewed-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Warning seen with make:
----------------------
$ make
[...]
In file included from utils.c:15:
utils.c: In function ‘glusterBlockSetLogLevel’:
utils.h:113:27: warning: implicit declaration of function ‘asprintf’; did you mean ‘vsprintf’? [-Wimplicit-function-de
claration]
int __ret=asprintf(ptr, ##fmt);__ret;})
^~~~~~~~
utils.h:131:19: note: in expansion of macro ‘GB_ASPRINTF’
len = GB_ASPRINTF(&buf, fmt, ##__VA_ARGS__); \
^~~~~~~~~~~
utils.c:41:5: note: in expansion of macro ‘MSG’
MSG(stderr, "unknown LOG-LEVEL: '%d'", logLevel);
^~~
CC libgb_la-lru.lo
[...]
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Reviewed-by: Amar Tumballi <amarts@redhat.com>
Reviewed-by: Xiubo Li <xiubli@redhat.com>
If there is no any variable argument in the logger callout, it
will hit the following compile errors:
../utils/utils.h:175:34: error: expected expression before ‘,’ token
__VA_ARGS__, __FILE__, __LINE__, __FUNCTION__); \
^
gluster-blockd.c:268:5: note: in expansion of macro ‘LOG’
LOG ("mgmt", GB_LOG_ERROR, "Test................");
^
More detail please see the GNU doc:
https://gcc.gnu.org/onlinedocs/cpp/Variadic-Macros.html
Signed-off-by: Xiubo Li <xiubli@redhat.com>
Reviewed-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
glibc has removed the rpc functions from current releases. Instead of
relying on glibc providing these, the modern libtirpc library should be
used instead.
Change-Id: I46c979e52147abce956255de5ad16b01b5621f52
Updates: #56
Signed-off-by: Niels de Vos <ndevos@redhat.com>
Reviewed-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Reviewed-by: Xiubo Li <xiubli@redhat.com>
Reviewed-by: Amar Tumballi <amarts@redhat.com>
Problem:
-------
Currently cli process Loads Config on every operation in cli process
context. Hence log msgs:
DEBUG: logLevel now is TRACE [at utils.c+50:<glusterBlockSetLogLevel>]
DEBUG: glfsLruCount now is 5 [at lru.c+43:<glusterBlockSetLruCount>]
are dumped to gluster-blockd.log on every single cli operation. Apart
from the inotify triggers to Load Config.
Solution:
--------
Print the Key:Value config file parameters to respective log files based
on the process context.
That is,
- if cli process calls Load Config, then dump the logs to
gluster-block-cli.log (only DEBUG or higher level)
- if dameon process calls Load Config ( via DynConfig thread), then dump
the logs to gluster-blockd.log (in CRIT level)
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Reviewed-by: Xiubo Li <xiubli@redhat.com>
Tested-by: Xiubo Li <xiubli@redhat.com>
Editors (such as vim, nano ..) follow different approaches to save conf file.
The two commonly followed techniques are to overwrite the existing file, or to
write to a new file (.swp, .tmp ..) and move it to actual file name later. In
the later case, the inotify fails, because the file it's been intended to watch
no longer exists, as the new file is a different file with just a same name.
To handle both the file save approaches mentioned above, it is better we watch
the directory and filter for MODIFY events.
Signed-off-by: Xiubo Li <xiubli@redhat.com>
Reviewed-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Tested-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
passing the strlen() of the source string as the destination
length is pointless, and gcc-8 now warns about it:
utils.c: In function ‘gbStrcpy’:
utils.c:432:11: warning: ‘strncpy’ specified bound depends on the
length of the source argument [-Wstringop-overflow=]
ret = strncpy(dest, src, n);
^~~~~~~~~~~~~~~~~~~~~
utils.c:427:16: note: length computed here
size_t n = strlen(src);
^~~~~~~~~~~
block_svc_routines.c:184:7: warning: ‘strncat’ specified bound 1
equals source length [-Wstringop-overflow=]
strncat(out, " ", 1);
^~~~~~~~~~~~~~~~~~~~
block_svc_routines.c:183:7: warning: ‘strncat’ specified bound
depends on the length of the source argument [-Wstringop-overflow=]
strncat(out, element, strlen(element));
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Signed-off-by: Xiubo Li <xiubli@redhat.com>
Reviewed-by: Amar Tumballi <amarts@redhat.com>
Reviewed-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
gluster-block (0.3)
usage:
gluster-block [timeout <seconds>] <command> <volname[/blockname]> [<args>] [--json*]
commands:
[...]
common cli options: (fixed formats)
timeout <seconds>
it is the time in seconds that cli can wait for daemon to respond.
[default: timeout 300]
--json*
used to request the output result in json format [default: plain text]
supported JSON formats: --json|--json-plain|--json-spaced|--json-pretty
Precedence of various methods to set cli timeout:
1. Options set through config file /etc/sysconfig/gluster-blockd [Top Prio]
eg: uncommenting and adjusting key:value at /etc/sysconfig/gluster-blockd
2. Argument passed at cli
eg: timeout 900
3. Environment variable
eg: export GB_CLI_TIMEOUT=900
4. Code level defaults (300 sec) [Least Prio]
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Reviewed & Tested-by: Xiubo Li <xiubli@redhat.com>
Reviewed-by: Amar Tumballi <amarts@redhat.com>
There are two rpc calls involved in the design,
Channel A: From cli process to local daemon (cli thread)
Channel B: From local daemon (cli thread) to remote daemon (remote thread)
Timeout for channel A is configurable now, using
/etc/sysconfig/gluster-blockd, by adjusting option 'GB_CLI_TIMEOUT' (in seconds)
$ cat /etc/sysconfig/gluster-blockd
GB_CLI_TIMEOUT=900
Changes to this value takes effect every time we start running/triggering new
cli command/request.
At the moment making Timeout configurable for channel B is not important, it is
hardcoded as 300 seconds in the code for now.
Some more details:
-----------------
It would have been an easy fix calling clnt_control() right after clnt_create(),
unfortunately there is a bug in glibc, which is why we had to hack the rpc
generated code instead of directly calling clnt_control(), see my very old
commit [1] which explains why we had to override (in a hacky-way) the default
generated TIMEOUT.
[1] 1dbbd7d457
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Reviewed & Tested-by: Xiubo Li <xiubli@redhat.com>
Reviewed-by: Amar Tumballi <amarts@redhat.com>
LOG() macro was designed to compare for specific strings, and based
on the result, log to different log files. This was done using
'strcmp()', but was using static strings. It made sense to compare
the static string directly as they resolve to same pointer while
compiling.
With this fix, was able to resolve 58 coverity issues in one shot.
Signed-off-by: Amar Tumballi <amarts@redhat.com>
Reviewed-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Reviewed-by: Xiubo Li <xiubli@redhat.com>
In-case if your editor (vim) follows write to a new file (.swp, .tmp ..) and
move it to actual file name later. There is a window, where we encounter
'No such file or directory' error on open, because of the editors savefile
transaction.
This patch, simply give few more retries to load the conf file.
Reviewed-by: Xiubo Li <xiubli@redhat.com>
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
* gluster-blockd: support volfile server setting through Env
This patch is majorly for GCS and kube use-case, where the discovery service,
whos name look like FQDN will resolve the backend volfile servers path, for
each and every block hosting volume from all the associated pods/nodes in the
cluster.
As of today gluster-blockd and tcmu-runner use localhost as volfile server
address to communicate with block hosting volumes, with this patch series we
will provide a way to set volfile server address (discovery service) of a
remote/separate gluster cluster, where the block hosting volume is running out-
side the gluster-block (daemon running) nodes.
This way, we achieve the isolation between the glusterd and gluster-blockd
daemons, and hence they both can now run in different nodes/pods.
Thanks to Amar Tumballi <amarts@redhat.com> for all the discussions
around this subject.
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
* replace: support volfile server setting through Env
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
* genconfig: support volfile server setting through Env
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
* config: support volfile server setting through config file
Just uncommnet below line at /etc/sysconfig/gluster-blockd and change the
value to fit discovery service name,
Ex:
> #GB_BHV_VOLSERVER="localhost"
GB_BHV_VOLSERVER="dhcpXX-YYY.lab.github.com"
followed by restart of gluster-blockd.service to take changes into effect.
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Reviewed & Tested-by: Xiubo Li <xiubli@redhat.com>
Reviewed-by: Amar Tumballi <amarts@redhat.com>
