10:51 a.m.
See patch 6/8 for the reasoning.
This series applies on top of the v2 of splitting checkpoints into a
separate file.
Peter Krempa (8):
qemu: checkpoint: Refactor cleanup in qemuCheckpointCreateXML
qemu: checkpoint: Remove open-ended TODOs
qemu: Simplify argument list of qemuDomainBlockPullCommon
qemu: Don't repeat virDomainObjEndAPI in qemuDomainBlockPull
qemu: caps: Add capability for incremental backup support
qemu: checkpoint: Forbid creating checkpoints until we support backups
qemu: Aggregate interlocking of blockjobs by checkpoints in one place
qemu: domain: Base block job interlocking on
QEMU_CAPS_INCREMENTAL_BACKUP
src/qemu/qemu_capabilities.c | 3 +++
src/qemu/qemu_capabilities.h | 3 +++
src/qemu/qemu_checkpoint.c | 42 +++++++++++++-----------------
src/qemu/qemu_domain.c | 24 +++++++++++++++++
src/qemu/qemu_domain.h | 4 +++
src/qemu/qemu_driver.c | 50 +++++++++++++-----------------------
6 files changed, 70 insertions(+), 56 deletions(-)
--
2.21.0
10:51 a.m.
New subject: [libvirt] [PATCH 1/8] qemu: checkpoint: Refactor cleanup in qemuCheckpointCreateXML
Use VIR_AUTO* helpers and get rid of the 'cleanup' label.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_checkpoint.c | 22 +++++++++-------------
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
index 30d1b6ae3a..4430f69911 100644
--- a/src/qemu/qemu_checkpoint.c
+++ b/src/qemu/qemu_checkpoint.c
@@ -363,9 +363,9 @@ qemuCheckpointCreateXML(virDomainPtr domain,
bool redefine = flags & VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE;
unsigned int parse_flags = 0;
virDomainMomentObjPtr other = NULL;
- virQEMUDriverConfigPtr cfg = NULL;
- virCapsPtr caps = NULL;
- virJSONValuePtr actions = NULL;
+ VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL;
+ VIR_AUTOUNREF(virCapsPtr) caps = NULL;
+ VIR_AUTOPTR(virJSONValue) actions = NULL;
int ret;
VIR_AUTOUNREF(virDomainCheckpointDefPtr) def = NULL;
@@ -380,32 +380,32 @@ qemuCheckpointCreateXML(virDomainPtr domain,
if (virDomainSnapshotObjListNum(vm->snapshots, NULL, 0) > 0) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("cannot create checkpoint while snapshot exists"));
- goto cleanup;
+ return NULL;
}
if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BITMAP_MERGE)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("qemu binary lacks persistent bitmaps support"));
- goto cleanup;
+ return NULL;
}
if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
- goto cleanup;
+ return NULL;
if (!virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("cannot create checkpoint for inactive domain"));
- goto cleanup;
+ return NULL;
}
if (!(def = virDomainCheckpointDefParseString(xmlDesc, caps, driver->xmlopt,
priv->qemuCaps, parse_flags)))
- goto cleanup;
+ return NULL;
/* Unlike snapshots, the RNG schema already ensured a sane filename. */
/* We are going to modify the domain below. */
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
- goto cleanup;
+ return NULL;
if (redefine) {
if (virDomainCheckpointRedefinePrep(vm, &def, &chk,
@@ -484,10 +484,6 @@ qemuCheckpointCreateXML(virDomainPtr domain,
qemuDomainObjEndJob(driver, vm);
- cleanup:
- virJSONValueFree(actions);
- virObjectUnref(caps);
- virObjectUnref(cfg);
return checkpoint;
}
--
2.21.0
10:51 a.m.
New subject: [libvirt] [PATCH 2/8] qemu: checkpoint: Remove open-ended TODOs
Once somebody is motivated enough to add the support for the quiesce
flag or offline checkpoint deletion they are welcome to do so but we
don't need to have a reminder.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_checkpoint.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
index 4430f69911..59f700e1d3 100644
--- a/src/qemu/qemu_checkpoint.c
+++ b/src/qemu/qemu_checkpoint.c
@@ -370,7 +370,6 @@ qemuCheckpointCreateXML(virDomainPtr domain,
VIR_AUTOUNREF(virDomainCheckpointDefPtr) def = NULL;
virCheckFlags(VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE, NULL);
- /* TODO: VIR_DOMAIN_CHECKPOINT_CREATE_QUIESCE */
if (redefine) {
parse_flags |= VIR_DOMAIN_CHECKPOINT_PARSE_REDEFINE;
@@ -570,8 +569,6 @@ qemuCheckpointDelete(virDomainObjPtr vm,
return -1;
if (!metadata_only) {
- /* Until qemu-img supports offline bitmap deletion, we are stuck
- * with requiring a running guest */
if (!virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("cannot delete checkpoint for inactive domain"));
--
2.21.0
10:51 a.m.
New subject: [libvirt] [PATCH 3/8] qemu: Simplify argument list of qemuDomainBlockPullCommon
Drop the 'driver' argument since it can be extracted from private data
to shorten the argument list.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_driver.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5852df2a53..71503898e4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17534,14 +17534,14 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver,
/* bandwidth in MiB/s per public API. Caller must lock vm beforehand,
* and not access it afterwards. */
static int
-qemuDomainBlockPullCommon(virQEMUDriverPtr driver,
- virDomainObjPtr vm,
+qemuDomainBlockPullCommon(virDomainObjPtr vm,
const char *path,
const char *base,
unsigned long bandwidth,
unsigned int flags)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
+ virQEMUDriverPtr driver = priv->driver;
const char *device = NULL;
const char *jobname = NULL;
virDomainDiskDefPtr disk;
@@ -18355,7 +18355,6 @@ static int
qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base,
unsigned long bandwidth, unsigned int flags)
{
- virQEMUDriverPtr driver = dom->conn->privateData;
virDomainObjPtr vm;
int ret = -1;
unsigned long long speed = bandwidth;
@@ -18384,7 +18383,7 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const
char *base,
/* For normal rebase (enhanced blockpull), the common code handles
* everything, including vm cleanup. */
if (!(flags & VIR_DOMAIN_BLOCK_REBASE_COPY))
- return qemuDomainBlockPullCommon(driver, vm, path, base, bandwidth, flags);
+ return qemuDomainBlockPullCommon(vm, path, base, bandwidth, flags);
/* If we got here, we are doing a block copy rebase. */
if (!(dest = virStorageSourceNew()))
@@ -18540,8 +18539,7 @@ qemuDomainBlockPull(virDomainPtr dom, const char *path, unsigned
long bandwidth,
return -1;
}
- return qemuDomainBlockPullCommon(dom->conn->privateData,
- vm, path, NULL, bandwidth, flags);
+ return qemuDomainBlockPullCommon(vm, path, NULL, bandwidth, flags);
}
--
2.21.0
10:51 a.m.
New subject: [libvirt] [PATCH 4/8] qemu: Don't repeat virDomainObjEndAPI in qemuDomainBlockPull
Add a 'cleanup' label and use jumps as we do in other places.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_driver.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 71503898e4..3ad1699eb1 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18522,24 +18522,27 @@ qemuDomainBlockPull(virDomainPtr dom, const char *path, unsigned
long bandwidth,
unsigned int flags)
{
virDomainObjPtr vm;
+ int ret = -1;
+
virCheckFlags(VIR_DOMAIN_BLOCK_PULL_BANDWIDTH_BYTES, -1);
if (!(vm = qemuDomainObjFromDomain(dom)))
return -1;
- if (virDomainBlockPullEnsureACL(dom->conn, vm->def) < 0) {
- virDomainObjEndAPI(&vm);
- return -1;
- }
+ if (virDomainBlockPullEnsureACL(dom->conn, vm->def) < 0)
+ goto cleanup;
if (virDomainListCheckpoints(vm->checkpoints, NULL, dom, NULL, 0) > 0) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("cannot perform block pull while checkpoint
exists"));
- virDomainObjEndAPI(&vm);
- return -1;
+ goto cleanup;
}
- return qemuDomainBlockPullCommon(vm, path, NULL, bandwidth, flags);
+ ret = qemuDomainBlockPullCommon(vm, path, NULL, bandwidth, flags);
+
+ cleanup:
+ virDomainObjEndAPI(&vm);
+ return ret;
}
--
2.21.0
5:02 p.m.
New subject: [libvirt] [PATCH 4/8] qemu: Don't repeat virDomainObjEndAPI in qemuDomainBlockPull
On 9/26/19 10:51 AM, Peter Krempa wrote:
Add a 'cleanup' label and use jumps as we do in other
places.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_driver.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
+ cleanup:
+ virDomainObjEndAPI(&vm);
+ return ret;
}
Is there any way to make virDomainObjEndAPI() replaceable by AUTO framework?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
1:33 a.m.
New subject: [libvirt] [PATCH 4/8] qemu: Don't repeat virDomainObjEndAPI in qemuDomainBlockPull
On Thu, Sep 26, 2019 at 17:02:49 -0500, Eric Blake wrote:
On 9/26/19 10:51 AM, Peter Krempa wrote:
> Add a 'cleanup' label and use jumps as we do in other places.
>
> Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
> ---
> src/qemu/qemu_driver.c | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
> + cleanup:
> + virDomainObjEndAPI(&vm);
> + return ret;
> }
Is there any way to make virDomainObjEndAPI() replaceable by AUTO framework?
It will probably require a new macro or a new type to avoid breaking
semantics we've established.
VIR_AUTOPTR is unuitable because the semantics say that the object will
be freed.
VIR_AUTOCLEAN is similarly unsuitable because we don't clean anything
inside, but AUTOCLEAN can be used internally in some cases as e.g. with
the autmatic XPath context cleanup code.
VIR_AUTOUNREF is not enough in this case because we are dealing with a
lockable object.
We could introduce something like VIR_AUTORELEASE and VIR_AUTOUNLOCK for
use with virOjbectLockable-based objects. The former would then do the
required virObjectUnlock and then virObjectUnref. The latter is for
parity in cases when the lock should be dropped.
I'll propose these and see how it will go.
7:48 a.m.
New subject: [libvirt] [PATCH 4/8] qemu: Don't repeat virDomainObjEndAPI in qemuDomainBlockPull
On 9/27/19 1:33 AM, Peter Krempa wrote:
On Thu, Sep 26, 2019 at 17:02:49 -0500, Eric Blake wrote:
> On 9/26/19 10:51 AM, Peter Krempa wrote:
>> Add a 'cleanup' label and use jumps as we do in other places.
>>
>> Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
>> ---
>> src/qemu/qemu_driver.c | 17 ++++++++++-------
>> 1 file changed, 10 insertions(+), 7 deletions(-)
>
>> + cleanup:
>> + virDomainObjEndAPI(&vm);
>> + return ret;
>> }
>
> Is there any way to make virDomainObjEndAPI() replaceable by AUTO framework?
It will probably require a new macro or a new type to avoid breaking
semantics we've established.
We could introduce something like VIR_AUTORELEASE and VIR_AUTOUNLOCK for
use with virOjbectLockable-based objects. The former would then do the
required virObjectUnlock and then virObjectUnref. The latter is for
parity in cases when the lock should be dropped.
I'll propose these and see how it will go.
glibc has GMutexLocker which is a way to release a lock on all exit
paths after its use (or to release the lock early).
https://developer.gnome.org/glib/stable/glib-Threads.html#g-mutex-locker-new
You can use that for inspiration.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
7:59 a.m.
New subject: [libvirt] [PATCH 4/8] qemu: Don't repeat virDomainObjEndAPI in qemuDomainBlockPull
On Fri, Sep 27, 2019 at 07:48:55 -0500, Eric Blake wrote:
On 9/27/19 1:33 AM, Peter Krempa wrote:
> On Thu, Sep 26, 2019 at 17:02:49 -0500, Eric Blake wrote:
> > On 9/26/19 10:51 AM, Peter Krempa wrote:
> > > Add a 'cleanup' label and use jumps as we do in other places.
> > >
> > > Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
> > > ---
> > > src/qemu/qemu_driver.c | 17 ++++++++++-------
> > > 1 file changed, 10 insertions(+), 7 deletions(-)
> >
> > > + cleanup:
> > > + virDomainObjEndAPI(&vm);
> > > + return ret;
> > > }
> >
> > Is there any way to make virDomainObjEndAPI() replaceable by AUTO framework?
>
> It will probably require a new macro or a new type to avoid breaking
> semantics we've established.
>
>
> We could introduce something like VIR_AUTORELEASE and VIR_AUTOUNLOCK for
> use with virOjbectLockable-based objects. The former would then do the
> required virObjectUnlock and then virObjectUnref. The latter is for
> parity in cases when the lock should be dropped.
>
> I'll propose these and see how it will go.
glibc has GMutexLocker which is a way to release a lock on all exit paths
after its use (or to release the lock early).
https://developer.gnome.org/glib/stable/glib-Threads.html#g-mutex-locker-new
You can use that for inspiration.
I opted to do something similar to VIR_AUTOUNREF in this iteration, but
we already use a very similar approach to what you've suggested in
VIR_XPATH_NODE_AUTORESTORE so that might also be a way forward.
10:51 a.m.
New subject: [libvirt] [PATCH 5/8] qemu: caps: Add capability for incremental backup support
Add a new all-covering capability which will be used to interlock
incremental backup support until all bits are ready.
---
src/qemu/qemu_capabilities.c | 3 +++
src/qemu/qemu_capabilities.h | 3 +++
2 files changed, 6 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 72e070e8ab..2edbb3fdfe 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -540,6 +540,9 @@ VIR_ENUM_IMPL(virQEMUCaps,
"dbus-vmstate",
"vhost-user-gpu",
"vhost-user-vga",
+
+ /* 340 */
+ "incremental-backup",
);
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 137b7161a5..5e990ce01e 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -522,6 +522,9 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check
*/
QEMU_CAPS_DEVICE_VHOST_USER_GPU, /* -device vhost-user-gpu */
QEMU_CAPS_DEVICE_VHOST_USER_VGA, /* -device vhost-user-vga */
+ /* 340 */
+ QEMU_CAPS_INCREMENTAL_BACKUP, /* incremental backup is supported */
+
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
--
2.21.0
10:51 a.m.
New subject: [libvirt] [PATCH 6/8] qemu: checkpoint: Forbid creating checkpoints until we support backups
Checkpoints by themselves are not very useful for anything else than
testing the few bitmap interactions that are currently implemented.
It's very unlikely that anybody used this feature and thus we can
disable it until we have a more complete implementation ready.
Additionally the code for deleting checkpoints has many broken failure
scenarios which should be fixed first. This will require support of
deleting a bitmap in a qemu 'transaction' which was not released yet.
Curious users obviously can use the qemu namespace in the XML to enable
this for experiments:
<domain type='kvm'
xmlns:qemu='http://libvirt.org/schemas/domain/qemu/1.0'>
...
<qemu:capabilities>
<qemu:add capability='incremental-backup'/>
</qemu:capabilities>
</domain>
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_checkpoint.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
index 59f700e1d3..46a10cbeba 100644
--- a/src/qemu/qemu_checkpoint.c
+++ b/src/qemu/qemu_checkpoint.c
@@ -382,9 +382,9 @@ qemuCheckpointCreateXML(virDomainPtr domain,
return NULL;
}
- if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BITMAP_MERGE)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("qemu binary lacks persistent bitmaps support"));
+ if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_INCREMENTAL_BACKUP)) {
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("incremental backup is not supported yet"));
return NULL;
}
@@ -569,14 +569,15 @@ qemuCheckpointDelete(virDomainObjPtr vm,
return -1;
if (!metadata_only) {
- if (!virDomainObjIsActive(vm)) {
+ if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_INCREMENTAL_BACKUP)) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
- _("cannot delete checkpoint for inactive domain"));
+ _("incremental backup is not supported yet"));
goto endjob;
}
- if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BITMAP_MERGE)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("qemu binary lacks persistent bitmaps support"));
+
+ if (!virDomainObjIsActive(vm)) {
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("cannot delete checkpoint for inactive domain"));
goto endjob;
}
}
--
2.21.0
5:19 p.m.
New subject: [libvirt] [PATCH 6/8] qemu: checkpoint: Forbid creating checkpoints until we support backups
On 9/26/19 10:51 AM, Peter Krempa wrote:
Checkpoints by themselves are not very useful for anything else than
testing the few bitmap interactions that are currently implemented.
It's very unlikely that anybody used this feature and thus we can
disable it until we have a more complete implementation ready.
Additionally the code for deleting checkpoints has many broken failure
scenarios which should be fixed first.
I suspect you are probably right.
This will require support of
deleting a bitmap in a qemu 'transaction' which was not released yet.
However, I'm not yet convinced on this point. Deleting a bitmap does
not have to be atomic with other actions from the standpoint of correct
contents of remaining bitmaps. I _do_ see that an atomic delete makes
it easier to delete multiple bitmaps at once, without having to worry
about failure states if one delete fails after another succeeded; but it
may still be possible to make things work without transactioned delete.
On the flip side, waiting to support checkpoints until qemu makes it
nice, and not worrying about older qemu, makes for less code to maintain.
Curious users obviously can use the qemu namespace in the XML to enable
this for experiments:
<domain type='kvm'
xmlns:qemu='http://libvirt.org/schemas/domain/qemu/1.0'>
...
<qemu:capabilities>
<qemu:add capability='incremental-backup'/>
</qemu:capabilities>
</domain>
Useful to know, for testing things in the meantime. Does this need to
go on any of our published documentation, perhaps in
docs/formatcheckpoint.html.in?
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_checkpoint.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
10:51 a.m.
New subject: [libvirt] [PATCH 7/8] qemu: Aggregate interlocking of blockjobs by checkpoints in one place
Rather than having to fix 5 places once we support the combination add a
function called by all the blockjob/snapshot APIs.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 21 +++++++++++++++++++++
src/qemu/qemu_domain.h | 4 ++++
src/qemu/qemu_driver.c | 25 +++++--------------------
3 files changed, 30 insertions(+), 20 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 19fa5420e7..e8a82fee0b 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -15513,3 +15513,24 @@ qemuDomainDefHasManagedPR(virDomainObjPtr vm)
return jobPR;
}
+
+
+/**
+ * qemuDomainSupportsCheckpointsBlockjobs:
+ * @vm: domain object
+ *
+ * Checks whether a block job is supported in possible combination with
+ * checkpoints (qcow2 bitmaps). Returns -1 if unsupported and reports an error
+ * 0 in case everything is supported.
+ */
+int
+qemuDomainSupportsCheckpointsBlockjobs(virDomainObjPtr vm)
+{
+ if (virDomainListCheckpoints(vm->checkpoints, NULL, NULL, NULL, 0) > 0) {
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("cannot perform block operations while checkpoint
exists"));
+ return -1;
+ }
+
+ return 0;
+}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index f45da882a8..01a54d4265 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -1217,3 +1217,7 @@ qemuDomainPausedReasonToSuspendedEvent(virDomainPausedReason
reason);
int
qemuDomainValidateActualNetDef(const virDomainNetDef *net,
virQEMUCapsPtr qemuCaps);
+
+int
+qemuDomainSupportsCheckpointsBlockjobs(virDomainObjPtr vm)
+ ATTRIBUTE_RETURN_CHECK;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3ad1699eb1..3847c727cf 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15846,11 +15846,8 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
if (virDomainSnapshotCreateXMLEnsureACL(domain->conn, vm->def, flags) < 0)
goto cleanup;
- if (virDomainListCheckpoints(vm->checkpoints, NULL, domain, NULL, 0) > 0) {
- virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
- _("cannot create snapshot while checkpoint exists"));
+ if (qemuDomainSupportsCheckpointsBlockjobs(vm) < 0)
goto cleanup;
- }
if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
goto cleanup;
@@ -18374,11 +18371,8 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const
char *base,
if (virDomainBlockRebaseEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
- if (virDomainListCheckpoints(vm->checkpoints, NULL, dom, NULL, 0) > 0) {
- virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
- _("cannot perform block rebase while checkpoint
exists"));
+ if (qemuDomainSupportsCheckpointsBlockjobs(vm) < 0)
goto cleanup;
- }
/* For normal rebase (enhanced blockpull), the common code handles
* everything, including vm cleanup. */
@@ -18464,11 +18458,8 @@ qemuDomainBlockCopy(virDomainPtr dom, const char *disk, const
char *destxml,
if (virDomainBlockCopyEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
- if (virDomainListCheckpoints(vm->checkpoints, NULL, dom, NULL, 0) > 0) {
- virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
- _("cannot perform block copy while checkpoint
exists"));
+ if (qemuDomainSupportsCheckpointsBlockjobs(vm) < 0)
goto cleanup;
- }
for (i = 0; i < nparams; i++) {
virTypedParameterPtr param = ¶ms[i];
@@ -18532,11 +18523,8 @@ qemuDomainBlockPull(virDomainPtr dom, const char *path, unsigned
long bandwidth,
if (virDomainBlockPullEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
- if (virDomainListCheckpoints(vm->checkpoints, NULL, dom, NULL, 0) > 0) {
- virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
- _("cannot perform block pull while checkpoint
exists"));
+ if (qemuDomainSupportsCheckpointsBlockjobs(vm) < 0)
goto cleanup;
- }
ret = qemuDomainBlockPullCommon(vm, path, NULL, bandwidth, flags);
@@ -18591,11 +18579,8 @@ qemuDomainBlockCommit(virDomainPtr dom,
if (virDomainBlockCommitEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
- if (virDomainListCheckpoints(vm->checkpoints, NULL, dom, NULL, 0) > 0) {
- virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
- _("cannot perform block commit while checkpoint
exists"));
+ if (qemuDomainSupportsCheckpointsBlockjobs(vm) < 0)
goto cleanup;
- }
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
goto cleanup;
--
2.21.0
5:05 p.m.
New subject: [libvirt] [PATCH 7/8] qemu: Aggregate interlocking of blockjobs by checkpoints in one place
On 9/26/19 10:51 AM, Peter Krempa wrote:
Rather than having to fix 5 places once we support the combination
add a
s/ add/, add/
function called by all the blockjob/snapshot APIs.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
10:51 a.m.
New subject: [libvirt] [PATCH 8/8] qemu: domain: Base block job interlocking on QEMU_CAPS_INCREMENTAL_BACKUP
The QEMU_CAPS_INCREMENTAL_BACKUP will be enabled once all bits of the
incremental backup feature work as expected which means also properly
interacting with blockjobs and snapshots.
Thus we can allow blockjobs and snapshots if QEMU_CAPS_INCREMENTAL_BACKUP
is present even when checkpoints exist.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index e8a82fee0b..6d42530db0 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -15526,7 +15526,10 @@ qemuDomainDefHasManagedPR(virDomainObjPtr vm)
int
qemuDomainSupportsCheckpointsBlockjobs(virDomainObjPtr vm)
{
- if (virDomainListCheckpoints(vm->checkpoints, NULL, NULL, NULL, 0) > 0) {
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+
+ if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_INCREMENTAL_BACKUP) &&
+ virDomainListCheckpoints(vm->checkpoints, NULL, NULL, NULL, 0) > 0) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("cannot perform block operations while checkpoint
exists"));
return -1;
--
2.21.0
5:07 p.m.
On 9/26/19 10:51 AM, Peter Krempa wrote:
See patch 6/8 for the reasoning.
This series applies on top of the v2 of splitting checkpoints into a
separate file.
Peter Krempa (8):
qemu: checkpoint: Refactor cleanup in qemuCheckpointCreateXML
qemu: checkpoint: Remove open-ended TODOs
qemu: Simplify argument list of qemuDomainBlockPullCommon
qemu: Don't repeat virDomainObjEndAPI in qemuDomainBlockPull
qemu: caps: Add capability for incremental backup support
qemu: checkpoint: Forbid creating checkpoints until we support backups
qemu: Aggregate interlocking of blockjobs by checkpoints in one place
qemu: domain: Base block job interlocking on
QEMU_CAPS_INCREMENTAL_BACKUP
Couple of minor comments, but overall the series makes sense (even if
it's rough to see a regression in support, I agree with the comment that
you can at least override it with custom xml namespace, and we ARE
working towards re-enabling things as soon as possible).
Reviewed-by: Eric Blake <eblake(a)redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
1919
days inactive
1920
days old
15 comments
2 participants
participants (2)
-
Eric Blake -
Peter Krempa