[libvirt] [PATCH] Report error when both live and offline flags are used for migration.

[Nitesh Konkar]

Hello Jiri,

On Tue, Mar 8, 2016 at 8:31 PM, Jiri Denemark <jdenemar at 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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160309/ab27806b/attachment-0001.htm>