viced: Set FS_STATE_DUMP_MODE earlier

When the fileserver saves its callback state to disk on shutdown,
fs_stateCreateDump sets state->mode to FS_STATE_DUMP_MODE. But this is
after some code paths have already looked at state->mode, such as
fs_stateSave -> h_stateVerify -> h_Enumerate_r -> h_stateVerifyHost ->
h_stateVerifyAddrHash. This doesn't cause any problems right now,
since the value of FS_STATE_DUMP_MODE is 0, and we zero the whole
'state' struct when it is initialized.

This is confusing, though, so move the assignment of
FS_STATE_DUMP_MODE to earlier in the state-saving process, before
anything looks at it.

To try to avoid possible uninitialized use of state->mode in the
future, change the FS_STATE_*_MODE constants so that none of them are
0, and make sure that code checking the 'mode' checks that the mode is
a valid constant.

[adeason@sinenomine.net: Added state mode checking.]

Change-Id: Ifa5e5aabdb7c7b4a50a1b397620aafa7c48c223e
Reviewed-on: https://gerrit.openafs.org/14726
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>

src/viced/host.c

@@ -3391,10 +3391,15 @@ h_stateVerifyAddrHash(struct fs_dump_state * state, struct host * h,
 	                tmp, (unsigned)htons(port)));
 	    ret = 1;
 	    goto done;
-	} else {
+	} else if (state->mode == FS_STATE_DUMP_MODE) {
 	    ViceLog(0, ("h_stateVerifyAddrHash: warning: addr %s:%u not found in hash\n",
 	                tmp, (unsigned)htons(port)));
 	    state->flags.warnings_generated = 1;
+	} else {
+	    ViceLog(0, ("h_stateVerifyAddrHash: error: bad state mode %d\n",
+			state->mode));
+	    ret = 1;
+	    goto done;
 	}
     }
 
@@ -3448,10 +3453,15 @@ h_stateVerifyUuidHash(struct fs_dump_state * state, struct host * h)
 		    tmp.buffer));
 	ret = 1;
 	goto done;
-    } else {
+    } else if (state->mode == FS_STATE_DUMP_MODE) {
 	ViceLog(0, ("h_stateVerifyUuidHash: warning: uuid %s not found in hash\n",
 		    tmp.buffer));
 	state->flags.warnings_generated = 1;
+    } else {
+	ViceLog(0, ("h_stateVerifyUuidHash: bad state mode %d\n",
+		    state->mode));
+	ret = 1;
+	goto done;
     }
 
  done:

src/viced/serialize_state.c

@@ -123,6 +123,8 @@ fs_stateSave(void)
 	goto done;
     }
 
+    state.mode = FS_STATE_DUMP_MODE;
+
     /* XXX
      * on busy servers, these checks will inevitably fail since stuff drops H_LOCK
      * all over the place (with structs left in inconsistent states) while RPCs to
@@ -328,7 +330,6 @@ fs_stateCreateDump(struct fs_dump_state * state)
     }
 
     state->fd = fd;
-    state->mode = FS_STATE_DUMP_MODE;
     memset(state->hdr, 0, sizeof(struct fs_state_header));
     fs_stateIncEOF(state, sizeof(struct fs_state_header));
 
@@ -769,7 +770,7 @@ fs_stateMapFile(struct fs_dump_state * state, int preserve_flag )
 	flags = PROT_WRITE;
 	break;
     default:
-	ViceLog(0, ("fs_stateMapFile: invalid dump state mode\n"));
+	ViceLog(0, ("fs_stateMapFile: invalid dump state mode %d\n", state->mode));
 	return 1;
     }
 

src/viced/serialize_state.h

@@ -230,8 +230,8 @@ struct idx_map_entry_t {
 
 struct fs_dump_state {
     enum {
-	FS_STATE_DUMP_MODE,
+	FS_STATE_DUMP_MODE = 0x1,
-	FS_STATE_LOAD_MODE
+	FS_STATE_LOAD_MODE = 0x2
     } mode;
     struct {
 	byte do_host_restore;              /* whether host restore should be done */