Consolidate code for reading/writing vol headers
authorAndrew Deason <adeason@sinenomine.net>
Thu, 4 Feb 2010 22:26:21 +0000 (16:26 -0600)
committerDerrick Brashear <shadow@dementia.org>
Tue, 9 Feb 2010 19:53:00 +0000 (11:53 -0800)
The code for reading in and overwriting/replacing volume headers is
duplicated many times throughout the volume package. Consolidate the
code into the functions VReadVolumeHeader, VWriteVolumeHeader,
VCreateVolumeHeader, and VDestroyVolumeHeader. This makes it easy to
change the semantics of how headers are read/written, though this commit
should not change them.

Change-Id: Id395ee25fc2db92de9301b9cdbe18f30830e18d3
Reviewed-on: http://gerrit.openafs.org/1238
Reviewed-by: Derrick Brashear <shadow@dementia.org>
Tested-by: Derrick Brashear <shadow@dementia.org>

src/vol/listinodes.c
src/vol/namei_ops.c
src/vol/nuke.c
src/vol/purge.c
src/vol/vol-salvage.c
src/vol/volume.c
src/vol/volume.h
src/vol/vutil.c

index 54ff779..b16cb6b 100644 (file)
@@ -1454,21 +1454,6 @@ inode_ConvertROtoRWvolume(char *pname, afs_uint32 volumeId)
 
     memset(&specinos, 0, sizeof(specinos));
           
-    (void)afs_snprintf(headername, sizeof headername, VFORMAT, afs_printable_uint32_lu(volumeId));
-    (void)afs_snprintf(oldpath, sizeof oldpath, "%s/%s", pname, headername);
-    fd = open(oldpath, O_RDONLY);
-    if (fd < 0) {
-        Log("1 inode_ConvertROtoRWvolume: Couldn't open header for RO-volume %lu.\n", volumeId);
-        return ENOENT;
-    }
-    if (read(fd, &h, sizeof(h)) != sizeof(h)) {
-        Log("1 inode_ConvertROtoRWvolume: Couldn't read header for RO-volume %lu.\n", volumeId);
-        close(fd);
-        return EIO;
-    }
-    close(fd);
-    FSYNC_VolOp(volumeId, pname, FSYNC_VOL_BREAKCBKS, 0, NULL);
-
     /* now do the work */
           
     for (partP = DiskPartitionList; partP && strcmp(partP->name, pname);
@@ -1478,6 +1463,14 @@ inode_ConvertROtoRWvolume(char *pname, afs_uint32 volumeId)
         return EIO;
     }
 
+    if (VReadVolumeDiskHeader(volumeId, partP, &h)) {
+       Log("1 inode_ConvertROtoRWvolume: Couldn't read header for RO-volume %lu.\n",
+           afs_printable_uint32_lu(volumeId));
+       return EIO;
+    }
+
+    FSYNC_VolOp(volumeId, pname, FSYNC_VOL_BREAKCBKS, 0, NULL);
+
     strcpy(tmpDevName, partP->devName);
     name = getDevName(tmpDevName, wpath);
 
@@ -1565,22 +1558,17 @@ inode_ConvertROtoRWvolume(char *pname, afs_uint32 volumeId)
     }
 #endif
 
