> ss_thread_create now returns the thread ID on success, and zero on failure.
> Previously returned SS_SUCCESS or SS_NO_THREAD. User must now test the
> return value to determine result.
> 
> ss_thread_kill added to kill the passed thread ID. Returns SS_SUCCESS or
> SS_NO_THREAD.
> 
> Any thread creation must be verified now, and old code must be examined to
> ensure the return value is checked.

Thank you for that post. Internally, threads are not use in midas, so there 
should be no problem. Only experiments using threads explicitly should take 
care.

I had to add a 'umask' opiton to the loggers (lazy and mlogger) for the new 
PSI archive. One can now put a filename into the settings like:

archive,21,user,pw,dir,run%05d.mid,026

where the optional last parameter is used for a "umask 026" command just 
sent to the FTP server after the connection has been established. This 
changes the mode bits of the newly transferred file. We needed that so that 
the files are group readable, since several people from one group want to 
read the data.

I committed mlogger.c and ybos.c which contains the ftp code (should 
actually go into lazylogger.c instead of ybos.c).

I just realized that db_set_value, db_set_data, db_set_num_values and 
db_merge_data do not check for num_values == 0. With such a parameter the 
ODB can become corrupted, since zero length ODB entries are not allowed. I 
fixed the according places in odb.c and committed the changes. Everyone 
with ODB corruption problems should update that code.

> Stephan, why did you prohibit building mana.c without ROOT and HBOOK
> support? I think such a configuration is valid and should be allowed.

Oops, sorry, my fault. I forgto that people use mana.c without ROOT and 
HBOOK. The reason I made the change was that people forgot the -DHVAE_HBOOK 
in their makefile. In that case, no HBOOK init is done in mana.c and the 
first histogram booking in the user code crashes HBOOK.

So please take the #error statement out of mana.c (I'm away in two hours for 
one week), but think about preventing the above mentionend problem. I don't 
know any way for the makefile or mana.c to figure out if there is any HF1 
call in the user code. Actually HF1 should return a "proper" error message 
than just crashing.

One possibility is that we put an additional layer on top of the histogram 
boooking/filling. These macros are converted to their HBOOK or ROOT 
equivalents depending on the HAVE_HBOOK/HAVE_ROOT. If none of both is 
present, the histogram booking macro can produce a runtime error. This has 
the additional advantage that users can switch from HBOOK to ROOT without 
change of their user code.

> I found where we tickle the race condition in db_create_record().
> 
> 1) in mhttpd.c,  every time we show the status page, we call
> db_create_record(hDB, 0, "/Runinfo", strcomb(runinfo_str));
> 2) internally db_create_record() deletes /RunInfo
> 3) other programs read "/runinfo/run number" while it is deleted do not
> check for the db_get_value() error code and happily get a zero run number.
> 
> Stephan fixed the race condition, and now I commited an mhttpd.c change that
> only calls db_create_record(hDB, 0, "/Runinfo", strcomb(runinfo_str)); if
> /runinfo does not exist. This seems to be redundant with a similar call in
> cm_connect_experiment1(), called each time a new client starts up.

The reason for the db_create_record() is the following: Assume that we change 
the /runinfo structure, by adding an additional variable in the future. If we 
run a "new" mhttpd on an "old" experiment, the "runinfo" C structure does not 
match the ODB contents. The db_create_record() ensures that the ODB structure 
exactly matches the C structure. I agree with you that this can cause 
potential problems. But most of them should be fixed by the additional lock() 
I added recently. So other programs cannot read the run number while it is 
deleted.

One could think of checking the record size, and re-creating the runinfo if 
the ODB record size does not match the C record size. But this does not 
prevent the potential error that some variable are reversed in order. They 
are then mapped wrongly to the C runinfo structure.

I see that you work very hard now on all possible checks for the run number. 
But I would not commit that and make it part of the distribution, since all 
experiments at PSI for example do not have this run number problem. Run it 
locally, determine the cause of your problem (the discovery of the race 
condition was already very good, I'm glad that your found it, should make the 
system much more stable), and we'll fix it. Puttin ASSERT's is a good idea, I 
should have done it from the very beginning. But if you start now, please put 
it in all other 100000 places (;-)

I would not add a db_get_value_cannot_possibly_fail() into the standard 
distribution, because it probably cannot correct the initial problem and then 
just will go into an infinite loop. We should tackle problems always at their 
source. 

