Re: [libvirt] [PATCH v8 18/21] backup: qemu: Implement framework for backup job APIs
On Wed, Apr 17, 2019 at 09:09:18 -0500, Eric Blake wrote:
Still needs to actually kick off the right QMP commands, but at
least allows validation of backup XML, including the fact that
a backup job can survive a libvirtd restart. Atomically creating
a checkpoint alongside the backup still needs implementing.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/qemu/qemu_domain.h | 4 +
src/qemu/qemu_domain.c | 29 ++++++-
src/qemu/qemu_driver.c | 185 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 214 insertions(+), 4 deletions(-)
[...]
# define QEMU_DOMAIN_PRIVATE(vm) \
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 16dec68302..f4be948bae 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
[...]
- return virXMLFormatElement(buf, "blockjobs", &attrBuf, NULL);
+ if (virXMLFormatElement(buf, "blockjobs", &attrBuf, NULL) < 0)
+ goto cleanup;
+
+ /* TODO: merge other blockjobs and backups into uniform space? */
I can see only one way to do this since the backup job spans multiple
disks. We need a new set of APIs which will work with job names rather
than disk names as blockjobs do. This will be a middle ground between
the API for the domain job which is basically usable only for migration
and blockjobs as there's possiblity to have multiple of them at the same
time.
That way you'll be able to cancel a whole job rather than have weird
semantics intermixed as a result of the domain job API and the blockjob
API and the custom backup job API
Additionally a general API will require a somewhat better naming than
just a number. The job name should also encode the operation in the
name. For blockjobs with blockdev I'm using e.g 'pull-NODENAME'. We can
prefix this e.g. by 'block-' for the blockjob or 'backup' for the backup
job if you decide that the new api is worth it.
The jobs can obviously be described by XML as is the case here. We can
synthetize a XML for blockjobs and also allow easy abort/complete
through the new API. This will also include a new set of events ... or
better a new one event which will be emitted for the full job rather
than a plethora of disks.
We only then need a general API to list all running jobs.
+ if (priv->backup && virDomainBackupDefFormat(buf,
priv->backup, true) < 0)
+ goto cleanup;
Last time I've requested that this is NOT formatted here. It formats a
separate element so put it into a separate function.
+
+ ret = 0;
+ cleanup:
+ virBufferFreeAndReset(&attrBuf);
+ return ret;
}
[...]
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 86310b6e92..cca0b550b8 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
[...]
@@ -17692,6 +17693,187 @@
qemuDomainCheckpointDelete(virDomainCheckpointPtr checkpoint,
return ret;
}
+static int qemuDomainBackupBegin(virDomainPtr domain, const char *diskXml,
+ const char *checkpointXml, unsigned int flags)
This does not conform to our header styling rules.
+{
Attachments:
- signature.asc (application/pgp-signature — 833 bytes)