From da0a19786e6a3866055b051a4b86785e78fa5b36 Mon Sep 17 00:00:00 2001 From: surface-camera-build Date: Tue, 22 Sep 2026 02:20:31 -0400 Subject: [PATCH] =?UTF-8?q?cube(2):=20one=20frame=20for=20every=20walk=20?= =?UTF-8?q?=E2=80=94=20the=20class=20mask=20is=20in=20it?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CUBE_OP_ENUM and CUBE_OP_RANGE returned `key | value_len | value`, so a caller could walk a store and not learn what any record it walked past *was*. A store could not answer `entries_flagged` from a kernel store at all, and the only way to find out was to open the device and read the format directly — which is the second reader of the format this interface exists to make unnecessary. Both walks now return the flag scan's frame: `key(24) | flags(2) | value_len | value`, with the space ahead of it when the scope is every space. One frame, one packer, one `Batch::offer`; the separate `offer_flagged` and `pack_record` are gone, and so is the reason for them to disagree. The class comes from wherever the value did: an addressed image's index entry (the binary search already read it and used to throw it away), a log entry's mask, or zero for a packed v1/v2 record, which has no field to carry one. The merge in `SpaceWalker` hands it back alongside the key and the value for the same reason — a record the log supplied carries the class its writer stamped, and dropping it there is what left a listing unable to say what it was listing. --- drivers/cube/cubelinux_store.rs | 121 +++++++++++++++----------------- include/uapi/linux/cube.h | 21 ++++-- 2 files changed, 70 insertions(+), 72 deletions(-) diff --git a/drivers/cube/cubelinux_store.rs b/drivers/cube/cubelinux_store.rs index 0678b50a4..dfe224192 100644 --- a/drivers/cube/cubelinux_store.rs +++ b/drivers/cube/cubelinux_store.rs @@ -2330,7 +2330,13 @@ impl<'a> SpaceWalker<'a> { } /// The next live record, or nothing when the space is walked out. - fn next(&mut self) -> Option<(&'a [u8; RAW_KEY_LEN], &'a [u8])> { + /// The next live record: `(key, its class, its value)`. + /// + /// The class comes back because a listing has to be able to say what a record *is*. A packed + /// image's records carry no mask field, so they read as "no class" — but a record the log + /// supplied carries the mask its writer stamped, and dropping it here is what left a + /// kernel-backed store unable to answer `entries_flagged`. + fn next(&mut self) -> Option<(&'a [u8; RAW_KEY_LEN], u16, &'a [u8])> { loop { // The entries of one key arrive in log order, so the last of a key's group is the // newest word on it and the earlier ones are history. @@ -2346,7 +2352,7 @@ impl<'a> SpaceWalker<'a> { self.edit_at += 1; self.image_next = next_in_space(&mut self.records, &self.wanted); if !e.deleted { - return Some((e.key, e.value)); + return Some((e.key, e.flags, e.value)); } } // A record only the log holds, in its key's place. @@ -2354,19 +2360,20 @@ impl<'a> SpaceWalker<'a> { self.edit_at += 1; self.image_next = Some((key, value)); if !e.deleted { - return Some((e.key, e.value)); + return Some((e.key, e.flags, e.value)); } } - // The image's own record, which the log says nothing about. + // The image's own record, which the log says nothing about. A packed image has no + // mask field, so it answers none. (_, Some((key, value))) => { self.image_next = next_in_space(&mut self.records, &self.wanted); - return Some((key, value)); + return Some((key, 0, value)); } // The image is walked out; the rest of the log still has records to report. (Some(e), None) => { self.edit_at += 1; if !e.deleted { - return Some((e.key, e.value)); + return Some((e.key, e.flags, e.value)); } } (None, None) => return None, @@ -2493,43 +2500,22 @@ struct Batch<'a> { with_space: bool, } -/// The fixed part of a flag-scan frame, before the value. +/// The fixed part of a walk's frame, before the value. fn flagged_frame(with_space: bool) -> usize { RAW_KEY_LEN + FLAGS_LEN + 4 + if with_space { SPACE_ID_LEN } else { 0 } } impl Batch<'_> { - /// Offer one record. `false` means nothing more will fit in this batch. - fn offer(&mut self, key: &[u8], value: &[u8]) -> bool { - self.seen += 1; - if self.seen <= self.cursor { - return true; - } - match pack_record(key, value, self.out, self.written) { - Some(n) => { - self.written += n; - self.returned += 1; - true - } - None => { - self.too_big = RAW_KEY_LEN + 4 + value.len(); - false - } - } - } - - /// Offer one *flagged* record. The counting rule is `offer`'s, and it is what makes the cursor - /// mean "matches already returned" rather than an index position: only a record that passed the - /// mask is counted, so a filtered walk resumes where it left off. + /// Offer one record, class and all. `false` means nothing more will fit in this batch. + /// + /// One frame for every walk. It used to be two — `key | len | value` for the plain walks and + /// `key | flags | len | value` for the scan — which meant a caller could not ask a listing what + /// class its records were, and a *store* could not answer `entries_flagged` from a kernel store + /// at all. A walk that cannot say what a record is forces a second reader of the format to find + /// out, and that is the thing this whole arrangement exists to avoid. /// /// `space` is carried only when `with_space` is set — see [`Batch::with_space`]. - fn offer_flagged( - &mut self, - space: &[u8; SPACE_ID_LEN], - key: &[u8], - flags: u16, - value: &[u8], - ) -> bool { + fn offer(&mut self, space: &[u8; SPACE_ID_LEN], key: &[u8], flags: u16, value: &[u8]) -> bool { self.seen += 1; if self.seen <= self.cursor { return true; @@ -3020,8 +3006,8 @@ unsafe fn v3_enum( let mut entry = KVVec::::new(); let mut at = first + cursor; while at < first + records { - let (value_off, value_len, _) = match image.index_entry(at, &mut entry) { - Ok(pair) => pair, + let (value_off, value_len, flags) = match image.index_entry(at, &mut entry) { + Ok(triple) => triple, Err(e) => return -(e.to_errno() as i32), }; let value = match image.value_at(value_off, value_len) { @@ -3032,7 +3018,7 @@ unsafe fn v3_enum( Ok(k) => k, Err(_) => return -22, }; - if !batch.offer(key, value.as_slice()) { + if !batch.offer(&wanted, key, flags, value.as_slice()) { break; } at += 1; @@ -3061,38 +3047,32 @@ unsafe fn v3_enum( None => None, }; match (edit, image_entry, image_key) { - (Some(e), Some((_, value_len, _)), Some(key)) if e.key == key => { - let _ = value_len; + (Some(e), Some((_, _, _)), Some(key)) if e.key == key => { edit_at += 1; at += 1; - if !e.deleted && !batch.offer(e.key, e.value) { + if !e.deleted && !batch.offer(&wanted, e.key, e.flags, e.value) { break; } } - (Some(e), Some((value_off, value_len, _)), Some(key)) if e.key < key => { + (Some(e), Some((_, _, _)), Some(key)) if e.key < key => { edit_at += 1; - if !e.deleted && !batch.offer(e.key, e.value) { + if !e.deleted && !batch.offer(&wanted, e.key, e.flags, e.value) { break; } - let _ = (value_off, value_len); } - (_, Some((value_off, value_len, _)), Some(_)) => { + (_, Some((value_off, value_len, flags)), Some(key)) => { let value = match image.value_at(value_off, value_len) { Ok(v) => v, Err(e) => return -(e.to_errno() as i32), }; - let key = match entry.as_slice()[..RAW_KEY_LEN].try_into() { - Ok(k) => k, - Err(_) => return -22, - }; at += 1; - if !batch.offer(key, value.as_slice()) { + if !batch.offer(&wanted, key, flags, value.as_slice()) { break; } } (Some(e), None, _) => { edit_at += 1; - if !e.deleted && !batch.offer(e.key, e.value) { + if !e.deleted && !batch.offer(&wanted, e.key, e.flags, e.value) { break; } } @@ -3235,31 +3215,40 @@ unsafe fn v3_range( (Some(e), Some(_), Some(key)) if e.key == key => { edit_at += 1; at += 1; - if !e.deleted && region.holds(e.key) && !batch.offer(e.key, e.value) { + if !e.deleted + && region.holds(e.key) + && !batch.offer(&wanted, e.key, e.flags, e.value) + { break; } } // A record only the log holds, in its key's place. (Some(e), Some(_), Some(key)) if e.key < key => { edit_at += 1; - if !e.deleted && region.holds(e.key) && !batch.offer(e.key, e.value) { + if !e.deleted + && region.holds(e.key) + && !batch.offer(&wanted, e.key, e.flags, e.value) + { break; } } // The image's own record, which the log says nothing about. - (_, Some((value_off, value_len, _)), Some(key)) => { + (_, Some((value_off, value_len, flags)), Some(key)) => { let value = match image.value_at(value_off, value_len) { Ok(v) => v, Err(e) => return -(e.to_errno() as i32), }; at += 1; - if region.holds(key) && !batch.offer(key, value.as_slice()) { + if region.holds(key) && !batch.offer(&wanted, key, flags, value.as_slice()) { break; } } (Some(e), None, _) => { edit_at += 1; - if !e.deleted && region.holds(e.key) && !batch.offer(e.key, e.value) { + if !e.deleted + && region.holds(e.key) + && !batch.offer(&wanted, e.key, e.flags, e.value) + { break; } } @@ -3390,7 +3379,7 @@ fn flag_scan_space( at += 1; if !e.deleted && flag_matches(e.flags, mask, mode) - && !batch.offer_flagged(wanted, e.key, e.flags, e.value) + && !batch.offer(wanted, e.key, e.flags, e.value) { return Ok(false); } @@ -3400,7 +3389,7 @@ fn flag_scan_space( edit_at += 1; if !e.deleted && flag_matches(e.flags, mask, mode) - && !batch.offer_flagged(wanted, e.key, e.flags, e.value) + && !batch.offer(wanted, e.key, e.flags, e.value) { return Ok(false); } @@ -3410,7 +3399,7 @@ fn flag_scan_space( let value = image.value_at(value_off, value_len)?; at += 1; if flag_matches(flags, mask, mode) - && !batch.offer_flagged(wanted, key, flags, value.as_slice()) + && !batch.offer(wanted, key, flags, value.as_slice()) { return Ok(false); } @@ -3419,7 +3408,7 @@ fn flag_scan_space( edit_at += 1; if !e.deleted && flag_matches(e.flags, mask, mode) - && !batch.offer_flagged(wanted, e.key, e.flags, e.value) + && !batch.offer(wanted, e.key, e.flags, e.value) { return Ok(false); } @@ -3459,7 +3448,7 @@ fn flag_scan_log_space( edit_at += 1; if !e.deleted && flag_matches(e.flags, mask, mode) - && !batch.offer_flagged(space, e.key, e.flags, e.value) + && !batch.offer(space, e.key, e.flags, e.value) { return Ok(false); } @@ -3805,8 +3794,8 @@ pub unsafe extern "C" fn cubelinux_kernel_enum( too_big: 0, with_space: false, }; - while let Some((key, value)) = walker.next() { - if !batch.offer(key, value) { + while let Some((key, flags, value)) = walker.next() { + if !batch.offer(&wanted, key, flags, value) { break; } } @@ -3908,7 +3897,7 @@ pub unsafe extern "C" fn cubelinux_kernel_range( too_big: 0, with_space: false, }; - while let Some((key, value)) = walker.next() { + while let Some((key, flags, value)) = walker.next() { match region.place(key) { // Below the span: not in the box, and skipping it is the seek's substitute on a layout // that has no index to search. @@ -3917,7 +3906,7 @@ pub unsafe extern "C" fn cubelinux_kernel_range( Span::Above => break, Span::Inside => {} } - if region.holds(key) && !batch.offer(key, value) { + if region.holds(key) && !batch.offer(&wanted, key, flags, value) { break; } } diff --git a/include/uapi/linux/cube.h b/include/uapi/linux/cube.h index f2a14a25b..4cab24dc8 100644 --- a/include/uapi/linux/cube.h +++ b/include/uapi/linux/cube.h @@ -66,10 +66,15 @@ struct cube_args { * The walk's argument block: its own block rather than a wider `cube_args`, because it needs a * cursor and a buffer, and the coordinate would otherwise be both an input and an output. * - * Records are packed as `key(24) | value_len(u32, little-endian) | value`, in the store's own - * order — space first, then key — which is the order a checkpoint writes them and the order the - * userspace store returns them, so a kernel listing and a userspace listing can be compared - * directly. The space is not repeated per record: the caller named it. + * Records are packed as `key(24) | flags(2, little-endian) | value_len(u32, little-endian) | + * value`, in the store's own order — space first, then key — which is the order a checkpoint writes + * them and the order the userspace store returns them, so a kernel listing and a userspace listing + * can be compared directly. The space is not repeated per record: the caller named it. + * + * The class mask is in the frame because a listing has to be able to say what a record *is*, and a + * caller that cannot see it has to open the store itself to find out — which is the second reader + * of the format this interface exists to make unnecessary. A record whose layout carries no mask (a + * packed v1/v2 image) reads as zero: "no class", the same answer every other reader gives. * * A walk ends when the cursor stops moving, and that is the only end signal: a batch holds as * many whole records as fit, so most batches come back short, and reading a short batch as the @@ -98,8 +103,9 @@ struct cube_enum_args { * replaced. * * A region is a box, inclusive on both corners. Records come back packed exactly as CUBE_OP_ENUM - * packs them — `key(24) | value_len(u32, little-endian) | value`, in the store's own order — so a - * kernel region answer and a userspace one can be compared byte for byte. + * packs them — `key(24) | flags(2, little-endian) | value_len(u32, little-endian) | value`, in the + * store's own order — so a kernel region answer and a userspace one can be compared byte for + * byte. * * The cursor is a COUNT OF RECORDS ALREADY RETURNED, and it is the end-of-walk signal for the same * reason as the walk's: a batch holds as many whole records as fit, so most batches come back @@ -107,6 +113,9 @@ struct cube_enum_args { * counts — a walk counts the records of the space, a region walk counts the records *in the box*, * because those are the records it returns. * + * Records come back in the same frame the walk uses, class mask included: + * `key(24) | flags(2) | value_len(u32) | value`. + * * Implementation, because it is what makes this cheap: **the kernel seeks.** The box's two corner * keys bound every key inside it (`cube_format::key_span`), so a v3 image's sorted index is * binary-searched for the foot of that span and read forward to its head. The span is a BOUND, not