Skip to content

Commit

Permalink
Fix zettacache compatibility checks for old releases (openzfs#492)
Browse files Browse the repository at this point in the history
Earlier this month the following commit was merged:
```
commit 75c74ba
Author: Serapheim Dimitropoulos <[email protected]>
Date:   Mon Jun 13 14:00:24 2022 -0700

    DLPX-81000 Generate Disk GUID for ZettaCache Devices (openzfs#434)

    Side-Change: Also record sector_size on-disk
```
In that commit, I renamed the `SuperblockPhys::guid` field to
`cache_guid` and even though upgrades to the newer on-disk format happen
without blowing up the cache's contents, I missed trying to open the new
on-disk format with older bits.

On that latter scenario we would ideally want to do a feature check with
the old bits and proceed to take further actions depending on the
feature flags introduced in the new on-disk format.  Unfortunately this
doesn't happen and instead we blow up because the old bits cannot find
the `guid` field in the superblock because it has been renamed.

This commit changes the name of that field back to its old version
while keeping the name of the in-memory struct the same. This way
we still keep the more readable name in the code while ensuring
on-disk format compatibility.

Side-Changes:
* Do feature checks in zcachedb's open() path.
* Fix `--clear-incompatible-cache` that somehow over time stopped
  working
  • Loading branch information
sdimitro authored Jun 23, 2022
1 parent 930f2fb commit aa55e0d
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 4 deletions.
4 changes: 2 additions & 2 deletions cmd/zfs_object_agent/zettacache/src/superblock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub const SUPERBLOCK_SIZE: u64 = util::message::SUPERBLOCK_SIZE as u64;
pub struct SuperblockPhys {
pub primary: Option<PrimaryPhys>,
pub disk: DiskId,
#[serde(alias = "guid")]
#[serde(rename = "guid")]
pub cache_guid: CacheGuid,
#[serde(default)]
pub disk_guid: Option<DiskGuid>,
Expand All @@ -36,7 +36,7 @@ pub struct SuperblockPhys {
struct SuperblockFeaturesPhys {
primary: Option<PrimaryFeaturesPhys>,
disk: DiskId,
#[serde(alias = "guid")]
#[serde(rename = "guid")]
cache_guid: CacheGuid,
}

Expand Down
1 change: 1 addition & 0 deletions cmd/zfs_object_agent/zettacache/src/zettacache/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,7 @@ impl ZettaCache {
let inner = match Inner::open(mode).await {
Ok(inner) => Some(inner),
Err(CacheOpenError::NoDevices) => None,
Err(e @ CacheOpenError::IncompatibleFeatures(_, _)) => return Err(e),
Err(e) => {
error!("could not open zettacache: {e}");
None
Expand Down
10 changes: 8 additions & 2 deletions cmd/zfs_object_agent/zettacache/src/zettacache/zcdb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@ use crate::block_access::BlockAccess;
use crate::block_access::Disk;
use crate::block_allocator::zcdb::zcachedb_dump_slabs;
use crate::block_allocator::zcdb::zcachedb_dump_spacemaps;
use crate::features::check_features;
use crate::slab_allocator::SlabAllocatorBuilder;
use crate::superblock::PrimaryPhys;
use crate::superblock::SuperblockPhys;
use crate::superblock::SUPERBLOCK_SIZE;
use crate::CacheOpenError;
use crate::DumpSlabsOptions;
use crate::DumpStructuresOptions;

Expand Down Expand Up @@ -49,11 +51,15 @@ impl ZCacheDBHandle {

pub async fn open(paths: Vec<PathBuf>) -> Result<ZCacheDBHandle> {
let mut disks: Vec<Disk> = Vec::with_capacity(paths.len());
for path in paths {
disks.push(Disk::new(&path, true)?);
for path in &paths {
disks.push(Disk::new(path, true)?);
}
let block_access = Arc::new(BlockAccess::new(disks, true));

let feature_flags = PrimaryPhys::read_features(&block_access).await?;
check_features(&feature_flags)
.map_err(|e| CacheOpenError::IncompatibleFeatures(paths, e))?;

let (primary, primary_disk, guid, _extra_disks) = PrimaryPhys::read(&block_access).await?;
let checkpoint = Arc::new(CheckpointPhys::read(&block_access, &primary.checkpoint).await?);

Expand Down

0 comments on commit aa55e0d

Please sign in to comment.