If you cannot resolve your zero run number problem, do the following: There 
is a cm_msg(MDEBUG, ...) which only puts a message into the shared memory, 
but not in midas.log. This can be used for real time debugging. Add those 
message temporarily in db_get_value() etc. to see what is going on. As soon 
as the run number goes to zero, stop all processes immediately (for example 
by locking the database with db_lock_database), and the look backwards in the 
sysmsg buffer to see what happened *before* the run number went to zero.

- Stefan

> I added error checking to the places where we read "/runinfo/run number". In
> general, I do this:

> Affected files:
> src/lazylogger.c
> src/odbedit.c
> src/mlogger.c
> src/mfe.c
> src/odb.c
> src/mana.c
> src/midas.c
> src/mhttpd.c

Now YOU broke the system by editing all these files with something I consider 
temporary debugging code. A run number of zero is *VALILD*. If I want to make 
sure a new experiment starts with run number #1, I put a run number of 0 into 
the ODB. So on the first start the number is incremented by one which results 
in run number from one. So please remove those checks which prevents me of 
doing that. Again, your "run number zero" problem is soemhow specific to your 
environment, and I would not put all these tests into the distribution, 
because this can have side effects, like that one I described above.

- Stefan

Ok, I apologize. It's all ok. Thanks for clearifying. Concerning the assert's, it 
would be nice to be able to disable them in release code. Under Windows, the 
assert() is actually a macro which expands to zero if NDEBUG is defined. I 
believe it's the same under linux, but I don't know about VxWorks. So we have 
three options:

1) Keep asserts always. This might possible slow down a DAQ system, but I'm not 
sure how much. Might be negligible.

2) Disable asserts by default (standard make). Only the "experts" can enable it 
in the make file (by removing NDEBUG), since only they know what to do with the 
assertation messages.

3) Let the user decide on the standard installation. Maybe have two libraries, 
one debug, one no-debug. The no-debug can even have the compiler optimization 
disabled, which makes debugging easier.

So what is your opinion (comments from others are welcome as well) of which way 
to go? 

I have seen the same behaviour and it annoys me, too. What I did in the past 
is a "cleanup" in ODBEdit which removes these open records. I have soem code 
in cm_watchdog(), which should take care of that. If a client is dead, it 
gets removed from the ODB, and its open records should get its notify_count 
decremented. So obviously this code has some bug. I plan to do in the 
following week (now I got some spare time) the following:

- exchange most db_create_record() by something better. Maybe 
db_check_record(..., correct_flag), which creates the record only if it does 
not exist at all, otherwise checks the structure. If correct_flag is TRUE, it 
corrects the strucure (by calling db_create_record()), if it's false it just 
returns an error code. This way one can decide from case to case which option 
is better. Like for the /Runinfo, the flag would be FALSE, maybe with a 
notification that the /Runinfo is different from the compiled-in structure, 
and one hast to recompile the application.

- revisit the open record issue from dying frontends. I remember vaguely that 
I tried to kill a frontend (kill -9), wait until the watchdog cleans up its 
entries, and it worked fine. So it's more the problem to reproduce the issue 
described in the previous elog entry. 

Let me propose a revised scheme for midas standard VME calls (mvmestd.h). 

Pierre mentioned some limitations before, and I find now also some fields 
to improve. Right now, the vme_open() call retrieves a handle. For some 
interfaces (like SBS/Bit3), one has to obtain separate handles for 
different addressing modes A24D32/A32D32 and so on, which I find a bit 
troublesome. I would rather keep the handle internally, invisible to the 
user, and use ioctl() statments to change the address/data mode. 

So the API could look like:

vme_open()       Deprecated, will be removed
vme_init(void)   Standard initialization, open device(s), stores handles
                 internally in a table
vme_exit(void)   Deallocates any memory, close handles

vme_read(void *dst, DWORD vme_addr, DWORD size)
vme_write(void *src, DWORD vme_addr, DWORD size)

vme_ioctl(int request, int *param)

                 Request is one of 
                   VME_IOCTL_CRATE_SET/GET
                     Sets VME crate (in case several interfaces are
                     plugged into singlePC, meaningless for embedded CPUs)
                   VME_IOCTL_DEST_SET/GET
                     VME_BUS/VME_RAM/VME_LM for VME bus, RAM in VME 
                     interface, or LM for local memory (used in Bit3 
                     interface)
                   VME_IOCTL_AMOD_SET/GET
                     Sets/Retrieves VME AMOD (= VME_AMOD_xxx as currently
                     defined in mvmestd.h)
                   VME_IOCTL_DSIZE_SET/GET
                     Sets/Retrieves VME data size (D8/D16/D32/D64)
                   VME_IOCTL_DMA_SET/GET
                     Enable/Disable DMA, should be independent of AMOD
                   VME_IOCTL_INTR_ATTACH/DETACH/ENABLE/DISABLE
                     Set VME interrupts
                   VME_IOCTL_AUTO_INCR_SET/GET
                     Set autoincremet of source pointer, can be disabled
                     for FIFO readout

vme_mmap(void **ptr, DWORD vme_addr, DWORD size)
vme_unmap(void *ptr, DWORD size)
                  Map/Unmap VME to local memory

vme_read2(void *dst, DWORD vme_addr, DWORD size, DWORD flags)
vme_write2(void *src, DWORD vme_addr, DWORD size, DWORD flags)
                 With these functions one can directly specify the flags
                 usually managed by vme_ioctl(). Usefule for applications
                 where the address modifier for example has to be
                 different in each read/write operation.  

Note that the vme_read/write functions do not have a VME handle any more, 
nor an address modifier. This is all accomplished with vme_ioctl() calls.

Please have a look at this proposal, compare it with what you do currently 
in VME, and let me know if we should add/modify something. I volunteer to 
implement the API for the SBS/Bit3 617 and the Struck SIS1100/3100 
interfaces, for VxWorks somebody at TRIUMF should take care.

As Konstantin pointed out correctly, the db_create_record() call is pretty 
heavy since it copies whole structures around the ODB. Therefore, it 
should not used frequently. It might be that several problems are caused 
by that, for example the "phantom" records reported in elog:40 .

I have therefore implemented the function 

db_check_record(HNDLE hDB, HNDLE hKey, char *keyname, char *rec_str, 
                BOOL correct)

which takes an ASCII structure in the same way as db_create_record(), but 
only checks this ASCII structure against the ODB contents without writing 
anything to the ODB. 

If the record does not exist at all, it is created via db_create_record(). 
This is useful for example with the /Runinfo structure on a virgin ODB.

If the parameter "correct" is FALSE, the function returns 
DB_STRUCT_MISMATCH if the ODB contents is wrong (wrong order of variables, 
wrong name of variables, wrong type or array size). The calling function 
should then abort, since a subsequent db_open_record() would fail. Note 
that although abort() is useful, one should add cm_disconnect_experiment() 
just before the abort() in order to have the application "log out" from 
the ODB gracefully. If the parameter "correct" is TRUE, the function 
db_create_record() is called internally to correct a mismatching record.

I have changed most calls of db_create_record() in mhttpd.c, mfe.c, mana.c 
and mlogger.c. Pierre, could you do the same for lazylogger.c?

I also started to put assert()'s everywhere and encourage everyone to 
follow. Under Windows, the asserts() are removed automatically if 
compiling in "Release" mode.

So I committed many changes, did some quick tests, but am not 100% 
convinced that all the changes are good. So please use the new code 
cautiously, and let me know if there is any new problem. I also would like 
to get some feedback if the whole thing becomes more stable now.

I tried to reproduce the problem, but without success. So in case this happens 
again, one should debug the code im cm_watchdog() next to the line

/* decrement notify_count for open records and clear exclusive mode */
...

So if a killed client is removed from the ODB via the watchdog (or a "cleanup" 
is done in ODBEdit), the notify_count should be decreased and thus the "open 
records" should be closed.

> 1) shutdown from mhttpd "programs" page -> "cannot shutdown client"
> 2) "sh mhttpd" from odbedit -> 
>    [midas.c:5298:cm_shutdown] cannot connect to client mhttpd on host
>    midtig01.triumf.ca, port 32853
>    Client mhttpd not active
> 3) in odbedit: "cd /system/clients; rm xxxx"
>    refuses to delete the key

Have you tried a "cleanup" in ODBEdit?

