Re: [PATCH v7 20/23] backup: qemu: Implement framework for backup job APIs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
> ---
>  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.

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux