12:08 p.m.
---
src/libvirt-domain.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 9491845..dc11945 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -3617,6 +3617,15 @@ virDomainMigrate(virDomainPtr domain,
error);
+ if (flags & VIR_MIGRATE_OFFLINE) {
+ if (flags & VIR_MIGRATE_LIVE) {
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+ _("Live and offline migration flags are "
+ "mutually exclusive"));
+ goto error;
+ }
+ }
+
if (flags & VIR_MIGRATE_OFFLINE) {
if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
VIR_DRV_FEATURE_MIGRATION_OFFLINE)) {
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
@@ -3841,6 +3850,15 @@ virDomainMigrate2(virDomainPtr domain,
error);
+ if (flags & VIR_MIGRATE_OFFLINE) {
+ if (flags & VIR_MIGRATE_LIVE) {
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+ _("Live and offline migration flags are "
+ "mutually exclusive"));
+ goto error;
+ }
+ }
+
if (flags & VIR_MIGRATE_OFFLINE) {
if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
VIR_DRV_FEATURE_MIGRATION_OFFLINE)) {
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
@@ -4014,6 +4032,15 @@ virDomainMigrate3(virDomainPtr domain,
VIR_MIGRATE_NON_SHARED_INC,
error);
+ if (flags & VIR_MIGRATE_OFFLINE) {
+ if (flags & VIR_MIGRATE_LIVE) {
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+ _("Live and offline migration flags are "
+ "mutually exclusive"));
+ goto error;
+ }
+ }
+
if (flags & VIR_MIGRATE_PEER2PEER) {
virReportInvalidArg(flags, "%s",
_("use virDomainMigrateToURI3 for peer-to-peer "
--
1.8.3.1
6:21 a.m.
Hello All,
Any comments on this small patch?
Warm Regards,
Nitesh Konkar.
On Thu, Mar 3, 2016 at 4:38 PM, Nitesh Konkar <
niteshkonkar.libvirt(a)gmail.com> wrote:
---
src/libvirt-domain.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 9491845..dc11945 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -3617,6 +3617,15 @@ virDomainMigrate(virDomainPtr domain,
error);
+ if (flags & VIR_MIGRATE_OFFLINE) {
+ if (flags & VIR_MIGRATE_LIVE) {
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+ _("Live and offline migration flags are "
+ "mutually exclusive"));
+ goto error;
+ }
+ }
+
if (flags & VIR_MIGRATE_OFFLINE) {
if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
VIR_DRV_FEATURE_MIGRATION_OFFLINE))
{
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
@@ -3841,6 +3850,15 @@ virDomainMigrate2(virDomainPtr domain,
error);
+ if (flags & VIR_MIGRATE_OFFLINE) {
+ if (flags & VIR_MIGRATE_LIVE) {
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+ _("Live and offline migration flags are "
+ "mutually exclusive"));
+ goto error;
+ }
+ }
+
if (flags & VIR_MIGRATE_OFFLINE) {
if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
VIR_DRV_FEATURE_MIGRATION_OFFLINE))
{
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
@@ -4014,6 +4032,15 @@ virDomainMigrate3(virDomainPtr domain,
VIR_MIGRATE_NON_SHARED_INC,
error);
+ if (flags & VIR_MIGRATE_OFFLINE) {
+ if (flags & VIR_MIGRATE_LIVE) {
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+ _("Live and offline migration flags are "
+ "mutually exclusive"));
+ goto error;
+ }
+ }
+
if (flags & VIR_MIGRATE_PEER2PEER) {
virReportInvalidArg(flags, "%s",
_("use virDomainMigrateToURI3 for
peer-to-peer "
--
1.8.3.1
4:01 p.m.
On Thu, Mar 03, 2016 at 06:08:20 -0500, Nitesh Konkar wrote:
---
src/libvirt-domain.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 9491845..dc11945 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -3617,6 +3617,15 @@ virDomainMigrate(virDomainPtr domain,
error);
+ if (flags & VIR_MIGRATE_OFFLINE) {
I asked you to move the following if () {...} block...
+ if (flags & VIR_MIGRATE_LIVE) {
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+ _("Live and offline migration flags are "
+ "mutually exclusive"));
+ goto error;
+ }
+ }
+
if (flags & VIR_MIGRATE_OFFLINE) {
... here.
if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver,
domain->conn,
VIR_DRV_FEATURE_MIGRATION_OFFLINE)) {
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
I wanted to do that and push the patch, but I realized the patch is
incomplete (it doesn't cover MigrateToURI* APIs) and there is even a
better place to add these checks...
Something like the following (untested):
diff --git i/src/qemu/qemu_migration.c w/src/qemu/qemu_migration.c
index 64cbffa..36a939d 100644
--- i/src/qemu/qemu_migration.c
+++ w/src/qemu/qemu_migration.c
@@ -3081,6 +3081,12 @@ qemuMigrationBeginPhase(virQEMUDriverPtr driver,
goto cleanup;
if (flags & VIR_MIGRATE_OFFLINE) {
+ if (flags & VIR_MIGRATE_LIVE) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("live and offline migration flags are "
+ "mutually exclusive"));
+ goto cleanup;
+ }
if (flags & (VIR_MIGRATE_NON_SHARED_DISK |
VIR_MIGRATE_NON_SHARED_INC)) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
@@ -3335,6 +3341,12 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
virNWFilterReadLockFilterUpdates();
if (flags & VIR_MIGRATE_OFFLINE) {
+ if (flags & VIR_MIGRATE_LIVE) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("live and offline migration flags are "
+ "mutually exclusive"));
+ goto cleanup;
+ }
if (flags & (VIR_MIGRATE_NON_SHARED_DISK |
VIR_MIGRATE_NON_SHARED_INC)) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
4:11 p.m.
Hello Jiri,
On Tue, Mar 8, 2016 at 8:31 PM, Jiri Denemark <jdenemar(a)redhat.com> wrote:
On Thu, Mar 03, 2016 at 06:08:20 -0500, Nitesh Konkar wrote:
> ---
> src/libvirt-domain.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 9491845..dc11945 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -3617,6 +3617,15 @@ virDomainMigrate(virDomainPtr domain,
> error);
>
> + if (flags & VIR_MIGRATE_OFFLINE) {
I asked you to move the following if () {...} block...
Sorry, I missed this out. Will rectify it in the next patch.
> + if (flags & VIR_MIGRATE_LIVE) {
> + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> + _("Live and offline migration flags are "
> + "mutually exclusive"));
> + goto error;
> + }
> + }
> +
> if (flags & VIR_MIGRATE_OFFLINE) {
... here.
> if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver,
domain->conn,
>
VIR_DRV_FEATURE_MIGRATION_OFFLINE)) {
> virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
I wanted to do that and push the patch, but I realized the patch is
incomplete (it doesn't cover MigrateToURI* APIs) and there is even a
better place to add these checks...
Something like the following (untested):
But, changes in qemuMigrationBeginPhase and qemuMigrationPrepareAny would
necessitate changes to all drivers domainMigrate* specific implementations
right ?
I feel what we have is better. What is your opinion on it ? :-)
diff --git i/src/qemu/qemu_migration.c w/src/qemu/qemu_migration.c
index 64cbffa..36a939d 100644
--- i/src/qemu/qemu_migration.c
+++ w/src/qemu/qemu_migration.c
@@ -3081,6 +3081,12 @@ qemuMigrationBeginPhase(virQEMUDriverPtr driver,
goto cleanup;
if (flags & VIR_MIGRATE_OFFLINE) {
+ if (flags & VIR_MIGRATE_LIVE) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("live and offline migration flags are "
+ "mutually exclusive"));
+ goto cleanup;
+ }
if (flags & (VIR_MIGRATE_NON_SHARED_DISK |
VIR_MIGRATE_NON_SHARED_INC)) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
@@ -3335,6 +3341,12 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
virNWFilterReadLockFilterUpdates();
if (flags & VIR_MIGRATE_OFFLINE) {
+ if (flags & VIR_MIGRATE_LIVE) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("live and offline migration flags are "
+ "mutually exclusive"));
+ goto cleanup;
+ }
if (flags & (VIR_MIGRATE_NON_SHARED_DISK |
VIR_MIGRATE_NON_SHARED_INC)) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
Looking forward to your reply.
Warm Regards,
Nitesh Konkar.
5:45 p.m.
On Wed, Mar 09, 2016 at 20:41:45 +0530, Nitesh Konkar wrote:
Hello Jiri,
On Tue, Mar 8, 2016 at 8:31 PM, Jiri Denemark <jdenemar(a)redhat.com> wrote:
> On Thu, Mar 03, 2016 at 06:08:20 -0500, Nitesh Konkar wrote:
> > ---
> > src/libvirt-domain.c | 27 +++++++++++++++++++++++++++
> > 1 file changed, 27 insertions(+)
> >
> > diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> > index 9491845..dc11945 100644
> > --- a/src/libvirt-domain.c
> > +++ b/src/libvirt-domain.c
> > @@ -3617,6 +3617,15 @@ virDomainMigrate(virDomainPtr domain,
> > error);
> >
> > + if (flags & VIR_MIGRATE_OFFLINE) {
>
> I asked you to move the following if () {...} block...
>
>
Sorry, I missed this out. Will rectify it in the next patch.
> > + if (flags & VIR_MIGRATE_LIVE) {
> > + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> > + _("Live and offline migration flags are
"
> > + "mutually exclusive"));
> > + goto error;
> > + }
> > + }
> > +
> > if (flags & VIR_MIGRATE_OFFLINE) {
>
> ... here.
>
> > if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver,
> domain->conn,
> >
> VIR_DRV_FEATURE_MIGRATION_OFFLINE)) {
> > virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
>
> I wanted to do that and push the patch, but I realized the patch is
> incomplete (it doesn't cover MigrateToURI* APIs) and there is even a
> better place to add these checks...
>
> Something like the following (untested):
>
>
But, changes in qemuMigrationBeginPhase and qemuMigrationPrepareAny would
necessitate changes to all drivers domainMigrate* specific implementations
right ?
Yes, but the problem with your approach is that it isn't complete (more
APIs would need to be updated) and it's client-side (these high level
migration APIs only run on the client). It doesn't seem to be the right
place either since these high level APIs use flags only to distinguish
what RPC calls they need to make.
My approach is better since you only need to add the check to source and
destination entry points (even though you need to do it for each driver,
but not really) and the checks are next to the other similar checks for
flags. Drivers with no support for either of the flags don't need to be
updated at all.
Jirka
3110
days inactive
3122
days old
4 comments
2 participants
participants (2)
-
Jiri Denemark -
Nitesh Konkar