Also enforce the enum type for MigrateSetMaxSpeed and MigrateGetMaxSpeed
functions which previously accepted generic uint32 type since the flags
haven't been in use.

Signed-off-by: Erik Skultety <eskultet@redhat.com>
---
 domain.go       | 10 ++++++++--
 domain_compat.h |  4 ++++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/domain.go b/domain.go
index f4546b8..91b8399 100644
--- a/domain.go
+++ b/domain.go
@@ -811,6 +811,12 @@ const (
 	MIGRATE_TLS               = DomainMigrateFlags(C.VIR_MIGRATE_TLS)
 )
 
+type DomainMigrateMaxSpeedFlags int
+
+const (
+	MIGRATE_MAX_SPEED_POSTCOPY = DomainMigrateMaxSpeedFlags(C.VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY)
+)
+
 type VcpuState int
 
 const (
@@ -2484,7 +2490,7 @@ func (d *Domain) MigrateSetCompressionCache(size uint64, flags uint32) error {
 }
 
 // See also https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainMigrateGetMaxSpeed
-func (d *Domain) MigrateGetMaxSpeed(flags uint32) (uint64, error) {
+func (d *Domain) MigrateGetMaxSpeed(flags DomainMigrateMaxSpeedFlags) (uint64, error) {
 	var maxSpeed C.ulong
 
 	var err C.virError
@@ -2497,7 +2503,7 @@ func (d *Domain) MigrateGetMaxSpeed(flags uint32) (uint64, error) {
 }
 
 // See also https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainMigrateSetMaxSpeed
-func (d *Domain) MigrateSetMaxSpeed(speed uint64, flags uint32) error {
+func (d *Domain) MigrateSetMaxSpeed(speed uint64, flags DomainMigrateMaxSpeedFlags) error {
 	var err C.virError
 	ret := C.virDomainMigrateSetMaxSpeedWrapper(d.ptr, C.ulong(speed), C.uint(flags), &err)
 	if ret == -1 {
diff --git a/domain_compat.h b/domain_compat.h
index 7589f41..9a30e8f 100644
--- a/domain_compat.h
+++ b/domain_compat.h
@@ -944,4 +944,8 @@ struct _virDomainInterface {
 #define VIR_MIGRATE_PARAM_BANDWIDTH_POSTCOPY "bandwidth.postcopy"
 #endif
 
+#ifndef VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY
+#define VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY (1 << 0)
+#endif
+
 #endif /* LIBVIRT_GO_DOMAIN_COMPAT_H__ */
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Mon, Feb 11, 2019 at 04:11:55PM +0100, Erik Skultety wrote:
> Also enforce the enum type for MigrateSetMaxSpeed and MigrateGetMaxSpeed
> functions which previously accepted generic uint32 type since the flags
> haven't been in use.

FWIW, Erik asked me about this change off-list.

Technically changing the flags parameter from uint32 to the named
enum type is considered an API incompatible change. In practice
though, the flags parameter was unused in the past, so applications
should normally have been passing an untyped '0' inline.eg

   dom.MigrateSetMaxSpeed(1024*1024, 0)

Go will happily do type inference there, so with this change it will
simply interpret '0' as being a value in the enum type. There should
not be any compile breakage in this case.

Where it could be a problem is if the app used an intermediate typed
variable

  var flags uint32
  flags = 0
  dom.MigrateSetMaxSpeed(1024*1024, flags)

This could conceivably affect some app, but I think it is fairly
unlikely since there's no compelling reason for the app to have used
a variable for flags given that it was unused. On balance with previous
cases like this I took the view that it is preferrable to change the
enum type so we get better type checking over the long term, despite
this small risk/

> 
> Signed-off-by: Erik Skultety <eskultet@redhat.com>
> ---
>  domain.go       | 10 ++++++++--
>  domain_compat.h |  4 ++++
>  2 files changed, 12 insertions(+), 2 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Mon, Feb 11, 2019 at 03:23:31PM +0000, Daniel P. Berrangé wrote:
> On Mon, Feb 11, 2019 at 04:11:55PM +0100, Erik Skultety wrote:
> > Also enforce the enum type for MigrateSetMaxSpeed and MigrateGetMaxSpeed
> > functions which previously accepted generic uint32 type since the flags
> > haven't been in use.
>
> FWIW, Erik asked me about this change off-list.
>
> Technically changing the flags parameter from uint32 to the named
> enum type is considered an API incompatible change. In practice
> though, the flags parameter was unused in the past, so applications
> should normally have been passing an untyped '0' inline.eg
>
>    dom.MigrateSetMaxSpeed(1024*1024, 0)
>
> Go will happily do type inference there, so with this change it will
> simply interpret '0' as being a value in the enum type. There should
> not be any compile breakage in this case.
>
> Where it could be a problem is if the app used an intermediate typed
> variable
>
>   var flags uint32
>   flags = 0
>   dom.MigrateSetMaxSpeed(1024*1024, flags)
>
> This could conceivably affect some app, but I think it is fairly
> unlikely since there's no compelling reason for the app to have used
> a variable for flags given that it was unused. On balance with previous
> cases like this I took the view that it is preferrable to change the
> enum type so we get better type checking over the long term, despite
> this small risk/

I'll include this in the commit message so that it doesn't get lost on the list
and is easier to be found.

Thanks,
Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list