9:14 a.m.
Improve error reporting when users select disk's to migrate but don't
enable non-shared storage migration.
Peter Krempa (4):
qemuMigrationSrcBeginPhase: Properly report error when non-shared
storage migration is requested over tunnel
qemuMigrationSrcBeginPhase: Require storage migration when
'migrate_disks' parameter is specified
virsh: doMigrate: Rework virsh option to migration flag conversion
virsh: doMigrate: Require --copy-storage-(all|inc) with
--migrate-disks
src/qemu/qemu_migration.c | 7 +++
tools/virsh-domain.c | 113 ++++++++++++++++----------------------
2 files changed, 54 insertions(+), 66 deletions(-)
--
2.41.0
9:14 a.m.
New subject: [PATCH 1/4] qemuMigrationSrcBeginPhase: Properly report error when non-shared storage migration is requested over tunnel
When VIR_MIGRATE_TUNNELLED is used without
VIR_MIGRATE_NON_SHARED_DISK/VIR_MIGRATE_NON_SHARED_INC
an error was reported without actually returning failure.
This was caused by a refactor which dropped many error paths.
Fixes: 6111b235224
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_migration.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index ed41a03851..a3fe6be4e9 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2617,6 +2617,7 @@ qemuMigrationSrcBeginPhase(virQEMUDriver *driver,
if (flags & VIR_MIGRATE_TUNNELLED) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("migration of non-shared storage is not supported with
tunnelled migration and this QEMU"));
+ return NULL;
}
if (nmigrate_disks) {
--
2.41.0
9:41 a.m.
New subject: [PATCH 1/4] qemuMigrationSrcBeginPhase: Properly report error when non-shared storage migration is requested over tunnel
On a Wednesday in 2023, Peter Krempa wrote:
When VIR_MIGRATE_TUNNELLED is used without
VIR_MIGRATE_NON_SHARED_DISK/VIR_MIGRATE_NON_SHARED_INC
an error was reported without actually returning failure.
This was caused by a refactor which dropped many error paths.
Fixes: 6111b235224
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_migration.c | 1 +
1 file changed, 1 insertion(+)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
9:14 a.m.
New subject: [PATCH 2/4] qemuMigrationSrcBeginPhase: Require storage migration when 'migrate_disks' parameter is specified
If a user passes a list of disks to migrate but don't actually use
'VIR_MIGRATE_NON_SHARED_DISK' or 'VIR_MIGRATE_NON_SHARED_INC' flags the
parameter would be simply ignored without informing the user of the
error.
Add a proper error in such case.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_migration.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index a3fe6be4e9..9057590c94 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2644,6 +2644,12 @@ qemuMigrationSrcBeginPhase(virQEMUDriver *driver,
migrate_disks,
nmigrate_disks))
cookieFlags |= QEMU_MIGRATION_COOKIE_NBD;
+ } else {
+ if (nmigrate_disks > 0) {
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("use of the 'VIR_MIGRATE_PARAM_MIGRATE_DISKS'
requires use of 'VIR_MIGRATE_NON_SHARED_DISK' or
'VIR_MIGRATE_NON_SHARED_INC' flag"));
+ return NULL;
+ }
}
if (virDomainDefHasMemoryHotplug(vm->def) ||
--
2.41.0
9:28 a.m.
New subject: [PATCH 2/4] qemuMigrationSrcBeginPhase: Require storage migration when 'migrate_disks' parameter is specified
On a Wednesday in 2023, Peter Krempa wrote:
If a user passes a list of disks to migrate but don't actually
use
'VIR_MIGRATE_NON_SHARED_DISK' or 'VIR_MIGRATE_NON_SHARED_INC' flags the
parameter would be simply ignored without informing the user of the
error.
Add a proper error in such case.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_migration.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index a3fe6be4e9..9057590c94 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2644,6 +2644,12 @@ qemuMigrationSrcBeginPhase(virQEMUDriver *driver,
migrate_disks,
nmigrate_disks))
cookieFlags |= QEMU_MIGRATION_COOKIE_NBD;
+ } else {
+ if (nmigrate_disks > 0) {
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("use of the 'VIR_MIGRATE_PARAM_MIGRATE_DISKS'
requires use of 'VIR_MIGRATE_NON_SHARED_DISK' or
'VIR_MIGRATE_NON_SHARED_INC' flag"));
d/the/
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
9:14 a.m.
New subject: [PATCH 3/4] virsh: doMigrate: Rework virsh option to migration flag conversion
Convert the flags declaratively as in the vast majority of cases it's a
simple binary addition if the flag exists.
In one instance there was also an additional check, which was moved up
after the new code, and the error message was fixed.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
tools/virsh-domain.c | 108 +++++++++++++++++--------------------------
1 file changed, 42 insertions(+), 66 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index f8758f18a3..84bf62057b 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11082,6 +11082,11 @@ static const vshCmdOptDef opts_migrate[] = {
{.name = NULL}
};
+struct doMigrateFlagMapping {
+ const char *optionname;
+ unsigned int migflag;
+};
+
static void
doMigrate(void *opaque)
{
@@ -11099,6 +11104,32 @@ doMigrate(void *opaque)
unsigned long long ullOpt = 0;
int rv;
virConnectPtr dconn = data->dconn;
+ size_t i;
+
+ static const struct doMigrateFlagMapping flagmap[] = {
+ { "live", VIR_MIGRATE_LIVE },
+ { "p2p", VIR_MIGRATE_PEER2PEER },
+ { "tunnelled", VIR_MIGRATE_TUNNELLED },
+ { "persistent", VIR_MIGRATE_PERSIST_DEST },
+ { "undefinesource", VIR_MIGRATE_UNDEFINE_SOURCE },
+ { "copy-storage-all", VIR_MIGRATE_NON_SHARED_DISK },
+ { "copy-storage-inc", VIR_MIGRATE_NON_SHARED_INC },
+ { "copy-storage-synchronous-writes",
VIR_MIGRATE_NON_SHARED_SYNCHRONOUS_WRITES },
+ { "change-protection", VIR_MIGRATE_CHANGE_PROTECTION },
+ { "unsafe", VIR_MIGRATE_UNSAFE },
+ { "compressed", VIR_MIGRATE_COMPRESSED },
+ { "auto-converge", VIR_MIGRATE_AUTO_CONVERGE },
+ { "rdma-pin-all", VIR_MIGRATE_RDMA_PIN_ALL },
+ { "offline", VIR_MIGRATE_OFFLINE },
+ { "abort-on-error", VIR_MIGRATE_ABORT_ON_ERROR },
+ { "postcopy", VIR_MIGRATE_POSTCOPY },
+ { "postcopy-resume", VIR_MIGRATE_POSTCOPY_RESUME },
+ { "zerocopy", VIR_MIGRATE_ZEROCOPY },
+ { "tls", VIR_MIGRATE_TLS },
+ { "parallel", VIR_MIGRATE_PARALLEL },
+ { "suspend", VIR_MIGRATE_PAUSED },
+ };
+
#ifndef WIN32
sigset_t sigmask, oldsigmask;
@@ -11108,6 +11139,17 @@ doMigrate(void *opaque)
goto out_sig;
#endif /* !WIN32 */
+ for (i = 0; i < G_N_ELEMENTS(flagmap); i++) {
+ if (vshCommandOptBool(cmd, flagmap[i].optionname))
+ flags |= flagmap[i].migflag;
+ }
+
+ if (flags & VIR_MIGRATE_NON_SHARED_SYNCHRONOUS_WRITES &&
+ !(flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_DISK))) {
+ vshError(ctl, "'--copy-storage-synchronous-writes' requires one of
'--copy-storage-all', '--copy-storage-inc'");
+ goto out;
+ }
+
if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
goto out;
@@ -11325,72 +11367,6 @@ doMigrate(void *opaque)
VIR_MIGRATE_PARAM_TLS_DESTINATION, opt) < 0)
goto save_error;
- if (vshCommandOptBool(cmd, "live"))
- flags |= VIR_MIGRATE_LIVE;
- if (vshCommandOptBool(cmd, "p2p"))
- flags |= VIR_MIGRATE_PEER2PEER;
- if (vshCommandOptBool(cmd, "tunnelled"))
- flags |= VIR_MIGRATE_TUNNELLED;
-
- if (vshCommandOptBool(cmd, "persistent"))
- flags |= VIR_MIGRATE_PERSIST_DEST;
- if (vshCommandOptBool(cmd, "undefinesource"))
- flags |= VIR_MIGRATE_UNDEFINE_SOURCE;
-
- if (vshCommandOptBool(cmd, "suspend"))
- flags |= VIR_MIGRATE_PAUSED;
-
- if (vshCommandOptBool(cmd, "copy-storage-all"))
- flags |= VIR_MIGRATE_NON_SHARED_DISK;
-
- if (vshCommandOptBool(cmd, "copy-storage-inc"))
- flags |= VIR_MIGRATE_NON_SHARED_INC;
-
- if (vshCommandOptBool(cmd, "copy-storage-synchronous-writes")) {
- if (!(flags & VIR_MIGRATE_NON_SHARED_DISK) &&
- !(flags & VIR_MIGRATE_NON_SHARED_INC)) {
- vshError(ctl, "'--copy-storage-synchronous-writes' requires one
of '--copy-storage-all', 'copy-storage-inc'");
- goto out;
- }
- flags |= VIR_MIGRATE_NON_SHARED_SYNCHRONOUS_WRITES;
- }
-
- if (vshCommandOptBool(cmd, "change-protection"))
- flags |= VIR_MIGRATE_CHANGE_PROTECTION;
-
- if (vshCommandOptBool(cmd, "unsafe"))
- flags |= VIR_MIGRATE_UNSAFE;
-
- if (vshCommandOptBool(cmd, "compressed"))
- flags |= VIR_MIGRATE_COMPRESSED;
-
- if (vshCommandOptBool(cmd, "auto-converge"))
- flags |= VIR_MIGRATE_AUTO_CONVERGE;
-
- if (vshCommandOptBool(cmd, "rdma-pin-all"))
- flags |= VIR_MIGRATE_RDMA_PIN_ALL;
-
- if (vshCommandOptBool(cmd, "offline"))
- flags |= VIR_MIGRATE_OFFLINE;
-
- if (vshCommandOptBool(cmd, "abort-on-error"))
- flags |= VIR_MIGRATE_ABORT_ON_ERROR;
-
- if (vshCommandOptBool(cmd, "postcopy"))
- flags |= VIR_MIGRATE_POSTCOPY;
-
- if (vshCommandOptBool(cmd, "postcopy-resume"))
- flags |= VIR_MIGRATE_POSTCOPY_RESUME;
-
- if (vshCommandOptBool(cmd, "zerocopy"))
- flags |= VIR_MIGRATE_ZEROCOPY;
-
- if (vshCommandOptBool(cmd, "tls"))
- flags |= VIR_MIGRATE_TLS;
-
- if (vshCommandOptBool(cmd, "parallel"))
- flags |= VIR_MIGRATE_PARALLEL;
-
if (flags & VIR_MIGRATE_PEER2PEER || vshCommandOptBool(cmd, "direct"))
{
if (virDomainMigrateToURI3(dom, desturi, params, nparams, flags) == 0)
data->ret = 0;
--
2.41.0
9:30 a.m.
New subject: [PATCH 3/4] virsh: doMigrate: Rework virsh option to migration flag conversion
On a Wednesday in 2023, Peter Krempa wrote:
Convert the flags declaratively as in the vast majority of cases
it's a
simple binary addition if the flag exists.
In one instance there was also an additional check, which was moved up
after the new code, and the error message was fixed.
I don't see the difference between the old and new messages.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
tools/virsh-domain.c | 108 +++++++++++++++++--------------------------
1 file changed, 42 insertions(+), 66 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
9:14 a.m.
New subject: [PATCH 4/4] virsh: doMigrate: Require --copy-storage-(all|inc) with --migrate-disks
Users need to enable non-shared-storage migration, otherwise the disks
specified via '--migrate-disks' will be ignored.
Add an error message to inform the users of their wrong config.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
tools/virsh-domain.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 84bf62057b..e0776c991f 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11204,6 +11204,11 @@ doMigrate(void *opaque)
if (opt) {
g_autofree char **val = NULL;
+ if (!(flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC))) {
+ vshError(ctl, "'--migrate-disks' requires one of
'--copy-storage-all', '--copy-storage-inc'");
+ goto out;
+ }
+
val = g_strsplit(opt, ",", 0);
if (virTypedParamsAddStringList(¶ms,
--
2.41.0
9:32 a.m.
New subject: [PATCH 4/4] virsh: doMigrate: Require --copy-storage-(all|inc) with --migrate-disks
On a Wednesday in 2023, Peter Krempa wrote:
Users need to enable non-shared-storage migration, otherwise the
disks
specified via '--migrate-disks' will be ignored.
Add an error message to inform the users of their wrong config.
Wouldn't it be friendlier to imply, for example, --copy-storage-all?
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
tools/virsh-domain.c | 5 +++++
1 file changed, 5 insertions(+)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
473
days inactive
473
days old
8 comments
2 participants
participants (2)
-
Ján Tomko -
Peter Krempa