The "last_activity" is a 32-bit int, filled with milliseconds. So indeed it 
wraps around after about one month. So if a all clients are stopped 
simultaneously the hard way (such that nobody's watchdog can clean any other 
client from the ODB), like with a power off, and you start the thing one 
month later, there might be a problem. I never tried that before. So next 
time to a cleanup. If that does not help, we should change last_activity 
from INT to DWORD. This way it's alway positive and the wraparound does not 
hurt.

> dio- is required to be setuid-root to gain I/O permissions. I looked at it a
> few times, and it is probably safe, but I would like to get a second
> opinion. Stephan, can you should it to your local security geeks?
> 
> mhttpd- definitely unsafe. It has more buffer overflows than I can shake a
> stick at. Why is it suid-root anyway?
> 
> webpaw- what is it?!?

dio was written by Pierre. 

mhttpd and webpaw both are web servers. webpaw is used to display PAW 
pictures over the web. If you run these programs at a port <1024, and most 
people do run them at port 80 (at least at PSI), they need to be setuid-root. 
Unless you know a better way to do that...

> INT == "int", wraparound in 1 month
> DWORD == "unsigned int", wraparound in 2 months
> 
> should we make it the 64-bit "long long" (or C98's "int64_t")?

Won't work on all supported compilers. The point is that DWORD wraps around in 
2 months, but the difference of two DWORDs is alywas positive, never negative 
like you had it. We only have to distinguish if the difference of the current 
time (im ms) minus the last_activity of a client is larget than the timeout, 
typically 10 seconds or so. If you have a wraparound on 32-bit DWORD, the 
difference is still ok. Like

current "time" : 0x0000 0100
last_activity:   0xFFFF FF00

then current_time - last_activity = 0x00000100 - 0xFFFFFF00 = 0x00000200 if 
calculated with 32-bit values.

Thanks for your contribution. Let me try to map your functionality to mvmestd calls:

> A) The VMIC vme_slave_xxx() options are not considered.

We could maybe do that through mvme_mmap(SLAVE, ...) instead of mvme_mmap(MASTER, ...)

> B) The interrupt handling can certainly match the 4 entries required in the user frontend
>     code i.e. Attach, Detach, Enable, Disable.

vmve_ioctl(VME_IOCTL_INTR_ATTACHE/DETACH/ENABLE/DISABLE, func())

> I don't understand your argument that the handle should be hidden. In case of multiple
> interfaces, how do you refer to a particular one if not specified? 
> The following scheme does require a handle for refering to the proper (device AND window).

Four reasons for that:

1) For the SBS/Bit3, you need a handle for each address mode. So if I have two crates (and I do in our 
current experiment), and have to access modules in A16, A24 and A32 mode, I need in total 6 handles. 
Sometimes I mix them up by mistake, and wonder why I get bus errors. 

2) Most installations will only have single crates (as your VMIC). So if there is only one crate, why 
bother with a handle? If you have hunderds of accesses in your code, you save some redundant typing work.

3) A handle is usually kept global, which is considered not good coding style.

4) Our MCSTD and MFBSTD functions also do not use a handle, so people used to those libraries will find it 
more natural not to use one.

> 1 ) deviceHandle = vme_init(int devNumber);
>     Even though the VMIC doesn't deal with multiple devices,
>     the SIS/PCI does and needs to init on a specific PCI card.
>     Internally:
>       opening of the device (/dev/sisxxxx_1) (ignored in case of VMIC).
>       Possible including a mapping to a default VME region of default size with default AM
>       (VMIC :16MB, A24). This way in a single call you get a valid handle for full VME access
>       in A24 mode. Needs to be elaborate this option. But in principle you need to declare the 
>      VME region that you want to work on (vme_map).

Just vme_init(); (like fb_init()).

This function takes the first device, opens it, and stores the handle internally. Sets the AM to a default 
value, and creates a mapping table which is initially empty or mapped to a default VME region. If one wants 
to access a secondary crate, one does a vme_ioctl(VME_IOCTL_CRATE_SET, 2), which opens the secondary crate, 
and stores the new handle in the internal table if applicable.

