Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(backups): fix size of NBD backups #6599

Merged
merged 3 commits into from
Jan 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions @xen-orchestra/backups/RemoteAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const { isMetadataFile } = require('./_backupType.js')
const { isValidXva } = require('./_isValidXva.js')
const { listPartitions, LVM_PARTITION_TYPE } = require('./_listPartitions.js')
const { lvs, pvs } = require('./_lvm.js')
const { watchStreamSize } = require('./_watchStreamSize')
// @todo : this import is marked extraneous , sould be fixed when lib is published
const { mount } = require('@vates/fuse-vhd')
const { asyncEach } = require('@vates/async-each')
Expand Down Expand Up @@ -661,7 +662,7 @@ class RemoteAdapter {
const handler = this._handler
if (this.#useVhdDirectory()) {
const dataPath = `${dirname(path)}/data/${uuidv4()}.vhd`
await createVhdDirectoryFromStream(handler, dataPath, input, {
const size = await createVhdDirectoryFromStream(handler, dataPath, input, {
concurrency: writeBlockConcurrency,
compression: this.#getCompressionType(),
async validator() {
Expand All @@ -671,12 +672,14 @@ class RemoteAdapter {
nbdClient,
})
await VhdAbstract.createAlias(handler, path, dataPath)
return size
} else {
await this.outputStream(path, input, { checksum, validator })
return this.outputStream(path, input, { checksum, validator })
}
}

async outputStream(path, input, { checksum = true, validator = noop } = {}) {
const container = watchStreamSize(input)
await this._handler.outputStream(path, input, {
checksum,
dirMode: this._dirMode,
Expand All @@ -685,6 +688,7 @@ class RemoteAdapter {
return validator.apply(this, arguments)
},
})
return container.size
}

// open the hierarchy of ancestors until we find a full one
Expand Down
10 changes: 5 additions & 5 deletions @xen-orchestra/backups/writers/DeltaBackupWriter.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ exports.DeltaBackupWriter = class DeltaBackupWriter extends MixinBackupWriter(Ab
}
}

async _transfer({ timestamp, deltaExport, sizeContainers }) {
async _transfer({ timestamp, deltaExport }) {
const adapter = this._adapter
const backup = this._backup

Expand Down Expand Up @@ -172,6 +172,7 @@ exports.DeltaBackupWriter = class DeltaBackupWriter extends MixinBackupWriter(Ab
}

const { size } = await Task.run({ name: 'transfer' }, async () => {
let transferSize = 0
await Promise.all(
map(deltaExport.vdis, async (vdi, id) => {
const path = `${this._vmBackupDir}/${vhds[id]}`
Expand Down Expand Up @@ -217,7 +218,8 @@ exports.DeltaBackupWriter = class DeltaBackupWriter extends MixinBackupWriter(Ab
} else {
debug('useNbd is disabled', { vdi: id, path })
}
await adapter.writeVhd(path, deltaExport.streams[`${id}.vhd`], {

transferSize += await adapter.writeVhd(path, deltaExport.streams[`${id}.vhd`], {
// no checksum for VHDs, because they will be invalidated by
// merges and chainings
checksum: false,
Expand All @@ -238,9 +240,7 @@ exports.DeltaBackupWriter = class DeltaBackupWriter extends MixinBackupWriter(Ab
})
})
)
return {
size: Object.values(sizeContainers).reduce((sum, { size }) => sum + size, 0),
}
return { size: transferSize }
})
metadataContent.size = size
this._metadataFileName = await adapter.writeVmBackupMetadata(vm.uuid, metadataContent)
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

- [REST API] Fix 5 minutes timeouts on VDI/VM uploads [#6568](https://github.com/vatesfr/xen-orchestra/issues/6568)
- [Backup] Fix NBD configuration (PR [#6597](https://github.com/vatesfr/xen-orchestra/pull/6597))
- [NBD Backups] Fix transfer size [#6599](https://github.com/vatesfr/xen-orchestra/issues/6599)

### Packages to release

Expand Down
11 changes: 11 additions & 0 deletions packages/vhd-lib/Vhd/VhdAbstract.integ.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,17 @@ test('it can create a vhd stream', async () => {
await vhd.writeFooter()
const stream = vhd.stream()

// size and stream must have the same result
expect(stream.length).toEqual(vhd.streamSize())

expect(stream.length).toEqual(
512 /* footer */ +
1024 /* header */ +
512 /* BAT */ +
512 /* parentlocator */ +
3 * (2 * 1024 * 1024 + 512) /* blocs */ +
512 /* end footer */
)
// read all the stream into a buffer

const buffer = await streamToBuffer(stream)
Expand Down
20 changes: 20 additions & 0 deletions packages/vhd-lib/Vhd/VhdAbstract.js
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,26 @@ exports.VhdAbstract = class VhdAbstract {
await handler.writeFile(aliasPath, relativePathToTarget)
}

streamSize() {
const { header, batSize } = this
let fileSize = FOOTER_SIZE + HEADER_SIZE + batSize + FOOTER_SIZE /* the footer at the end */

// add parentlocator size
for (let i = 0; i < PARENT_LOCATOR_ENTRIES; i++) {
fileSize += header.parentLocatorEntry[i].platformDataSpace * SECTOR_SIZE
}

// add block size
for (let i = 0; i < header.maxTableEntries; i++) {
if (this.containsBlock(i)) {
fileSize += this.fullBlockSize
}
}

assert.strictEqual(fileSize % SECTOR_SIZE, 0)
return fileSize
}

stream() {
const { footer, batSize } = this
const { ...header } = this.header // copy since we don't ant to modifiy the current header
Expand Down
4 changes: 3 additions & 1 deletion packages/vhd-lib/createVhdDirectoryFromStream.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ const buildVhd = Disposable.wrap(async function* (handler, path, inputStream, {
}
)
await Promise.all([vhd.writeFooter(), vhd.writeHeader(), vhd.writeBlockAllocationTable()])
return vhd.streamSize()
})

exports.createVhdDirectoryFromStream = async function createVhdDirectoryFromStream(
Expand All @@ -47,10 +48,11 @@ exports.createVhdDirectoryFromStream = async function createVhdDirectoryFromStre
{ validator, concurrency = 16, compression, nbdClient } = {}
) {
try {
await buildVhd(handler, path, inputStream, { concurrency, compression, nbdClient })
const size = await buildVhd(handler, path, inputStream, { concurrency, compression, nbdClient })
if (validator !== undefined) {
await validator.call(this, path)
}
return size
} catch (error) {
// cleanup on error
await handler.rmtree(path).catch(warn)
Expand Down