virDomainNetDefCheckABIStability: Check for MTU change too

[libvirt] [PATCH] virDomainNetDefCheckABIStability: Check for MTU change too

Posted by Michal Privoznik 5 years, 7 months ago

https://bugzilla.redhat.com/show_bug.cgi?id=1623157

Changing MTU on a running guest is not possible and trying to do
so made us face many problems. That's why we forbid it in
5f44d7e357f61f7. However, there is still one possible path where
users can sneak in change: migration XML.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/conf/domain_conf.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 38cac07913..59ca9f5888 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -21901,6 +21901,13 @@ virDomainNetDefCheckABIStability(virDomainNetDefPtr src,
         return false;
     }
 
+    if (src->mtu != dst->mtu) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("Target network card MTU %d does not match source %d"),
+                       dst->mtu, src->mtu);
+        return false;
+    }
+
     if (src->virtio && dst->virtio &&
         !virDomainVirtioOptionsCheckABIStability(src->virtio, dst->virtio))
         return false;
-- 
2.16.4

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

Re: [libvirt] [PATCH] virDomainNetDefCheckABIStability: Check for MTU change too

Posted by Daniel P. Berrangé 5 years, 7 months ago

On Thu, Aug 30, 2018 at 02:14:27PM +0200, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1623157
> 
> Changing MTU on a running guest is not possible and trying to do
> so made us face many problems. That's why we forbid it in
> 5f44d7e357f61f7. However, there is still one possible path where
> users can sneak in change: migration XML.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/conf/domain_conf.c | 7 +++++++
>  1 file changed, 7 insertions(+)

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


> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 38cac07913..59ca9f5888 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -21901,6 +21901,13 @@ virDomainNetDefCheckABIStability(virDomainNetDefPtr src,
>          return false;
>      }
>  
> +    if (src->mtu != dst->mtu) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("Target network card MTU %d does not match source %d"),
> +                       dst->mtu, src->mtu);
> +        return false;
> +    }
> +
>      if (src->virtio && dst->virtio &&
>          !virDomainVirtioOptionsCheckABIStability(src->virtio, dst->virtio))
>          return false;

The virDomainNetDefCheckABIStability function feels like it need more
work to it, so I'm not convinced it is safe to change the def->driver.*
fields either - especially number of queues. That can be dealy with
separately though.

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