On 09/10/2015 09:20 AM, Nikolay Shirokovskiy wrote:
> Introduce a new function for the check. virDomainMigrateUnmanagedParams is not
> a good candidate for this functionality as it is used by migrate family
> functions too and its have its own checks that are superset of extracted and we
> don't need to check twice.
>
> Actually name of the function is slightly misleading as there is also a check
> for consitensy of flags parameter alone. So it could be refactored further and
s/consitensy/consistency
> reused by all migrate functions bu for now let it be a matter of a different
> patchset.
>
> It is *not* a pure refactoring patch as it introduces offline check for all
> versions. Looks like it must be done that way and no one will be broken too.
>
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@xxxxxxxxxxxxx>
> ---
> src/libvirt-domain.c | 81 +++++++++++++++++++------------------------------
> 1 files changed, 32 insertions(+), 49 deletions(-)
>
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 483537a..2d98997 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -4108,6 +4108,35 @@ virDomainMigrate3(virDomainPtr domain,
> return NULL;
> }
>
> +static
> +int virDomainMigrateUnmanagedCheckCompat(virDomainPtr domain, unsigned int flags)
Should be:
static int
virDomainMigrateUnmanagedCheckCompat(virDomainPtr domain,
unsigned int flags)
The "unsigned int flags" should be a separate line
Be sure to keep 2 blank lines between functions...
Also I now see where you lifted the changes in patch 10 from. Makes me
wonder if the two should change slightly such that patch 10 becomes
create the new API with the only caller virDomainMigrateToURI. Then
patch 11 becomes, modify virDomainMigrateToURI2 && URI3 to call it
rather than compressing the checks.
BTW: I still think the local duri in current patch 10 will do enough to
make it so Coverity doesn't whine.
> +{
> + VIR_EXCLUSIVE_FLAGS_RET(VIR_MIGRATE_NON_SHARED_DISK,
> + VIR_MIGRATE_NON_SHARED_INC,
> + -1);
> +
> + if (flags & VIR_MIGRATE_OFFLINE &&
> + !VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
> + VIR_DRV_FEATURE_MIGRATION_OFFLINE)) {
> + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> + _("offline migration is not supported by "
> + "the source host"));
> + return -1;
> + }
> +
> + if (((flags & VIR_MIGRATE_PEER2PEER) &&
> + !VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
> + VIR_DRV_FEATURE_MIGRATION_P2P)) ||
> + (!(flags & VIR_MIGRATE_PEER2PEER) &&
> + !VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
> + VIR_DRV_FEATURE_MIGRATION_DIRECT))) {
> + virReportUnsupportedError();
> + return -1;
> + }
> +
> + return 0;
> +}
> +
>
> /**
> * virDomainMigrateToURI:
> @@ -4195,34 +4224,12 @@ virDomainMigrateToURI(virDomainPtr domain,
>
> virResetLastError();
>
> - /* First checkout the source */
any particular reason this line gets delete in each of the 3 functions?
John
> virCheckDomainReturn(domain, -1);
> virCheckReadOnlyGoto(domain->conn->flags, error);
> -
> virCheckNonNullArgGoto(duri, error);
>
> - VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MIGRATE_NON_SHARED_DISK,
> - VIR_MIGRATE_NON_SHARED_INC,
> - error);
> -
> - if (flags & VIR_MIGRATE_OFFLINE &&
> - !VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
> - VIR_DRV_FEATURE_MIGRATION_OFFLINE)) {
> - virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> - _("offline migration is not supported by "
> - "the source host"));
> - goto error;
> - }
> -
> - if (((flags & VIR_MIGRATE_PEER2PEER) &&
> - !VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
> - VIR_DRV_FEATURE_MIGRATION_P2P)) ||
> - (!(flags & VIR_MIGRATE_PEER2PEER) &&
> - !VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
> - VIR_DRV_FEATURE_MIGRATION_DIRECT))) {
> - virReportUnsupportedError();
> + if (virDomainMigrateUnmanagedCheckCompat(domain, flags) < 0)
> goto error;
> - }
>
> if (flags & VIR_MIGRATE_PEER2PEER)
> dconnuri = duri;
> @@ -4343,23 +4350,11 @@ virDomainMigrateToURI2(virDomainPtr domain,
>
> virResetLastError();
>
> - /* First checkout the source */
> virCheckDomainReturn(domain, -1);
> virCheckReadOnlyGoto(domain->conn->flags, error);
>
> - VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MIGRATE_NON_SHARED_DISK,
> - VIR_MIGRATE_NON_SHARED_INC,
> - error);
> -
> - if (((flags & VIR_MIGRATE_PEER2PEER) &&
> - !VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
> - VIR_DRV_FEATURE_MIGRATION_P2P)) ||
> - (!(flags & VIR_MIGRATE_PEER2PEER) &&
> - !VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
> - VIR_DRV_FEATURE_MIGRATION_DIRECT))) {
> - virReportUnsupportedError();
> + if (virDomainMigrateUnmanagedCheckCompat(domain, flags) < 0)
> goto error;
> - }
>
> if (!(flags & VIR_MIGRATE_PEER2PEER))
> dconnuri = NULL;
> @@ -4426,23 +4421,11 @@ virDomainMigrateToURI3(virDomainPtr domain,
>
> virResetLastError();
>
> - /* First checkout the source */
> virCheckDomainReturn(domain, -1);
> virCheckReadOnlyGoto(domain->conn->flags, error);
>
> - VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MIGRATE_NON_SHARED_DISK,
> - VIR_MIGRATE_NON_SHARED_INC,
> - error);
> -
> - if (((flags & VIR_MIGRATE_PEER2PEER) &&
> - !VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
> - VIR_DRV_FEATURE_MIGRATION_P2P)) ||
> - (!(flags & VIR_MIGRATE_PEER2PEER) &&
> - !VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
> - VIR_DRV_FEATURE_MIGRATION_DIRECT))) {
> - virReportUnsupportedError();
> + if (virDomainMigrateUnmanagedCheckCompat(domain, flags) < 0)
> goto error;
> - }
>
> if (!(flags & VIR_MIGRATE_PEER2PEER))
> dconnuri = NULL;
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list