report: Print NULL strings as "" in _string_disp (instead of a SEGV).
PATCH 1/4 report: Print NULL strings as "" in _string_disp (instead of a SEGV).
--- lib/report/report.c | 2 ++ 1 file changed, 2 insertions(+)
lib/report/report.c +2 -0
@@ -179,6 +179,8 @@ static int _string_disp(struct dm_report *rh, struct dm_pool *mem __attribute__(struct dm_report_field *field, const void *data, void *private __attribute__((unused))) { +if (!*(void **) data) +return _field_set_value (field,"" ,NULL );return dm_report_field_string (rh, field, (const char *const *) data); }
Signature · 1 line
2.1.2
PATCH 2/4 report: Add cache_policy and cache_settings (LV) segment fields.
--- lib/report/columns.h | 2 ++ lib/report/properties.c | 5 +++++ lib/report/report.c | 38 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+)
lib/report/columns.h +2 -0
@@ -178,6 +178,8 @@ FIELD(SEGS, seg, STR_LIST, "Seg Tags", tags, 8, tags, seg_tags, "Tags, if any.", FIELD(SEGS, seg, STR, "PE Ranges", list, 9, peranges, seg_pe_ranges, "Ranges of Physical Extents of underlying devices in command line format.", 0) FIELD(SEGS, seg, STR, "Devices", list, 7, devices, devices, "Underlying devices used with starting extent numbers.", 0) FIELD(SEGS, seg, STR, "Monitor", list, 7, segmonitor, seg_monitor, "Dmeventd monitoring status of the segment.", 0) +FIELD(SEGS, seg, STR, "Cache Policy", policy_name, 11, string, cache_policy, "The cache policy (cached segments only).", 0) +FIELD(SEGS, seg, STR_LIST, "Cache Settings", policy_settings, 20, cache_settings, cache_settings, "Cache settings/parameters (cached segments only).", 0) FIELD(PVSEGS, pvseg, NUM, "Start", pe, 5, uint32, pvseg_start, "Physical Extent number of start of segment.", 0)FIELD (PVSEGS , pvseg,NUM ,"SSize" , len,5 , uint32, pvseg_size,"Number of extents in segment." ,0 )
lib/report/properties.c +5 -0
@@ -440,6 +440,11 @@ GET_LVSEG_STR_PROPERTY_FN(devices, lvseg_devices(lvseg->lv->vg->vgmem, lvseg)) GET_LVSEG_STR_PROPERTY_FN(seg_monitor, lvseg_monitor_dup(lvseg->lv->vg->vgmem, lvseg))#define _seg_monitor_set prop_not_implemented_set +#define _cache_policy_get prop_not_implemented_get +#define _cache_policy_set prop_not_implemented_set +#define _cache_settings_get prop_not_implemented_get +#define _cache_settings_set prop_not_implemented_set +/* PVSEG */ GET_PVSEG_NUM_PROPERTY_FN(pvseg_start, pvseg->pe)#define _pvseg_start_set prop_not_implemented_set
lib/report/report.c +38 -0
@@ -233,6 +233,44 @@ static int _tags_disp(struct dm_report *rh, struct dm_pool *mem,return _field_set_string_list (rh, field, tagsl, private,1 ); } +struct _str_list_append_baton { +struct dm_pool *mem; +struct dm_list *result; +}; + +static int _str_list_append (const char *line,void *baton) +{ +struct _str_list_append_baton *b = baton; +const char *dup =dm_pool_strdup (b->mem, line); +if (!dup) + return_0; +if (!str_list_add (b->mem, b->result, dup)) + return_0; +return 1 ; +} + +static int _cache_settings_disp (struct dm_report *rh,struct dm_pool *mem, +struct dm_report_field *field, +const void *data,void *private) +{ +const struct dm_config_node *settings = *(const struct dm_config_node **) data; +struct dm_list *result =str_list_create (mem); +struct _str_list_append_baton baton = { mem, result }; + +if (!result) + return_0; + +if (settings) + settings = settings->child; + +while (settings) { +dm_config_write_one_node (settings, _str_list_append, &baton); + settings = settings->sib; + }; + +return _field_set_string_list (rh, field, result, private,0 ); +} +static int _modules_disp (struct dm_report *rh,struct dm_pool *mem,struct dm_report_field *field,const void *data,void *private)
Signature · 1 line
2.1.2
PATCH 3/4 test: Add a test for reports of cache policy & settings.
In reply to Petr Rockai--- test/shell/lvs-cache.sh | 50 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 test/shell/lvs-cache.sh
test/shell/lvs-cache.sh new file +50 -0
@@ -0,0 +1,50 @@ +#!/bin/sh +# Copyright (C) 2014 Red Hat, Inc. All rights reserved. +# +# This copyrighted material is made available to anyone wishing to use, +# modify, copy, or redistribute it subject to the terms and conditions +# of the GNU General Public License v.2. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software Foundation, +# Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + +# Exercise creation of cache and cache pool volumes + +# Full CLI uses --type +# Shorthand CLI uses -H | --cache + +. lib/inittest + +aux have_cache 1 3 0 || skip +aux prepare_vg 5 8000 + +lvcreate --type cache-pool -L10 $vg/cpool +lvcreate --type cache -l 1 --cachepool $vg/cpool -n corigin $vg +lvs -a -o lv_name,cache_policy +lvs -a -o lv_name,cache_settings + +lvremove -f $vg + +lvcreate --type cache-pool -L10 $vg/cpool +lvcreate --type cache -l 1 --cachepool $vg/cpool -n corigin $vg --cachepolicy mq \ + --cachesettings migration_threshold=233 +lvs -a -o lv_name,cache_policy | grep mq +lvs -a -o lv_name,cache_settings | grep migration_threshold=233 + +lvremove -f $vg + +lvcreate --type cache-pool -L10 --cachepolicy mq --cachesettings migration_threshold=233 $vg/cpool +lvcreate --type cache -l 1 --cachepool $vg/cpool -n corigin $vg +lvs -a -o lv_name,cache_policy | grep mq +lvs -a -o lv_name,cache_settings | grep migration_threshold=233 + +lvremove -f $vg + +lvcreate --type cache-pool -L10 --cachepolicy mq --cachesettings migration_threshold=233 --cachesettings sequential_threshold=13 $vg/cpool +lvcreate --type cache -l 1 --cachepool $vg/cpool -n corigin $vg +lvs -a -o lv_name,cache_policy | grep mq +lvs -a -o lv_name,cache_settings | grep migration_threshold=233 +lvs -a -o lv_name,cache_settings | grep sequential_threshold=13 + +lvremove -f $vg
Signature · 1 line
2.1.2
PATCH 4/4 test: Check that cache settings properly survive LV reactivation.
In reply to Petr Rockai--- test/shell/lvcreate-cache.sh | 3 +++ 1 file changed, 3 insertions(+)
test/shell/lvcreate-cache.sh +3 -0
@@ -230,6 +230,9 @@ lvcreate --type cache-pool -L10 $vg/cpool lvcreate --type cache -l 1 --cachepool $vg/cpool -n corigin $vg --cachepolicy mq --cachesettings migration_threshold=233 dmsetup status | grep $vg dmsetup status | grep $vg-corigin | grep 'migration_threshold 233' +lvchange -an $vg +lvchange -ay $vg +dmsetup status | grep $vg-corigin | grep 'migration_threshold 233' lvremove -f $vg
Signature · 1 line
2.1.2
PATCH 1/4 report: Print NULL strings as "" in _string_disp (instead of a SEGV).
In reply to Petr RockaiQuoted 9 lines · Petr Rockai
--- lib/report/report.c | 2 ++ 1 file changed, 2 insertions(+)lib/report/report.c +0
-0
lib/report/report.c static int _string_disp(struct dm_report *rh, struct dm_pool *mem __attribute__( · line 185
Quoted 7 lines
lib/report/report.c +2
-0@@ -179,6 +179,8 @@ static int _string_disp(struct dm_report *rh, struct dm_pool *mem __attribute__(struct dm_report_field *field, const void *data, void *private __attribute__((unused))) { +if (!*(void **) data) +return _field_set_value (field,"" ,NULL );return dm_report_field_string (rh, field, (const char *const *) data); }
...we could do this, but I think more appropriate here would be to define field-specific reserved value to represent the "undefined" value so we can match against this too when using selection (-S|--select).
So, in lib/report/values.h, you can just define:
FIELD_RESERVED_VALUE(cache_policy, cache_policy_undefined, "", "", "undefined", ... add as many synonym to "undefined" here as you need ...)
With this, any "" or "undefined" (or any other synonym) can be used in selection criteria to match against (the first synonym on the list is the one printed - in this case it's blank value "").
But that would mean defining own display function for the cache_policy, not using the generic _string_disp (but that's not a problem, of course).
So the patch above is OK, but it depends how you need the selection to behave when matching the cache_policy field.
(I'm thinking whether the "" STR value should be a global reserved value with that "undefined" and other synonyms, but it's possible that wouldn't probably suit all STR fields... I'd probably go with the field-specific reserved value for now. We can make that generic anytime later...)
Signature · 1 line
Peter
PATCH 1/4 report: Print NULL strings as "" in _string_disp (instead of a SEGV).
Quoted 23 lines · Peter Rajnoha
Quoted 9 lines · Petr Rockai
--- lib/report/report.c | 2 ++ 1 file changed, 2 insertions(+)lib/report/report.c +0
-0lib/report/report.c static int _string_disp(struct dm_report *rh, struct dm_pool *mem __attribute__( · line 185
Quoted 7 lines
lib/report/report.c +2
-0@@ -179,6 +179,8 @@ static int _string_disp(struct dm_report *rh, struct dm_pool *mem __attribute__(struct dm_report_field *field, const void *data, void *private __attribute__((unused))) { +if (!*(void **) data) +return _field_set_value (field,"" ,NULL );return dm_report_field_string (rh, field, (const char *const *) data); }...we could do this, but I think more appropriate here would be to define field-specific reserved value to represent the "undefined" value so we can match against this too when using selection (-S|--select).
Quoted 4 lines
So, in
lib/report/values.h, you can just define:FIELD_RESERVED_VALUE(cache_policy,
cache_policy_undefined, "", "", "undefined", ... add as many synonym to "undefined" here as you need ...)
Thinking about this more, we have actually three situations to display (correct me if I'm wrong please).
So in the end, we need:
- if cache LV and NULL policy -> we should report "default" (or similar keyword to denote "default" policy used)
- if cache LV and non-NULL policy -> we should report the policy name directly
- if non-cache LV (which has always the policy NULL, if course) -> we should report "" (with "undefined", "undef" ... synonyms to match against in selection criteria)
Signature · 1 line
Peter