On May 21, 2013, at 1:02 PM, Igor Galić <[email protected]> wrote:
> > > ----- Original Message ----- >> Updated Branches: >> refs/heads/issues/TS-1728 [created] 1d8179df5 >> >> >> TS-1728: Assign storage entries to volumes >> >> Introduces the volume assignment in storage.config, on an optional >> basis. No change is needed in the config if you don't want to use >> this feature. >> >> * Add an optional reference to the volume number in storage.config, >> and the Span object, in Store.h and Store.cc >> * Add the volume reference to DiskVol, CacheProcessor::start_internal >> (in Cache.cc) >> * Change cplist_reconfigure (in Cache.cc) to constrain mapping >> * Fix bytes_used and percent_used per volume stats > [snip] > > n.b.: This codebase is too big/complex for *me* to review, the comments here > are only things that I noticed while reading the diff on a train ride. > >> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/1d8179df/iocore/cache/Cache.cc >> ---------------------------------------------------------------------- >> diff --git a/iocore/cache/Cache.cc b/iocore/cache/Cache.cc >> index fbb97f3..4c7b9e4 100644 >> --- a/iocore/cache/Cache.cc >> +++ b/iocore/cache/Cache.cc > >> @@ -2456,7 +2508,16 @@ cplist_reconfigure() >> /* go through volume config and grow and create volumes */ >> >> config_vol = config_volumes.cp_queue.head; >> + for (; config_vol; config_vol = config_vol->link.next) { > > form/consistency question: why not have config_vol = > config_volumes.cp_queue.head as first part of the for loop? > >> + // if volume is given exclusive disks, fill here and continue >> + volume_number = config_vol->number; >> + if (!config_vol->cachep) { >> + continue; >> + } >> + fillExclusiveDisks(config_vol->cachep); >> + } >> >> + config_vol = config_volumes.cp_queue.head; >> for (; config_vol; config_vol = config_vol->link.next) { >> >> size = config_vol->size; >> @@ -2467,6 +2528,11 @@ cplist_reconfigure() > [snip] >> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/1d8179df/iocore/cache/Store.cc >> ---------------------------------------------------------------------- >> diff --git a/iocore/cache/Store.cc b/iocore/cache/Store.cc >> index 9076dc1..c843cbb 100644 >> --- a/iocore/cache/Store.cc >> +++ b/iocore/cache/Store.cc >> @@ -28,6 +28,31 @@ >> // Global >> Store theStore; >> >> +#define VOL_STR "volume=" > > consider making this a C++ constant > >> +int getVolume(char* line) { > > consider defining this function static. +1 on making everything static > >> + int v = 0; >> + if(!line) return 0; >> + char* str = strstr(line, VOL_STR); >> + char* vol_start = str; >> + if(!str) return 0; >> + while (*str && !ParseRules::is_digit(*str)) >> + str++; >> + v = ink_atoi(str); >> + >> + while (*str && ParseRules::is_digit(*str)) >> + str++; >> + while(*str) { >> + *vol_start = *str; >> + vol_start++; >> + str++; >> + } >> + *vol_start = 0; >> + Debug("cache_init", "returning %d and '%s'", v, line); >> + >> + if(v < 0) return 0; >> + return v; >> +} >> + >> >> // >> // Store >> @@ -291,6 +316,8 @@ Store::read_config(int fd) >> if (!*n) >> continue; >> >> + int volume_id = getVolume(n); >> + >> // parse >> Debug("cache_init", "Store::read_config: \"%s\"", n); >> >> @@ -310,7 +337,9 @@ Store::read_config(int fd) >> n[len] = 0; >> char *pp = Layout::get()->relative(n); >> ns = NEW(new Span); >> - Debug("cache_init", "Store::read_config - ns = NEW (new Span); >> ns->init(\"%s\",%" PRId64 ")", pp, size); >> + ns->vol_num = volume_id; >> + Debug("cache_init", "Store::read_config - ns = NEW (new Span); >> ns->init(\"%s\",%" PRId64 "), ns->vol_num=%d", >> + pp, size, ns->vol_num); >> if ((err = ns->init(pp, size))) { >> char buf[4096]; >> snprintf(buf, sizeof(buf), "could not initialize storage >> \"%s\" [%s]", pp, err); > > -- > Igor Galić > > Tel: +43 (0) 664 886 22 883 > Mail: [email protected] > URL: http://brainsware.org/ > GPG: 6880 4155 74BD FD7C B515 2EA5 4B1D 9E08 A097 C9AE >
