Repository navigation
[WIP] Ceph improvements - #147
Draft
weizhouapache wants to merge 61 commits into
Draft
weizhouapache wants to merge 61 commits into
weizhouapache wants to merge 61 commits into
Conversation
LibvirtStorageAdaptor#deletePhysicalDisk manually connected to Ceph via librados/librbd to unprotect and remove every snapshot of an RBD image before asking libvirt to delete the volume. libvirt's RBD storage backend has supported VIR_STORAGE_VOL_DELETE_WITH_SNAPSHOTS since 1.2.20, which does the same unprotect/remove internally. Pass that flag instead and drop the manual cleanup.
…pache#13835) * kvm: fix RBD exclusive-lock leak that breaks revertSnapshot on Ceph takeRbdVolumeSnapshotOfStoppedVm() called image.snapCreate(snapshotName) twice. The first call creates the RBD snapshot, the second one always throws RbdException ("Failed to create snapshot <uuid>") because the snapshot already exists. The duplicate is a merge artifact: 30d3066 ("Merge branch '4.20' into 4.22") resolved a conflict by keeping the call from both sides - each parent had exactly one. Because there was no finally block, that exception skipped rbd.close(image) and r.ioCtxDestroy(io), so the agent kept the image open and held its RBD exclusive-lock indefinitely. The exception is only logged, so the snapshot job still reported success and the fault stayed invisible. Consequences observed on a KVM + Ceph/RBD cluster: - revertSnapshot fails with "com.ceph.rbd.RbdException: Failed to rollback snapshot <uuid>". librbd returns EROFS because a live peer holds the exclusive-lock; 'rbd snap rollback' only succeeds once that client dies and librbd can break the lock, which makes the failure look intermittent. - getRbdSnapshotSize() is never reached, so every snapshot is reported with physical size 0 when snapshot.backup.to.secondary is false. - The leaked watchers keep the image busy, so 'rbd rm' fails and the volume cannot be expunged - it stays stuck in state Destroy. Note the method also runs for RUNNING VMs: createSnapshot() branches on "RUNNING && !primaryPool.isExternalSnapshot()", and RBD is an external-snapshot pool, so every RBD volume snapshot took this path. Remove the duplicated call and move the image/IO-context cleanup into a finally block so the lock is released even if the snapshot itself fails. * kvm: release RBD handles on every path when cloning a volume from a snapshot createRBDvolumeFromRBDSnapshot() closed the source image, the cloned image and the RADOS IO context only on the success path, and called snapUnprotect() only there too. Two paths escaped that cleanup: - the early "Could not find snapshot ... on RBD" return, and - any RadosException/RbdException from clone(), resize() or flatten(), which is caught and turned into a null disk. Both leave the images open, so this client keeps the RBD exclusive-lock. That later makes 'rbd snap rollback' (revertSnapshot) fail with EROFS from another host, and keeps the image busy so 'rbd rm' cannot remove it - the volume then stays stuck in state Destroy. The failure paths after snapProtect() are worse: the snapshot stays protected, and a protected snapshot can be deleted neither on its own nor together with its volume. Move the cleanup into a finally block, tracking whether the snapshot was actually protected so it is unprotected exactly when it needs to be. Failures during cleanup are logged and never mask the original outcome; a failed snapUnprotect is logged at ERROR since it needs manual intervention. This is the same class of defect as the leak fixed in takeRbdVolumeSnapshotOfStoppedVm(); no behaviour changes on the success path. * kvm: add regression tests for the RBD snapshot handle leak Two tests around takeRbdVolumeSnapshotOfStoppedVm, using the MockedConstruction pattern already used in this test class (the Rbd instance is created inside the method under test, so it cannot be injected): - createsSnapshotExactlyOnce guards the duplicated snapCreate call from coming back, and checks the image and IO context are released. - releasesHandlesWhenSnapshotFails makes snapCreate throw and asserts the image is still closed and the IO context destroyed, so a future failure cannot leak the RBD exclusive-lock again. takeRbdVolumeSnapshotOfStoppedVm, radosConnect and getRbdSnapshotSize widened from private to protected so the test can stub the Ceph interactions. * kvm: extract RBD handle teardown into null-safe helper methods Address review feedback: the nested try-catch teardown blocks in the finally clauses of takeRbdVolumeSnapshotOfStoppedVm and createRBDvolumeFromRBDSnapshot are extracted into two reusable, null-safe helpers that log but never throw: - closeRbdImage (3 call sites) - destroyRadosIoCtx (2 call sites) No behavior change. * kvm: extract RBD snapshot unprotect into a helper method Follow-up to the teardown helpers: the snapUnprotect block in the finally clause of createRBDvolumeFromRBDSnapshot moves into a never-throwing unprotectRbdSnapshot helper, so the finally clause is now free of inline try-catch constructions entirely. No behavior change. --------- Co-authored-by: calvix <7136358+calvix@users.noreply.github.com>
Member
|
[SF] Trillian test result (tid-60)
|
Member
|
[SF] Trillian test result (tid-61)
|
Member
Author
|
@blueorangutan package |
Member
|
@weizhouapache a [SL] Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
Member
|
[SF] Trillian test result (tid-67)
|
Member
Author
|
@blueorangutan package |
Member
Author
the test failures (see below) with hostha have been fixed by kvm: support Host HA on Ceph RBD primary storage 
|
Member
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19427 |
Member
|
[SF] Trillian test result (tid-71)
|
Member
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19454 |
Member
|
[SF] Trillian test result (tid-78)
|
Member
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19486 |
Member
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19497 |
Member
|
[SF] Trillian test result (tid-88)
|
Member
|
[SF] Trillian test result (tid-89)
|
Restoring a volume from a backup and attaching it to a VM has been broken since the restore commands were changed to run without a shell, in three independent ways. getDeviceToAttachDisk pipes virsh domblklist through awk, but passes the awk program still wrapped in the single quotes a shell would have stripped. Run directly, awk fails with "invalid char" and returns nothing, so the device name is empty and charAt throws StringIndexOutOfBoundsException before any attach is attempted. This affects every storage type. The exit value was also never checked, and the output not trimmed, so even a working awk would leave the trailing line separator and increment that instead of the device letter. The RBD branch passes the literal string "<<EOF%sEOF" as a virsh argument. The placeholder is never substituted with the disk XML, and a here-document cannot work without a shell, so virsh is handed a bogus argument and fails. The XML is now written to a temporary file that virsh reads. The Linstor branch declares "--subdriver qcow2", inverting the previous behaviour where Linstor got a raw attach and every other pool got qcow2. A Linstor volume is a raw DRBD block device, so libvirt rejects it with "Image is not in qcow2 format". The condition is restored, along with the "--driver qemu" that was dropped.
…BD (apache#13991) Ceph Tentacle 20.2.4 removed the legacy `auth_supported` librados option. CloudStack builds RBD connection strings with `auth_supported=cephx`/`none`, so every RBD operation through the KVM agent fails with "failed to set RADOS option: auth_supported". Switch the RBD string builder to `auth_client_required`, the modern option used by the qemu rbd driver and accepted by current Ceph releases. Co-authored-by: Wei Zhou <weizhou@apache.org>
… on RBD (apache#13361) RBD erasure-coded pool support (apache#9808) added handling of the rbd_default_data_pool storage-pool detail to RBDStringBuilder (qemu-img path) and to createPhysicalDisk (blank volumes), but not to createDiskFromTemplateOnRBD. As a result, ROOT volumes created from a template via rados-java rbd.clone()/rbd.create() are created without a data pool: all of their data objects land in the (replicated) metadata pool instead of the erasure-coded data pool, defeating the point of EC and consuming ~3x raw space. Set rbd_default_data_pool on the Rados connection (before connect) in both the same-cluster clone/copy branch and the cross-cluster copy branch of createDiskFromTemplateOnRBD, using the destination pool's detail. librbd then honors it as the default data pool when the new image is created, so template-derived volumes get data_pool set, the same way blank volumes already do.
…storeBackupCommandWrapper (apache#14006) * kvm: detect mount failures and honour the configured timeouts on backup restore Script.executeCommand returns null when the command fails, it does not throw, so the try/catch around the mount and umount of the backup repository could never fire and the return value was discarded. A repository that fails to mount was therefore treated as mounted, and the restore carried on against an empty directory until it failed later with a misleading "backup file not found". A failed umount was ignored the same way, leaking the mount. Both now go through executeCommandForExitValue and check the exit value. The same refactor also dropped the timeouts. mountTimeout was still passed into mountBackupDirectory but never used, and the rsync of the volume lost the command timeout, so both fell back to the one hour default in Script instead of the configured values. An unresponsive repository could hold a restore up for an hour rather than failing after nas.backup.restore.mount.timeout seconds. * kvm: clean up after a failed mount and bound the unmount of a backup repository The directory created for the mount is removed by the caller in a finally block, but that block is only reached once the mount has succeeded, so a repository that cannot be mounted left an empty directory behind on every attempt. It is now removed before the failure is reported, best effort and logged if it cannot be. The unmount ran without a timeout and so fell back to the one hour default in Script. Unmounting a repository that has become unreachable blocks as easily as mounting one, and this runs in the cleanup path of a restore, so it is now bounded by the configured mount timeout like the mount itself.
* storage: enable RBD/Ceph volume encryption support (shared base) Flip StoragePoolType.RBD from EncryptionSupport.Unsupported to Hypervisor so the existing encryption control plane (allocator, endpoint selector, offerings) treats RBD pools as encryption-capable. The agent-side encrypted RBD create path is not implemented yet; it is delivered by two follow-up tracks (qemu-native engine='qemu' and ceph-native engine='librbd'). Until then, fail closed at the two RBD create chokepoints in LibvirtStorageAdaptor (createPhysicalDisk and createDiskFromTemplate) when a passphrase is present, so we never silently produce a plaintext volume that the control plane believes is encrypted. No change for existing unencrypted RBD volumes (guards only fire when a passphrase is set; supportsEncryption() only affects volumes that require encryption). * kvm: Ceph-native LUKS2 encryption for RBD volumes (engine='librbd') Implements encrypted RBD data and root disks using librbd's native LUKS2 encryption, decrypted at runtime by libvirt/qemu via <encryption engine='librbd'>. CloudStack manages the passphrase (existing model). - RbdEncryption: isolated helper wrapping `rbd encryption format luks2`, cephx via --id + keyfile (secret not on the command line), LUKS passphrase via KeyFile. Kept separate so the CLI can later be swapped for a JNA binding (rados-java has no rbd_encryption_format API). - LibvirtStorageAdaptor: create/clone the raw RBD image, then apply `rbd encryption format luks2`; mark the disk LUKS2 so encrypt_format propagates to the volume. Replaces the fail-closed guards. - QemuObject.EncryptFormat: add LUKS2. - LibvirtVMDef: render <encryption format='luks2' engine='librbd'>; the encrypt details now carry an optional engine. - attach (KVMStorageProcessor) and boot (LibvirtComputingResource): set engine='librbd' for RBD-backed encrypted volumes. NOTE: the CoW-clone-then-format path (encrypted root from an unencrypted template) needs live-cluster validation for the parent-grow / usable-size behaviour described in the Ceph image-encryption docs. Builds: api + plugins/hypervisors/kvm (JDK11). * kvm: gate host encryption probe on librbd support for RBD hostSupportsVolumeEncryption() now advertises encryption capability if the host supports EITHER qemu-native LUKS (qemu-img LUKS + cryptsetup) OR librbd native encryption (rbd CLI with the encryption subcommand). Previously a Ceph-only host that lacked cryptsetup would not advertise encryption even though librbd can encrypt RBD volumes. Split into hostSupportsQemuNativeVolumeEncryption() and hostSupportsRbdVolumeEncryption(); kept HOST_VOLUME_ENCRYPTION as the single host-wide flag (documented limitation: not per-pool). * kvm: resize support for librbd-encrypted RBD volumes (#5) Encrypted RBD volumes are encrypted natively by librbd and must be resized with `rbd resize --encryption-passphrase-file` so librbd grows the encrypted payload and keeps the LUKS header consistent. The existing encrypted-resize path (resizeEncryptedQcowFile) uses qemu-img --object secret, which is for qemu-native LUKS and does not fit the librbd LUKS2 layout. - RbdEncryption.resize(): new `rbd resize` wrapper (cephx via --id + keyfile, passphrase via KeyFile, optional --allow-shrink). - LibvirtResizeVolumeCommandWrapper: detect encrypted RBD and route to the rbd resize path, bypassing the libvirt v.resize and qemu-img paths. Snapshot/revert, RBD<->RBD copy, and migration of encrypted RBD volumes need no code changes: they operate on the raw (LUKS-containing) image at the block level, and the destination passphrase secret is already created engine-agnostic in LibvirtPrepareForMigrationCommandWrapper. These still require live validation. Builds: plugins/hypervisors/kvm (JDK11). * kvm: route online resize of encrypted RBD through virsh blockresize For a running VM, an librbd-encrypted RBD volume must be resized in-band by qemu/librbd, not out-of-band by the rbd CLI. Gate the CLI rbd-resize path on !vmIsRunning so: - offline -> `rbd resize --encryption-passphrase-file` (librbd-aware), and - online -> existing NOTIFYONLY path -> virsh blockresize, where qemu's block_resize delegates to librbd to grow the encrypted payload and notify the guest in one step (no passphrase needed; qemu holds the secret). This avoids notify-less out-of-band growth and qemu/librbd size divergence while the image is open. Online behaviour still needs live validation that blockresize resizes the encrypted payload for engine='librbd' disks. * kvm: encrypted RBD root disks (thin CoW clone + full-copy fallback) Root disks could not be encrypted: cloning a plaintext template and then `rbd encryption format`ing the clone leaves the inherited OS data unreadable (the LUKS header offsets it), so the guest could not mount root. Fix, in createDiskFromTemplateOnRBD, with two paths: - Option A (same-cluster cached RBD template): grow the template base to reserve LUKS2 header space, snapshot+protect it (cloudstack-base-snap-luks), clone from it, apply the LUKS2 header, resize the clone to the requested size. Inherited template data stays readable through the clone's encryption and the clone is a thin CoW image (only the header is written). - Option B (first-use / non-RBD template): create an empty image, apply a LUKS2 header, then import the template THROUGH the encryption layer via RbdEncryption.importTemplate (qemu-img convert -n into encrypt.key-secret). Correct but a full copy. Validated end-to-end on Ubuntu 26.04 / libvirt 12.0.0: both boot; A is thin (3.5 GiB provisioned, ~120 MiB used); LUKS2 verified at rest on Ceph. * kvm: harden and align librbd-encrypted RBD volume code Review pass over the librbd LUKS2 encryption feature to fix latent issues and bring it in line with CloudStack conventions: - RbdEncryption: reject empty/null passphrase with a clear error; round rbd --size up to MiB so a non-aligned request never shrinks the volume below what was asked for; create the temporary cephx conf/keyring 0600 explicitly instead of relying on the umask. - LibvirtStorageAdaptor: close Rados/IoCTX/RbdImage in a finally block on the encrypted-root paths (mirrors deleteVolume) so handles are not leaked on exceptions; use parameterized log messages instead of string concatenation; extract the encrypted-root Option A/B logic into createEncryptedRootCoWClone / createEncryptedRootFullCopy. - RbdEncryption: use an instance logger (matching the plugin convention) and split argv construction into build{Format,Resize,Convert}Script so the generated commands can be unit-tested. * kvm: add RbdEncryption unit tests Assert the rbd/qemu-img argv built for format, resize and convert-through-encryption (RBD and file sources), and that empty/null passphrases are rejected. Command construction is verified without a live Ceph cluster. * kvm: refuse encrypted RBD hot-plug on libvirt < 10.1.0 libvirt 10.0.0 has an object apply-order bug (fixed in 10.1.0) that breaks hot-plug of an encrypted rbd blockdev: on attach the disk is opened before its LUKS secret object is defined, so the attach fails with "No secret with id '...-format-encryption-secret0'". Booting a VM from an encrypted RBD disk is unaffected (the QEMU command line resolves all -object before -blockdev). Refuse the attach up front with a clear error (mirroring the existing openvswitch/io_uring libvirt-version gates) instead of letting libvirt fail opaquely. Only the RBD hot-plug path is gated; boot/root/detach are untouched. * docs: add PendingReleaseNotes entry for librbd-encrypted RBD volumes * kvm: route the encrypted RBD template import through QemuImg RbdEncryption built its own 'qemu-img convert' command line, which duplicated qemu-img knowledge outside of QemuImg. QemuImg could only write to a plain filename destination, so importing a template through the librbd encryption layer was not expressible with it. QemuImg now supports a destination described by image options (--target-image-opts, with -n implied since such a target always exists already), exposed as convertIntoExistingTarget(). QemuImageOptions can render its parameters under either image-opts flag. RbdEncryption.importTemplate now composes QemuImageOptions and a QemuObject secret and delegates to QemuImg; its hand-built convert script is removed. The rbd CLI calls (encryption format, resize, support probe) stay, as qemu-img cannot perform them. No functional change to the generated command. * kvm: use readable variable names in the encrypted RBD root helpers Review feedback: single-letter and abbreviated names are hard to read. Renamed in the two methods added by this PR only (renaming the rest of the class is out of scope here): r -> radosConnection, io -> ioContext, rbd -> rbdClient, base -> templateImage, s -> snapshotInfo, encSnap -> luksReservedSnapshotName, haveEncSnap -> luksSnapshotExists, createSize -> imageSizeWithLuksHeader, srcIsRbd -> sourceIsRbdPool. No functional change. * server, kvm: report and require RBD volume encryption separately Review feedback: distinguish the two volume encryption mechanisms instead of advertising them under one host flag. host.volume.encryption goes back to meaning qemu-native LUKS only (qemu-img LUKS + cryptsetup), as it did before this PR, and hosts now additionally report host.volume.encryption.rbd for librbd encryption (rbd encryption format). The deployment planner requires the flag matching the pool type of each encrypted volume - librbd for volumes on RBD pools, qemu-native for any other pool type - at all three places it validated encryption support before. The pool is taken from the pools proposed alongside the host when present, so first deployments are matched accurately too; an encrypted volume with no pool yet accepts either mechanism and the storage pool allocator picks a pool the host can serve. This also stops a host whose librbd is too old for 'rbd encryption format' from being selected for encrypted RBD volumes; it previously advertised encryption through the qemu stack and the VM failed to start. --------- Co-authored-by: Václav Rozsypálek <vaclav.rozsypalek@master.cz> Co-authored-by: calvix <7136358+calvix@users.noreply.github.com>
findByID will fail for dummy template as it is stored in the deleted state.
…ion (apache#14162) * kvm: allow importVm importsource=shared from an RBD pool The CheckVolumeCommand wrapper on the KVM agent only accepted file based pools, so importing a root disk straight from Ceph failed on the agent with "Unsupported Storage Pool" and surfaced as "Disk not found or is invalid" on the management server. Add RBD to the supported pool types, take the virtual size from the disk libvirt resolved (qemu-img cannot open a bare RBD image name), skip the QCOW2 header check for raw RBD images, and build the rbd: URI when running qemu-img info, the same way LibvirtGetVolumesOnStorageCommandWrapper already does for listVolumesForImport. * kvm: record the real image format on an imported volume importVolume and updateImportedVolume both stamped the cluster default format for the hypervisor, so a volume imported from an RBD pool was recorded as QCOW2 while a natively deployed volume on the same pool is RAW. This affected both entry points: importVm importsource=shared for a root disk, and importVolume for a data disk. Pass the format the hypervisor reported for the existing image, from the check answer for a root disk and from the volume listed on the pool for a data disk, and fall back to the hypervisor default only when no format is reported. This also corrects a raw image imported from a file based pool. * vm import: plan the instance inside the pod and cluster of the requested pool importKVMInstanceFromDiskImage planned with the pod and cluster unset, so the planner was free to pick any host in the zone by capacity. When it picked a host in a cluster that cannot see the pool the caller named, the volume check ran against whichever pool that cluster does have, and the import failed with "Disk not found or is invalid" although the image was fine. Take the pod and cluster from the pool the caller passed, the same way importVolume already derives its host from the pool's scope * verify qemu is able to read the rbd image since check can not be done on a raw file * use computed volumeDetails in success path as well. * ui: do not call the imported disk a QCOW2 image The import wizard for local and shared storage described the disk as a QCOW2 image. An image on an RBD pool is raw, and a raw image on a file based pool can be imported as well, so the wording is wrong for both. Call it a disk image instead. Only the English strings are changed; the other locales come from Transifex.
apache#14188) getPhysicalDisk() asks libvirt for the volume. A volume that was just created by the management server is not in this host's libvirt pool cache, so the lookup misses and getVolume() falls back to refreshing the whole pool. Refreshing an RBD pool opens and stats every image in it, so the cost grows with the number of volumes in the pool and is paid on every VM start. On a pool holding 950 images that is 11.7 seconds added to each start, against 0.005 seconds to list the image names. Look RBD volumes up directly through librbd instead. Creating, cloning, resizing, copying and deleting RBD volumes in this class already use librbd directly; only the lookup went through libvirt. Because this runs on every VM start, the connection is handled more carefully than at the existing call sites: - the rados connection is shut down in a finally, so a client is not left for the finalizer. KVMStoragePoolManager.getPhysicalDisk retries a missing volume 100 times, so leaking here would mean 101 live librados clients for one absent volume. - the image is opened read only, since it is only stat'ed, and so cannot take the exclusive lock. - the image is closed through a helper that does not throw, so a failed close cannot discard a successful lookup or hide the original error. - the cephx key is only set when the pool has a user. librados aborts the process if it is handed a null value.
…not be saved When the new activity state can not be written to the RADOS object, the next check would use an old state. Report it as not known whether the host is alive (exit status 2), as when the watchers can not be read.
Incremental NAS backups so far relied on QEMU dirty bitmaps / libvirt checkpoints, which only exist on file-based qcow2 storage. RBD images are raw and have nowhere to persist a bitmap, so any VM with a volume on Ceph was forced onto the full-only path and re-uploaded the whole image on every cycle. Ceph provides the equivalent natively, and this adds a second chain format that uses it: an `rbd snap` per cycle as the anchor plus `rbd export-diff --from-snap`, which computes the changed extents inside Ceph from the image object-map. NASBackupProvider now selects the mechanism per VM via getIncrementalFlavor() - QCOW2 for checkpoint-capable pools, RBD for Ceph, UNSUPPORTED (legacy full-only) for mixed or other storage, since the two formats cannot be mixed inside one backup. Everything else - chain identity, the nas.backup.full.every cadence, delete/tombstone semantics and resource accounting - is shared unchanged. Restore differs because an export-diff stream carries no pointer to the diff before it, unlike a rebased qcow2. The provider therefore sends the whole chain from its full anchor up to the backup being restored, and the agent recreates the image and replays each diff with `rbd import-diff` in order. The image has to be recreated: a full diff only carries the extents that were allocated when it was taken, so any block still allocated in the target would otherwise show through as stale data. That also drops RBD snapshots the volume carried, which is inherent to rewinding a volume wholesale. Two behaviours follow from the anchor living in Ceph rather than in the QEMU process: a stopped VM keeps its incremental cadence (no live QEMU needed), and a chain survives a VM restart or host migration. Only the newest anchor is kept per image; a full additionally sweeps every older snapshot carrying the csbackup- prefix, so a deleted chain, an unassigned offering or a restore cannot leak snapshots. The feature stays behind the existing nas.backup.incremental.enabled zone setting, which still defaults to false. Note: the agent drives the `rbd` CLI, so KVM hosts backing up Ceph volumes need ceph-common installed. Verified on a Ceph Reef 18.2.8 cluster with KVM hosts on Ceph Quincy 17.2.7 clients: FULL 1.86 GB followed by incrementals of 71 KB - 2.6 MB, restores replaying 2- and 3-diff chains across two RBD pools including through a tombstoned ancestor. test_backup_recovery_nas.py: the blanket RBD skip is replaced by an _is_rbd_storage() predicate, so the five chain tests now run on Ceph. test_stopped_vm_falls_back_to_full keeps a targeted skip because RBD has the opposite behaviour by design, and a new test_stopped_rbd_vm_keeps_incremental_cadence asserts that other side.
The suite registered the legacy CentOS 5.5 tiny template via get_template(). Its kernel/udev does not reliably enumerate hot-attached virtio disks and it ships no mkfs.ext4, so the chain tests needed guest-side workarounds to reach the data they back up and restore. Register the Ubuntu 24.04 cloud-init template instead, which enumerates hot-attached disks, carries the usual filesystem tools and grows its root filesystem on first boot by itself. The cloud image disables SSH password authentication outright (60-cloudimg-settings.conf), so guest access moves to an SSH keypair and the stock "ubuntu" user. That user lands unprivileged, so commands touching /root or a raw block device now go through a _sudo() helper which wraps the whole command in `sh -c` under sudo, keeping redirects and ';'-separated commands root-privileged rather than just the first word. Cleanup ordering changes with it: a template cannot be deleted while a VM still uses it, so it is appended to _cleanup after the account that owns the VM, since cleanup_resources() deletes in list order. The in-place multi-disk restore regression test that accompanied this change upstream is left out: it asserts the uuid-based volume mapping in LibvirtRestoreBackupCommandWrapper, which this branch does not carry.
The existing chain tests each check one property of one backup: the full/incremental cadence, a restore from the tip, a delete that repairs or defers. None of them walks the chain as a whole, and none asserts on backup size. Add a test that builds FULL + 3 INCREMENTALs with a marker file per backup, restores from every backup and creates a VM from every backup, then deletes the 2nd incremental and repeats both sweeps over what survives. The mid-chain delete is what earns the runtime on RBD. An `rbd export-diff` stream carries no pointer to the diff before it, so a restore replays the whole sequence from the full anchor, and a diff whose descendant still needs it cannot be dropped when it is deleted, only hidden. Restoring the tip across that gap is what breaks if the provider ever sends a chain with a hole in it, and nothing else covered it. Sizes are asserted throughout: an incremental must be a small fraction of the full. That is what distinguishes a real export-diff delta from the whole image being re-exported every cycle, and no other test in this file looks at size at all, so a provider that silently fell back to full-every-time still passed. Each restore also asserts that markers written after that backup are gone. Checking only that the expected markers are present would pass even if the restore had quietly done nothing. VMs built from backups are expunged as the test goes rather than through _cleanup: seven of them are created, and holding them all to the end risks exhausting the environment's capacity partway through a run.
…t boots The chain test checked an Instance created from a backup only for reaching Running. That proves the image boots; it says nothing about whether the diff chain was replayed into the new volume correctly, so a create-from-backup that produced a stale or empty image would still have passed. Check the guest the same way an in-place restore is checked: the markers taken up to that backup must be present, and markers written after it must be absent. The absent half matters as much as the present half -- without it, a VM that somehow came up with the source Instance's current disk would pass. Access uses the pattern already proven by test_incremental_after_vm_restart: the backup carries the source Instance's authorized_keys, so the new VM accepts the same keypair and the stock cloud-image "ubuntu" user.
A disk-only VM snapshot does not contain the guest page cache, so the file written by the test may be missing after the revert.
Commit 26e7c3f (apache#13763, let libvirt remove RBD snapshots on volume delete) removed the com.ceph.rados.exceptions.ErrorCode import together with its last uses. Commit 08a706d (apache#14188, look RBD volumes up through librbd instead of refreshing the pool) brought the uses back without the import, so plugins/hypervisors/kvm failed to compile.
weizhouapache
force-pushed
the
4.23.0-ceph
branch
from
October 12, 2026 10:08
5baaf36 to
937542d
Compare
Member
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19520 |
Member
|
[SF] Trillian Build Failed (tid-92) |
Member
|
[SF] Trillian test result (tid-96)
|
…vity check Follow-up to the review of the Host HA on Ceph RBD support: - kvmheartbeat_rbd.sh and kvmvmactivity_rbd.sh are now kvmheartbeat_rbd.py and kvmvmactivity_rbd.py, with the same options, output and exit status - the Ceph key is no longer an argument of the scripts: the agent gives it in the environment variable CEPH_KEY, and the scripts give it to rados and rbd in CEPH_ARGS, so it is not in the process list and no keyfile is needed; the heartbeat and the activity state are given to rados on standard input instead of temporary files - kvmvmactivity_rbd.py lists the watchers of the volumes over one connection with the Python bindings of librados and librbd (16 s instead of 97 s for 1000 volumes without a watcher), and runs rbd status for each volume if the bindings are not installed or can not be used - for a RBD pool, the volumes are given to kvmvmactivity_rbd.py in a temporary file with the new option -U, as they can be more than the 128 KiB which an argument of a command can have (about 3500 volumes), and as the whole command is written to the agent log at each check - the warnings of a failed heartbeat or VM-activity check no longer say that it may shut the host down: it is considered dead, which may trigger HA actions, such as fencing it and restarting its VMs on other hosts - add unit tests for the environment of the scripts, the Ceph user and key and the file with the volumes
…e pool LibvirtCheckVMActivityOnStoragePoolCommandWrapper gave the script of the agent, kvmvmactivity.sh, to every pool, and LibvirtStoragePool replaced it with the script of RBD by itself. Choose the script in the wrapper: kvmvmactivity_rbd.py for a RBD pool, and the script of the agent for the other pools. If the script of RBD can not be found, the check fails with a message, instead of running without a script. LibvirtStoragePool uses the script which it is given.
…HA scripts on Ceph RBD The Host HA scripts for Ceph RBD read and write the heartbeat, the activity state and the watchers of the volumes over one connection, with the Python bindings of librados and librbd, instead of running the rados and rbd commands. The code is shared in kvmha_rbd.py. - Each operation has a timeout of 30 seconds (rados_osd_op_timeout, rados_mon_op_timeout, client_mount_timeout and the connect), so that an unreachable cluster does not block a check. - The cephx key is given to the bindings in their configuration. It is not an argument, and it is not in the environment of any other command. - Nothing is written to the standard error output, as the agent reads only the first line of the output of the scripts, and a message of librados would hide the DEAD verdict. - If the bindings are missing, the heartbeat is not written (exit 1), and the heartbeat check and the VM activity check are inconclusive (exit 2, no DEAD), so that a host is not considered dead because of it. - The cloudstack-agent packages require python3-rados and python3-rbd.
When qemu-img convert into an RBD pool failed or timed out, the half-written image was left behind. Every later copy of the same template then failed with "rbd create: File exists", so deployments from that template broke on the pool until the image was removed by hand. - check whether the destination RBD image exists before converting, and refuse to overwrite it, as it may be a complete image used by clones - remove the image again if the copy created it and then failed - do not print the Ceph key in the "Failed to convert" error
Member
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19534 |
…onUtils The wrapper checked each optional argument by hand, as "x == null || x.isEmpty()" or its negation. That states less than the predicate it stands for, and it counts a whitespace-only value as present, which none of these callers mean: a blank mode, bitmap name or stdout is an absent one. Use StringUtils for the arguments that are strings -- the mode, the two bitmap names and the script's output -- and CollectionUtils for the two that are lists, the parent paths and the disk paths. Blank now reads as absent, and the script's trailing output and fallback marker are read through StringUtils too, so neither needs its own null check. Take CollectionUtils from commons-collections rather than from the AWS SDK, which this wrapper alone was reaching for. Cherry-picked from the LibvirtTakeBackupCommandWrapper part of cffa6e3 on 4.22, the backport of PR 13074 as PR 13877.
The RBD path overloaded ChainDecision.mode with rbd-full and rbd-incremental, the same way the content-based path upstream overloaded it with content-full and content-incremental. Split the two concerns as that path already did: mode stays full or incremental, and a new method field says how the backup is taken -- qcow2 (push mode with dirty bitmaps) or rbd (Ceph-native, anchored on an rbd snapshot and exported with rbd export-diff). TakeBackupCommand gets a method field and nasbackup.sh a --method option. Both default to qcow2 when absent, so a management server that does not send a method keeps working against an upgraded agent. legacy-full is unchanged: rewriting it as full plus a "none" method would break incremental-disabled backups on agents that have not been upgraded yet. CHAIN_FORMAT stays as it is. The method says how a backup is taken and lives only for the duration of the request; the chain format is persisted and says what is on the repository, which is what restore and delete read later. The two coincide for the rbd method today but answer different questions. This mirrors c144d09 from apache#14194, which introduced the method for the content path and named an RBD diff as the next one to adopt it. Kept as close to that commit as the branch allows so the two merge cleanly: this branch carries the RBD path but not the content one, so the METHOD_CONTENT token is defined without a user here.
Member
|
[SF] Trillian test result (tid-99)
|
Member
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19549 |
Everything that talks to Ceph needs the pool's monitors as a list the client can parse, and the rules are the same wherever that list is built: give each monitor the pool's port unless it names one already, and put a bare IPv6 address in square brackets so its own colons stay distinguishable from the separator before the port. The mon_host option of a disk's rbd: string wants the same list again, with ':' and ';' escaped, because those two separate the options of that string and its monitors. None of it depends on a storage pool, so collect it in RbdUtils as static methods with their unit tests, rather than leaving each caller to grow a copy that is right in a different set of cases.
The agent assembled the monitor list in several places, each with its own idea of the rules. Opening a RADOS connection, or writing a ceph.conf for one, appended the pool's port to the host string as a whole, so with several monitors only the last was given it and a monitor that already named its port was handed a second. The rbd: option string of a disk escaped the colons of an IPv6 address only when it was bracketed already, which left a bare one reading as though it carried options of its own. Use the shared helpers for all of them. Two inputs therefore come out differently, both previously wrong: a monitor that names its port keeps just that one, and a bare IPv6 address is bracketed before anything is appended to it or escaped in it. The iscsiadm portals in IscsiAdmStorageAdaptor read the same way but are not monitor lists, so they keep their own formatting.
LibvirtStoragePool carried its own copy of the monitor-list logic. Call the shared one instead; the cases its tests covered are covered by RbdUtilsTest.
…BD HA scripts Both checks treated every Ceph failure alike, so two very different answers came out the same. A host that has never written a heartbeat, and a cluster this host cannot reach, are not the same thing: only the first says anything about the host being checked. Raise ObjectNotFoundError when the RADOS object is absent and keep CephError for a cluster that cannot be read. The heartbeat check then reports the host DEAD only for the missing or empty object, and exits unknown when Ceph itself is out of reach, as the VM activity check already did. Let the activity check survive a volume that is gone. Asking an image that no longer exists for its watchers now yields nothing instead of failing the whole check, so a volume the management server still lists does not decide the fate of the others. If none of the images exist the host's state is genuinely unknown, which is a failure rather than an answer of "no activity".
The monitors with a port, and the IPv6 monitors, are not valid arguments of rbd -m when the brackets are removed and the port is added to each monitor.
The field listed qcow2 and content but not rbd, the method this branch adds, so the one method a reader of this command is most likely to be chasing was the one the comment left out.
A restore rewinds the volume wholesale: the image is removed and rebuilt from its diff chain, so every snapshot it carried goes with it. The chain anchors this path takes are its own to drop, but a CloudStack volume snapshot kept on primary storage, or anything else taken outside this path, is content someone asked to keep, and destroying it quietly is not a trade the operator was offered. Ask what the image carries before touching it. A snapshot that is not one of ours refuses the restore, naming the ones in the way so they can be deleted or the backup restored into a new volume. Anything else is purged as before, except that an image which carries nothing is no longer handed a purge with nothing to do; when the listing itself fails we cannot tell, so it is purged as it always was. The failure to remove the image goes back to saying only that. Explaining it by a snapshot with clones made sense while a restore purged whatever it found, but the snapshot those clones belong to is now refused earlier, so a removal that still fails is failing for some other reason. The test that replays a whole chain stubbed every piped rbd call with the output of `rbd info`, whose "name" field would now read as a snapshot; tell the two commands apart there, and cover the refusal.
The key the script hands the rbd CLI lives in a temporary file for the length of a call. cleanup() removes it, but only the failure paths call cleanup, so an exit that does not reach it left the key behind. Leave its removal to the shell as well, which covers every exit it still gets to run code on.
Member
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19558 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR is used to test several ceph changes, which includes
Bug fixes
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?