-    (void)afs_snprintf(headername, sizeof headername, VFORMAT, afs_printable_uint32_lu(h.id));
-    (void)afs_snprintf(newpath, sizeof newpath, "%s/%s", pname, headername);
-    fd = open(newpath, O_CREAT | O_EXCL | O_RDWR, 0644);
-    if (fd < 0) {
-        Log("1 inode_ConvertROtoRWvolume: Couldn't create header for RW-volume %lu.\n", h.id);
+    if (VCreateVolumeDiskHeader(&h, partP)) {
+        Log("1 inode_ConvertROtoRWvolume: Couldn't write header for RW-volume %lu\n",
+           afs_printable_uint32_lu(h.id));
         return EIO;
     }
-    if (write(fd, &h, sizeof(h)) != sizeof(h)) {
-        Log("1 inode_ConvertROtoRWvolume: Couldn't write header for RW-volume %lu.\n", h.id);
-        close(fd);
-        return EIO;
-    }
-    close(fd);
-    if (unlink(oldpath) < 0) {
-        Log("1 inode_ConvertROtoRWvolume: Couldn't unlink RO header, error = %d\n", errno);
+
+    if (VDestroyVolumeDiskHeader(partP, volumeId, h.parent)) {
+        Log("1 inode_ConvertROtoRWvolume: Couldn't unlink header for RO-volume %lu\n",
+           afs_printable_uint32_lu(volumeId));
     }
+
     FSYNC_VolOp(volumeId, pname, FSYNC_VOL_DONE, 0, NULL);
     FSYNC_VolOp(h.id, pname, FSYNC_VOL_ON, 0, NULL);
     return 0;
index 84d22d0..25edb7e 100644 (file)
@@ -1641,22 +1641,6 @@ namei_ConvertROtoRWvolume(char *pname, afs_uint32 volumeId)
     struct DiskPartition64 *partP;
     struct ViceInodeInfo info;
     struct VolumeDiskHeader h;
-    char headername[16];
-
-    (void)afs_snprintf(headername, sizeof headername, VFORMAT, afs_printable_uint32_lu(volumeId));
-    (void)afs_snprintf(oldpath, sizeof oldpath, "%s/%s", pname, headername);
-    fd = open(oldpath, O_RDONLY);
-    if (fd < 0) {
-        Log("1 namei_ConvertROtoRWvolume: Couldn't open header for RO-volume %lu.\n", volumeId);
-        return ENOENT;
-    }
-    if (read(fd, &h, sizeof(h)) != sizeof(h)) {
-        Log("1 namei_ConvertROtoRWvolume: Couldn't read header for RO-volume %lu.\n", volumeId);
-        close(fd);
-        return EIO;
-    }
-    close(fd);
-    FSYNC_VolOp(volumeId, pname, FSYNC_VOL_BREAKCBKS, 0, NULL);
 
     for (partP = DiskPartitionList; partP && strcmp(partP->name, pname);
          partP = partP->next);
@@ -1664,6 +1648,15 @@ namei_ConvertROtoRWvolume(char *pname, afs_uint32 volumeId)
         Log("1 namei_ConvertROtoRWvolume: Couldn't find DiskPartition for %s\n", pname);
         return EIO;
     }
+
+    if (VReadVolumeDiskHeader(volumeId, partP, &h)) {
+       Log("1 namei_ConvertROtoRWvolume: Couldn't read header for RO-volume %lu.\n",
+           afs_printable_uint32_lu(volumeId));
+       return EIO;
+    }
+
+    FSYNC_VolOp(volumeId, pname, FSYNC_VOL_BREAKCBKS, 0, NULL);
+
     ino = namei_MakeSpecIno(h.parent, VI_LINKTABLE);
     IH_INIT(ih, partP->device, h.parent, ino);
 
@@ -1793,25 +1786,18 @@ namei_ConvertROtoRWvolume(char *pname, afs_uint32 volumeId)
     h.smallVnodeIndex_hi = h.id;
     h.largeVnodeIndex_hi = h.id;
     h.linkTable_hi = h.id;
-    (void)afs_snprintf(headername, sizeof headername, VFORMAT, afs_printable_uint32_lu(h.id));
-    (void)afs_snprintf(newpath, sizeof newpath, "%s/%s", pname, headername);
-    fd = open(newpath, O_CREAT | O_EXCL | O_RDWR, 0644);
-    if (fd < 0) {
-        Log("1 namei_ConvertROtoRWvolume: Couldn't create header for RW-volume %lu.\n", h.id);
-        return EIO;
-    }
-    if (write(fd, &h, sizeof(h)) != sizeof(h)) {
-        Log("1 namei_ConvertROtoRWvolume: Couldn't write header for RW-volume\
- %lu.\n", h.id);
-        close(fd);
+
+    if (VCreateVolumeDiskHeader(&h, partP)) {
+        Log("1 namei_ConvertROtoRWvolume: Couldn't write header for RW-volume %lu\n",
+           afs_printable_uint32_lu(h.id));
         return EIO;
     }
-    close(fd);
-    (void)afs_snprintf(headername, sizeof headername, VFORMAT, afs_printable_uint32_lu(volumeId));
-    (void)afs_snprintf(oldpath, sizeof oldpath, "%s/%s", pname, headername);
-    if (unlink(oldpath) < 0) {
-        Log("1 namei_ConvertROtoRWvolume: Couldn't unlink RO header, error = %d\n", errno);
+
+    if (VDestroyVolumeDiskHeader(partP, volumeId, h.parent)) {
+        Log("1 namei_ConvertROtoRWvolume: Couldn't unlink header for RO-volume %lu\n",
+           afs_printable_uint32_lu(volumeId));
     }
+
     FSYNC_VolOp(volumeId, pname, FSYNC_VOL_DONE, 0, NULL);
     FSYNC_VolOp(h.id, pname, FSYNC_VOL_ON, 0, NULL);
 #endif
index a5f74cb..a79d64d 100644 (file)
@@ -119,8 +119,9 @@ nuke(char *aname, afs_int32 avolid)
     struct ilist *ti, *ni, *li=NULL;
     register afs_int32 code;
     int i, forceSal;
-    char devName[64], wpath[100];
+    char wpath[100];
     char *lastDevComp;
+    struct DiskPartition64 *dp;
 #ifdef AFS_NAMEI_ENV
 #ifdef AFS_NT40_ENV
     char path[MAX_PATH];
@@ -129,14 +130,20 @@ nuke(char *aname, afs_int32 avolid)
     namei_t ufs_name;
 #endif
 #endif /* AFS_NAMEI_ENV */
+#ifndef AFS_NAMEI_ENV
+    char devName[64]
+#endif /* !AFS_NAMEI_ENV */
     IHandle_t *fileH;
     struct ilist *allInodes = 0;
 
     if (avolid == 0)
        return EINVAL;
     code = afs_stat(aname, &tstat);
-    if (code) {
+    if (code || (dp = VGetPartition(aname, 0)) == NULL) {
        printf("volnuke: partition %s does not exist.\n", aname);
+       if (!code) {
+           code = EINVAL;
+       }
        return code;
     }
     /* get the device name for the partition */
@@ -227,17 +234,7 @@ nuke(char *aname, afs_int32 avolid)
         * system, and is a normal file.  As such, it is not stamped with the
         * volume's ID in its inode, and has to be removed explicitly.
         */
-       /* reuse devName buffer now */
-#ifdef AFS_NT40_ENV
-       afs_snprintf(devName, sizeof devName, "%c:\\%s", *lastDevComp,
-                    VolumeExternalName(avolid));
-#else
-       afs_snprintf(devName, sizeof devName, "%s/%s", aname,
-                    VolumeExternalName(avolid));
-#endif /* AFS_NT40_ENV */
-       code = unlink(devName);
-       if (code)
-           code = errno;
+       code = VDestroyVolumeDiskHeader(dp, avolid, 0);
     } else {
        /* just free things */
        for (ti = allInodes; ti; ti = ni) {
index 3248e46..17029ab 100644 (file)
@@ -60,6 +60,8 @@ static void PurgeHeader(Volume * vp);
 static void PurgeIndex_r(Volume * vp, VnodeClass class);
 static void PurgeHeader_r(Volume * vp);
 
+/*@printflike@*/ extern void Log(const char *format, ...);
+
 /* No lock needed. Only the volserver will call this, and only one transaction
  * can have a given volume (volid/partition pair) in use at a time 
  */
@@ -67,7 +69,11 @@ void
 VPurgeVolume(Error * ec, Volume * vp)
 {
     struct DiskPartition64 *tpartp = vp->partition;
-    char purgePath[MAXPATHLEN];
+    VolumeId volid, parent;
+    afs_int32 code;
+
+    volid = V_id(vp);
+    parent = V_parentId(vp);
 
     /* so VCheckDetach doesn't try to update the volume header and
      * dump spurious errors into the logs */
@@ -77,13 +83,17 @@ VPurgeVolume(Error * ec, Volume * vp)
      * volume header. This routine can, under some circumstances, be called
      * when two volumes with the same id exist on different partitions.
      */
-    (void)afs_snprintf(purgePath, sizeof purgePath, "%s/%s",
-                      VPartitionPath(vp->partition),
-                      VolumeExternalName(V_id(vp)));
     PurgeIndex_r(vp, vLarge);
     PurgeIndex_r(vp, vSmall);
     PurgeHeader_r(vp);
-    unlink(purgePath);
+
+    code = VDestroyVolumeDiskHeader(tpartp, volid, parent);
+    if (code) {
+       Log("VPurgeVolume: Error %ld when destroying volume %lu header\n",
+           afs_printable_int32_ld(code),
+           afs_printable_uint32_lu(volid));
+    }
+
     /*
      * Call the fileserver to break all call backs for that volume
      */
index 5434b00..ff331ff 100644 (file)
@@ -880,7 +880,18 @@ DeleteExtraVolumeHeaderFile(register struct VolumeSummary *vsp)
     if (!Showmode)
        Log("The volume header file %s is not associated with any actual data (%sdeleted)\n", path, (Testing ? "would have been " : ""));
     if (!Testing) {
-       if (unlink(path)) {
+       afs_int32 code;
+       code = VDestroyVolumeDiskHeader(fileSysPartition, vsp->header.id, vsp->header.parent);
+       if (code) {
+           Log("Error %ld destroying volume disk header for volume %lu\n",
+               afs_printable_int32_ld(code),
+               afs_printable_uint32_lu(vsp->header.id));
+       }
+
+       /* make sure we actually delete the fileName file; ENOENT
+        * is fine, since VDestroyVolumeDiskHeader probably already
+        * unlinked it */
+       if (unlink(path) && errno != ENOENT) {
            Log("Unable to unlink %s (errno = %d)\n", path, errno);
        }
     }
@@ -1687,11 +1698,11 @@ SalvageVolumeHeaderFile(register struct InodeSummary *isp,
                        register struct ViceInodeInfo *inodes, int RW,
                        int check, int *deleteMe)
 {
-    int headerFd = 0;
     int i;
     register struct ViceInodeInfo *ip;
     int allinodesobsolete = 1;
     struct VolumeDiskHeader diskHeader;
+    afs_int32 (*writefunc)(VolumeDiskHeader_t *, struct DiskPartition64 *) = NULL;
     int *skip;
 
     /* keeps track of special inodes that are probably 'good'; they are
@@ -1872,11 +1883,11 @@ SalvageVolumeHeaderFile(register struct InodeSummary *isp,
            Log("No header file for volume %u; %screating %s\n",
                isp->volumeId, (Testing ? "it would have been " : ""),
                path);
-       headerFd = afs_open(path, O_RDWR | O_CREAT | O_TRUNC, 0644);
-       assert(headerFd != -1);
        isp->volSummary = (struct VolumeSummary *)
            malloc(sizeof(struct VolumeSummary));
        isp->volSummary->fileName = ToString(headerName);
+
+       writefunc = VCreateVolumeDiskHeader;
     } else {
        char path[64];
        char headerName[64];
@@ -1901,11 +1912,10 @@ SalvageVolumeHeaderFile(register struct InodeSummary *isp,
            if (check)
                return -1;
 
-           headerFd = afs_open(path, O_RDWR | O_TRUNC, 0644);
-           assert(headerFd != -1);
+           writefunc = VWriteVolumeDiskHeader;
        }
     }
-    if (headerFd) {
+    if (writefunc) {
        memcpy(&isp->volSummary->header, &tempHeader,
               sizeof(struct VolumeHeader));
        if (Testing) {
@@ -1913,15 +1923,16 @@ SalvageVolumeHeaderFile(register struct InodeSummary *isp,
                Log("It would have written a new header file for volume %u\n",
                    isp->volumeId);
        } else {
+           afs_int32 code;
            VolumeHeaderToDisk(&diskHeader, &tempHeader);
-           if (write(headerFd, &diskHeader, sizeof(struct VolumeDiskHeader))
-               != sizeof(struct VolumeDiskHeader)) {
-               Log("Couldn't rewrite volume header file!\n");
-               close(headerFd);
+           code = (*writefunc)(&diskHeader, fileSysPartition);
+           if (code) {
+               Log("Error %ld writing volume header file for volume %lu\n",
+                   afs_printable_int32_ld(code),
+                   afs_printable_uint32_lu(diskHeader.id));
                return -1;
            }
        }
-       close(headerFd);
     }
     IH_INIT(isp->volSummary->volumeInfoHandle, fileSysDevice, isp->RWvolumeId,
            isp->volSummary->header.volumeInfo);
@@ -3315,9 +3326,21 @@ MaybeZapVolume(register struct InodeSummary *isp, char *message, int deleteMe,
                    Log("it will be deleted instead.  It should be recloned.\n");
            }
            if (!Testing) {
+               afs_int32 code;
                char path[64];
                sprintf(path, "%s/%s", fileSysPath, isp->volSummary->fileName);
-               if (unlink(path)) {
+
+               code = VDestroyVolumeDiskHeader(fileSysPartition, isp->volumeId, isp->RWvolumeId);
+               if (code) {
+                   Log("Error %ld destroying volume disk header for volume %lu\n",
+                       afs_printable_int32_ld(code),
+                       afs_printable_uint32_lu(isp->volumeId));
+               }
+
+               /* make sure we actually delete the fileName file; ENOENT
+                * is fine, since VDestroyVolumeDiskHeader probably already
+                * unlinked it */
+               if (unlink(path) && errno != ENOENT) {
                    Log("Unable to unlink %s (errno = %d)\n", path, errno);
                }
            }
@@ -3376,27 +3399,15 @@ AskOffline(VolumeId volumeId, char * partition)
      * schedule another salvage while we are salvaging, which would be
      * annoying. */
     if (!Testing) {
-       int fd;
        IHandle_t *h;
-       char name[VMAXPATHLEN];
        struct VolumeHeader header;
        struct VolumeDiskHeader diskHeader;
        struct VolumeDiskData volHeader;
 
-       afs_snprintf(name, sizeof(name), "%s/" VFORMAT, fileSysPathName,
-           afs_printable_uint32_lu(volumeId));
-
-       fd = afs_open(name, O_RDONLY);
-       if (fd < 0) {
-           return;
-       }
-       if (read(fd, &diskHeader, sizeof(diskHeader)) != sizeof(diskHeader) ||
-           diskHeader.stamp.magic != VOLUMEHEADERMAGIC) {
-
-           close(fd);
+       code = VReadVolumeDiskHeader(volumeId, fileSysPartition, &diskHeader);
+       if (code) {
            return;
        }
-       close(fd);
 
        DiskToVolumeHeader(&header, &diskHeader);
 
index d966ad7..94417ef 100644 (file)
@@ -205,9 +205,6 @@ static void LoadVolumeHeader(Error * ec, Volume * vp);
 static int VCheckOffline(register Volume * vp);
 static int VCheckDetach(register Volume * vp);
 static Volume * GetVolume(Error * ec, Error * client_ec, VolId volumeId, Volume * hint, int flags);
-#ifdef AFS_DEMAND_ATTACH_FS
-static int VolumeExternalName_r(VolumeId volumeId, char * name, size_t len);
-#endif
 
 int LogLevel;                  /* Vice loglevel--not defined as extern so that it will be
                                 * defined when not linked with vice, XXXX */
@@ -5138,16 +5135,12 @@ VolumeExternalName(VolumeId volumeId)
  * @see afs_snprintf
  *
  * @note re-entrant equivalent of VolumeExternalName
- *
- * @internal volume package internal use only.
  */
-#ifdef AFS_DEMAND_ATTACH_FS
-static int
+int
 VolumeExternalName_r(VolumeId volumeId, char * name, size_t len)
 {
     return afs_snprintf(name, len, VFORMAT, afs_printable_uint32_lu(volumeId));
 }
-#endif
 
 
 /***************************************************/
index 509e7a3..3981719 100644 (file)
@@ -783,6 +783,7 @@ extern void VFreeBitMapEntry_r(Error * ec, register struct vnodeIndex *index,
                               unsigned bitNumber);
 extern int VolumeNumber(char *name);
 extern char *VolumeExternalName(VolumeId volumeId);
+extern int VolumeExternalName_r(VolumeId volumeId, char *name, size_t len);
 extern Volume *VAttachVolumeByName(Error * ec, char *partition, char *name,
                                   int mode);
 extern Volume *VAttachVolumeByName_r(Error * ec, char *partition, char *name,
@@ -854,6 +855,15 @@ extern void VPurgeVolume(Error * ec, Volume * vp);
 extern afs_int32 VCanScheduleSalvage(void);
 extern afs_int32 VCanUseFSSYNC(void);
 extern afs_int32 VCanUseSALVSYNC(void);
+extern afs_int32 VReadVolumeDiskHeader(VolumeId volid,
+                                      struct DiskPartition64 * dp,
+                                      VolumeDiskHeader_t * hdr);
+extern afs_int32 VWriteVolumeDiskHeader(VolumeDiskHeader_t * hdr,
+                                       struct DiskPartition64 * dp);
+extern afs_int32 VCreateVolumeDiskHeader(VolumeDiskHeader_t * hdr,
+                                        struct DiskPartition64 * dp);
+extern afs_int32 VDestroyVolumeDiskHeader(struct DiskPartition64 * dp,
+                                         VolumeId volid, VolumeId parent);
 
 /* Naive formula relating number of file size to number of 1K blocks in file */
 /* Note:  we charge 1 block for 0 length files so the user can't store
index f508e39..f11d88d 100644 (file)
@@ -114,8 +114,8 @@ VCreateVolume_r(Error * ec, char *partname, VolId volumeId, VolId parentId)
 {                              /* Should be the same as volumeId if there is
                                 * no parent */
     VolumeDiskData vol;
-    int fd, i;
-    char headerName[32], volumePath[64];
+    int i, rc;
+    char headerName[VMAXPATHLEN], volumePath[VMAXPATHLEN];
     Device device;
     struct DiskPartition64 *partition;
     struct VolumeDiskHeader diskHeader;
@@ -123,6 +123,7 @@ VCreateVolume_r(Error * ec, char *partname, VolId volumeId, VolId parentId)
     FdHandle_t *fdP;
     Inode nearInode = 0;
     char *part, *name;
+    struct stat st;
 
     *ec = 0;
     memset(&vol, 0, sizeof(vol));
@@ -156,6 +157,7 @@ VCreateVolume_r(Error * ec, char *partname, VolId volumeId, VolId parentId)
            return NULL;
        }
     }
+    *ec = 0;
     VLockPartition_r(partname);
     memset(&tempHeader, 0, sizeof(tempHeader));
     tempHeader.stamp.magic = VOLUMEHEADERMAGIC;
@@ -168,14 +170,15 @@ VCreateVolume_r(Error * ec, char *partname, VolId volumeId, VolId parentId)
     (void)afs_snprintf(headerName, sizeof headerName, VFORMAT, afs_printable_uint32_lu(vol.id));
     (void)afs_snprintf(volumePath, sizeof volumePath, "%s/%s",
                       VPartitionPath(partition), headerName);
-    fd = afs_open(volumePath, O_CREAT | O_EXCL | O_WRONLY, 0600);
-    if (fd == -1) {
-       if (errno == EEXIST) {
+    rc = stat(volumePath, &st);
+    if (rc == 0 || errno != ENOENT) {
+       if (rc == 0) {
            Log("VCreateVolume: Header file %s already exists!\n",
                volumePath);
            *ec = VVOLEXISTS;
        } else {
-           Log("VCreateVolume: Couldn't create header file %s for volume %u\n", volumePath, vol.id);
+           Log("VCreateVolume: Error %d trying to stat header file %s\n",
+               errno, volumePath);
            *ec = VNOVOL;
        }
        return NULL;
@@ -228,8 +231,10 @@ VCreateVolume_r(Error * ec, char *partname, VolId volumeId, VolId parentId)
            if (handle)
                IH_RELEASE(handle);
            RemoveInodes(device, vol.id);
-           *ec = VNOVOL;
-           close(fd);
+           if (!*ec) {
+               *ec = VNOVOL;
+           }
+           VDestroyVolumeDiskHeader(partition, volumeId, parentId);
            return NULL;
        }
        IH_INIT(handle, device, vol.parentId, *(p->inode));
@@ -262,34 +267,35 @@ VCreateVolume_r(Error * ec, char *partname, VolId volumeId, VolId parentId)
     if (fdP == NULL) {
        Log("VCreateVolume:  Problem iopen inode %s (err=%d)\n",
            PrintInode(NULL, tempHeader.volumeInfo), errno);
-       unlink(volumePath);
        goto bad;
     }
     if (FDH_SEEK(fdP, 0, SEEK_SET) < 0) {
        Log("VCreateVolume:  Problem lseek inode %s (err=%d)\n",
            PrintInode(NULL, tempHeader.volumeInfo), errno);
        FDH_REALLYCLOSE(fdP);
-       unlink(volumePath);
        goto bad;
     }
     if (FDH_WRITE(fdP, (char *)&vol, sizeof(vol)) != sizeof(vol)) {
        Log("VCreateVolume:  Problem writing to  inode %s (err=%d)\n",
            PrintInode(NULL, tempHeader.volumeInfo), errno);
        FDH_REALLYCLOSE(fdP);
-       unlink(volumePath);
        goto bad;
     }
     FDH_CLOSE(fdP);
     IH_RELEASE(handle);
 
     VolumeHeaderToDisk(&diskHeader, &tempHeader);
-    if (write(fd, &diskHeader, sizeof(diskHeader)) != sizeof(diskHeader)) {
-       Log("VCreateVolume: Unable to write volume header %s; volume %u not created\n", volumePath, vol.id);
-       unlink(volumePath);
+    rc = VCreateVolumeDiskHeader(&diskHeader, partition);
+    if (rc) {
+       Log("VCreateVolume: Error %d trying to write volume header for "
+           "volume %u on partition %s; volume not created\n", rc,
+           vol.id, VPartitionPath(partition));
+       if (rc == EEXIST) {
+           *ec = VVOLEXISTS;
+       }
        goto bad;
     }
-    fsync(fd);
-    close(fd);
+
     return (VAttachVolumeByName_r(ec, partname, headerName, V_SECRETLY));
 }
 
@@ -366,3 +372,187 @@ ClearVolumeStats_r(register VolumeDiskData * vol)
     vol->dayUse = 0;
     vol->dayUseDate = 0;
 }
+
+/**
+ * read an existing volume disk header.
+ *
+ * @param[in]  volid  volume id
+ * @param[in]  dp     disk partition object
+ * @param[out] hdr    volume disk header
+ *
+ * @return operation status
+ *    @retval 0 success
+ *    @retval -1 volume header doesn't exist
+ *    @retval EIO failed to read volume header
+ *
+ * @internal
+ */
+afs_int32
+VReadVolumeDiskHeader(VolumeId volid,
+                     struct DiskPartition64 * dp,
+                     VolumeDiskHeader_t * hdr)
+{
+    afs_int32 code = 0;
+    int fd;
+    char path[MAXPATHLEN];
+
+    (void)afs_snprintf(path, sizeof(path),
+                      "%s/" VFORMAT,
+                      VPartitionPath(dp), afs_printable_uint32_lu(volid));
+    fd = open(path, O_RDONLY);
+    if (fd < 0) {
+       Log("VReadVolumeDiskHeader: Couldn't open header for volume %lu.\n",
+           afs_printable_uint32_lu(volid));
+       code = -1;
+    } else if (read(fd, hdr, sizeof(*hdr)) != sizeof(*hdr)) {
+       Log("VReadVolumeDiskHeader: Couldn't read header for volume %lu.\n",
+           afs_printable_uint32_lu(volid));
+       code = EIO;
+    }
+
+    if (fd >= 0) {
+       close(fd);
+    }
+    return code;
+}
+
+/**
+ * write an existing volume disk header.
+ *
+ * @param[in] hdr   volume disk header
+ * @param[in] dp    disk partition object
+ * @param[in] cr    assert if O_CREAT | O_EXCL should be passed to open()
+ *
+ * @return operation status
+ *    @retval 0 success
+ *    @retval -1 volume header doesn't exist
+ *    @retval EIO failed to write volume header
+ *
+ * @internal
+ */
+static afs_int32
+_VWriteVolumeDiskHeader(VolumeDiskHeader_t * hdr,
+                       struct DiskPartition64 * dp,
+                       int flags)
+{
+    afs_int32 code = 0;
+    int fd;
+    char path[MAXPATHLEN];
+
+    flags |= O_RDWR;
+
+    (void)afs_snprintf(path, sizeof(path),
+                      "%s/" VFORMAT,
+                      VPartitionPath(dp), afs_printable_uint32_lu(hdr->id));
+    fd = open(path, flags, 0644);
+    if (fd < 0) {
+       code = errno;
+       Log("_VWriteVolumeDiskHeader: Couldn't open header for volume %lu, "
+           "error = %d\n", afs_printable_uint32_lu(hdr->id), errno);
+    } else if (write(fd, hdr, sizeof(*hdr)) != sizeof(*hdr)) {
+       Log("_VWriteVolumeDiskHeader: Couldn't write header for volume %lu, "
+           "error = %d\n", afs_printable_uint32_lu(hdr->id), errno);
+       code = EIO;
+    }
+
+    if (fd >= 0) {
+       if (close(fd) != 0) {
+           Log("_VWriteVolumeDiskHeader: Error closing header for volume "
+               "%lu, errno %d\n", afs_printable_uint32_lu(hdr->id), errno);
+       }
+    }
+
+    return code;
+}
+
+/**
+ * write an existing volume disk header.
+ *
+ * @param[in] hdr   volume disk header
+ * @param[in] dp    disk partition object
+ *
+ * @return operation status
+ *    @retval 0 success
+ *    @retval ENOENT volume header doesn't exist
+ *    @retval EIO failed to write volume header
+ */
+afs_int32
+VWriteVolumeDiskHeader(VolumeDiskHeader_t * hdr,
+                      struct DiskPartition64 * dp)
+{
+    afs_int32 code;
+
+    code = _VWriteVolumeDiskHeader(hdr, dp, 0);
+    if (code) {
+       goto done;
+    }
+
+ done:
+    return code;
+}
+
+/**
+ * create and write a volume disk header to disk.
+ *
+ * @param[in] hdr   volume disk header
+ * @param[in] dp    disk partition object
+ *
+ * @return operation status
+ *    @retval 0 success
+ *    @retval EEXIST volume header already exists
+ *    @retval EIO failed to write volume header
+ *
+ * @internal
+ */
+afs_int32
+VCreateVolumeDiskHeader(VolumeDiskHeader_t * hdr,
+                       struct DiskPartition64 * dp)
+{
+    afs_int32 code = 0;
+
+    code = _VWriteVolumeDiskHeader(hdr, dp, O_CREAT | O_EXCL);
+    if (code) {
+       goto done;
+    }
+
+ done:
+    return code;
+}
+
+
+/**
+ * destroy a volume disk header.
+ *
+ * @param[in] dp      disk partition object
+ * @param[in] volid   volume id
+ * @param[in] parent  parent's volume id, 0 if unknown
+ *
+ * @return operation status
+ *    @retval 0 success
+ *
+ * @note if parent is 0, the parent volume ID will be looked up from the
+ * fileserver
+ *
+ * @note for non-DAFS, parent is currently ignored
+ */
+afs_int32
+VDestroyVolumeDiskHeader(struct DiskPartition64 * dp,
+                        VolumeId volid,
+                        VolumeId parent)
+{
+    afs_int32 code = 0;
+    char path[MAXPATHLEN];
+
+    (void)afs_snprintf(path, sizeof(path),
+                       "%s/" VFORMAT,
+                       VPartitionPath(dp), afs_printable_uint32_lu(volid));
+    code = unlink(path);
+    if (code) {
+       Log("VDestroyVolumeDiskHeader: Couldn't unlink disk header, error = %d\n", errno);
+       goto done;
+    }
+
+ done:
+    return code;
+}
+