7:43 a.m.
Peter Krempa (4):
qemu: Always save status XML in qemuDomainBlockJobAbort
qemu: Move shareable disk check for block copy
qemu: Remove unused 'cfg' qemuDomainBlockPivot
qemu: Use data in qemuBlockJobDataPtr instead of re-generating job
name
src/qemu/qemu_driver.c | 46 +++++++++++++++---------------------------
1 file changed, 16 insertions(+), 30 deletions(-)
--
2.20.1
7:43 a.m.
New subject: [libvirt] [PATCH 1/4] qemu: Always save status XML in qemuDomainBlockJobAbort
For copy and active commit jobs we record the state of the mirror so
that we can recover. The status XML was not saved in case of
qemuDomainBlockPivot due to an oversight.
Save the XML always when invoking qemuDomainBlockJobAbort even if
the job is not currently tracking any state. This will change later and
also this is not a particularly hot code path.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_driver.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2485dd93eb..e422a44b2e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17311,7 +17311,6 @@ qemuDomainBlockJobAbort(virDomainPtr dom,
char *device = NULL;
virDomainDiskDefPtr disk = NULL;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
- bool save = false;
bool pivot = !!(flags & VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT);
bool async = !!(flags & VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC);
qemuBlockJobDataPtr job = NULL;
@@ -17363,10 +17362,8 @@ qemuDomainBlockJobAbort(virDomainPtr dom,
if ((ret = qemuDomainBlockPivot(driver, vm, device, disk)) < 0)
goto endjob;
} else {
- if (disk->mirror) {
+ if (disk->mirror)
disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_ABORT;
- save = true;
- }
qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorBlockJobCancel(qemuDomainGetMonitor(vm), device);
@@ -17382,11 +17379,7 @@ qemuDomainBlockJobAbort(virDomainPtr dom,
}
}
- /* If we have made changes to XML due to a copy job, make a best
- * effort to save it now. But we can ignore failure, since there
- * will be further changes when the event marks completion. */
- if (save)
- ignore_value(virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm,
driver->caps));
+ ignore_value(virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm,
driver->caps));
/*
* With the ABORT_ASYNC flag we don't need to do anything, the event will
--
2.20.1
5:30 a.m.
New subject: [libvirt] [PATCH 1/4] qemu: Always save status XML in qemuDomainBlockJobAbort
On Thu, Jan 31, 2019 at 02:43:47PM +0100, Peter Krempa wrote:
For copy and active commit jobs we record the state of the mirror so
that we can recover. The status XML was not saved in case of
qemuDomainBlockPivot due to an oversight.
Save the XML always when invoking qemuDomainBlockJobAbort even if
the job is not currently tracking any state. This will change later and
also this is not a particularly hot code path.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_driver.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
7:43 a.m.
New subject: [libvirt] [PATCH 2/4] qemu: Move shareable disk check for block copy
The writing to an image actually starts when the copy job is initiated,
so checking this at the time of the pivot operation is too late.
Move the check to qemuDomainBlockCopyCommon. Note that modern qemu would
have prevented two writers with qcow2 so the slim possibility of a job
started with libvirtd without this patch missing the check is not really
worth worrying about.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_driver.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e422a44b2e..6674db7d20 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17140,16 +17140,6 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver,
goto cleanup;
}
- /* When pivoting to a shareable disk we need to make sure that the disk can
- * be safely shared, since block copy might have changed the format. */
- if (disk->src->shared && !disk->src->readonly &&
- !qemuBlockStorageSourceSupportsConcurrentAccess(disk->mirror)) {
- virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
- _("can't pivot a shared disk to a storage volume not
"
- "supporting sharing"));
- goto cleanup;
- }
-
/* Attempt the pivot. Record the attempt now, to prevent duplicate
* attempts; but the actual disk change will be made when emitting
* the event.
@@ -17780,6 +17770,16 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
}
}
+ /* When copying a shareable disk we need to make sure that the disk can
+ * be safely shared, since block copy may change the format. */
+ if (disk->src->shared && !disk->src->readonly &&
+ !qemuBlockStorageSourceSupportsConcurrentAccess(mirror)) {
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("can't pivot a shared disk to a storage volume not
"
+ "supporting sharing"));
+ goto endjob;
+ }
+
/* pre-create the image file */
if (!reuse) {
if (virStorageFileCreate(mirror) < 0) {
--
2.20.1
6:58 a.m.
New subject: [libvirt] [PATCH 2/4] qemu: Move shareable disk check for block copy
On Thu, Jan 31, 2019 at 02:43:48PM +0100, Peter Krempa wrote:
The writing to an image actually starts when the copy job is
initiated,
so checking this at the time of the pivot operation is too late.
Move the check to qemuDomainBlockCopyCommon. Note that modern qemu would
have prevented two writers with qcow2 so the slim possibility of a job
started with libvirtd without this patch missing the check is not really
worth worrying about.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_driver.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
7:43 a.m.
New subject: [libvirt] [PATCH 3/4] qemu: Remove unused 'cfg' qemuDomainBlockPivot
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_driver.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6674db7d20..c80d126276 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17124,7 +17124,6 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver,
{
int ret = -1;
qemuDomainObjPrivatePtr priv = vm->privateData;
- virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
if (!disk->mirror) {
virReportError(VIR_ERR_OPERATION_INVALID,
@@ -17165,7 +17164,6 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver,
}
cleanup:
- virObjectUnref(cfg);
return ret;
}
--
2.20.1
6:59 a.m.
New subject: [libvirt] [PATCH 3/4] qemu: Remove unused 'cfg' qemuDomainBlockPivot
On Thu, Jan 31, 2019 at 02:43:49PM +0100, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_driver.c | 2 --
1 file changed, 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
7:43 a.m.
New subject: [libvirt] [PATCH 4/4] qemu: Use data in qemuBlockJobDataPtr instead of re-generating job name
qemuDomainBlockPivot and qemuDomainBlockJobAbort need the job name for
cancelling or pivoting but were generating it locally instead of
accessing the existing copy in the job data structure.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_driver.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c80d126276..f790ffdf36 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17119,7 +17119,7 @@ qemuDomainOpenChannel(virDomainPtr dom,
static int
qemuDomainBlockPivot(virQEMUDriverPtr driver,
virDomainObjPtr vm,
- const char *device,
+ qemuBlockJobDataPtr job,
virDomainDiskDefPtr disk)
{
int ret = -1;
@@ -17151,7 +17151,7 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver,
* overall return value. */
disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_PIVOT;
qemuDomainObjEnterMonitor(driver, vm);
- ret = qemuMonitorDrivePivot(priv->mon, device);
+ ret = qemuMonitorDrivePivot(priv->mon, job->name);
if (qemuDomainObjExitMonitor(driver, vm) < 0) {
ret = -1;
goto cleanup;
@@ -17296,7 +17296,6 @@ qemuDomainBlockJobAbort(virDomainPtr dom,
unsigned int flags)
{
virQEMUDriverPtr driver = dom->conn->privateData;
- char *device = NULL;
virDomainDiskDefPtr disk = NULL;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
bool pivot = !!(flags & VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT);
@@ -17326,9 +17325,6 @@ qemuDomainBlockJobAbort(virDomainPtr dom,
if (!(disk = qemuDomainDiskByName(vm->def, path)))
goto endjob;
- if (!(device = qemuAliasDiskDriveFromDisk(disk)))
- goto endjob;
-
if (!(job = qemuBlockJobDiskGetJob(disk))) {
virReportError(VIR_ERR_INVALID_ARG,
_("disk %s does not have an active block job"),
disk->dst);
@@ -17347,14 +17343,14 @@ qemuDomainBlockJobAbort(virDomainPtr dom,
qemuBlockJobSyncBegin(job);
if (pivot) {
- if ((ret = qemuDomainBlockPivot(driver, vm, device, disk)) < 0)
+ if ((ret = qemuDomainBlockPivot(driver, vm, job, disk)) < 0)
goto endjob;
} else {
if (disk->mirror)
disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_ABORT;
qemuDomainObjEnterMonitor(driver, vm);
- ret = qemuMonitorBlockJobCancel(qemuDomainGetMonitor(vm), device);
+ ret = qemuMonitorBlockJobCancel(qemuDomainGetMonitor(vm), job->name);
if (qemuDomainObjExitMonitor(driver, vm) < 0) {
ret = -1;
goto endjob;
@@ -17394,7 +17390,6 @@ qemuDomainBlockJobAbort(virDomainPtr dom,
cleanup:
virObjectUnref(job);
virObjectUnref(cfg);
- VIR_FREE(device);
virDomainObjEndAPI(&vm);
return ret;
}
--
2.20.1
6:59 a.m.
New subject: [libvirt] [PATCH 4/4] qemu: Use data in qemuBlockJobDataPtr instead of re-generating job name
On Thu, Jan 31, 2019 at 02:43:50PM +0100, Peter Krempa wrote:
qemuDomainBlockPivot and qemuDomainBlockJobAbort need the job name
for
cancelling or pivoting but were generating it locally instead of
accessing the existing copy in the job data structure.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_driver.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
2080
days inactive
2087
days old
8 comments
2 participants
participants (2)
-
Ján Tomko -
Peter Krempa