> 2) mapHandle = vme_map(int deviceHandle, int vmeAddress, int am, int size);
>     Return a mapHandle specific to a device and window. The am has to be specified.
>     What ever are the operation to get there, the mapHandle is a reference to thas setting.
>     It could just fill a map structure.
>     Internally:
>       WindowHandle[deviceHandle] = vme_master_create(BusHandle[deviceHandle], ...
>       WindowPtr[WindowHandle] = vme_master_window(BusHandle[deviceHandle]
>                               , WindowHandle[deviceHandle]...

The best would be if a mvme_read(...) to an unmapped region would automatically (internally) trigger a 
vme_map() call, and store the WindowHandle and WindowPtr internally. The advantage of this is that code 
written for the SIS for example (which does not require this kind of mapping) would work without change 
under the VMIC. The disadvantage is that for each mvme_read(), the code has to scan the internal mapping 
table to find the proper window handle. Now I don't know how much overhead this would be, but I guess a 
single for() loop over a couple of entries in the mapping table is still faster than a microsecond or so, 
thus making it negligible in a block transfer. 

> 3) vme_setmode(mapHandle, const int DATA_SIZE, const int AM
>                            , const BOOL ENA_DMA, const BOOL ENA_FIFO);
>     Mainly used for the vme_block_read/write. Define for following read the data size and 
>     am in case of DMA (could use orther DMA mode than window definition for optimal
>     transfer).
> 
>     Predefine the mode of access:
>     DATA_SIZE : D8, D16, D32
>     AM             : A16, A24, A32, etc...
>     enaDMA     : optional if available.
>     enaFIFO     : optional for block read for autoincrement source pointer.
> 
> Remark:
> PAA- I can imagine this function to be a vme_ioctl (int mapHandle, int *param)
>         such that extension of functionality is possible. But by passing cons int
>         arguments, the optimizer is able to substitute and reduce the internal code.

Right. mvme_ioctl(VME_IOCTL_AMOD_SET/DSIZE_SET/DMA_SET/AUTO_INCR_SET, ...)

>    uint_8Value   = vme_readD8  (int mapHandle, uint_64 vmeSrceOffset)
>    uint_16Value = vme_readD16 (int mapHandle, uint_64 vmeSrceOffset)
>    uint_32Value = vme_readD32 (int mapHandle, uint_64 vmeSrceOffset)
>    Single VME read access. In the VMIC case, this access is always through mapping.
>    Value = *(WindowPtr[WindowHandle] + vmeSrceOffset) 
>    or 
>    Value = *(WindowStruct->WindowPtr[WindowHandle] + vmeSrceOffset) 

mvme_read(*dst, DWORD vme_addr, DWORD size); would cover this in a single call. Note that the SIS for 
example does not have memory mapping, so if one consistently uses mvme_read(), it will work on both 
architectures. Again, this takes some overhead. Consider for example a possible VMIC implementation

mvme_read(char *dst, DWORD vme_addr, DWORD size)
{
  for (i=0 ; table[i].valid ; i++)
    {
    if (table[i].start >= vme_addr && table[i].end < vme_addr+size)
      break;
    }

  if (!table[i].valid)
    {
    vme_master_crate(...)
    table[i].window_handle = vme_master_window(...)
    }

  if (size == 2)
    mvme_ioctl(VME_IOCTL_DSIZE_SET, D16);
  else if (size == 1)
    mvme_ioctl(VME_IOCTL_DSIZE_SET, D8);

  memcpy(dst, table[i].window_handle + vme_addr - table[i].start, size);
}

Note this is only some rough code, would need more checking etc. But you see that for each access the for() 
loop has to be evaluated. Now I know that for the SBS/Bit3 and for the SIS a single VME access takes 
~0.5us. So the for() loop could be much faster than that. But one has to try. If one experiment needs the 
ultimate speed, it can use the native VMIC API, but then looses the portability. I'm not sure if one needs 
the automatic DSIZE_SET, maybe it works without.

>    status  = vme_writeD8   (int mapHandle, uint_64 vmeSrceOffset, uint_8 Value)
>    status  = vme_writeD16 (int mapHandle, uint_64 vmeSrceOffset, uint_16 Value)
>    status  = vme_writeD32 (int mapHandle, uint_64 vmeSrceOffset, uint_32 Value)
>    Single VME write access.

Dito. mvme_write(void *dst, DWORD vme_addr, DWORD size);

>    nBytes = vme_block_read(mapHandle, char * pDest, uint_64 vmeSrceOffset, int size);
>    Multiple read access. Can be done through standard do loop or DMA if available.
>    nBytes < 0 :  error
>    Incremented pDest  = (pDest + nBytes); Don't need to pass **pDest for autoincrement.

vmve_ioctl(VME_IOCTL_DMA_SET, TRUE);
n = mvme_read(char *pDest, DWORD vmd_addr, DWORD size);

>    nBytes = vme_block_write(mapHandle, uint_64 vmeSrceOffset, char *pSrce, int size);
>    Multiple write access.
>    nBytes < 0 :  error
>    Incremented pSrce  = (pSrce + nBytes); Don't need to pass **pSrce for autoincrement.

Dito.

> 8) status = vme_unmap(int mapHandle)
>    Cleanup internal pointers or structure of given mapHandle only.

mvme_unmap(DWORD vme_addr, DWORD size)

Scan through internal table to find handle, then calls vme_unmap(mapHandle);

> 9) status = vme_exit()
>    Cleanup deviceHandle and release device.

mvme_exit();

Let me know if this all makes sense to you...

- Stefan

> But there is one problem with "cleanup". It has a hardwired timeout of
> 2 seconds.  This is a problem for tasks like lazylogger which set a timeout
> of 60 seconds when moving the tape. So BEWARE, if you issue the "cleanup"
> command, it might kill some clients who have setup their timeout to longer
> than 2 seconds. 
> 
> I have asked Stefan to change this before. He said that, to be effective,
> the timeout value used for "cleanup" has to be rather short. 
> One possibility, would be to allow for a user entered "cleanup" timeout.
> The default could stay at 2 seconds. 

I changed the behaviour of cleanup by adding an extra parameter 
ignore_timeout to cm_cleanup(). Now, in ODBEdit, a "clanup" obeys the 
timeout set by the clients. The problem with that is if the logger crashes 
for example, and it's timeout is set o 5 min., it cannot be clean-up'ed any 
more for the next five minutes, and therefor not be restarted wasting 
precious beam time. That's why I hard-wired originally the "cleanup" timout 
to 2 sec. Now I added a flag "-f" to the ODBEdit cleanup command which works 
in the old fashion with a 2 sec. timeout. So a "cleanup" alone won't kill a 
looger which currently rewinds a tape or so, but a "cleanup -f" does.

I also changed internal timeouts from INT to DWORD, which should fix the 
problem Konstantin reported recently (re-starting an experiment after 
several weeks). New changes are commited, but I only did basic tests. So 
please try the new code and tell me if there is any problem.

- Stefan

> fal.c does not compile: it calls cm_cleanup() with one argument when there
> should be two arguments. K.O.

Fixed and committed.

Fixed and committed. Can you check if it's working?

> I have noticed a problem with deleting a key to an array in odb, then
> recreating the record as in the code below. The record is recreated
> successfully, but when viewing it with mhttpd, a spurious blank line
> (coloured orange) is visible, followed by the rest of the data as normal.
> 
> db_create_record(hDB,0,"/Equipment/Cycle_scalers/Settings/",strcomb(type1_str));
>     }
>   else {
>     exp_mode = 2; /* TDmusr types - noscans */
>     status =
> db_create_record(hDB,0,"/Equipment/Cycle_scalers/Settings/",strcomb(type2_str));
>   }

The first problem is that the db_create_record has a trailing "/" in the key name 
after Settings. This causes the (empty) subsirectory which causes your trouble. 
Simple removing it fixes the problem. I agree that this is not obvious, so I 
added some code in db_create_record() which removes such a trailing slash if 
present. New version under CVS.

Second, the db_create_record() call is deprecated. You should use the new 
function db_check_record() instead, and remove your db_delete_key(). This avoids 
possible ODB trouble since the structure is not re-created each time, but only 
when necessary.

- Stefan

Hi Paul,

sorry my late reply, I had to find some time for debugging your problem. 
Thank you very much for the detailed description of the problem, I wish all 
bug reports would be such elaborate!

You were right that there was a bug in the RPC system. The function 
db_remove_open_record() got a new parameter recently, which was not changed 
in the RPC call, and caused the mserver side to crash on any 
db_close_record() call.

I fixed it and the update is under CVS (http://midas.psi.ch/cgi-
bin/cvsweb/midas/src/). Since you need to update many files, I wonder if I 
should enable anonymous CVS read access. Does anybody know how to set this 
up using "ssh" as the protocol (via CVS_RSH=ssh)?

Please note that db_close_record() is not necessary as 
cm_disconnect_experiment() takes care of this, but having it there does not 
hurt.