On Wed, Mar 27, 2019 at 05:10:51 -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@redhat.com> --- src/qemu/qemu_domain.h | 4 + src/qemu/qemu_domain.c | 32 ++++++- src/qemu/qemu_driver.c | 185 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 217 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 9b29628107..7e3641c6d0 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -376,6 +376,10 @@ struct _qemuDomainObjPrivate {
/* true if global -mem-prealloc appears on cmd line */ bool memPrealloc; + + /* Any currently running backup job. + * FIXME: allow jobs in parallel. For now, at most one job, always id 1. */ + virDomainBackupDefPtr backup; };
# define QEMU_DOMAIN_PRIVATE(vm) \ diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 8a2b951dde..6648240dc4 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -57,6 +57,7 @@ #include "locking/domain_lock.h" #include "virdomainsnapshotobjlist.h" #include "virdomaincheckpointobjlist.h" +#include "backup_conf.h"
#ifdef MAJOR_IN_MKDEV # include <sys/mkdev.h> @@ -2313,13 +2314,25 @@ static int qemuDomainObjPrivateXMLFormatBlockjobs(virBufferPtr buf, virDomainObjPtr vm) { + qemuDomainObjPrivatePtr priv = vm->privateData; virBuffer attrBuf = VIR_BUFFER_INITIALIZER; bool bj = qemuDomainHasBlockjob(vm, false); + int ret = -1;
virBufferAsprintf(&attrBuf, " active='%s'", virTristateBoolTypeToString(virTristateBoolFromBool(bj)));
- return virXMLFormatElement(buf, "blockjobs", &attrBuf, NULL); + if (virXMLFormatElement(buf, "blockjobs", &attrBuf, NULL) < 0) + goto cleanup; + + /* TODO: merge other blockjobs and backups into uniform space? */ + if (priv->backup && virDomainBackupDefFormat(buf, priv->backup, true) < 0) + goto cleanup;
If this is a separate element please don't put it into this function. This should format just <blockjobs>. If you wish to declare it as a blockjob format it as a child of <blockjobs>.
+ + ret = 0; + cleanup: + virBufferFreeAndReset(&attrBuf); + return ret; }
@@ -2666,18 +2679,29 @@ qemuDomainObjPrivateXMLParseAutomaticPlacement(xmlXPathContextPtr ctxt,
static int -qemuDomainObjPrivateXMLParseBlockjobs(qemuDomainObjPrivatePtr priv, +qemuDomainObjPrivateXMLParseBlockjobs(virQEMUDriverPtr driver, + qemuDomainObjPrivatePtr priv, xmlXPathContextPtr ctxt) { + xmlNodePtr node; char *active; int tmp; + int ret = -1;
if ((active = virXPathString("string(./blockjobs/@active)", ctxt)) && (tmp = virTristateBoolTypeFromString(active)) > 0) priv->reconnectBlockjobs = tmp;
+ if ((node = virXPathNode("./domainbackup", ctxt)) && + !(priv->backup = virDomainBackupDefParseNode(ctxt->doc, node, + driver->xmlopt, + VIR_DOMAIN_BACKUP_PARSE_INTERNAL)))
Same here. Put it into a separate function if it's considered separate. Also I'd strongly suggest to put it into a container if you ever expect more than one element present. so that it does not pollute the private data part.