5:59 a.m.
This is basically just rebase of [1] as it was not get any attention at that
time.
[1] [PATCH 0/8] qemu: support renaming domains with snapshots/checkpoints
https://www.redhat.com/archives/libvir-list/2020-March/msg00018.html
Nikolay Shirokovskiy (10):
qemu: qemuDomainRenameCallback: fix sending false undefined event
qemu: rename: send events only on success
qemu: rename: return instead of goto if no cleanup required
qemu: remove duplicate code for removing remnant files
qemu: rename: support renaming snapshots directory
qemu: rename: support renaming checkpoints directory
qemu: update name on reverting from snapshot
qemu: rename: remove snapshot/checkpoint restriction
qemu: qemuDomainDefineXMLFlags: move cleanup logic to cleanup section
qemu: remove possible garbage left from previous rename/undefine
src/qemu/qemu_checkpoint.c | 2 +-
src/qemu/qemu_checkpoint.h | 6 ++
src/qemu/qemu_domain.c | 43 ++++++++++++
src/qemu/qemu_domain.h | 5 ++
src/qemu/qemu_driver.c | 158 ++++++++++++++++++++++++++-------------------
src/qemu/qemu_migration.c | 3 +
src/qemu/qemu_snapshot.c | 10 +++
7 files changed, 160 insertions(+), 67 deletions(-)
--
1.8.3.1
5:59 a.m.
New subject: [PATCH v2 01/10] qemu: qemuDomainRenameCallback: fix sending false undefined event
For example if saving config file with new name fails we send false undefine
event currently.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/qemu/qemu_driver.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index cb56fbb..2f2a55f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19152,10 +19152,6 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
}
}
- event_old = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_UNDEFINED,
- VIR_DOMAIN_EVENT_UNDEFINED_RENAMED);
-
/* Switch name in domain definition. */
old_dom_name = vm->def->name;
vm->def->name = new_dom_name;
@@ -19182,6 +19178,9 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
}
}
+ event_old = virDomainEventLifecycleNew(vm->def->id, old_dom_name,
vm->def->uuid,
+ VIR_DOMAIN_EVENT_UNDEFINED,
+ VIR_DOMAIN_EVENT_UNDEFINED_RENAMED);
event_new = virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_DEFINED,
VIR_DOMAIN_EVENT_DEFINED_RENAMED);
--
1.8.3.1
6 a.m.
New subject: [PATCH v2 02/10] qemu: rename: send events only on success
We can simplify cleanup section by moving sending events to success path only
because only on sucess path events are not NULL.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/qemu/qemu_driver.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2f2a55f..22df0f6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19184,11 +19184,11 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
event_new = virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_DEFINED,
VIR_DOMAIN_EVENT_DEFINED_RENAMED);
+ virObjectEventStateQueue(driver->domainEventState, event_old);
+ virObjectEventStateQueue(driver->domainEventState, event_new);
ret = 0;
cleanup:
- virObjectEventStateQueue(driver->domainEventState, event_old);
- virObjectEventStateQueue(driver->domainEventState, event_new);
return ret;
rollback:
--
1.8.3.1
6 a.m.
New subject: [PATCH v2 03/10] qemu: rename: return instead of goto if no cleanup required
Going to cleanup label is mere return -1 thus let's just return
instead of goto to this label.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/qemu/qemu_driver.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 22df0f6..621facc 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19135,20 +19135,20 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
new_dom_name)) ||
!(old_dom_cfg_file = virDomainConfigFile(cfg->configDir,
vm->def->name)))
- goto cleanup;
+ return -1;
if (vm->autostart) {
if (!(new_dom_autostart_link = virDomainConfigFile(cfg->autostartDir,
new_dom_name)) ||
!(old_dom_autostart_link = virDomainConfigFile(cfg->autostartDir,
vm->def->name)))
- goto cleanup;
+ return -1;
if (symlink(new_dom_cfg_file, new_dom_autostart_link) < 0) {
virReportSystemError(errno,
_("Failed to create symlink '%s to
'%s'"),
new_dom_autostart_link, new_dom_cfg_file);
- goto cleanup;
+ return -1;
}
}
--
1.8.3.1
6 a.m.
New subject: [PATCH v2 04/10] qemu: remove duplicate code for removing remnant files
This patch also changes functionality a bit.
First if unlinking of old config file is failed we rollback and return error
previously and now we return success. I don't think this makes much difference.
I guess in both cases on libvirtd restart we have to deal with both new and old
config existing on disk with different names but same uuid.
Second if unlinking of old autolink is failed we rollback previously which
was not right as at this point we already unlink old config file. So this
is fixed now.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/qemu/qemu_domain.c | 29 +++++++++++++++++++++++++++++
src/qemu/qemu_domain.h | 5 +++++
src/qemu/qemu_driver.c | 39 ++++++++-------------------------------
3 files changed, 42 insertions(+), 31 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d7dbca4..7b2c165 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -11094,3 +11094,32 @@ qemuDomainInterfaceSetDefaultQDisc(virQEMUDriverPtr driver,
return 0;
}
+
+
+int
+qemuDomainNamePathsCleanup(virQEMUDriverConfigPtr cfg,
+ const char *name,
+ bool bestEffort)
+{
+ g_autofree char *cfg_file = NULL;
+ g_autofree char *autostart_link = NULL;
+
+ cfg_file = virDomainConfigFile(cfg->configDir, name);
+ autostart_link = virDomainConfigFile(cfg->autostartDir, name);
+
+ if (virFileExists(cfg_file) &&
+ unlink(cfg_file) < 0) {
+ virReportSystemError(errno, _("Failed to unlink '%s'"),
cfg_file);
+ if (!bestEffort)
+ return -1;
+ }
+
+ if (virFileIsLink(autostart_link) == 1 &&
+ unlink(autostart_link) < 0) {
+ virReportSystemError(errno, _("Failed to unlink '%s'"),
autostart_link);
+ if (!bestEffort)
+ return -1;
+ }
+
+ return 0;
+}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index fc69678..a2dfe86 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -1050,3 +1050,8 @@ qemuDomainFileWrapperFDClose(virDomainObjPtr vm,
int
qemuDomainInterfaceSetDefaultQDisc(virQEMUDriverPtr driver,
virDomainNetDefPtr net);
+
+int
+qemuDomainNamePathsCleanup(virQEMUDriverConfigPtr cfg,
+ const char *name,
+ bool bestEffort);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 621facc..52c42cf 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19112,6 +19112,7 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
virObjectEventPtr event_new = NULL;
virObjectEventPtr event_old = NULL;
int ret = -1;
+ virErrorPtr err = NULL;
g_autofree char *new_dom_name = NULL;
g_autofree char *old_dom_name = NULL;
g_autofree char *new_dom_cfg_file = NULL;
@@ -19158,25 +19159,7 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
new_dom_name = NULL;
if (virDomainDefSave(vm->def, driver->xmlopt, cfg->configDir) < 0)
- goto rollback;
-
- if (virFileExists(old_dom_cfg_file) &&
- unlink(old_dom_cfg_file) < 0) {
- virReportSystemError(errno,
- _("cannot remove old domain config file %s"),
- old_dom_cfg_file);
- goto rollback;
- }
-
- if (vm->autostart) {
- if (virFileIsLink(old_dom_autostart_link) &&
- unlink(old_dom_autostart_link) < 0) {
- virReportSystemError(errno,
- _("Failed to delete symlink '%s'"),
- old_dom_autostart_link);
- goto rollback;
- }
- }
+ goto cleanup;
event_old = virDomainEventLifecycleNew(vm->def->id, old_dom_name,
vm->def->uuid,
VIR_DOMAIN_EVENT_UNDEFINED,
@@ -19189,23 +19172,17 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
ret = 0;
cleanup:
- return ret;
-
- rollback:
- if (old_dom_name) {
+ if (old_dom_name && ret < 0) {
new_dom_name = vm->def->name;
vm->def->name = old_dom_name;
old_dom_name = NULL;
}
- if (virFileExists(new_dom_cfg_file))
- unlink(new_dom_cfg_file);
-
- if (vm->autostart &&
- virFileExists(new_dom_autostart_link))
- unlink(new_dom_autostart_link);
-
- goto cleanup;
+ if (ret < 0)
+ virErrorPreserveLast(&err);
+ qemuDomainNamePathsCleanup(cfg, ret < 0 ? new_dom_name : old_dom_name, true);
+ virErrorRestore(&err);
+ return ret;
}
static int qemuDomainRename(virDomainPtr dom,
--
1.8.3.1
6 a.m.
New subject: [PATCH v2 05/10] qemu: rename: support renaming snapshots directory
This is basically just saves snapshots metadata on disk after name is changed
in memory as path to domain snapshot directory depends on name. After that
old snapshot directory is deleted with snapshot metadata files.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/qemu/qemu_domain.c | 7 +++++++
src/qemu/qemu_driver.c | 33 +++++++++++++++++++++++++++++++++
2 files changed, 40 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 7b2c165..258c8c5 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -11103,9 +11103,11 @@ qemuDomainNamePathsCleanup(virQEMUDriverConfigPtr cfg,
{
g_autofree char *cfg_file = NULL;
g_autofree char *autostart_link = NULL;
+ g_autofree char *snap_dir = NULL;
cfg_file = virDomainConfigFile(cfg->configDir, name);
autostart_link = virDomainConfigFile(cfg->autostartDir, name);
+ snap_dir = g_strdup_printf("%s/%s", cfg->snapshotDir, name);
if (virFileExists(cfg_file) &&
unlink(cfg_file) < 0) {
@@ -11121,5 +11123,10 @@ qemuDomainNamePathsCleanup(virQEMUDriverConfigPtr cfg,
return -1;
}
+ if (virFileIsDir(snap_dir) &&
+ virFileDeleteTree(snap_dir) < 0 &&
+ !bestEffort)
+ return -1;
+
return 0;
}
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 52c42cf..2664603 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19101,6 +19101,30 @@ qemuDomainSetUserPassword(virDomainPtr dom,
}
+struct qemuDomainMomentWriteMetadataData {
+ virQEMUDriverPtr driver;
+ virDomainObjPtr vm;
+};
+
+
+static int
+qemuDomainSnapshotWriteMetadataIter(void *payload,
+ const char *name G_GNUC_UNUSED,
+ void *opaque)
+{
+ struct qemuDomainMomentWriteMetadataData *data = opaque;
+ virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(data->driver);
+ int ret;
+
+ ret = qemuDomainSnapshotWriteMetadata(data->vm, payload,
+ data->driver->xmlopt,
+ cfg->snapshotDir);
+
+ virObjectUnref(cfg);
+ return ret;
+}
+
+
static int
qemuDomainRenameCallback(virDomainObjPtr vm,
const char *new_name,
@@ -19119,6 +19143,10 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
g_autofree char *old_dom_cfg_file = NULL;
g_autofree char *new_dom_autostart_link = NULL;
g_autofree char *old_dom_autostart_link = NULL;
+ struct qemuDomainMomentWriteMetadataData data = {
+ .driver = driver,
+ .vm = vm,
+ };
virCheckFlags(0, ret);
@@ -19158,6 +19186,11 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
vm->def->name = new_dom_name;
new_dom_name = NULL;
+ if (virDomainSnapshotForEach(vm->snapshots,
+ qemuDomainSnapshotWriteMetadataIter,
+ &data) < 0)
+ goto cleanup;
+
if (virDomainDefSave(vm->def, driver->xmlopt, cfg->configDir) < 0)
goto cleanup;
--
1.8.3.1
7:42 a.m.
New subject: [PATCH v2 05/10] qemu: rename: support renaming snapshots directory
On 11/3/20 9:00 AM, Nikolay Shirokovskiy wrote:
This is basically just saves snapshots metadata on disk after name is
changed
Extra 'is'. "This basically just saves ...."
in memory as path to domain snapshot directory depends on name. After
that
old snapshot directory is deleted with snapshot metadata files.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/qemu/qemu_domain.c | 7 +++++++
src/qemu/qemu_driver.c | 33 +++++++++++++++++++++++++++++++++
2 files changed, 40 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 7b2c165..258c8c5 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -11103,9 +11103,11 @@ qemuDomainNamePathsCleanup(virQEMUDriverConfigPtr cfg,
{
g_autofree char *cfg_file = NULL;
g_autofree char *autostart_link = NULL;
+ g_autofree char *snap_dir = NULL;
cfg_file = virDomainConfigFile(cfg->configDir, name);
autostart_link = virDomainConfigFile(cfg->autostartDir, name);
+ snap_dir = g_strdup_printf("%s/%s", cfg->snapshotDir, name);
if (virFileExists(cfg_file) &&
unlink(cfg_file) < 0) {
@@ -11121,5 +11123,10 @@ qemuDomainNamePathsCleanup(virQEMUDriverConfigPtr cfg,
return -1;
}
+ if (virFileIsDir(snap_dir) &&
+ virFileDeleteTree(snap_dir) < 0 &&
+ !bestEffort)
+ return -1;
+
return 0;
}
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 52c42cf..2664603 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19101,6 +19101,30 @@ qemuDomainSetUserPassword(virDomainPtr dom,
}
+struct qemuDomainMomentWriteMetadataData {
+ virQEMUDriverPtr driver;
+ virDomainObjPtr vm;
+};
+
+
+static int
+qemuDomainSnapshotWriteMetadataIter(void *payload,
+ const char *name G_GNUC_UNUSED,
+ void *opaque)
+{
+ struct qemuDomainMomentWriteMetadataData *data = opaque;
+ virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(data->driver);
+ int ret;
+
+ ret = qemuDomainSnapshotWriteMetadata(data->vm, payload,
+ data->driver->xmlopt,
+ cfg->snapshotDir);
+
+ virObjectUnref(cfg);
+ return ret;
+}
+
+
static int
qemuDomainRenameCallback(virDomainObjPtr vm,
const char *new_name,
@@ -19119,6 +19143,10 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
g_autofree char *old_dom_cfg_file = NULL;
g_autofree char *new_dom_autostart_link = NULL;
g_autofree char *old_dom_autostart_link = NULL;
+ struct qemuDomainMomentWriteMetadataData data = {
+ .driver = driver,
+ .vm = vm,
+ };
virCheckFlags(0, ret);
@@ -19158,6 +19186,11 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
vm->def->name = new_dom_name;
new_dom_name = NULL;
+ if (virDomainSnapshotForEach(vm->snapshots,
+ qemuDomainSnapshotWriteMetadataIter,
+ &data) < 0)
+ goto cleanup;
+
if (virDomainDefSave(vm->def, driver->xmlopt, cfg->configDir) < 0)
goto cleanup;
6 a.m.
New subject: [PATCH v2 06/10] qemu: rename: support renaming checkpoints directory
This is basically just saves checkpoints metadata on disk after name is changed
in memory as path to domain checkpoints directory depends on name. After that
old checkpoint directory is deleted with checkpoint metadata files.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/qemu/qemu_checkpoint.c | 2 +-
src/qemu/qemu_checkpoint.h | 6 ++++++
src/qemu/qemu_domain.c | 7 +++++++
src/qemu/qemu_driver.c | 23 +++++++++++++++++++++++
4 files changed, 37 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
index fb76c21..4a496d5 100644
--- a/src/qemu/qemu_checkpoint.c
+++ b/src/qemu/qemu_checkpoint.c
@@ -76,7 +76,7 @@ qemuCheckpointObjFromCheckpoint(virDomainObjPtr vm,
}
-static int
+int
qemuCheckpointWriteMetadata(virDomainObjPtr vm,
virDomainMomentObjPtr checkpoint,
virDomainXMLOptionPtr xmlopt,
diff --git a/src/qemu/qemu_checkpoint.h b/src/qemu/qemu_checkpoint.h
index 0d267a1..d58657e 100644
--- a/src/qemu/qemu_checkpoint.h
+++ b/src/qemu/qemu_checkpoint.h
@@ -79,3 +79,9 @@ qemuCheckpointDiscardDiskBitmaps(virStorageSourcePtr src,
virJSONValuePtr actions,
const char *diskdst,
GSList **reopenimages);
+
+int
+qemuCheckpointWriteMetadata(virDomainObjPtr vm,
+ virDomainMomentObjPtr checkpoint,
+ virDomainXMLOptionPtr xmlopt,
+ const char *checkpointDir);
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 258c8c5..422b534 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -11104,10 +11104,12 @@ qemuDomainNamePathsCleanup(virQEMUDriverConfigPtr cfg,
g_autofree char *cfg_file = NULL;
g_autofree char *autostart_link = NULL;
g_autofree char *snap_dir = NULL;
+ g_autofree char *chk_dir = NULL;
cfg_file = virDomainConfigFile(cfg->configDir, name);
autostart_link = virDomainConfigFile(cfg->autostartDir, name);
snap_dir = g_strdup_printf("%s/%s", cfg->snapshotDir, name);
+ chk_dir = g_strdup_printf("%s/%s", cfg->checkpointDir, name);
if (virFileExists(cfg_file) &&
unlink(cfg_file) < 0) {
@@ -11128,5 +11130,10 @@ qemuDomainNamePathsCleanup(virQEMUDriverConfigPtr cfg,
!bestEffort)
return -1;
+ if (virFileIsDir(chk_dir) &&
+ virFileDeleteTree(chk_dir) < 0 &&
+ !bestEffort)
+ return -1;
+
return 0;
}
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2664603..4e0186b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19126,6 +19126,24 @@ qemuDomainSnapshotWriteMetadataIter(void *payload,
static int
+qemuDomainCheckpointWriteMetadataIter(void *payload,
+ const char *name G_GNUC_UNUSED,
+ void *opaque)
+{
+ struct qemuDomainMomentWriteMetadataData *data = opaque;
+ virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(data->driver);
+ int ret;
+
+ ret = qemuCheckpointWriteMetadata(data->vm, payload,
+ data->driver->xmlopt,
+ cfg->snapshotDir);
+
+ virObjectUnref(cfg);
+ return ret;
+}
+
+
+static int
qemuDomainRenameCallback(virDomainObjPtr vm,
const char *new_name,
unsigned int flags,
@@ -19191,6 +19209,11 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
&data) < 0)
goto cleanup;
+ if (virDomainCheckpointForEach(vm->checkpoints,
+ qemuDomainCheckpointWriteMetadataIter,
+ &data) < 0)
+ goto cleanup;
+
if (virDomainDefSave(vm->def, driver->xmlopt, cfg->configDir) < 0)
goto cleanup;
--
1.8.3.1
7:44 a.m.
New subject: [PATCH v2 06/10] qemu: rename: support renaming checkpoints directory
On 11/3/20 9:00 AM, Nikolay Shirokovskiy wrote:
This is basically just saves checkpoints metadata on disk after name
is changed
"This basically just saves ..."
in memory as path to domain checkpoints directory depends on name.
After that
old checkpoint directory is deleted with checkpoint metadata files.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/qemu/qemu_checkpoint.c | 2 +-
src/qemu/qemu_checkpoint.h | 6 ++++++
src/qemu/qemu_domain.c | 7 +++++++
src/qemu/qemu_driver.c | 23 +++++++++++++++++++++++
4 files changed, 37 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
index fb76c21..4a496d5 100644
--- a/src/qemu/qemu_checkpoint.c
+++ b/src/qemu/qemu_checkpoint.c
@@ -76,7 +76,7 @@ qemuCheckpointObjFromCheckpoint(virDomainObjPtr vm,
}
-static int
+int
qemuCheckpointWriteMetadata(virDomainObjPtr vm,
virDomainMomentObjPtr checkpoint,
virDomainXMLOptionPtr xmlopt,
diff --git a/src/qemu/qemu_checkpoint.h b/src/qemu/qemu_checkpoint.h
index 0d267a1..d58657e 100644
--- a/src/qemu/qemu_checkpoint.h
+++ b/src/qemu/qemu_checkpoint.h
@@ -79,3 +79,9 @@ qemuCheckpointDiscardDiskBitmaps(virStorageSourcePtr src,
virJSONValuePtr actions,
const char *diskdst,
GSList **reopenimages);
+
+int
+qemuCheckpointWriteMetadata(virDomainObjPtr vm,
+ virDomainMomentObjPtr checkpoint,
+ virDomainXMLOptionPtr xmlopt,
+ const char *checkpointDir);
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 258c8c5..422b534 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -11104,10 +11104,12 @@ qemuDomainNamePathsCleanup(virQEMUDriverConfigPtr cfg,
g_autofree char *cfg_file = NULL;
g_autofree char *autostart_link = NULL;
g_autofree char *snap_dir = NULL;
+ g_autofree char *chk_dir = NULL;
cfg_file = virDomainConfigFile(cfg->configDir, name);
autostart_link = virDomainConfigFile(cfg->autostartDir, name);
snap_dir = g_strdup_printf("%s/%s", cfg->snapshotDir, name);
+ chk_dir = g_strdup_printf("%s/%s", cfg->checkpointDir, name);
if (virFileExists(cfg_file) &&
unlink(cfg_file) < 0) {
@@ -11128,5 +11130,10 @@ qemuDomainNamePathsCleanup(virQEMUDriverConfigPtr cfg,
!bestEffort)
return -1;
+ if (virFileIsDir(chk_dir) &&
+ virFileDeleteTree(chk_dir) < 0 &&
+ !bestEffort)
+ return -1;
+
return 0;
}
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2664603..4e0186b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19126,6 +19126,24 @@ qemuDomainSnapshotWriteMetadataIter(void *payload,
static int
+qemuDomainCheckpointWriteMetadataIter(void *payload,
+ const char *name G_GNUC_UNUSED,
+ void *opaque)
+{
+ struct qemuDomainMomentWriteMetadataData *data = opaque;
+ virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(data->driver);
+ int ret;
+
+ ret = qemuCheckpointWriteMetadata(data->vm, payload,
+ data->driver->xmlopt,
+ cfg->snapshotDir);
+
+ virObjectUnref(cfg);
+ return ret;
+}
+
+
+static int
qemuDomainRenameCallback(virDomainObjPtr vm,
const char *new_name,
unsigned int flags,
@@ -19191,6 +19209,11 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
&data) < 0)
goto cleanup;
+ if (virDomainCheckpointForEach(vm->checkpoints,
+ qemuDomainCheckpointWriteMetadataIter,
+ &data) < 0)
+ goto cleanup;
+
if (virDomainDefSave(vm->def, driver->xmlopt, cfg->configDir) < 0)
goto cleanup;
6 a.m.
New subject: [PATCH v2 07/10] qemu: update name on reverting from snapshot
If domain name is changed since snapshot we need to update it to current in
config taken from snapshot.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/qemu/qemu_snapshot.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index a6241ab..c9f2be0 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -1924,6 +1924,11 @@ qemuSnapshotRevert(virDomainObjPtr vm,
driver->xmlopt, priv->qemuCaps, true);
if (!config)
goto endjob;
+
+ if (STRNEQ(config->name, vm->def->name)) {
+ VIR_FREE(config->name);
+ config->name = g_strdup(vm->def->name);
+ }
}
if (snap->def->inactiveDom) {
@@ -1931,6 +1936,11 @@ qemuSnapshotRevert(virDomainObjPtr vm,
driver->xmlopt, priv->qemuCaps, true);
if (!inactiveConfig)
goto endjob;
+
+ if (STRNEQ(inactiveConfig->name, vm->def->name)) {
+ VIR_FREE(inactiveConfig->name);
+ inactiveConfig->name = g_strdup(vm->def->name);
+ }
} else {
/* Inactive domain definition is missing:
* - either this is an old active snapshot and we need to copy the
--
1.8.3.1
6 a.m.
New subject: [PATCH v2 08/10] qemu: rename: remove snapshot/checkpoint restriction
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/qemu/qemu_driver.c | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4e0186b..86ff74f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19284,18 +19284,6 @@ static int qemuDomainRename(virDomainPtr dom,
goto endjob;
}
- if (virDomainSnapshotObjListNum(vm->snapshots, NULL, 0) > 0) {
- virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("cannot rename domain with
snapshots"));
- goto endjob;
- }
-
- if (virDomainListCheckpoints(vm->checkpoints, NULL, dom, NULL, flags) > 0) {
- virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("cannot rename domain with
checkpoints"));
- goto endjob;
- }
-
if (virDomainObjListRename(driver->domains, vm, new_name, flags,
qemuDomainRenameCallback, driver) < 0)
goto endjob;
--
1.8.3.1
6 a.m.
New subject: [PATCH v2 09/10] qemu: qemuDomainDefineXMLFlags: move cleanup logic to cleanup section
Let's move objlist restoring to cleanup section so that we can handle failure
of actions between virDomainObjListAdd and virDomainDefSave. We are going
to add such actions in next patch.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/qemu/qemu_driver.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 86ff74f..6c353a5 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6705,7 +6705,7 @@ qemuDomainDefineXMLFlags(virConnectPtr conn,
if (!(def = virDomainDefParseString(xml, driver->xmlopt,
NULL, parse_flags)))
- goto cleanup;
+ return NULL;
if (virXMLCheckIllegalChars("name", def->name, "\n") < 0)
goto cleanup;
@@ -6719,10 +6719,23 @@ qemuDomainDefineXMLFlags(virConnectPtr conn,
goto cleanup;
def = NULL;
+ if (virDomainDefSave(vm->newDef ? vm->newDef : vm->def,
+ driver->xmlopt, cfg->configDir) < 0)
+ goto cleanup;
+
vm->persistent = 1;
- if (virDomainDefSave(vm->newDef ? vm->newDef : vm->def,
- driver->xmlopt, cfg->configDir) < 0) {
+ event = virDomainEventLifecycleNewFromObj(vm,
+ VIR_DOMAIN_EVENT_DEFINED,
+ !oldDef ?
+ VIR_DOMAIN_EVENT_DEFINED_ADDED :
+ VIR_DOMAIN_EVENT_DEFINED_UPDATED);
+
+ VIR_INFO("Creating domain '%s'", vm->def->name);
+ dom = virGetDomain(conn, vm->def->name, vm->def->uuid,
vm->def->id);
+
+ cleanup:
+ if (!dom && !def) {
if (oldDef) {
/* There is backup so this VM was defined before.
* Just restore the backup. */
@@ -6735,22 +6748,9 @@ qemuDomainDefineXMLFlags(virConnectPtr conn,
} else {
/* Brand new domain. Remove it */
VIR_INFO("Deleting domain '%s'", vm->def->name);
- vm->persistent = 0;
qemuDomainRemoveInactiveJob(driver, vm);
}
- goto cleanup;
}
-
- event = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_DEFINED,
- !oldDef ?
- VIR_DOMAIN_EVENT_DEFINED_ADDED :
- VIR_DOMAIN_EVENT_DEFINED_UPDATED);
-
- VIR_INFO("Creating domain '%s'", vm->def->name);
- dom = virGetDomain(conn, vm->def->name, vm->def->uuid,
vm->def->id);
-
- cleanup:
virDomainDefFree(oldDef);
virDomainDefFree(def);
virDomainObjEndAPI(&vm);
--
1.8.3.1
6 a.m.
New subject: [PATCH v2 10/10] qemu: remove possible garbage left from previous rename/undefine
Due to failures to unlink on previous rename/undefine we can already have
autolink etc files for the domain to be defined. Remove them.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/qemu/qemu_driver.c | 6 ++++++
src/qemu/qemu_migration.c | 3 +++
2 files changed, 9 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6c353a5..3b7d1d7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6719,6 +6719,9 @@ qemuDomainDefineXMLFlags(virConnectPtr conn,
goto cleanup;
def = NULL;
+ if (!oldDef && qemuDomainNamePathsCleanup(cfg, vm->def->name, false)
< 0)
+ goto cleanup;
+
if (virDomainDefSave(vm->newDef ? vm->newDef : vm->def,
driver->xmlopt, cfg->configDir) < 0)
goto cleanup;
@@ -19184,6 +19187,9 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
vm->def->name)))
return -1;
+ if (qemuDomainNamePathsCleanup(cfg, new_name, false) < 0)
+ goto cleanup;
+
if (vm->autostart) {
if (!(new_dom_autostart_link = virDomainConfigFile(cfg->autostartDir,
new_dom_name)) ||
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 6f764b0..b4fb586 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5137,6 +5137,9 @@ qemuMigrationDstPersist(virQEMUDriverPtr driver,
priv->qemuCaps)))
goto error;
+ if (!oldDef && qemuDomainNamePathsCleanup(cfg, vmdef->name, false) <
0)
+ goto error;
+
if (virDomainDefSave(vmdef, driver->xmlopt, cfg->configDir) < 0 &&
!ignoreSaveError)
goto error;
--
1.8.3.1
7:51 a.m.
On 11/3/20 8:59 AM, Nikolay Shirokovskiy wrote:
This is basically just rebase of [1] as it was not get any attention
at that
time.
[1] [PATCH 0/8] qemu: support renaming domains with snapshots/checkpoints
https://www.redhat.com/archives/libvir-list/2020-March/msg00018.html
Code LGTM:
Reviewed-by: Daniel Henrique Barboza <danielhb413(a)gmail.com>
Shouldn't you add some test cases for this new behavior though? I'm a bit
nervous with pushing this upstream without any coverage.
Thanks,
DHB
Nikolay Shirokovskiy (10):
qemu: qemuDomainRenameCallback: fix sending false undefined event
qemu: rename: send events only on success
qemu: rename: return instead of goto if no cleanup required
qemu: remove duplicate code for removing remnant files
qemu: rename: support renaming snapshots directory
qemu: rename: support renaming checkpoints directory
qemu: update name on reverting from snapshot
qemu: rename: remove snapshot/checkpoint restriction
qemu: qemuDomainDefineXMLFlags: move cleanup logic to cleanup section
qemu: remove possible garbage left from previous rename/undefine
src/qemu/qemu_checkpoint.c | 2 +-
src/qemu/qemu_checkpoint.h | 6 ++
src/qemu/qemu_domain.c | 43 ++++++++++++
src/qemu/qemu_domain.h | 5 ++
src/qemu/qemu_driver.c | 158 ++++++++++++++++++++++++++-------------------
src/qemu/qemu_migration.c | 3 +
src/qemu/qemu_snapshot.c | 10 +++
7 files changed, 160 insertions(+), 67 deletions(-)
--
1.8.3.1
1:58 a.m.
On 09.11.2020 16:51, Daniel Henrique Barboza wrote:
On 11/3/20 8:59 AM, Nikolay Shirokovskiy wrote:
> This is basically just rebase of [1] as it was not get any attention at that
> time.
>
> [1] [PATCH 0/8] qemu: support renaming domains with snapshots/checkpoints
> https://www.redhat.com/archives/libvir-list/2020-March/msg00018.html
Code LGTM:
Reviewed-by: Daniel Henrique Barboza <danielhb413(a)gmail.com>
Shouldn't you add some test cases for this new behavior though? I'm a bit
nervous with pushing this upstream without any coverage.
So basically we need to test how qemuDomainRenameCallback works. Currently
there is no test for this function or virDomainRename. I spent some time
looking at existing tests that need to mock driver/vm objects and it looks like
it requires a good deal of effort in order to prepare the test in this case.
At the same time those tests has many inputs so it looks like worth heavy
preparation. In case of rename the code we test does not depend greatly on
inputs so may be it does not worth adding such test given heavy preparation we
have to do.
Of course I give the patch series some manual testing.
Nikolay
5:55 a.m.
On 10.11.2020 10:58, Nikolay Shirokovskiy wrote:
On 09.11.2020 16:51, Daniel Henrique Barboza wrote:
>
>
> On 11/3/20 8:59 AM, Nikolay Shirokovskiy wrote:
>> This is basically just rebase of [1] as it was not get any attention at that
>> time.
>>
>> [1] [PATCH 0/8] qemu: support renaming domains with snapshots/checkpoints
>> https://www.redhat.com/archives/libvir-list/2020-March/msg00018.html
>
> Code LGTM:
>
> Reviewed-by: Daniel Henrique Barboza <danielhb413(a)gmail.com>
>
>
> Shouldn't you add some test cases for this new behavior though? I'm a bit
> nervous with pushing this upstream without any coverage.
>
>
So basically we need to test how qemuDomainRenameCallback works. Currently
there is no test for this function or virDomainRename. I spent some time
looking at existing tests that need to mock driver/vm objects and it looks like
it requires a good deal of effort in order to prepare the test in this case.
At the same time those tests has many inputs so it looks like worth heavy
preparation. In case of rename the code we test does not depend greatly on
inputs so may be it does not worth adding such test given heavy preparation we
have to do.
Of course I give the patch series some manual testing.
Thanx for the review!
Pushed with next hunk squashed into the last patch:
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index c831ae6..fef0be6 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5137,7 +5137,7 @@ qemuMigrationDstPersist(virQEMUDriverPtr driver,
priv->qemuCaps)))
goto error;
- if (!oldDef && qemuDomainNamePathsCleanup(cfg, vmdef->name, false) < 0)
+ if (!oldPersist && qemuDomainNamePathsCleanup(cfg, vmdef->name, false)
< 0)
goto error;
if (virDomainDefSave(vmdef, driver->xmlopt, cfg->configDir) < 0 &&
Nikolay
1500
days inactive
1510
days old
15 comments
2 participants
participants (2)
-
Daniel Henrique Barboza -
Nikolay Shirokovskiy