Re: [libvirt] [PATCH] Report error when both live and offline flags are used for migration.
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